diff --git a/docs/installation.md b/docs/installation.md index 1bc9fa286..2028ea38d 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -574,7 +574,8 @@ rea uninstall --purge-data Uninstall preserves Hopper, Node.js, Evidence files, captures, unrelated skills and other MCP servers. Purging removes only REA's cache and state under `~/.rea`. A client configuration that is malformed, unreadable, or at an unsafe -path stops the operation before anything is removed; a purge path that is a +path stops the operation before anything is removed, and a client that fails +while being updated stops the remaining removals. A purge path that is a symbolic link is retained and reported rather than followed. See the [CLI guide](cli.md#output-and-exit-status) for exit statuses. diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index 0f926d440..789bdc34e 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -91,7 +91,8 @@ export interface UninstallHost { /** * Remove only REA-owned registrations and managed files, optionally including * local state. A client configuration that cannot be read safely stops the - * operation before anything is removed. + * operation before anything is removed; one that fails during removal stops + * the remaining removals. */ export const runUninstall = async ( purgeData: boolean, @@ -127,13 +128,29 @@ export const runUninstall = async ( }; }; +/** + * Each client is reread when it is edited, so a concurrent change is never + * overwritten. A client that fails then still stops the remaining removals. + */ const removeAll = async ( clients: readonly SetupClient[], purgeData: boolean, host: UninstallHost, ): Promise => { const items: UninstallItem[] = []; - for (const client of clients) items.push(await host.removeClient(client)); + for (const client of clients) { + const removed = await host.removeClient(client); + items.push(removed); + if (removed.status === "failed") + return [ + ...items, + item( + "uninstall", + "skipped", + "Uninstall stopped at this failure; the items above report what changed. Repair the failed configuration, then rerun uninstall.", + ), + ]; + } items.push(await host.removeSkill()); if (purgeData) items.push(...(await host.purgeData())); return items; diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 2c66bd42d..bf277a24f 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -1467,6 +1467,65 @@ describe("client configuration filesystem removal", () => { }); }); +describe("client configuration changed during uninstall", () => { + it("stops the remaining removals when a configuration fails after preflight", async () => { + const home = await createTestTempDirectory("rea-uninstall-race-"); + roots.push(home); + const codex = join(home, ".codex/config.toml"); + const cursor = join(home, ".cursor/mcp.json"); + const skill = join( + home, + ".agents/skills/reverse-engineer-anything/SKILL.md", + ); + const cursorConfig = JSON.stringify({ + mcpServers: { rea: { command: "rea", args: ["mcp"] } }, + }); + for (const path of [codex, cursor, skill]) + await mkdir(dirname(path), { recursive: true }); + await writeFile( + codex, + '[mcp_servers.rea]\ncommand = "rea"\nargs = ["mcp"]\n', + ); + await writeFile(cursor, cursorConfig); + await writeFile(skill, "managed"); + let codexReads = 0; + const fileSystem: UninstallFileSystem = { + ...testFileSystem, + // The preflight read succeeds; the removal reread finds a broken file. + readText: (path) => { + if (path !== codex) return readFile(path, "utf8"); + codexReads += 1; + return codexReads === 1 + ? readFile(path, "utf8") + : Promise.resolve("[mcp_servers.rea]\ncommand = [broken\n"); + }, + }; + + const result = await runUninstall( + false, + systemUninstallHost(home, fileSystem), + ); + + expect(result.status).toBe("failed"); + // Clients ordered before Codex have no configuration in this home. + expect( + result.items + .filter(({ status }) => status !== "skipped") + .map(({ name, status }) => [name, status]), + ).toEqual([ + ["codex", "failed"], + ["analysis_engine", "retained"], + ]); + expect(result.items.map(({ name }) => name)).not.toContain("cursor"); + expect(result.items.at(-2)).toMatchObject({ + name: "uninstall", + status: "skipped", + }); + expect(await readFile(cursor, "utf8")).toBe(cursorConfig); + expect(await readFile(skill, "utf8")).toBe("managed"); + }); +}); + describe("client configuration filesystem failure reporting", () => { it.each([ [