fix(setup): stop remaining uninstall removals after a client fails
A configuration that passes the preflight can still fail when uninstall rereads it to edit, for example when it changes concurrently. Rereading keeps that edit from overwriting the newer file; stopping at the failure keeps later clients, the skill, and REA data from being removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
5cc841d5c1
commit
fd975c75fa
|
|
@ -574,7 +574,8 @@ rea uninstall --purge-data
|
||||||
Uninstall preserves Hopper, Node.js, Evidence files, captures, unrelated skills
|
Uninstall preserves Hopper, Node.js, Evidence files, captures, unrelated skills
|
||||||
and other MCP servers. Purging removes only REA's cache and state under
|
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
|
`~/.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
|
symbolic link is retained and reported rather than followed. See the
|
||||||
[CLI guide](cli.md#output-and-exit-status) for exit statuses.
|
[CLI guide](cli.md#output-and-exit-status) for exit statuses.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -91,7 +91,8 @@ export interface UninstallHost {
|
||||||
/**
|
/**
|
||||||
* Remove only REA-owned registrations and managed files, optionally including
|
* Remove only REA-owned registrations and managed files, optionally including
|
||||||
* local state. A client configuration that cannot be read safely stops the
|
* 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 (
|
export const runUninstall = async (
|
||||||
purgeData: boolean,
|
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 (
|
const removeAll = async (
|
||||||
clients: readonly SetupClient[],
|
clients: readonly SetupClient[],
|
||||||
purgeData: boolean,
|
purgeData: boolean,
|
||||||
host: UninstallHost,
|
host: UninstallHost,
|
||||||
): Promise<UninstallItem[]> => {
|
): Promise<UninstallItem[]> => {
|
||||||
const items: UninstallItem[] = [];
|
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());
|
items.push(await host.removeSkill());
|
||||||
if (purgeData) items.push(...(await host.purgeData()));
|
if (purgeData) items.push(...(await host.purgeData()));
|
||||||
return items;
|
return items;
|
||||||
|
|
|
||||||
|
|
@ -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", () => {
|
describe("client configuration filesystem failure reporting", () => {
|
||||||
it.each([
|
it.each([
|
||||||
[
|
[
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue