From 4cd22d4ec2f6ec9e6c8ebf98bf97e1d9d7f8ac23 Mon Sep 17 00:00:00 2001 From: Nikhil Bhagat Date: Sun, 18 Oct 2026 12:45:47 +0545 Subject: [PATCH] fix(setup): keep OpenCode registrations whose V1 fallback REA does not own Beside a native V2 table, OpenCode loads the V1 mcp.rea member once the native mcp.servers.rea entry is gone. Uninstall removed an owned native entry even when that V1 member was foreign, so it activated a launcher REA does not own and still reported the registration removed. This already happened on main with one native entry; removing repeated native entries made it reachable there too. The shared ownership decision now treats a foreign V1 fallback as something native removal would reveal: preflight reports why and nothing is changed or backed up. Owned V1 members are still removed together with the native entry. Co-Authored-By: Claude Opus 5.5 --- docs/installation.md | 2 + src/application/Uninstall.ts | 44 +++++++++++-------- .../setup/duplicateJsonMemberSetup.test.ts | 35 +++++++++++++++ 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index 359f21a8e0..6a48ae5ea9 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -380,6 +380,8 @@ while being updated stops the remaining removals. JSON clients read the last of repeated `rea` members, so uninstall removes every repeat in the table the client reads when REA owns each one; if an earlier repeat is not REA-owned, removal would activate it, so the operation stops before anything is removed. +The same applies to an OpenCode V1 `mcp.rea` entry that REA does not own, +which OpenCode loads once the native `mcp.servers.rea` entry is gone. Repeats inside an inactive earlier copy of the server table stay unchanged. A purge path that is a symbolic link is retained and reported rather than followed. Uninstall checks diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index d2fe431d16..5c985b15e3 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -285,8 +285,8 @@ export const systemUninstallHost = ( read.original, client, ); - if (removal.shadowsUnownedMember) - return item(client.name, "failed", SHADOWED_UNOWNED_MEMBER_DETAIL); + if (removal.blocked !== undefined) + return item(client.name, "failed", removal.blocked); if (!removal.removeNative && removal.legacyPath === undefined) return undefined; // A removal the running package would revert, or redirect into its @@ -463,21 +463,17 @@ const clientRegistrationRemoval = ( readonly removeNative: boolean; readonly legacyPath: readonly string[] | undefined; /** - * REA's effective entry repeats a member REA does not own. Removing the - * entry would make that duplicate active, so nothing is removed. + * Why nothing is removed: removing REA's entry would activate an entry REA + * does not own, either an earlier repeat or an OpenCode V1 fallback. */ - readonly shadowsUnownedMember: boolean; + readonly blocked: string | undefined; } => { const name = PRODUCT_IDENTITY.mcpServerKey; if ( parsed.dialect === "dsh" && dshDocument(parsed.document).hasUnknownStructure ) - return { - removeNative: false, - legacyPath: undefined, - shadowsUnownedMember: false, - }; + return { removeNative: false, legacyPath: undefined, blocked: undefined }; // Removal deletes every repeated member of the name, so each must be owned. // The repeats are located only once an owned entry needs this check. let shadowed: ShadowedClientServers | undefined; @@ -500,22 +496,30 @@ const clientRegistrationRemoval = ( legacyPath === undefined ? "absent" : removal(parsed.legacyServers, (members) => members.legacyServers); - const shadowsUnownedMember = - native === "shadows_unowned" || legacy === "shadows_unowned"; + // OpenCode loads the V1 member once the native entry is gone, so removing + // the native entry must not expose a V1 member REA cannot remove. + const blocked = + native === "shadows_unowned" || legacy === "shadows_unowned" + ? SHADOWED_UNOWNED_MEMBER_DETAIL + : native === "removable" && legacy === "foreign" + ? UNOWNED_LEGACY_FALLBACK_DETAIL + : undefined; return { removeNative: - !shadowsUnownedMember && + blocked === undefined && ((parsed.dialect === "dsh" && dshDocument(parsed.document).carriesEntry) || native === "removable"), legacyPath: - !shadowsUnownedMember && legacy === "removable" ? legacyPath : undefined, - shadowsUnownedMember, + blocked === undefined && legacy === "removable" ? legacyPath : undefined, + blocked, }; }; const SHADOWED_UNOWNED_MEMBER_DETAIL = `Configuration repeats the "${PRODUCT_IDENTITY.mcpServerKey}" member, and an earlier duplicate is not REA-owned; removing the registration the client reads would activate an earlier duplicate, so no change was made. Remove the duplicate members manually, then rerun uninstall.`; +const UNOWNED_LEGACY_FALLBACK_DETAIL = `OpenCode falls back to the V1 "mcp.${PRODUCT_IDENTITY.mcpServerKey}" entry, which is not REA-owned; removing REA's native registration would activate it, so no change was made. Remove or replace that V1 entry manually, then rerun uninstall.`; + /** Preserve the bytes uninstall is about to edit and return the backup holding them. */ const backupClientConfiguration = async ( client: SetupClient, @@ -569,10 +573,12 @@ const removeClient = async ( ); if (!registrationPresent(parsed)) return item(client.name, "skipped", "REA registration is absent."); - const { removeNative, legacyPath, shadowsUnownedMember } = - clientRegistrationRemoval(parsed, original, client); - if (shadowsUnownedMember) - return item(client.name, "failed", SHADOWED_UNOWNED_MEMBER_DETAIL); + const { removeNative, legacyPath, blocked } = clientRegistrationRemoval( + parsed, + original, + client, + ); + if (blocked !== undefined) return item(client.name, "failed", blocked); const removeLegacy = legacyPath !== undefined; if (!removeNative && !removeLegacy) return item( diff --git a/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts b/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts index a75a04ac6b..d233ba24ad 100644 --- a/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts +++ b/tests/acceptance/setup/duplicateJsonMemberSetup.test.ts @@ -261,3 +261,38 @@ cliTest( }); }, ); + +for (const [layout, servers] of [ + ["one native entry", `"rea": ${openCodeOwned}`], + [ + "repeated native entries", + `"rea": ${openCodeOwned}, "rea": ${openCodeOwned}`, + ], +] as const) + cliTest( + `uninstall keeps ${layout} when OpenCode would fall back to a foreign V1 entry`, + async ({ cli }) => { + const home = await createTestTempDirectory("rea-json-duplicate-"); + const config = join(home, ".config/opencode/opencode.json"); + // Without the native entry, OpenCode loads the V1 `mcp.rea` member. + const original = `{\n "mcp": {\n "servers": {${servers}},\n "rea": ${openCodeForeign}\n }\n}\n`; + await writeConfiguration(config, original); + + const removed = await runIn(cli, home)(["uninstall", "--json"]); + expect(removed.exitCode, removed.stdout).toBe(1); + expect(removed.json).toMatchObject({ + status: "failed", + items: expect.arrayContaining([ + expect.objectContaining({ + name: "opencode", + status: "failed", + detail: expect.stringContaining("would activate it"), + }), + ]), + }); + expect(await readFile(config, "utf8")).toBe(original); + await expect(readFile(`${config}.rea.backup`)).rejects.toMatchObject({ + code: "ENOENT", + }); + }, + );