feat(codex): write Codex artifacts with backup and atomic rename
This commit is contained in:
@@ -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 (<cwd>/.codex or <cwd> for AGENTS.md) scopes.
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
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/<name>.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 <cwd>/.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);
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
* <root>/codex-config-backups/<type>/, 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ĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
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 <root> 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 };
|
||||
}
|
||||
|
||||
/** <root>/codex-config-backups/<type>/, always rooted the same way as the read side's <root>. */
|
||||
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,
|
||||
};
|
||||
Reference in New Issue
Block a user