morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4976] [Bug] Repeated REA-owned JSON members still block uninstall after #4957, and the refusal skips preflight #3078

New issue

Closed

opened 2026-10-31 00:36:03 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-31 00:36:03 +00:00

Owner

Originally created by @NikeGunn on GitHub (Oct 18, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4976

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Setup, packaging, or upgrade

Observed behavior

On current main b83b9caf (after #4957), three repeated-member cases from #4952 still fail:

Repeated REA-owned rea members cannot be uninstalled. #4952 lists "repeated identical owned rea members" uninstall as a target failure. rea uninstall still exits 1 with Configuration repeats the member at ["mcpServers","rea"] ... would activate an earlier duplicate, although every repeat is REA-owned and nothing foreign would become active. The same happens for OpenCode V2 mcp.servers.rea repeats and minified or JSONC layouts.
The refusal is not in the shared preflight decision, so uninstall can be partial. The repeated-member refusal comes from the serializer while one client is being removed. If an earlier client in iteration order succeeds first, its registration is already gone when the later client refuses: exit 1 with claude_code: removed, cursor: failed.
OpenCode setup refuses repeated V1 entries that it replaces when there is only one. Beside a native V2 mcp.servers table, setup replaces a single V1 mcp.rea (owned or not) with the native entry. Two V1 mcp.rea members make setup fail with failed/shadowed_member, exit 1.

Reproduction

Built CLI, isolated HOME, CLAUDE_CONFIG_DIR, and XDG_CONFIG_HOME for each call. Owned launcher: {"command": "npx", "args": ["-y", "rea-agents@6.6.0", "mcp"]} (classified, not executed).

~/.claude.json = {"mcpServers": {"rea": OWNED, "peer": {"command": "/usr/bin/true"}, "rea": OWNED}}, then rea uninstall --json → exit 1, file unchanged.
~/.claude.json = {"mcpServers": {"rea": OWNED}} and ~/.cursor/mcp.json = {"mcpServers": {"rea": {"command": "/usr/bin/false"}, "rea": OWNED}}, then rea uninstall --json → exit 1, and REA is already removed from ~/.claude.json.
~/.config/opencode/opencode.json = {"mcp": {"servers": {"peer": {...}}, "rea": FOREIGN, "rea": FOREIGN}}, then rea setup --client opencode --skill=false --yes --json → exit 1 failed/shadowed_member. With one rea V1 member the same command returns configured.

Expected behavior

Uninstall removes every repeat of rea in the table the client reads when REA owns each one, so no repeat becomes active.
When an earlier repeat is not REA-owned, the refusal is part of the ownership decision that preflight shares, so uninstall stops before changing any client and creates no backup.
OpenCode setup applies its existing V1-replacement rule to every repeat.

Impact and scope

Deterministic for these layouts; external frequency NOT_ENOUGH_INFO. No destructive change was observed: a refused edit is written before backup and nothing foreign is removed. Case 2 is a partial uninstall that the output reports.

REA version

main b83b9caf (6.6.0 + #4957)

Runtime and platform

Linux x64 (WSL2 Ubuntu), Node 24.18.0, npm 11.16.0.

Interface

CLI

Earliest failure stage

Uninstall ownership decision (clientRegistrationRemoval) and the JSON text-edit boundary (editJsonMember).

Analysis or suspected cause

parseClientConfiguration keeps the last repeated member and drops the earlier ones, so the ownership rule in clientRegistrationRemoval cannot see what removal would reveal. #4957 therefore made the serializer refuse every repeated deletion without regard to ownership, and that refusal is only reached during removal, after preflight. Proposed fix: have the document owner report the earlier repeats from the source text, extend the shared ownership decision to cover them, and give the serializer explicit authority to remove every repeat once that decision admits it. I have a branch with tests and will open a PR.

Originally created by @NikeGunn on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4976 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Setup, packaging, or upgrade ### Observed behavior On current main `b83b9caf` (after #4957), three repeated-member cases from #4952 still fail: 1. **Repeated REA-owned `rea` members cannot be uninstalled.** #4952 lists "repeated identical owned `rea` members" uninstall as a target failure. `rea uninstall` still exits 1 with `Configuration repeats the member at ["mcpServers","rea"] ... would activate an earlier duplicate`, although every repeat is REA-owned and nothing foreign would become active. The same happens for OpenCode V2 `mcp.servers.rea` repeats and minified or JSONC layouts. 2. **The refusal is not in the shared preflight decision, so uninstall can be partial.** The repeated-member refusal comes from the serializer while one client is being removed. If an earlier client in iteration order succeeds first, its registration is already gone when the later client refuses: exit 1 with `claude_code: removed`, `cursor: failed`. 3. **OpenCode setup refuses repeated V1 entries that it replaces when there is only one.** Beside a native V2 `mcp.servers` table, setup replaces a single V1 `mcp.rea` (owned or not) with the native entry. Two V1 `mcp.rea` members make setup fail with `failed/shadowed_member`, exit 1. ### Reproduction Built CLI, isolated `HOME`, `CLAUDE_CONFIG_DIR`, and `XDG_CONFIG_HOME` for each call. Owned launcher: `{"command": "npx", "args": ["-y", "rea-agents@6.6.0", "mcp"]}` (classified, not executed). 1. `~/.claude.json` = `{"mcpServers": {"rea": OWNED, "peer": {"command": "/usr/bin/true"}, "rea": OWNED}}`, then `rea uninstall --json` → exit 1, file unchanged. 2. `~/.claude.json` = `{"mcpServers": {"rea": OWNED}}` and `~/.cursor/mcp.json` = `{"mcpServers": {"rea": {"command": "/usr/bin/false"}, "rea": OWNED}}`, then `rea uninstall --json` → exit 1, and REA is already removed from `~/.claude.json`. 3. `~/.config/opencode/opencode.json` = `{"mcp": {"servers": {"peer": {...}}, "rea": FOREIGN, "rea": FOREIGN}}`, then `rea setup --client opencode --skill=false --yes --json` → exit 1 `failed/shadowed_member`. With one `rea` V1 member the same command returns `configured`. ### Expected behavior - Uninstall removes every repeat of `rea` in the table the client reads when REA owns each one, so no repeat becomes active. - When an earlier repeat is not REA-owned, the refusal is part of the ownership decision that preflight shares, so uninstall stops before changing any client and creates no backup. - OpenCode setup applies its existing V1-replacement rule to every repeat. ### Impact and scope Deterministic for these layouts; external frequency NOT_ENOUGH_INFO. No destructive change was observed: a refused edit is written before backup and nothing foreign is removed. Case 2 is a partial uninstall that the output reports. ### REA version main `b83b9caf` (6.6.0 + #4957) ### Runtime and platform Linux x64 (WSL2 Ubuntu), Node 24.18.0, npm 11.16.0. ### Interface CLI ### Earliest failure stage Uninstall ownership decision (`clientRegistrationRemoval`) and the JSON text-edit boundary (`editJsonMember`). ### Analysis or suspected cause `parseClientConfiguration` keeps the last repeated member and drops the earlier ones, so the ownership rule in `clientRegistrationRemoval` cannot see what removal would reveal. #4957 therefore made the serializer refuse every repeated deletion without regard to ownership, and that refusal is only reached during removal, after preflight. Proposed fix: have the document owner report the earlier repeats from the source text, extend the shared ownership decision to cover them, and give the serializer explicit authority to remove every repeat once that decision admits it. I have a branch with tests and will open a PR.

bsakaguchi closed this issue

2026-10-31 00:36:03 +00:00

bsakaguchi commented

2026-10-31 00:36:04 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 18, 2026):

Agreed with the proposed fix shape — the root cause is that parseClientConfiguration collapses duplicates before ownership can be judged, so the serializer ends up refusing with no ownership context, after preflight. Extending the shared ownership decision to cover earlier repeats is the right move. One ordering point worth locking in: the ownership decision must complete before any client removal starts, so case 2's partial-uninstall state (claude_code: removed, cursor: failed) becomes impossible rather than merely reported. Also worth pinning down backup semantics — if the shared decision precedes the first edit, the existing backup flow should fire before that first edit, not after. For case 3, applying the existing V1-replacement rule to every repeat is consistent; just make sure the refusal message distinguishes a foreign repeat from an owned one, so users can tell an ownership conflict apart from a bug.

@XinwangTao commented on GitHub (Oct 18, 2026): Agreed with the proposed fix shape — the root cause is that parseClientConfiguration collapses duplicates before ownership can be judged, so the serializer ends up refusing with no ownership context, after preflight. Extending the shared ownership decision to cover earlier repeats is the right move. One ordering point worth locking in: the ownership decision must complete before any client removal starts, so case 2's partial-uninstall state (claude_code: removed, cursor: failed) becomes impossible rather than merely reported. Also worth pinning down backup semantics — if the shared decision precedes the first edit, the existing backup flow should fire before that first edit, not after. For case 3, applying the existing V1-replacement rule to every repeat is consistent; just make sure the refusal message distinguishes a foreign repeat from an owned one, so users can tell an ownership conflict apart from a bug.

bsakaguchi commented

2026-10-31 00:36:04 +00:00

Author

Owner

@NikeGunn commented on GitHub (Oct 18, 2026):

Thanks for reviewing. All three points are covered in #4977 (head 4cd22d4):

1. The decision finishes before any removal starts. runUninstall runs inspectClient for every client first (L144-L147), and removeAll starts only when nothing is blocked (L150-L160). The repeated-member check is part of clientRegistrationRemoval, the ownership decision that preflight and removal share (L288, L458). For an unchanged configuration, case 2's partial state can therefore no longer happen; it isn't just reported. The acceptance test uninstall refuses an unowned repeat before changing any client puts Claude Code first in iteration order and asserts it stays unchanged with no backup. One deliberate limit: removeClient re-reads each file and re-runs the same decision (L581), so a file edited after preflight is still refused rather than overwritten. That keeps REA's existing documented contract for failures during removal, and makes no claim of atomicity across files.

2. Backup timing. For each client the order is: decision, serialization (L602), backup (L625), then the single write (L645). The backup is taken right before the only edit, and a refused or unserializable edit takes none. Tests assert that the backup equals the original after a successful removal (L181) and that no backup exists after each refusal (L232, L294).

3. Owned and foreign repeats are told apart. REA-owned repeats are not refused at all; they are removed together. Only an ownership conflict is refused, and each kind has its own message:

Foreign earlier repeat: "Configuration repeats the "rea" member, and an earlier duplicate is not REA-owned; removing the registration the client reads would activate an earlier duplicate…"
Foreign OpenCode V1 fallback (found in review and already reachable on main with a single native entry): "OpenCode falls back to the V1 "mcp.rea" entry, which is not REA-owned; removing REA's native registration would activate it…"

For case 3, setup now applies its existing V1-replacement rule to every repeat, so it no longer refuses there. The serializer still defaults to the fail-closed shadowed_member refusal from #4957 for any edit made without that authority.

@NikeGunn commented on GitHub (Oct 18, 2026): Thanks for reviewing. All three points are covered in #4977 (head `4cd22d4`): **1. The decision finishes before any removal starts.** `runUninstall` runs `inspectClient` for every client first ([L144-L147](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L144-L147)), and `removeAll` starts only when nothing is blocked ([L150-L160](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L150-L160)). The repeated-member check is part of `clientRegistrationRemoval`, the ownership decision that preflight and removal share ([L288](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L288), [L458](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L458)). For an unchanged configuration, case 2's partial state can therefore no longer happen; it isn't just reported. The acceptance test [_uninstall refuses an unowned repeat before changing any client_](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts#L204) puts Claude Code first in iteration order and asserts it stays unchanged with no backup. One deliberate limit: `removeClient` re-reads each file and re-runs the same decision ([L581](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L581)), so a file edited after preflight is still refused rather than overwritten. That keeps REA's existing documented contract for failures during removal, and makes no claim of atomicity across files. **2. Backup timing.** For each client the order is: decision, serialization ([L602](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L602)), backup ([L625](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L625)), then the single write ([L645](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/src/application/Uninstall.ts#L645)). The backup is taken right before the only edit, and a refused or unserializable edit takes none. Tests assert that the backup equals the original after a successful removal ([L181](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts#L181)) and that no backup exists after each refusal ([L232](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts#L232), [L294](https://github.com/NikeGunn/rea/blob/4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts#L294)). **3. Owned and foreign repeats are told apart.** REA-owned repeats are not refused at all; they are removed together. Only an ownership conflict is refused, and each kind has its own message: - Foreign earlier repeat: _"Configuration repeats the "rea" member, and an earlier duplicate is not REA-owned; removing the registration the client reads would activate an earlier duplicate…"_ - Foreign OpenCode V1 fallback (found in review and already reachable on main with a single native entry): _"OpenCode falls back to the V1 "mcp.rea" entry, which is not REA-owned; removing REA's native registration would activate it…"_ For case 3, setup now applies its existing V1-replacement rule to every repeat, so it no longer refuses there. The serializer still defaults to the fail-closed `shadowed_member` refusal from #4957 for any edit made without that authority.

bsakaguchi commented

2026-10-31 00:36:05 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Fixed on main in 22ddd8201.

The semantic JSON parse kept only the effective member, so preflight missed earlier registrations that deletion could expose. The document owner now reads those facts from the effective JSONC syntax tree. Uninstall uses one typed ownership decision during both preflight and reread, including foreign OpenCode V1 fallbacks. One deletion planner removes admitted members and separators; edit authority is limited to exact paths. Inactive repeated parents, unrelated entries, comments and original backups remain intact. Setup grants replacement authority only for the superseded OpenCode V1 path.

Adapted the proposal in #4977 at 4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23, with contributor credit retained, then replaced handwritten JSON-string interpretation and broad edit permission. The PR itself was not merged or qualified as a whole.

Validation: final 86 focused cases and 9 setup acceptance cases passed, as did 104 additional filesystem consumer cases, full static/architecture checks, docs and test discovery. Compiled CLI controls verify owned-repeat removal, refusal before earlier-client edits/backups, foreign V1 fallback refusal and unchanged source bytes. A broader 186-case run had one OMP timeout; the isolated 9-case rerun passed, but the timeout cause remains unverified. Real developer-client execution was not tested. CI continues separately in the background.

Cost check: 20 completed compiled CLI workflows, comparing equivalent successful uninstalls with identical full reports and backups. Four alternating warm repetitions per variant/task followed separately recorded first-process runs. Full-response cl100k estimate stayed 1,053 tokens. For 8 KiB / 256 KiB configurations, baseline→candidate medians were elapsed 0.385→0.385 / 0.412→0.401 seconds, CPU 0.586→0.576 / 0.622→0.600 seconds, sampled process-family memory 352.5→345.8 / 356.4→350.0 MB. These controlled measurements do not establish a general speedup; model context, real clients, faults and other hosts remain unmeasured.

@morluto commented on GitHub (Oct 18, 2026): Fixed on main in [22ddd8201](https://github.com/morluto/rea/commit/22ddd820143b0ce455756b24acf60ed251286622). The semantic JSON parse kept only the effective member, so preflight missed earlier registrations that deletion could expose. The document owner now reads those facts from the effective JSONC syntax tree. Uninstall uses one typed ownership decision during both preflight and reread, including foreign OpenCode V1 fallbacks. One deletion planner removes admitted members and separators; edit authority is limited to exact paths. Inactive repeated parents, unrelated entries, comments and original backups remain intact. Setup grants replacement authority only for the superseded OpenCode V1 path. Adapted the proposal in #4977 at `4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23`, with contributor credit retained, then replaced handwritten JSON-string interpretation and broad edit permission. The PR itself was not merged or qualified as a whole. Validation: final 86 focused cases and 9 setup acceptance cases passed, as did 104 additional filesystem consumer cases, full static/architecture checks, docs and test discovery. Compiled CLI controls verify owned-repeat removal, refusal before earlier-client edits/backups, foreign V1 fallback refusal and unchanged source bytes. A broader 186-case run had one OMP timeout; the isolated 9-case rerun passed, but the timeout cause remains unverified. Real developer-client execution was not tested. CI continues separately in the background. Cost check: 20 completed compiled CLI workflows, comparing equivalent successful uninstalls with identical full reports and backups. Four alternating warm repetitions per variant/task followed separately recorded first-process runs. Full-response `cl100k` estimate stayed 1,053 tokens. For 8 KiB / 256 KiB configurations, baseline→candidate medians were elapsed 0.385→0.385 / 0.412→0.401 seconds, CPU 0.586→0.576 / 0.622→0.600 seconds, sampled process-family memory 352.5→345.8 / 356.4→350.0 MB. These controlled measurements do not establish a general speedup; model context, real clients, faults and other hosts remain unmeasured.

bsakaguchi referenced this issue

2026-10-31 01:07:52 +00:00

[PR #3078] [CLOSED] fix(javascript): retain directive literals in semantic function fingerprints #5598

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#3078

No description provided.

Rows
Columns