From 030fa886f94e894e34ed2539e9d8dedff19c3327 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 27 Oct 2026 06:43:26 +0800 Subject: [PATCH] fix(setup): verify published configuration content during uninstall Require publication readback in the owning check so in-place edits are detected and preserved with the original backup. --- .../ClientConfigurationPublication.ts | 4 +- src/application/Uninstall.ts | 8 ++-- .../clientConfigurationTransactions.test.ts | 2 +- .../setupConfigurationRollback.test.ts | 38 ++++++++++++++++++- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/application/ClientConfigurationPublication.ts b/src/application/ClientConfigurationPublication.ts index 1fb69059d9..960945bc19 100644 --- a/src/application/ClientConfigurationPublication.ts +++ b/src/application/ClientConfigurationPublication.ts @@ -169,12 +169,12 @@ export class ClientConfigurationPublication { return outcome.value; } - async assertCurrent(readback?: string): Promise { + async assertCurrent(readback: string): Promise { const owned = await this.owner.handle.stat(); const named = await lstat(this.path); if ( owned.ino === 0 || - (readback !== undefined && readback !== this.contents) || + readback !== this.contents || named.isSymbolicLink() || !sameRegularFileState(owned, named) ) diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index 5e10a9e2e0..60e2bc451b 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -963,11 +963,9 @@ const removeClient = async ( try { return await publication.verify(async () => { try { - const readback = parseClientConfiguration( - await fileSystem.readText(publication.path), - client.format, - ); - await publication.assertCurrent(); + const text = await fileSystem.readText(publication.path); + await publication.assertCurrent(text); + const readback = parseClientConfiguration(text, client.format); if ( (removeNative && Object.hasOwn(readback.servers, name)) || (readback.dialect === "dsh" && diff --git a/tests/boundary/filesystem/clientConfigurationTransactions.test.ts b/tests/boundary/filesystem/clientConfigurationTransactions.test.ts index 6802427ada..4e9679596b 100644 --- a/tests/boundary/filesystem/clientConfigurationTransactions.test.ts +++ b/tests/boundary/filesystem/clientConfigurationTransactions.test.ts @@ -55,7 +55,7 @@ it { requestedPath, state }, ); await publication.verify(async () => { - await publication.assertCurrent(); + await publication.assertCurrent(await readFile(publication.path, "utf8")); expect(await readFile(selected, "utf8")).toBe("updated"); expect(await readFile(join(second, "config.json"), "utf8")).toBe( "original", diff --git a/tests/process-global/setupConfigurationRollback.test.ts b/tests/process-global/setupConfigurationRollback.test.ts index 36fc48f259..2b0ee13a47 100644 --- a/tests/process-global/setupConfigurationRollback.test.ts +++ b/tests/process-global/setupConfigurationRollback.test.ts @@ -629,6 +629,40 @@ it.each(["symlink", "regular", "valid-readback"] as const)( }, ); +it("uninstall retains concurrent edits to the published inode and reports failure", async () => { + const root = await createTestTempDirectory("rea-uninstall-inode-edit-"); + const path = join(root, "client.json"); + const fs = + await vi.importActual( + "node:fs/promises", + ); + const original = + '{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}},"keep":"original"}\n'; + const edited = '{"keep":"concurrent edit"}\n'; + await fs.writeFile(path, original); + state.failReadback = false; + state.afterPublication = async (publishedPath) => { + state.afterPublication = undefined; + const before = await fs.stat(publishedPath); + await fs.writeFile(publishedPath, edited); + expect((await fs.stat(publishedPath)).ino).toBe(before.ino); + }; + const host = systemUninstallHost(root); + try { + const result = await host.removeClient({ + name: "cursor", + configPath: path, + format: "json", + }); + expect(result.status).toBe("failed"); + expect(result.detail).not.toContain("The original was restored"); + expect(await fs.readFile(path, "utf8")).toBe(edited); + expect(await fs.readFile(`${path}.rea.backup`, "utf8")).toBe(original); + } finally { + await host.close?.(); + } +}); + it("uninstall retains both readback and publication owners until cleanup retry", async () => { const root = await createTestTempDirectory("rea-uninstall-two-close-owners-"); const path = join(root, "client.json"); @@ -1394,7 +1428,9 @@ it("publication keeps its selected canonical parent when a directory symlink is "{}", original, ); - await publication.verify(() => publication.assertCurrent()); + await publication.verify(async () => + publication.assertCurrent(await fs.readFile(publication.path, "utf8")), + ); expect(await fs.readFile(join(selected, "client.json"), "utf8")).toBe("{}"); expect(await fs.readFile(join(foreign, "client.json"), "utf8")).toBe( unrelated,