mirror of
https://github.com/morluto/rea.git
synced 2026-10-18 20:10:13 -05:00
fix(uninstall): retain per-file partial outcomes
Preserve configuration paths and constituent actions independently of aggregate status. Derive summaries from all leaf outcomes, including after final cleanup reporting, so a later failure cannot hide an earlier removal. Closes #5079
This commit is contained in:
1 parent
a80575c736
commit
559af3d3ff
5 files changed
+225
-44
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<string, UninstallAction[]>();
|
||||
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":
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user