[PR #4284] [CLOSED] fix(clients): remove a sole JSONC registration with a trailing comma #6156

New issue

Closed

opened 2026-10-31 01:12:18 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:18 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4284
Author: @alfredo1491
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/clients-sole-jsonc-trailing-comma

📝 Commits (1)

19f4362 fix(clients): remove a sole JSONC registration with a trailing comma

📊 Changes

2 files changed (+80 additions, -29 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.ts (+54 -29)
📝 tests/boundary/filesystem/clientConfigurationDialects.test.ts (+26 -0)

📄 Description

Fixes #4244

Problem

When REA's registration is the only member of a JSONC server table and is followed by a trailing comma, rea uninstall fails: Cursor reports failed, the original file is restored, and REA stays registered. Removing the comma by hand makes the same uninstall succeed.

Root cause: for a sole property, deleteJsonPropertyPreservingComments returns undefined and the stock jsonc-parser removal edit runs. That edit (jsonc-parser 3.3.1) drops the property but keeps its trailing comma:

{ "mcpServers": { "rea": {…}, } }   →   { "mcpServers": { , } }

The serialized document then fails validation, so the uninstall transaction rolls back. Tables with another sibling are unaffected (the existing path or the stock edit leaves valid JSONC).

Change

deleteJsonPropertyPreservingComments handles the sole-member case: it drops the trailing comma, then applies the same stock removal edit, so the result matches the comma-less file.
The whitespace/comment scan that finds the comma is extracted into nextJsoncTokenOffset and shared with the existing next-sibling path; the JSONC edit formatting options are named once (JSONC_EDIT_FORMATTING). No other behavior changes.

Verification

New boundary test uninstalls a sole Cursor registration that has a trailing comma in tests/boundary/filesystem/clientConfigurationDialects.test.ts: real configureClientConfiguration registration, a trailing comma added after REA's entry, then systemUninstallHost(...).removeClient(...). It fails on main with expected 'failed' to be 'removed' and passes with this change; the result parses without errors and leaves mcpServers empty.
Client configuration suites (tests/boundary/filesystem/client*.test.ts, *ClientConfiguration.test.ts, src/application/{Setup,Uninstall,Client,Doctor}*.test.ts) compared before/after on the same Windows host: identical failure set (46 pre-existing Windows-host failures), 500 → 501 passing, no new failures.
tsc --noEmit and oxlint on the changed files: passed.

Unavailable coverage: run on Windows 11 x64 / Node 22.23.1 only; the 46 pre-existing failures in those suites on this host were not investigated, and the full CI suite was not run locally.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4284 **Author:** [@alfredo1491](https://github.com/alfredo1491) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/clients-sole-jsonc-trailing-comma` --- ### 📝 Commits (1) - [`19f4362`](https://github.com/morluto/rea/commit/19f43626a536464c171ce5eaf38791794369e47b) fix(clients): remove a sole JSONC registration with a trailing comma ### 📊 Changes **2 files changed** (+80 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.ts` (+54 -29) 📝 `tests/boundary/filesystem/clientConfigurationDialects.test.ts` (+26 -0) </details> ### 📄 Description Fixes #4244 ## Problem When REA's registration is the only member of a JSONC server table and is followed by a trailing comma, `rea uninstall` fails: Cursor reports `failed`, the original file is restored, and REA stays registered. Removing the comma by hand makes the same uninstall succeed. Root cause: for a sole property, `deleteJsonPropertyPreservingComments` returns `undefined` and the stock jsonc-parser removal edit runs. That edit (jsonc-parser 3.3.1) drops the property but keeps its trailing comma: ```text { "mcpServers": { "rea": {…}, } } → { "mcpServers": { , } } ``` The serialized document then fails validation, so the uninstall transaction rolls back. Tables with another sibling are unaffected (the existing path or the stock edit leaves valid JSONC). ## Change - `deleteJsonPropertyPreservingComments` handles the sole-member case: it drops the trailing comma, then applies the same stock removal edit, so the result matches the comma-less file. - The whitespace/comment scan that finds the comma is extracted into `nextJsoncTokenOffset` and shared with the existing next-sibling path; the JSONC edit formatting options are named once (`JSONC_EDIT_FORMATTING`). No other behavior changes. ## Verification - New boundary test `uninstalls a sole Cursor registration that has a trailing comma` in `tests/boundary/filesystem/clientConfigurationDialects.test.ts`: real `configureClientConfiguration` registration, a trailing comma added after REA's entry, then `systemUninstallHost(...).removeClient(...)`. It fails on `main` with `expected 'failed' to be 'removed'` and passes with this change; the result parses without errors and leaves `mcpServers` empty. - Client configuration suites (`tests/boundary/filesystem/client*.test.ts`, `*ClientConfiguration.test.ts`, `src/application/{Setup,Uninstall,Client,Doctor}*.test.ts`) compared before/after on the same Windows host: identical failure set (46 pre-existing Windows-host failures), 500 → 501 passing, no new failures. - `tsc --noEmit` and `oxlint` on the changed files: passed. Unavailable coverage: run on Windows 11 x64 / Node 22.23.1 only; the 46 pre-existing failures in those suites on this host were not investigated, and the full CI suite was not run locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:18 +00:00