[PR #4957] [MERGED] fix(setup): edit the duplicate JSON member that clients read #6694

Closed
opened 2026-11-01 21:42:20 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4957
Author: @kaoru0822-kitauji
Created: 10/18/2026
Status: ✅ Merged
Merged: 10/18/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/json-duplicate-member-edit


📝 Commits (2)

  • b6bb75d fix(setup): edit the duplicate JSON member that clients read
  • 59eaa2e Merge remote-tracking branch 'origin/main' into fix/json-duplicate-member-edit

📊 Changes

8 files changed (+365 additions, -92 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.capacity.test.ts (+13 -13)
📝 src/application/ClientConfigurationDocument.ts (+143 -54)
📝 src/application/SetupClientConfiguration.ts (+32 -18)
📝 src/application/SetupClients.ts (+2 -0)
📝 src/application/SetupTypes.ts (+6 -0)
📝 src/application/Uninstall.ts (+22 -6)
➕ tests/acceptance/setup/duplicateJsonMemberSetup.test.ts (+144 -0)
📝 tests/boundary/filesystem/clientConfigurationEnvironment.test.ts (+3 -1)

📄 Description

Summary

Fixes #4952. Setup and uninstall now edit the duplicate JSON member that the parsed document and JSON clients read (the last one), and they refuse an uninstall that would make an earlier shadowed duplicate active.

Problem and expected behavior

parseClientConfiguration reads JSON/JSONC with last-member semantics, the same as JSON.parse and native Claude Code. The shared serializer edited text through jsonc-parser's modify and findNodeAtLocation, and both of those pick the first matching property. As a result, setup on {"mcpServers": {...}, "mcpServers": {...}}, or on a repeated rea leaf, wrote into a member nobody reads, and the run ended in needs_human with failed/readback. Uninstall from a repeated parent table failed the same way. In every case the original file was restored.

Expected: edit the member the client reads, and leave shadowed duplicates and comments as they are. If removing the effective owned rea would make an earlier rea active (owned, foreign or unresolved), keep the configuration unchanged and say why.

Change and scope

  • ClientConfigurationDocument.ts now owns a single last-member locator (effectiveJsonProperty), and every JSON text edit goes through editJsonMember. The first-member modify path is gone. Formatting follows modify's own rule: jsonc-parser format runs over the touched lines.
  • serializeClientConfiguration returns Result<string, ClientConfigurationShadowedMember>. Deleting an effective member that has an earlier same-key sibling returns { kind: "shadowed_member", path }.
  • Uninstall serializes before taking a backup. On shadowed_member it reports failed with a specific explanation and writes nothing, not even a backup.
  • Setup serializes before its backup too. The only way setup deletes is OpenCode's legacy mcp.rea beside V2. That case now maps to a new shadowed_member failure reason with its own message.
  • Scope: an ordinary fix inside existing boundaries; no maintainer scope decision is needed. TOML, YAML and DSH serialization are unchanged.

Contract and boundary impact

  • Semantic owner and earliest changed stage: ClientConfigurationDocument JSON text-edit boundary.

  • Contribution scope fit and linked maintainer decision: not applicable (bug fix inside existing setup/uninstall authority).

  • CLI and MCP/tool-catalog contract: setup client results gain failed/shadowed_member with path. Uninstall keeps failed and adds a new detail text. Tool catalog unchanged.

  • Provider, bridge, target-format, or platform compatibility: JSON/JSONC client formats only.

  • Evidence, artifact, provenance, or reconstruction contract: none.

  • Target selection, authority, upstream behavior, and cleanup details: shadowed members are never deleted or activated. A refused uninstall leaves the configuration untouched and creates no backup.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

  • Change process memory, registers, files, or configuration.

Evidence and regression coverage

  • Tests added or updated: new tests/acceptance/setup/duplicateJsonMemberSetup.test.ts, which runs the public CLI:

    • Claude Code setup with a repeated server table and with a repeated rea leaf succeeds. The shadowed member stays byte-for-byte, the backup matches the original, and a second setup plans nothing and leaves the file unchanged.
    • Cursor JSONC setup with a repeated table keeps the comments and the shadowed table.
    • Uninstall from a repeated parent table succeeds.
    • Uninstall with a foreign rea followed by an owned rea is refused. The file is unchanged and no backup is created.

    Existing serializer callers in ClientConfigurationDocument.capacity.test.ts and clientConfigurationEnvironment.test.ts were updated to the Result return.

  • Base reproduction: on origin/main (2c4338db1) the issue's CLI repro gave exit 1 with failed/readback for duplicate-parent setup and duplicate-leaf setup, and a failed uninstall (original restored) for duplicate-parent and foreign-then-owned. With this branch the same script gave exit 0 configured for both setups, exit 0 removed for duplicate-parent uninstall, and exit 1 failed for foreign-then-owned, with the file unchanged. All 5 new tests fail when run against origin/main sources.

  • User-visible output for the refused uninstall: Configuration repeats the member at ["mcpServers","rea"]; removing the registration the client reads would activate an earlier duplicate, so no change was made. Remove the duplicate members manually, then rerun uninstall.

  • Remaining proof gaps: native Claude Code and other native clients were not run for this change; the issue's native last-member observation is the only native evidence. Not run: Windows/Linux, and the OpenCode legacy shadowed_member setup path (source-derived only, no dedicated test).

Validation performed

  • npm run build:cached — passed
  • Issue repro script (setup/uninstall with isolated HOME/CLAUDE_CONFIG_DIR) on main and on this branch — results as above
  • npm run test:focused -- tests/acceptance/setup/duplicateJsonMemberSetup.test.ts on origin/main sources — 5 failed (expected)
  • npm run test:focused -- the related setup and client-configuration tests (44 files in tests/acceptance/setup and tests/boundary/filesystem, plus src/application/ClientConfigurationDocument.{capacity,shape}.test.ts and src/application/Setup.{configuration,recovery,planning,initialSnapshot}.test.ts) — 50 files, 738 passed, 2 skipped
  • npm run check:fast — passed
  • npx oxfmt --check on changed files — clean after formatting

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Configurations without duplicate members are edited as before.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS only; native Claude Code not run.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the fix narrows mutation. It never removes or activates a shadowed member.

Author readiness

Human owner:

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 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/4957 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/18/2026 **Status:** ✅ Merged **Merged:** 10/18/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/json-duplicate-member-edit` --- ### 📝 Commits (2) - [`b6bb75d`](https://github.com/morluto/rea/commit/b6bb75dfe8f3659152fced1b4550bef06bca25d4) fix(setup): edit the duplicate JSON member that clients read - [`59eaa2e`](https://github.com/morluto/rea/commit/59eaa2edc96aaa8b4c111765ed3befcf8f713fae) Merge remote-tracking branch 'origin/main' into fix/json-duplicate-member-edit ### 📊 Changes **8 files changed** (+365 additions, -92 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.capacity.test.ts` (+13 -13) 📝 `src/application/ClientConfigurationDocument.ts` (+143 -54) 📝 `src/application/SetupClientConfiguration.ts` (+32 -18) 📝 `src/application/SetupClients.ts` (+2 -0) 📝 `src/application/SetupTypes.ts` (+6 -0) 📝 `src/application/Uninstall.ts` (+22 -6) ➕ `tests/acceptance/setup/duplicateJsonMemberSetup.test.ts` (+144 -0) 📝 `tests/boundary/filesystem/clientConfigurationEnvironment.test.ts` (+3 -1) </details> ### 📄 Description ## Summary Fixes #4952. Setup and uninstall now edit the duplicate JSON member that the parsed document and JSON clients read (the last one), and they refuse an uninstall that would make an earlier shadowed duplicate active. ## Problem and expected behavior `parseClientConfiguration` reads JSON/JSONC with last-member semantics, the same as `JSON.parse` and native Claude Code. The shared serializer edited text through jsonc-parser's `modify` and `findNodeAtLocation`, and both of those pick the **first** matching property. As a result, setup on `{"mcpServers": {...}, "mcpServers": {...}}`, or on a repeated `rea` leaf, wrote into a member nobody reads, and the run ended in `needs_human` with `failed/readback`. Uninstall from a repeated parent table failed the same way. In every case the original file was restored. Expected: edit the member the client reads, and leave shadowed duplicates and comments as they are. If removing the effective owned `rea` would make an earlier `rea` active (owned, foreign or unresolved), keep the configuration unchanged and say why. ## Change and scope - `ClientConfigurationDocument.ts` now owns a single last-member locator (`effectiveJsonProperty`), and every JSON text edit goes through `editJsonMember`. The first-member `modify` path is gone. Formatting follows `modify`'s own rule: jsonc-parser `format` runs over the touched lines. - `serializeClientConfiguration` returns `Result<string, ClientConfigurationShadowedMember>`. Deleting an effective member that has an earlier same-key sibling returns `{ kind: "shadowed_member", path }`. - Uninstall serializes before taking a backup. On `shadowed_member` it reports `failed` with a specific explanation and writes nothing, not even a backup. - Setup serializes before its backup too. The only way setup deletes is OpenCode's legacy `mcp.rea` beside V2. That case now maps to a new `shadowed_member` failure reason with its own message. - Scope: an ordinary fix inside existing boundaries; no maintainer scope decision is needed. TOML, YAML and DSH serialization are unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: `ClientConfigurationDocument` JSON text-edit boundary. - Contribution scope fit and linked maintainer decision: not applicable (bug fix inside existing setup/uninstall authority). - CLI and MCP/tool-catalog contract: setup client results gain `failed/shadowed_member` with `path`. Uninstall keeps `failed` and adds a new detail text. Tool catalog unchanged. - Provider, bridge, target-format, or platform compatibility: JSON/JSONC client formats only. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: shadowed members are never deleted or activated. A refused uninstall leaves the configuration untouched and creates no backup. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. - [x] Change process memory, registers, files, or configuration. ## Evidence and regression coverage - Tests added or updated: new `tests/acceptance/setup/duplicateJsonMemberSetup.test.ts`, which runs the public CLI: - Claude Code setup with a repeated server table and with a repeated `rea` leaf succeeds. The shadowed member stays byte-for-byte, the backup matches the original, and a second setup plans nothing and leaves the file unchanged. - Cursor JSONC setup with a repeated table keeps the comments and the shadowed table. - Uninstall from a repeated parent table succeeds. - Uninstall with a foreign `rea` followed by an owned `rea` is refused. The file is unchanged and no backup is created. Existing serializer callers in `ClientConfigurationDocument.capacity.test.ts` and `clientConfigurationEnvironment.test.ts` were updated to the `Result` return. - Base reproduction: on `origin/main` (`2c4338db1`) the issue's CLI repro gave exit 1 with `failed/readback` for duplicate-parent setup and duplicate-leaf setup, and a failed uninstall (original restored) for duplicate-parent and foreign-then-owned. With this branch the same script gave exit 0 `configured` for both setups, exit 0 `removed` for duplicate-parent uninstall, and exit 1 `failed` for foreign-then-owned, with the file unchanged. All 5 new tests fail when run against `origin/main` sources. - User-visible output for the refused uninstall: `Configuration repeats the member at ["mcpServers","rea"]; removing the registration the client reads would activate an earlier duplicate, so no change was made. Remove the duplicate members manually, then rerun uninstall.` - Remaining proof gaps: native Claude Code and other native clients were **not** run for this change; the issue's native last-member observation is the only native evidence. Not run: Windows/Linux, and the OpenCode legacy `shadowed_member` setup path (source-derived only, no dedicated test). ## Validation performed - `npm run build:cached` — passed - Issue repro script (setup/uninstall with isolated `HOME`/`CLAUDE_CONFIG_DIR`) on main and on this branch — results as above - `npm run test:focused -- tests/acceptance/setup/duplicateJsonMemberSetup.test.ts` on `origin/main` sources — 5 failed (expected) - `npm run test:focused --` the related setup and client-configuration tests (44 files in `tests/acceptance/setup` and `tests/boundary/filesystem`, plus `src/application/ClientConfigurationDocument.{capacity,shape}.test.ts` and `src/application/Setup.{configuration,recovery,planning,initialSnapshot}.test.ts`) — 50 files, 738 passed, 2 skipped - `npm run check:fast` — passed - `npx oxfmt --check` on changed files — clean after formatting ## Compatibility, safety, and release - Breaking changes or migration steps: none. Configurations without duplicate members are edited as before. - Real Hopper/Ghidra, browser, or OS coverage: macOS only; native Claude Code not run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the fix narrows mutation. It never removes or activates a shadowed member. ## Author readiness Human owner: - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 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-11-01 21:42:20 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6694
No description provided.