From 7572a7082e698321514540fd5900937a428043b9 Mon Sep 17 00:00:00 2001 From: 0xLLVEL Date: Mon, 19 Oct 2026 15:25:49 +0700 Subject: [PATCH] fix(uninstall): keep owned-row proof through removal refusals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasOwnershipEvidence treated any clientRegistrationRemoval refusal as no ownership: an effective REA-owned rea member shadowed by an earlier foreign duplicate (shadowed_unowned_member), or an owned native row beside a foreign V1 fallback (foreign_legacy_fallback), discarded the already-parsed positive row fact. With FreeBuff unverified and close failures recurring on the preflight/removal rereads, those failures were then downgraded to retained, so runUninstall(true) removed the skill and purged REA data while the active owned row remained. The refusal now carries the owned rows that motivated it — the refusal only refuses the edit — and hasOwnershipEvidence evaluates that fact on both paths; preflight and removal keep refusing the unsafe edit exactly as before. Covers the Codex P2 on Uninstall.ts:269. Validation: the new regression fails on 225cb214a (freebuff reports no blocking failure while the poisoned-scope error lands on claude_code) and passes after; setupClientConfigCleanupOwnership 26/26; the 21 Uninstall-consumer suites are byte-identical to the recorded 225cb214a baseline (61 pre-existing Windows-host failures: symlink EPERM, stale-vs-aligned); tsc --noEmit, oxlint (0 errors, same 6 warnings) and oxfmt --check clean. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- src/application/Uninstall.ts | 21 +++++--- .../setupClientConfigCleanupOwnership.test.ts | 48 +++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index dc9f2df84b..700900d16e 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -266,9 +266,11 @@ const hasOwnershipEvidence = async ( continue; } const removal = clientRegistrationRemoval(parsed, original, client); - if (!removal.ok) continue; - if (removal.value.removeNative || removal.value.legacyPath !== undefined) - return true; + // Refusing the edit never retracts the owned-row fact: an effective + // REA-owned row shadowed by a foreign duplicate still proves ownership, + // so later read failures stay blocking instead of softening to retained. + const owned = removal.ok ? removal.value : removal.error.owned; + if (owned.removeNative || owned.legacyPath !== undefined) return true; } return false; }; @@ -551,6 +553,8 @@ type RegistrationRemoval = { type RegistrationRemovalRefusal = { readonly kind: "shadowed_unowned_member" | "foreign_legacy_fallback"; readonly detail: string; + /** The owned rows that motivated removal; refusing the edit keeps them. */ + readonly owned: RegistrationRemoval; }; const clientRegistrationRemoval = ( @@ -572,6 +576,10 @@ const clientRegistrationRemoval = ( const removeLegacy = legacyPath !== undefined && isOwnedRegistration(parsed.legacyServers[name], client); + const ownership: RegistrationRemoval = { + removeNative, + legacyPath: removeLegacy ? legacyPath : undefined, + }; if (removeNative || removeLegacy) { const shadowed = shadowedClientServers(original, parsed, name); if ( @@ -587,17 +595,16 @@ const clientRegistrationRemoval = ( return err({ kind: "shadowed_unowned_member", detail: SHADOWED_UNOWNED_MEMBER_DETAIL, + owned: ownership, }); } if (removeNative && legacyPath !== undefined && !removeLegacy) return err({ kind: "foreign_legacy_fallback", detail: UNOWNED_LEGACY_FALLBACK_DETAIL, + owned: ownership, }); - return ok({ - removeNative, - legacyPath: removeLegacy ? legacyPath : undefined, - }); + return ok(ownership); }; 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.`; diff --git a/tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts b/tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts index 2702cf8631..3713d779d9 100644 --- a/tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts +++ b/tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts @@ -225,6 +225,54 @@ it("keeps shared-ownership proof when the ownership read completes but its close } }); +it("keeps shared-ownership proof when removal refuses a shadowed duplicate", async () => { + // The effective row is REA-owned while an earlier duplicate is foreign, so + // removal refuses the edit. That refusal must not erase ownership: without + // the positive row fact, recurring close failures soften to retained and + // purge runs while the owned row remains active. + const home = await createTestTempDirectory("rea-freebuff-shadowed-close-"); + const setupHost = systemSetupHost( + { platform: "linux", homeDirectory: home } as DoctorHost, + { HOME: home }, + ); + const client = (await setupHost.supportedClients()).find( + ({ name }) => name === "freebuff", + ); + if (client === undefined) throw new Error("freebuff client missing"); + await mkdir(dirname(client.configPath), { recursive: true }); + await writeFile( + client.configPath, + '{"mcpServers":{"rea":{"command":"foreign-server","args":[]},"rea":{"command":"rea","args":["mcp"]}}}', + ); + const state = join(home, ".rea", "state.json"); + await mkdir(dirname(state), { recursive: true }); + await writeFile(state, "{}\n"); + const { release } = holdReadHandle( + client.configPath, + "held ownership read handle", + ); + const host = systemUninstallHost(home, undefined, { HOME: home }, "linux"); + try { + const result = await runUninstall(true, host); + expect(result.status).toBe("failed"); + expect(result.items).toContainEqual( + expect.objectContaining({ name: "freebuff", status: "failed" }), + ); + expect(result.items).toContainEqual( + expect.objectContaining({ + name: "uninstall", + status: "skipped", + detail: expect.stringContaining( + "No REA registration, skill, or data was removed", + ), + }), + ); + await expect(readFile(state, "utf8")).resolves.toBe("{}\n"); + } finally { + await cleanupHeldHandle(host, release); + } +}); + describe("ZCode fallback cleanup ownership", () => { it.each(["preflight", "configure"])( "retains a failed ZCode fallback close during %s until host cleanup",