fix(setup): stop uninstall before removals when a client config is unsafe

The installation guide says malformed client configuration stops
`rea uninstall`, but uninstall reported that client as failed and still
removed other clients' registrations, the bundled skill, and with
--purge-data REA's cache and state.

Inspect every client configuration first, using the conditions setup
already treats as blocking (unsafe path, unreadable file, malformed
document), and stop before any removal when one fails. Purge paths that are
symbolic links are still retained and reported per item, as the guide now
states.

Fixes #1164

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kaoru0822 2026-10-09 03:07:40 +00:00
parent de5bfec79f
commit 5cc841d5c1
4 changed files with 148 additions and 33 deletions

View File

@ -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

View File

@ -81,21 +81,39 @@ export const isUninstallFailure = (result: UninstallResult): boolean => {
/** Filesystem boundary used by the contained uninstall workflow. */
export interface UninstallHost {
clients(): Promise<readonly SetupClient[]>;
/** Report a configuration that cannot be read safely, before any removal. */
inspectClient(client: SetupClient): Promise<UninstallItem | undefined>;
removeClient(client: SetupClient): Promise<UninstallItem>;
removeSkill(): Promise<UninstallItem>;
purgeData(): Promise<readonly UninstallItem[]>;
}
/** 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<UninstallResult> => {
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<UninstallItem[]> => {
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<UninstallItem> => {
): Promise<ClientConfigurationRead> => {
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<UninstallItem> => {
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);

View File

@ -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: () =>

View File

@ -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", () => {