diff --git a/docs/installation.md b/docs/installation.md index 628b6b600c..184778a6b3 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -376,10 +376,15 @@ 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, and a client that fails -while being updated stops the remaining removals. For JSON clients, uninstall -removes every repeated `rea` member in the effective server table only when -each registration is REA-owned. An unowned earlier member, or an unowned -OpenCode V1 entry that removing V2 would expose, stops preflight before any +while being updated stops the remaining removals. Uninstall reports each +candidate `configuration_path` with its outcome. A client with multiple files +keeps their individual `actions` alongside its aggregate status and detail, +including earlier removals when a later file fails. The paths name +the discovered configuration candidates, not resolved physical file identities. +For JSON clients, uninstall removes every repeated `rea` member in the effective +server table only when each registration is REA-owned. An unowned earlier +member, or an unowned OpenCode V1 entry that removing V2 would expose, stops +preflight before any removal. 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 70a705cc59..27f01419dd 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -100,11 +100,17 @@ const systemFileSystem = ( removeEmptyDirectory: (path) => rmdir(path), }); -/** One explicitly classified uninstall action. */ -export interface UninstallItem { - readonly name: string; +/** One completed or refused action, preserving its candidate configuration path. */ +export interface UninstallAction { readonly status: "removed" | "retained" | "skipped" | "failed"; readonly detail: string; + readonly configuration_path?: string; +} + +/** One client summary with its constituent actions when multiple paths apply. */ +export interface UninstallItem extends UninstallAction { + readonly name: string; + readonly actions?: readonly UninstallAction[]; } /** Structured, idempotent result of removing REA-owned state. */ @@ -143,7 +149,8 @@ export const runUninstall = async ( const blocked: UninstallItem[] = []; for (const client of clients) { const blocking = await host.inspectClient(client); - if (blocking !== undefined) blocked.push(blocking); + if (blocking !== undefined) + blocked.push({ ...blocking, configuration_path: client.configPath }); } const skillBlocker = await host.inspectSkill?.(); if (skillBlocker !== undefined) blocked.push(skillBlocker); @@ -211,7 +218,7 @@ const removeAll = async ( const items: UninstallItem[] = []; for (const client of clients) { const removed = await host.removeClient(client); - items.push(removed); + items.push({ ...removed, configuration_path: client.configPath }); if (removed.status === "failed") return [ ...items, @@ -1062,47 +1069,42 @@ const item = ( /** * Layered clients are removed file by file, but callers look up one status by - * client name. A missing earlier file must not hide a later removal. + * client name. Aggregate status does not invalidate any constituent outcome. */ const collapseUninstallItems = ( items: readonly UninstallItem[], ): UninstallItem[] => { - const collapsed: UninstallItem[] = []; + const grouped = new Map(); for (const next of items) { - const index = collapsed.findIndex(({ name }) => name === next.name); - const current = collapsed[index]; - if (current === undefined) { - collapsed.push(next); - continue; - } - collapsed[index] = mergeUninstallItems(current, next); + const { name, actions, ...action } = next; + const group = grouped.get(name) ?? []; + group.push( + ...(actions !== undefined && actions.length > 0 ? actions : [action]), + ); + grouped.set(name, group); } - return collapsed; + return [...grouped].map(([name, actions]) => { + const first = actions[0]; + if (first === undefined) + throw new Error("Uninstall outcome group has no actions."); + if (actions.length === 1) return { name, ...first }; + const status = actions.reduce( + (selected, action) => + uninstallStatusPriority(action.status) > + uninstallStatusPriority(selected) + ? action.status + : selected, + first.status, + ); + return { + name, + status, + detail: [...new Set(actions.map(({ detail }) => detail))].join(" "), + actions, + }; + }); }; -const mergeUninstallItems = ( - current: UninstallItem, - next: UninstallItem, -): UninstallItem => { - const status = preferredUninstallStatus(current.status, next.status); - const details = [current, next] - .filter( - (entry) => - entry.status === status || - (status === "removed" && entry.status === "retained"), - ) - .map(({ detail }) => detail); - return item(current.name, status, [...new Set(details)].join(" ")); -}; - -const preferredUninstallStatus = ( - current: UninstallItem["status"], - next: UninstallItem["status"], -): UninstallItem["status"] => - uninstallStatusPriority(next) > uninstallStatusPriority(current) - ? next - : current; - const uninstallStatusPriority = (status: UninstallItem["status"]): number => { switch (status) { case "failed": diff --git a/tests/boundary/cli/clientProfileLifecycle.test.ts b/tests/boundary/cli/clientProfileLifecycle.test.ts index e2bca171f2..4f684e7a8f 100644 --- a/tests/boundary/cli/clientProfileLifecycle.test.ts +++ b/tests/boundary/cli/clientProfileLifecycle.test.ts @@ -4,6 +4,7 @@ import { mkdir, readFile, realpath, + rm, symlink, writeFile, } from "node:fs/promises"; @@ -13,6 +14,89 @@ import { PRODUCT_IDENTITY } from "../../../src/identity.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; +cliTest( + "uninstall reports completed file removals beside a later OpenCode failure and recovers", + async ({ cli }) => { + const home = await createTestTempDirectory( + "rea-uninstall-layered-failure-", + ); + const directory = join(home, "opencode"); + await mkdir(directory, { recursive: true }); + const first = join(directory, "opencode.json"); + const second = join(directory, "opencode.jsonc"); + const original = JSON.stringify({ + mcp: { + rea: { + type: "local", + command: ["npx", "-y", "rea-agents@6.7.0", "mcp"], + }, + }, + unrelated: "keep", + }); + await writeFile(first, original); + await writeFile(second, original); + await mkdir(`${second}.rea.backup`); + const run = () => + cli.run({ + arguments: ["uninstall", "--json"], + cwd: home, + environment: { + HOME: home, + USERPROFILE: home, + OPENCODE_CONFIG_DIR: directory, + }, + }); + + const failed = await run(); + expect(failed.exitCode, failed.stderr).toBe(1); + expect(failed.json).toMatchObject({ + status: "failed", + items: expect.arrayContaining([ + expect.objectContaining({ + name: "opencode", + status: "failed", + detail: expect.stringContaining("Removed registration"), + actions: expect.arrayContaining([ + expect.objectContaining({ + configuration_path: first, + status: "removed", + }), + expect.objectContaining({ + configuration_path: second, + status: "failed", + }), + ]), + }), + ]), + }); + expect(failed.json).toMatchObject({ + items: expect.arrayContaining([ + expect.objectContaining({ + name: "opencode", + detail: expect.stringContaining("could not be backed up"), + }), + ]), + }); + expect(JSON.parse(await readFile(first, "utf8"))).toEqual({ + mcp: {}, + unrelated: "keep", + }); + expect(await readFile(`${first}.rea.backup`, "utf8")).toBe(original); + expect(await readFile(second, "utf8")).toBe(original); + + await rm(`${second}.rea.backup`, { recursive: true }); + const recovered = await run(); + expect(recovered.exitCode, recovered.stderr).toBe(0); + expect(recovered.json).toMatchObject({ status: "complete" }); + expect(JSON.parse(await readFile(second, "utf8"))).toEqual({ + mcp: {}, + unrelated: "keep", + }); + expect(await readFile(`${first}.rea.backup`, "utf8")).toBe(original); + expect(await readFile(`${second}.rea.backup`, "utf8")).toBe(original); + }, +); + const cases = [ { client: "gemini_cli", diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index dd229f9c83..850159418d 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -1642,7 +1642,9 @@ describe("client configuration filesystem removal", () => { } }, ); +}); +describe("uninstall configuration symlink ownership", () => { it("removes an owned entry through a symlink without replacing it", async () => { const home = await createTestTempDirectory("rea-uninstall-symlink-"); roots.push(home); @@ -1658,8 +1660,14 @@ describe("client configuration filesystem removal", () => { await writeFile(targetPath, original); await symlink(targetPath, configPath); - expect((await runUninstall(false, systemUninstallHost(home))).status).toBe( - "complete", + const result = await runUninstall(false, systemUninstallHost(home)); + expect(result.status).toBe("complete"); + expect(result.items).toContainEqual( + expect.objectContaining({ + name: "cursor", + status: "removed", + configuration_path: configPath, + }), ); expect((await lstat(configPath)).isSymbolicLink()).toBe(true); expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original); @@ -1680,6 +1688,7 @@ describe("client configuration filesystem removal", () => { expect(result.items).toContainEqual({ name: "cursor", status: "failed", + configuration_path: configPath, detail: "Configuration path could not be safely verified. Check its permissions and, if it is a symbolic link, verify that the link resolves to a regular file owned by the current user, then rerun uninstall.", }); @@ -1789,6 +1798,85 @@ describe("uninstall registration ownership preflight", () => { }); }); +describe("layered uninstall outcomes", () => { + it("reports an earlier layered OpenCode removal when a later backup is blocked", async () => { + const home = await createTestTempDirectory( + "rea-uninstall-opencode-layers-", + ); + roots.push(home); + const directory = join(home, ".config/opencode"); + const json = join(directory, "opencode.json"); + const jsonc = join(directory, "opencode.jsonc"); + const blocker = `${jsonc}.rea.backup`; + await mkdir(directory, { recursive: true }); + const registration = { + type: "local", + command: [PRODUCT_IDENTITY.cliBinary, "mcp"], + enabled: true, + }; + const jsonOriginal = JSON.stringify({ + mcp: { servers: { rea: registration } }, + }); + const jsoncOriginal = `// OpenCode's later JSONC layer\n${JSON.stringify({ + mcp: { servers: { rea: registration } }, + })}\n`; + await writeFile(json, jsonOriginal); + await writeFile(jsonc, jsoncOriginal); + await mkdir(blocker); + + const host = systemUninstallHost(home); + const blocked = await runUninstall(false, { + ...host, + close: async () => { + await host.close?.(); + return "Controlled final cleanup diagnostic."; + }, + }); + + expect(blocked.status).toBe("failed"); + expect(await readFile(json, "utf8")).not.toContain('"rea"'); + expect(await readFile(jsonc, "utf8")).toBe(jsoncOriginal); + const client = blocked.items.find(({ name }) => name === "opencode"); + expect(client?.status).toBe("failed"); + expect(client?.detail).toContain(`Removed registration from ${json}.`); + expect(client?.detail).toContain("Configuration could not be backed up"); + expect(client?.detail).toContain("Check file permissions"); + expect(client?.actions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + configuration_path: json, + status: "removed", + }), + expect.objectContaining({ + configuration_path: jsonc, + status: "failed", + }), + ]), + ); + expect( + client?.actions?.filter( + ({ configuration_path }) => configuration_path === json, + ), + ).toHaveLength(1); + expect(blocked.items).toContainEqual({ + name: "uninstall_cleanup", + status: "failed", + detail: "Controlled final cleanup diagnostic.", + }); + + await rm(blocker, { recursive: true }); + const recovered = await runUninstall(false, systemUninstallHost(home)); + + expect(recovered.status).toBe("complete"); + expect(recovered.items).toContainEqual( + expect.objectContaining({ name: "opencode", status: "removed" }), + ); + expect(parseJsonc(await readFile(jsonc, "utf8"))).toEqual({ + mcp: { servers: {} }, + }); + }); +}); + describe("uninstall configuration preflight", () => { it("fails closed on malformed client configuration", async () => { const home = await createTestTempDirectory("rea-uninstall-bad-"); @@ -1833,6 +1921,7 @@ describe("uninstall configuration preflight", () => { expect(result.items).toContainEqual({ name: "cursor", status: "failed", + configuration_path: config, detail: `Configuration was not changed: ${problem}. Repair it, then rerun uninstall.`, }); expect(await readFile(config, "utf8")).toBe(content); diff --git a/tests/boundary/filesystem/hermesClientConfiguration.test.ts b/tests/boundary/filesystem/hermesClientConfiguration.test.ts index 33c4369978..28fdb5e386 100644 --- a/tests/boundary/filesystem/hermesClientConfiguration.test.ts +++ b/tests/boundary/filesystem/hermesClientConfiguration.test.ts @@ -202,6 +202,7 @@ describe("Hermes YAML shape diagnostics", () => { expect(result.items).toContainEqual({ name: "hermes", status: "failed", + configuration_path: client.configPath, detail: `Configuration was not changed: ${problem}. Repair it, then rerun uninstall.`, }); expect(await readFile(client.configPath, "utf8")).toBe(original);