morluto/rea
1
0
Fork 0
mirror of https://github.com/morluto/rea.git synced 2026-10-12 02:19:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(setup): preserve initial backups and Codex comments (#1204)

Browse Source
This commit is contained in:
Ysopwnable
2026-10-09 12:57:00 +08:00
committed by GitHub
parent db675f1d52
commit 6051cb2c0c
6 changed files with 221 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/installation.md
+3 -2
View File
@@ -195,8 +195,9 @@ Before applying changes, REA checks your current configuration. The plan lists:
Malformed or unsafe existing configuration blocks the whole transaction before
Hopper installation or any file write. Declining or pressing Ctrl-C makes no
changes. Agent configuration writes preserve unrelated
entries, create backups, use atomic replacement, and verify their result.
changes. Agent configuration writes preserve unrelated entries and comments,
create backups, use atomic replacement, and verify their result. Setup and
uninstall retain an existing `.rea.backup` rather than replacing the first snapshot.
After setup, REA reports which agents, analysis tools, and workflow files passed
its final checks. Restart any agent named in the completion message, then begin
scripts/verify-package-setup.mjs
+17 -10
View File
@@ -31,6 +31,14 @@ const OPENCODE_ORIGINAL = `{
},
}\n`;
const CODEX_ORIGINAL = [
"# Keep Codex preferences and their spelling.",
'model = "gpt-5"',
'notify = ["a", "b"]',
"literal = 'C:\\demo\\path'",
"",
].join("\n");
const verifyMcpAdd = async ({ cli, environment, npxLog }) => {
await run(cli, ["mcp", "add"], environment);
const mcpRegistration = await readFile(npxLog, "utf8");
@@ -248,7 +256,7 @@ const verifySetupPlan = async ({
!planned.plannedActions.some(({ kind }) => kind === "configure_client") ||
!planned.plannedActions.some(({ kind }) => kind === "install_skill") ||
plannedClaudeConfig !== '{"existing":true}\n' ||
plannedCodexConfig !== 'model = "gpt-5"\n' ||
plannedCodexConfig !== CODEX_ORIGINAL ||
plannedCursorConfig !== '{"existing":true}\n' ||
plannedOpenCodeConfig !== opencodeOriginal
)
@@ -459,8 +467,8 @@ const assertCodexSymlink = async ({
!codex.includes("[mcp_servers.rea]") ||
!codex.includes(`command = "${cli}"`) ||
!codex.includes("startup_timeout_sec = 30") ||
(await readFile(`${codexConfig}.rea.backup`, "utf8")) !==
'model = "gpt-5"\n' ||
!codex.startsWith(CODEX_ORIGINAL) ||
(await readFile(`${codexConfig}.rea.backup`, "utf8")) !== CODEX_ORIGINAL ||
(await readFile(`${cursorConfig}.rea.backup`, "utf8")) !==
'{"existing":true}\n'
)
@@ -631,7 +639,6 @@ const verifyUninstall = async ({
codexTarget,
opencodeConfig,
}) => {
const openCodeBeforeUninstall = await readFile(opencodeConfig, "utf8");
const uninstallExecution = await runWithStatus(
cli,
["uninstall", "--json"],
@@ -657,7 +664,7 @@ const verifyUninstall = async ({
!(await lstat(codexConfig)).isSymbolicLink() ||
cursorAfterUninstall.existing !== true ||
cursorAfterUninstall.mcpServers?.rea !== undefined ||
!codexAfterUninstall.includes('model = "gpt-5"') ||
codexAfterUninstall !== CODEX_ORIGINAL ||
codexAfterUninstall.includes("mcp_servers.rea") ||
openCodeErrors.length !== 0 ||
openCodeAfterUninstall?.model !== "provider/model" ||
@@ -670,11 +677,10 @@ const verifyUninstall = async ({
"Keep this independently managed server",
) ||
(await readFile(`${opencodeConfig}.rea.backup`, "utf8")) !==
openCodeBeforeUninstall ||
!(await readFile(`${cursorConfig}.rea.backup`, "utf8")).includes('"rea"') ||
!(await readFile(`${codexConfig}.rea.backup`, "utf8")).includes(
"[mcp_servers.rea]",
)
OPENCODE_ORIGINAL ||
(await readFile(`${cursorConfig}.rea.backup`, "utf8")) !==
'{"existing":true}\n' ||
(await readFile(`${codexConfig}.rea.backup`, "utf8")) !== CODEX_ORIGINAL
)
throw new Error(
`packaged uninstall did not preserve config symlinks: ${JSON.stringify(uninstall)}`,
@@ -704,6 +710,7 @@ export async function verifyPackageSetup({
);
const siblingSkillPath = join(home, ".agents/skills/unrelated/SKILL.md");
if (supportedSetupHost) {
await writeFile(codexTarget, CODEX_ORIGINAL);
await mkdir(join(home, ".agents/skills/unrelated"), { recursive: true });
await writeFile(siblingSkillPath, "unrelated skill\n");
}
src/application/ClientConfigurationDocument.ts
+29 -25
View File
@@ -832,15 +832,15 @@ const disabledServerListLines = (
/**
* Replace REA's TOML server tables and assignments. Comments that introduce a
* following table stay. Text inside strings is not treated as a header. Only
* Grok Build reads the root `disabled_mcp_servers` list that registration
* also edits.
* following table stay. Codex also retains comments around REA's own table.
* Text inside strings is not treated as a header. Only Grok Build reads the
* root `disabled_mcp_servers` list that registration also edits.
*/
const upsertTomlServerSection = (
originalText: string,
serverKey: string,
entry: unknown,
editDisabledList: boolean,
format: "toml" | "grok",
): string => {
// smol-toml accepts a leading BOM. Leave it in place or the first header is missed.
const bom = originalText.startsWith("\uFEFF");
@@ -917,24 +917,25 @@ const upsertTomlServerSection = (
mode = "none";
continue;
}
const disabledLines = editDisabledList
? disabledServerListLines(
disabledServerListEdit({
text,
lineStart,
valueOffset,
valueEnd,
statementEnd,
tablePath,
segments: assignment.segments,
serverKey,
entry,
}),
lines,
lineIndex,
endLine,
)
: undefined;
const disabledLines =
format === "grok"
? disabledServerListLines(
disabledServerListEdit({
text,
lineStart,
valueOffset,
valueEnd,
statementEnd,
tablePath,
segments: assignment.segments,
serverKey,
entry,
}),
lines,
lineIndex,
endLine,
)
: undefined;
if (disabledLines !== undefined) {
flushPending();
kept.push(...disabledLines);
@@ -975,7 +976,10 @@ const upsertTomlServerSection = (
continue;
}
if (isBlankOrComment(line)) {
pending.push(physical);
if (format === "toml" && line.trimStart().startsWith("#")) {
flushPending();
kept.push(physical);
} else pending.push(physical);
lineIndex += 1;
continue;
}
@@ -1044,7 +1048,7 @@ const serializeGrokConfiguration = (
originalText,
PRODUCT_IDENTITY.mcpServerKey,
entry,
true,
"grok",
);
};
@@ -1077,7 +1081,7 @@ const serializeCodexConfiguration = (
originalText,
PRODUCT_IDENTITY.mcpServerKey,
tomlServerEntry(document, PRODUCT_IDENTITY.mcpServerKey),
false,
"toml",
);
// Removing a final table also drops the file's last line break.
const edited =
src/application/Uninstall.ts
+10 -7
View File
@@ -7,6 +7,7 @@ import {
type ClientConfigurationDocument,
} from "./ClientConfigurationDocument.js";
import { copyFile, lstat, readFile, realpath, rm } from "node:fs/promises";
import { constants as fsConstants } from "node:fs";
import { homedir } from "node:os";
import { join } from "node:path";
@@ -31,6 +32,7 @@ interface ManagedPathStats {
/** Injectable filesystem operations used to prove uninstall failure recovery. */
export interface UninstallFileSystem {
readText(path: string): Promise<string>;
/** Preserve the first backup; reject an existing destination with EEXIST. */
copy(source: string, destination: string): Promise<void>;
writeText(path: string, contents: string): Promise<void>;
stat(path: string): Promise<ManagedPathStats>;
@@ -40,7 +42,8 @@ export interface UninstallFileSystem {
const systemFileSystem: UninstallFileSystem = {
readText: (path) => readFile(path, "utf8"),
copy: (source, destination) => copyFile(source, destination),
copy: (source, destination) =>
copyFile(source, destination, fsConstants.COPYFILE_EXCL),
writeText: (path, contents) =>
writeFileAtomic(path, contents, { encoding: "utf8" }),
stat: (path) => lstat(path),
@@ -274,12 +277,12 @@ const removeClient = async (
try {
await fileSystem.copy(transactionPath, backupPath);
} catch (cause: unknown) {
void cause;
return item(
client.name,
"failed",
"Configuration could not be backed up, so no change was made. Check file permissions, then rerun uninstall.",
);
if (!(cause instanceof Error && "code" in cause && cause.code === "EEXIST"))
return item(
client.name,
"failed",
"Configuration could not be backed up, so no change was made. Check file permissions, then rerun uninstall.",
);
}
try {
await fileSystem.writeText(
tests/boundary/filesystem/clientConfigurationTransactions.test.ts
+139
View File
@@ -2,8 +2,10 @@ import { chmod, readFile, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { describe, expect, it } from "vitest";
import { parse as parseToml } from "smol-toml";
import { configureClientConfiguration } from "../../../src/application/SetupClientConfiguration.js";
import { systemUninstallHost } from "../../../src/application/Uninstall.js";
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
describe("client configuration write failures", () => {
@@ -55,3 +57,140 @@ describe("TOML client configuration comparison", () => {
});
});
});
const unrelatedToml = [
"# Keep this explanation.",
'notify = ["a", "b"]',
"literal = 'C:\\demo\\path'",
'disabled_mcp_servers = ["rea", "other"] # Not a Codex registration.',
"",
].join("\n");
const unrelatedServer = [
"# Keep the other server.",
'[mcp_servers."rea.env"]',
"command = 'other'",
"",
].join("\n");
const unrelatedValues = (text: string) => {
const document = parseToml(text);
const servers = document.mcp_servers;
if (typeof servers === "object" && servers !== null) {
Reflect.deleteProperty(servers, "rea");
if (Object.keys(servers).length === 0) delete document.mcp_servers;
}
return document;
};
describe("Codex configuration text preservation", () => {
it.each([
{
name: "new registration",
original: unrelatedToml,
preserved: [unrelatedToml],
},
{
name: "quoted tables and nested environment",
original: `${unrelatedToml}[mcp_servers."rea"]\ncommand = "old"\n[mcp_servers.rea.env]\nOLD = "value"\n${unrelatedServer}`,
preserved: [unrelatedToml, unrelatedServer],
},
{
name: "dotted registration keys",
original: `${unrelatedToml}mcp_servers.rea.command = "old"\n${unrelatedServer}`,
preserved: [unrelatedToml, unrelatedServer],
},
{
name: "inline server table",
original: `${unrelatedToml}mcp_servers = { rea = { command = "old" }, other = { command = 'other' } } # Shared table.\n`,
preserved: [
unrelatedToml,
"other = { command = 'other' }",
"# Shared table.",
],
},
{
name: "BOM, CRLF, and multiline text resembling a registration",
original:
`\uFEFF${unrelatedToml}banner = '''\n[mcp_servers.rea]\n# This is string content.\n'''\n`.replaceAll(
"\n",
"\r\n",
),
preserved: [
unrelatedToml.replaceAll("\n", "\r\n"),
"banner = '''\r\n[mcp_servers.rea]\r\n# This is string content.\r\n'''",
],
},
{
name: "comments without settings",
original: "# Keep this explanation.\n# And this trailing comment.\n\n",
preserved: ["# Keep this explanation.", "# And this trailing comment."],
},
])(
"preserves unrelated text through $name setup and uninstall",
async ({ original, preserved }) => {
const root = await createTestTempDirectory("rea-codex-preserve-");
const configPath = join(root, "config.toml");
const client = { name: "codex", format: "toml", configPath } as const;
await writeFile(configPath, original);
expect(
await configureClientConfiguration(client, {}, ["rea", "mcp"]),
).toMatchObject({ status: "configured" });
const configured = await readFile(configPath, "utf8");
for (const text of preserved) expect(configured).toContain(text);
expect(unrelatedValues(configured)).toEqual(unrelatedValues(original));
expect(configured.startsWith("\uFEFF")).toBe(
original.startsWith("\uFEFF"),
);
expect(
await configureClientConfiguration(client, {}, ["rea", "mcp"]),
).toEqual({ status: "unchanged" });
expect(await readFile(configPath, "utf8")).toBe(configured);
expect(
(await systemUninstallHost(root).removeClient(client)).status,
).toBe("removed");
const removed = await readFile(configPath, "utf8");
for (const text of preserved) expect(removed).toContain(text);
expect(unrelatedValues(removed)).toEqual(unrelatedValues(original));
expect(parseToml(removed)).not.toHaveProperty("mcp_servers.rea");
expect(removed.startsWith("\uFEFF")).toBe(original.startsWith("\uFEFF"));
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
},
);
it("retains the first backup after uninstall and a later setup", async () => {
const root = await createTestTempDirectory("rea-codex-backup-");
const configPath = join(root, "config.toml");
const client = { name: "codex", format: "toml", configPath } as const;
const original = 'notify = ["a", "b"]\n';
await writeFile(configPath, original);
await configureClientConfiguration(client, {}, ["rea", "mcp"]);
expect((await systemUninstallHost(root).removeClient(client)).status).toBe(
"removed",
);
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
await configureClientConfiguration(client, {}, ["rea", "mcp"]);
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
});
it("refuses malformed Codex TOML unchanged without creating a backup", async () => {
const root = await createTestTempDirectory("rea-codex-malformed-");
const configPath = join(root, "config.toml");
const client = { name: "codex", format: "toml", configPath } as const;
const original =
"# Keep this explanation.\n[mcp_servers.rea]\ncommand = [broken\n";
await writeFile(configPath, original);
expect(
await configureClientConfiguration(client, {}, ["rea", "mcp"]),
).toEqual({ status: "failed", reason: "readback" });
expect((await systemUninstallHost(root).removeClient(client)).status).toBe(
"failed",
);
expect(await readFile(configPath, "utf8")).toBe(original);
await expect(
readFile(`${configPath}.rea.backup`, "utf8"),
).rejects.toMatchObject({ code: "ENOENT" });
});
});
tests/boundary/filesystem/clientLifecycle.test.ts
+23 -2
View File
@@ -1,3 +1,4 @@
import { constants as fsConstants } from "node:fs";
import {
copyFile,
lstat,
@@ -1587,7 +1588,7 @@ describe("client configuration filesystem failure reporting", () => {
copy: (source, destination) =>
failure === "backup"
? Promise.reject(new Error("SECRET backup failure"))
: copyFile(source, destination),
: copyFile(source, destination, fsConstants.COPYFILE_EXCL),
writeText: async (path, contents) => {
writes += 1;
if ((failure === "update" && writes === 1) || failure === "restore")
@@ -1607,6 +1608,25 @@ describe("client configuration filesystem failure reporting", () => {
},
);
it("restores the current configuration without replacing its initial backup", async () => {
const { home, config } = await uninstallFixture();
const current = await readFile(config, "utf8");
const initial = '{"existing":true}\n';
await writeFile(`${config}.rea.backup`, initial);
let writes = 0;
const result = await systemUninstallHost(home, {
...testFileSystem,
writeText: async (path, contents) => {
writes += 1;
await writeFile(path, contents);
if (writes === 1) throw new Error("update failed after writing");
},
}).removeClient({ name: "cursor", format: "json", configPath: config });
expect(result.status).toBe("failed");
expect(await readFile(config, "utf8")).toBe(current);
expect(await readFile(`${config}.rea.backup`, "utf8")).toBe(initial);
});
it("reports a managed-path removal failure", async () => {
const home = await createTestTempDirectory("rea-uninstall-remove-");
roots.push(home);
@@ -1631,7 +1651,8 @@ describe("client configuration filesystem failure reporting", () => {
const testFileSystem: UninstallFileSystem = {
readText: (path) => readFile(path, "utf8"),
copy: (source, destination) => copyFile(source, destination),
copy: (source, destination) =>
copyFile(source, destination, fsConstants.COPYFILE_EXCL),
writeText: (path, contents) => writeFile(path, contents),
stat: (path) => lstat(path),
realpath: (path) => realpath(path),