diff --git a/docs/installation.md b/docs/installation.md index 17c9be092..1bc9fa286 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -573,8 +573,10 @@ 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`; malformed client configuration or purge-path symlinks stop the -operation. See the [CLI guide](cli.md#output-and-exit-status) for exit statuses. +`~/.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 +symbolic link is retained and reported rather than followed. See the +[CLI guide](cli.md#output-and-exit-status) for exit statuses. ## MCP Registry diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index d0203ae2f..0f926d440 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -81,21 +81,39 @@ export const isUninstallFailure = (result: UninstallResult): boolean => { /** Filesystem boundary used by the contained uninstall workflow. */ export interface UninstallHost { clients(): Promise; + /** Report a configuration that cannot be read safely, before any removal. */ + inspectClient(client: SetupClient): Promise; removeClient(client: SetupClient): Promise; removeSkill(): Promise; purgeData(): Promise; } -/** Remove only REA-owned registrations and managed files, optionally including local state. */ +/** + * 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. + */ export const runUninstall = async ( purgeData: boolean, host: UninstallHost = systemUninstallHost(), ): Promise => { - const items: UninstallItem[] = []; - for (const client of await host.clients()) - items.push(await host.removeClient(client)); - items.push(await host.removeSkill()); - if (purgeData) items.push(...(await host.purgeData())); + const clients = await host.clients(); + const blocked: UninstallItem[] = []; + for (const client of clients) { + const blocking = await host.inspectClient(client); + if (blocking !== undefined) blocked.push(blocking); + } + const items: UninstallItem[] = + blocked.length > 0 + ? [ + ...blocked, + item( + "uninstall", + "skipped", + "No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall.", + ), + ] + : await removeAll(clients, purgeData, host); items.push({ name: "analysis_engine", status: "retained", @@ -109,12 +127,30 @@ export const runUninstall = async ( }; }; +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)); + items.push(await host.removeSkill()); + if (purgeData) items.push(...(await host.purgeData())); + return items; +}; + /** Create uninstall effects contained to detected client configs and REA-owned paths. */ export const systemUninstallHost = ( home = homedir(), fileSystem: UninstallFileSystem = systemFileSystem, ): UninstallHost => ({ clients: () => Promise.resolve(supportedClients(home)), + inspectClient: async (client) => { + const read = await readClientConfiguration(client, fileSystem); + return read.kind === "item" && read.item.status === "failed" + ? read.item + : undefined; + }, removeClient: (client) => removeClient(client, fileSystem), removeSkill: () => removeManagedSkills(home, fileSystem), purgeData: async () => [ @@ -123,43 +159,72 @@ export const systemUninstallHost = ( ], }); -const removeClient = async ( +type ClientConfigurationRead = + | { readonly kind: "item"; readonly item: UninstallItem } + | { + readonly kind: "configuration"; + readonly transactionPath: string; + readonly original: string; + readonly parsed: ClientConfigurationDocument; + }; + +const readClientConfiguration = async ( client: SetupClient, fileSystem: UninstallFileSystem, -): Promise => { +): Promise => { if (client.format === "unsupported") - return item( - client.name, - "skipped", - manualRegistrationRemediation(client.name) ?? - "This client has no documented local MCP configuration boundary.", + return itemRead( + item( + client.name, + "skipped", + manualRegistrationRemediation(client.name) ?? + "This client has no documented local MCP configuration boundary.", + ), ); const resolved = await resolveUninstallConfigPath(client, fileSystem); - if (typeof resolved !== "string") return resolved; + if (typeof resolved !== "string") return itemRead(resolved); const transactionPath = resolved; let original: string; try { original = await fileSystem.readText(transactionPath); } catch (cause: unknown) { - return isMissing(cause) - ? item(client.name, "skipped", "Configuration does not exist.") - : item( - client.name, - "failed", - "Configuration could not be read. Check file permissions, then rerun uninstall.", - ); - } - let parsed: ClientConfigurationDocument; - try { - parsed = parseClientConfiguration(original, client.format); - } catch (cause: unknown) { - void cause; - return item( - client.name, - "failed", - `Configuration is not valid ${client.format === "toml" || client.format === "grok" ? "TOML" : client.format === "opencode" ? "JSONC" : "JSON"} and was not changed. Repair it, then rerun uninstall.`, + return itemRead( + isMissing(cause) + ? item(client.name, "skipped", "Configuration does not exist.") + : item( + client.name, + "failed", + "Configuration could not be read. Check file permissions, then rerun uninstall.", + ), ); } + try { + const parsed = parseClientConfiguration(original, client.format); + return { kind: "configuration", transactionPath, original, parsed }; + } catch (cause: unknown) { + void cause; + return itemRead( + item( + client.name, + "failed", + `Configuration is not valid ${client.format === "toml" || client.format === "grok" ? "TOML" : client.format === "opencode" ? "JSONC" : "JSON"} and was not changed. Repair it, then rerun uninstall.`, + ), + ); + } +}; + +const itemRead = (result: UninstallItem): ClientConfigurationRead => ({ + kind: "item", + item: result, +}); + +const removeClient = async ( + client: SetupClient, + fileSystem: UninstallFileSystem, +): Promise => { + const read = await readClientConfiguration(client, fileSystem); + if (read.kind === "item") return read.item; + const { transactionPath, original, parsed } = read; const name = PRODUCT_IDENTITY.mcpServerKey; // Beside a native OpenCode V2 table, OpenCode also loads a V1 `mcp.rea`. const legacyPath = legacyClientServerPath(parsed, name); diff --git a/src/cliLogging.test.ts b/src/cliLogging.test.ts index e275519e6..dc0544109 100644 --- a/src/cliLogging.test.ts +++ b/src/cliLogging.test.ts @@ -145,6 +145,7 @@ describe("CLI command outcome classification", () => { it("returns nonzero when the real uninstall outcome contains a failed action", async () => { const host: UninstallHost = { clients: () => Promise.resolve([]), + inspectClient: () => Promise.resolve(undefined), removeClient: () => Promise.resolve({ name: "client", status: "failed", detail: "write" }), removeSkill: () => diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 8ea5a9945..2c66bd42d 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -1418,6 +1418,53 @@ describe("client configuration filesystem removal", () => { ); expect(await readFile(config, "utf8")).toBe("not-json"); }); + + it("stops before any removal when one client configuration is malformed", async () => { + const home = await createTestTempDirectory("rea-uninstall-stop-"); + roots.push(home); + const codex = join(home, ".codex/config.toml"); + const cursor = join(home, ".cursor/mcp.json"); + const managed = [ + join(home, ".agents/skills/reverse-engineer-anything/marker.txt"), + join(home, ".rea/cache/marker.txt"), + join(home, ".rea/state/marker.txt"), + ]; + const cursorConfig = JSON.stringify({ + mcpServers: { + rea: { command: "npx", args: ["-y", "rea-agents@6.1.0", "mcp"] }, + other: { command: "echo", args: ["hello"] }, + }, + }); + for (const path of [codex, cursor, ...managed]) + await mkdir(dirname(path), { recursive: true }); + await writeFile(codex, "[mcp_servers.rea]\ncommand = [broken\n"); + await writeFile(cursor, cursorConfig); + for (const path of managed) await writeFile(path, "marker"); + + const stopped = await runUninstall(true, systemUninstallHost(home)); + + expect(stopped.status).toBe("failed"); + expect(stopped.items.map(({ name, status }) => [name, status])).toEqual([ + ["codex", "failed"], + ["uninstall", "skipped"], + ["analysis_engine", "retained"], + ]); + expect(await readFile(codex, "utf8")).toBe( + "[mcp_servers.rea]\ncommand = [broken\n", + ); + expect(await readFile(cursor, "utf8")).toBe(cursorConfig); + for (const path of managed) + expect(await readFile(path, "utf8")).toBe("marker"); + + await writeFile(codex, ""); + const repaired = await runUninstall(true, systemUninstallHost(home)); + expect(repaired.status).toBe("complete"); + expect(JSON.parse(await readFile(cursor, "utf8"))).toEqual({ + mcpServers: { other: { command: "echo", args: ["hello"] } }, + }); + for (const path of managed) + await expect(readFile(path, "utf8")).rejects.toThrow(); + }); }); describe("client configuration filesystem failure reporting", () => {