From c9541dc509b128d5b6dd77adc5820307b3d433a3 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:22:31 +0700 Subject: [PATCH] feat(codex): write Codex artifacts with backup and atomic rename --- server/__tests__/codex-mutate.test.js | 237 ++++++++++++++++++++++++++ server/lib/codex-mutate.js | 231 +++++++++++++++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 server/__tests__/codex-mutate.test.js create mode 100644 server/lib/codex-mutate.js diff --git a/server/__tests__/codex-mutate.test.js b/server/__tests__/codex-mutate.test.js new file mode 100644 index 00000000..0372dbf0 --- /dev/null +++ b/server/__tests__/codex-mutate.test.js @@ -0,0 +1,237 @@ +/** + * @file Tests for codex-mutate.js: create/edit/delete on Codex config + * surfaces (skills, agents, rules, memory, config) across the user + * (CODEX_HOME) and project (/.codex or for AGENTS.md) scopes. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const os = require("node:os"); + +const { describe, it, before, after } = require("node:test"); +const assert = require("node:assert/strict"); + +describe("codex-mutate", () => { + let TMP; + let REPO; + let prevCodexHome; + let cm; + + before(() => { + prevCodexHome = process.env.CODEX_HOME; + TMP = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-")); + REPO = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-repo-")); + process.env.CODEX_HOME = TMP; + delete require.cache[require.resolve("../lib/codex-home")]; + delete require.cache[require.resolve("../lib/codex-mutate")]; + cm = require("../lib/codex-mutate"); + }); + + after(() => { + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + fs.rmSync(TMP, { recursive: true, force: true }); + fs.rmSync(REPO, { recursive: true, force: true }); + }); + + it("creates a project skill as a directory with SKILL.md inside", () => { + const { file, created } = cm.writeArtifact({ + scope: "project", + type: "skills", + name: "release-guard", + content: "---\nname: x\n---\n", + cwd: REPO, + }); + assert.equal(created, true); + assert.equal(file, path.join(REPO, ".codex", "skills", "release-guard", "SKILL.md")); + }); + + it("backs the old body up before overwriting it", () => { + cm.writeArtifact({ scope: "user", type: "rules", name: "default", content: "old", cwd: REPO }); + const { backup } = cm.writeArtifact({ + scope: "user", + type: "rules", + name: "default", + content: "new", + cwd: REPO, + }); + assert.equal(fs.readFileSync(backup, "utf8"), "old"); + assert.equal(fs.readFileSync(path.join(TMP, "rules", "default.rules"), "utf8"), "new"); + }); + + it("refuses a name that climbs out of the root", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "agents", + name: "../../evil", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + }); + + it("writes AGENTS.md at the repo root, not inside .codex", () => { + const { file } = cm.writeArtifact({ + scope: "project", + type: "memory", + content: "# rules", + cwd: REPO, + }); + assert.equal(file, path.join(REPO, "AGENTS.md")); + }); + + it("keeps a deleted skill's backup out of the skills tree", () => { + cm.writeArtifact({ scope: "user", type: "skills", name: "gone", content: "x", cwd: REPO }); + const { backup } = cm.deleteArtifact({ + scope: "user", + type: "skills", + name: "gone", + cwd: REPO, + }); + assert.ok( + !backup.includes(path.join(TMP, "skills", "gone")), + "a backup must never be discoverable as a skill" + ); + assert.equal(fs.existsSync(path.join(TMP, "skills", "gone")), false); + }); + + it("writes and reads back a project agent under .codex/agents/.toml", () => { + const { file } = cm.writeArtifact({ + scope: "project", + type: "agents", + name: "reviewer", + content: 'name = "reviewer"\n', + cwd: REPO, + }); + assert.equal(file, path.join(REPO, ".codex", "agents", "reviewer.toml")); + }); + + it("writes user config.toml under CODEX_HOME and project config.toml under /.codex", () => { + const u = cm.writeArtifact({ + scope: "user", + type: "config", + content: 'model = "x"\n', + cwd: REPO, + }); + assert.equal(u.file, path.join(TMP, "config.toml")); + const p = cm.writeArtifact({ + scope: "project", + type: "config", + content: 'model = "y"\n', + cwd: REPO, + }); + assert.equal(p.file, path.join(REPO, ".codex", "config.toml")); + }); + + it("rejects a scope that is neither user nor project", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "project-local", + type: "rules", + name: "x", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADSCOPE" + ); + }); + + it("rejects a type outside TYPES", () => { + assert.throws( + () => + cm.writeArtifact({ scope: "user", type: "commands", name: "x", content: "x", cwd: REPO }), + (e) => e.code === "EBADTYPE" + ); + }); + + for (const badName of ["..", ".", "/etc/passwd", "a/b", "a\0b"]) { + it(`rejects the bad name ${JSON.stringify(badName)} with EBADNAME and writes nothing`, () => { + const target = path.join(TMP, "agents", `${badName.replace(/[/\0]/g, "_")}.toml`); + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "agents", + name: badName, + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + assert.equal(fs.existsSync(target), false); + }); + } + + it("rejects a name passed to memory", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "memory", + name: "AGENTS", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + }); + + it("rejects a name passed to config", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "config", + name: "config", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + }); + + it("rejects content over the size limit and leaves the existing file untouched", () => { + cm.writeArtifact({ scope: "user", type: "rules", name: "toobig", content: "small", cwd: REPO }); + const huge = "x".repeat(256 * 1024 + 1); + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "rules", + name: "toobig", + content: huge, + cwd: REPO, + }), + (e) => e.code === "ETOOLARGE" + ); + assert.equal(fs.readFileSync(path.join(TMP, "rules", "toobig.rules"), "utf8"), "small"); + }); + + it("refuses to delete something that does not exist, and creates no backup", () => { + const before = fs.existsSync(path.join(TMP, "codex-config-backups", "rules")) + ? fs.readdirSync(path.join(TMP, "codex-config-backups", "rules")).length + : 0; + assert.throws( + () => cm.deleteArtifact({ scope: "user", type: "rules", name: "never-existed", cwd: REPO }), + (e) => e.code === "ENOTFOUND" + ); + const after = fs.existsSync(path.join(TMP, "codex-config-backups", "rules")) + ? fs.readdirSync(path.join(TMP, "codex-config-backups", "rules")).length + : 0; + assert.equal(after, before, "no backup should be created for a missing target"); + }); + + it("lists backups for a given scope and type", () => { + cm.writeArtifact({ scope: "user", type: "rules", name: "listed", content: "v1", cwd: REPO }); + cm.writeArtifact({ scope: "user", type: "rules", name: "listed", content: "v2", cwd: REPO }); + const rows = cm.listBackups({ scope: "user", type: "rules" }); + assert.ok(rows.length >= 1); + assert.equal(rows[0].type, "rules"); + assert.equal(rows[0].name.startsWith("listed.rules."), true); + }); +}); diff --git a/server/lib/codex-mutate.js b/server/lib/codex-mutate.js new file mode 100644 index 00000000..cc95e691 --- /dev/null +++ b/server/lib/codex-mutate.js @@ -0,0 +1,231 @@ +/** + * @file codex-mutate.js + * @description Mutation helpers for the Codex Config panel. Handles + * create / overwrite / delete on Codex's text-file surfaces: skills, agents, + * rules, memory (AGENTS.md), and config (config.toml). Mirrors cc-mutate.js's + * structure (single `resolveTarget` chokepoint, regex-validated names, + * containment re-check, backup-before-mutation, atomic write) so the two + * panels share the same safety shape and error-code vocabulary. + * + * Hard constraints (do not relax without a follow-up review): + * - `memory` and `config` are fixed filenames (AGENTS.md / config.toml) — + * they take NO `name`. Passing one is a caller error (EBADNAME). + * - Every write/delete of an existing file creates a timestamped backup + * BEFORE the mutation. Backups land under + * /codex-config-backups//, a directory Codex never scans, so + * a deleted skill can never come back as a discoverable skill named + * after its backup. + * - Writes are atomic via temp file + fs.renameSync (see config-write.js). + * - Names are validated against a strict allowlist regex; resolved paths + * are double-checked to live under the expected containment root before + * any I/O. + * - Codex has no `project-local` scope and no auto-memory: only "user" and + * "project" are valid scopes. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("./codex-home"); +const { MAX_FILE_BYTES } = require("./cc-discovery"); +const { createBackup, atomicWriteFile, assertUnder } = require("./config-write"); + +const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/; + +const TYPES = { + skills: { kind: "dir", subdir: "skills", filename: "SKILL.md" }, + agents: { kind: "file", subdir: "agents", ext: ".toml" }, + rules: { kind: "file", subdir: "rules", ext: ".rules" }, + memory: { kind: "fixed", file: "AGENTS.md" }, // AGENTS.md, no `name` + config: { kind: "fixed", file: "config.toml" }, // config.toml, no `name` +}; + +function makeError(code, message) { + const err = new Error(message); + err.code = code; + return err; +} + +function assertScope(scope) { + if (scope !== "user" && scope !== "project") { + throw makeError("EBADSCOPE", `unknown scope: ${scope}`); + } +} + +/** The a (scope) resolves under for skills/agents/rules subdirs. */ +function rootForScope(scope, opts = {}) { + return scope === "user" ? getCodexHome() : getProjectCodexDir(opts.cwd); +} + +/** Fixed-filename path for memory/config, per scope's own layout rule. */ +function fixedPathFor(type, scope, opts = {}) { + if (type === "memory") { + return scope === "user" + ? path.join(getCodexHome(), "AGENTS.md") + : path.join(getProjectRoot(opts.cwd), "AGENTS.md"); + } + // config + return scope === "user" + ? path.join(getCodexHome(), "config.toml") + : path.join(getProjectCodexDir(opts.cwd), "config.toml"); +} + +/** + * Resolve the on-disk target for a (scope, type, name) tuple AND the + * containment root used for path-traversal checks. + * + * Returns: { kind: "file" | "dir", target, filePath, containmentRoot } + */ +function resolveTarget(scope, type, name, opts = {}) { + const spec = TYPES[type]; + if (!spec) throw makeError("EBADTYPE", `unknown type: ${type}`); + assertScope(scope); + + if (spec.kind === "fixed") { + if (name !== undefined && name !== null) { + throw makeError("EBADNAME", `${type} is a fixed file and takes no name`); + } + const filePath = fixedPathFor(type, scope, opts); + return { kind: "file", target: filePath, filePath, containmentRoot: path.dirname(filePath) }; + } + + if (typeof name !== "string" || !NAME_RE.test(name)) { + throw makeError("EBADNAME", `name must match ${NAME_RE}`); + } + + const subdirAbs = path.join(rootForScope(scope, opts), spec.subdir); + + if (spec.kind === "dir") { + const target = path.join(subdirAbs, name); + return { + kind: "dir", + target, + filePath: path.join(target, spec.filename), + containmentRoot: subdirAbs, + }; + } + + const target = path.join(subdirAbs, name + spec.ext); + return { kind: "file", target, filePath: target, containmentRoot: subdirAbs }; +} + +/** /codex-config-backups//, always rooted the same way as the read side's . */ +function backupRootFor(scope, type, opts = {}) { + return path.join(rootForScope(scope, opts), "codex-config-backups", type); +} + +function rmTreeSync(p) { + fs.rmSync(p, { recursive: true, force: true }); +} + +// ── Public API ───────────────────────────────────────────────────────── + +/** + * Create or overwrite a single text artifact. `name` is required for + * skills/agents/rules and forbidden for memory/config. + * + * @param {{scope:string, type:string, name?:string, content:string, cwd?:string}} args + */ +function writeArtifact(args) { + const { scope, type, name, content, cwd } = args; + if (typeof content !== "string") throw makeError("EBADCONTENT", "content must be a string"); + if (Buffer.byteLength(content, "utf8") > MAX_FILE_BYTES) { + throw makeError("ETOOLARGE", `content exceeds ${MAX_FILE_BYTES} bytes`); + } + + const r = resolveTarget(scope, type, name, { cwd }); + assertUnder(r.containmentRoot, r.target); + + const existedBefore = fs.existsSync(r.filePath); + const backup = existedBefore + ? createBackup({ + source: r.kind === "dir" ? r.target : r.filePath, + backupRoot: backupRootFor(scope, type, { cwd }), + type, + name: path.basename(r.target), + }) + : null; + + if (r.kind === "dir") { + fs.mkdirSync(r.target, { recursive: true }); + } + atomicWriteFile(r.filePath, content); + + return { file: r.filePath, created: !existedBefore, backup }; +} + +/** + * Delete a single text artifact. Backup is mandatory and runs first; if + * the target does not exist, no backup is attempted. + */ +function deleteArtifact(args) { + const { scope, type, name, cwd } = args; + const r = resolveTarget(scope, type, name, { cwd }); + assertUnder(r.containmentRoot, r.target); + + if (!fs.existsSync(r.target)) { + throw makeError("ENOTFOUND", `${type}/${name || TYPES[type].file} does not exist`); + } + + const backup = createBackup({ + source: r.target, + backupRoot: backupRootFor(scope, type, { cwd }), + type, + name: path.basename(r.target), + }); + + if (r.kind === "dir") { + rmTreeSync(r.target); + } else { + fs.unlinkSync(r.target); + } + + return { file: r.filePath, backup }; +} + +/** + * List backups for either all types or a specific (scope, type) bucket. + * Returns [{ type, name, file, mtime, size }]. + */ +function listBackups(opts = {}) { + const out = []; + const scopes = opts.scope ? [opts.scope] : ["user", "project"]; + const types = opts.type ? [opts.type] : Object.keys(TYPES); + for (const scope of scopes) { + for (const type of types) { + const root = backupRootFor(scope, type, opts); + let entries = []; + try { + entries = fs.readdirSync(root, { withFileTypes: true }); + } catch { + continue; + } + for (const ent of entries) { + const full = path.join(root, ent.name); + let stat; + try { + stat = fs.statSync(full); + } catch { + continue; + } + out.push({ + type, + name: ent.name, + file: full, + mtime: stat.mtimeMs, + size: ent.isDirectory() ? null : stat.size, + }); + } + } + } + return out.sort((a, b) => b.mtime - a.mtime); +} + +module.exports = { + writeArtifact, + deleteArtifact, + listBackups, + resolveTarget, // exported for tests + TYPES, + NAME_RE, +};