golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#2760): defensive Codex install — strip legacy agents blocks, default hooks to AoT, validate post-write schema (#2785)

Browse Source 
* fix(#2760): defensive Codex install — strip legacy agents blocks, default hooks to AoT, validate post-write schema

Three defects, three defensive fixes shipped together. Issue reporter
never returned with the requested diagnostic backup, but four additional
users have since confirmed the same Codex breakage and ZakAnun confirmed
manual cleanup is the only working workaround — defensive triple ships
without the original backup grep, justified by the corroborating reports.

Fix 1 (defect 3 — confirmed real). The Codex hooks emit path always
appended a top-level `[[hooks]]` AoT block, which collides with users
who already use the namespaced AoT form `[[hooks.SessionStart]]`. New
helper `hasUserNamespacedAotHooks()` detects the user's preferred shape
on parse and the install emits the GSD-managed hook in that same shape
when present. Default for fresh configs stays at top-level `[[hooks]]`
so status-quo behavior is preserved.

Fix 2 (defects 1+2 — defensive). `stripLeakedGsdCodexSections()` (the
install-time stripper) now always purges bare `[agents]` single-bracket
tables and `[[agents]]` sequence tables regardless of GSD marker
presence — both forms are invalid in current Codex schema and produce
"invalid type: ..., expected struct AgentsToml". Previously gated on
GSD-name lookup which missed marker-stripped configs and third-party
authored entries. The uninstall-time stripper (`stripCodexGsdAgentSections`)
keeps its old conservative behavior so user-authored entries survive
uninstall.

Fix 3 (defensive). Post-write schema validation parses the bytes about
to be committed and asserts no bare `[agents]`, no `[[agents]]`, and no
bare `[hooks.<Event>]` tables remain. On failure the install restores
the pre-install backup of config.toml and aborts loudly so the user is
never left with a Codex CLI that refuses to load. Pre-install snapshot
is captured before installCodexConfig runs (not after) so restore
returns the file to its true pre-GSD state.

Tests added (10 new, 1 updated):
  - bug-2760-codex-install-defensive.test.cjs (10 new tests across 4
    describes: hooks AoT preservation, strip robustness for both
    [agents] and [[agents]] without marker, schema validator behavior,
    abort+restore via test seam)
  - codex-config.test.cjs "case 2 ..." updated to reflect new defensive
    bare-[agents] purge

Full suite: 5747 pass / 0 fail.

Closes #2760

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(#2760): normalize Codex hooks emit field name across migration and managed paths

The migrateCodexHooksMapFormat path emitted `type = "<TYPE>"` for legacy
[hooks.TYPE] sections, while the GSD-managed Codex install emitted
`event = "SessionStart"` — same target [[hooks]] schema, two different
field names. Codex currently tolerates both via permissive parsing, but
the moment one path tightens this becomes a silent #2760-class regression.

Normalize both call sites on `event` (the existing GSD-managed
convention). Update migration emit, docstring, and existing migration
assertions to match. Add a parity regression test that drives both code
paths and asserts the [[hooks]] field key is identical.

* test(#2153): fix test isolation by building hooks/dist on demand

The "Codex install copies hook file (#2153)" regression depends on
hooks/dist/ being populated, but that directory is gitignored and only
built by `npm run build:hooks`. The npm pretest chain runs `build:sdk`
but not `build:hooks`, so when this file is run in isolation
(`node --test tests/codex-config.test.cjs`) the hook copy step skips
silently and the regression test fails on a stale-environment artifact
rather than a real bug.

Add a top-level before() hook that runs scripts/build-hooks.js when
hooks/dist/ is missing or empty. Matches the pattern already used by
bug-1834-sh-hooks-installed and other install integration tests, so the
suite passes regardless of runner ordering or which tests are targeted.

* fix(#2760): structural TOML validation, atomic writes, and behavioral test rewrites

Addresses CodeRabbit review on PR #2785 plus source-grep violations the
maintainer flagged in the regression test.

Fix 1 (CR 3149606220) — validateCodexConfigSchema now parses the TOML into
a structured object first via the new parseTomlToObject helper, then
runs schema-shape checks against both the parsed structure and the table
section headers. Malformed TOML with valid-looking headers no longer
slips past validation.

Fix 2 (CR 3149606224) — Replaced the four source-grep assertions in
tests/bug-2760-codex-install-defensive.test.cjs (lines 109, 125, 169,
201) with structural assertions against the parsed TOML object via the
exported parseTomlToObject helper. Tests now verify behavior (the file
parses and contains the expected structure) instead of literal byte
patterns. Robust to formatting changes — exactly what the regex-loosening
suggestion was reaching for, done correctly. Confirmed clean by
`npm run lint:tests` (0 violations).

Fix 3 (CR 3149606234) — The describe block that mutates
installModule.__codexSchemaValidator now runs with concurrency: false
so the test seam mutation cannot leak into sibling suites that also
call runCodexInstall.

Fix 4 (CR outside-diff) — Approach (b): atomic temp-file + renameSync.
Added atomicWriteFileSync helper used by mergeCodexConfig and the final
hooks-write. A mid-write failure leaves the .tmp-<pid>-<n> sibling
behind (cleaned up immediately) and never truncates the original
config.toml. Paired with try/catch wrapping around the entire
post-snapshot mutation sequence so any unexpected throw also triggers
restoreCodexSnapshot. Two layers of defense: atomic write prevents the
corruption window, snapshot restore handles non-atomic write paths.

Added behavioral test for fix 4: stubs fs.renameSync to throw on the
configPath rename, asserts the on-disk bytes match the pre-install
snapshot byte-for-byte, asserts the parsed structure is still the
user's [model] section (no half-written GSD agents block), and asserts
no stray .tmp-* files remain. Marked concurrency: false because it
monkey-patches a global.

Test results: 5749/5749 pass, 0 fail. lint:tests clean.

* test(#2760): TOML-parse based assertions for bare-agents purge and hook-field parity (CodeRabbit follow-up)

* fix(#2760): treat write failures as fatal, strip legacy hooks before guard, tighten TOML parser (CR4)

CR4 finding 1 (MAJOR) — Write failures silently succeeded. The inner catch
around atomicWriteFileSync restored the snapshot then re-threw, but the outer
catch only matched 'post-write Codex schema validation failed' and downgraded
everything else to a warn-and-continue. Install finished with "Done!" while
Codex had no GSD agents configured. Fix: wrap writeErr with a `post-write
Codex install failed:` prefix and broaden the outer guard to `.startsWith(
'post-write')` so both schema-validation and write failures abort install.

CR4 finding 2 (MAJOR) — Legacy flat [[hooks]] block prevented namespaced AoT
upgrade. The `!configContent.includes('gsd-check-update')` guard short-
circuited the new namespaced emit when an existing install had the legacy
flat [[hooks]] block, leaving users stuck in the mixed layout this fix is
designed to eliminate. Fix: strip ALL existing managed gsd-check-update
hook blocks (top-level [[hooks]] AND namespaced [[hooks.SessionStart]])
BEFORE evaluating the includes guard, so every install converges on the
right shape regardless of prior state.

CR4 finding 3 (MAJOR) — Homegrown TOML parser silently accepted malformed
input. parseTomlValue happily consumed the `0` prefix of `timeout = 0.5`
and parseTomlToObject did not verify the full RHS was consumed, so
`key = "x" junk` and date/time literals slipped through. Per CONTRIBUTING
("No external dependencies in core"), option (b) was chosen over adding
@iarna/toml: (a) parseTomlValue rejects any integer immediately followed
by `.`, `e`, `E`, `:`, `-`, `T`, or `Z` (floats / dates / times); (b)
parseTomlToObject scans from parsed.end to the next newline and throws
`trailing bytes after value` if anything other than whitespace + optional
`# comment` is present.

* test(#2760): add CR4 regression tests + scope GSD_TEST_MODE + rename rename-fault test

CR4 finding 5 (NIT) — GSD_TEST_MODE leak. Saved previous value, set '1' for
the require, then restored (delete if undefined). No more test-only env var
leaking to siblings in the same node process.

CR4 finding 4 (NIT) — Renamed the existing fix-4 test from 'fs.writeFileSync'
to 'fs.renameSync' (the only call actually faulted) and added a sibling test
that stubs fs.writeFileSync to throw on the .tmp- target — exercising the
pre-rename branch of atomicWriteFileSync that was previously untested. Both
serialize via concurrency: false on the existing describe block.

CR4 finding 1 (MAJOR test) — New behavioral test asserts install throws with
a `post-write Codex install failed` message AND never prints "Done!" when
the hook-block atomic rename fails. Captures stdout via console.log stub,
asserts byte equality of restored snapshot. Faults only the rename whose
temp source contains gsd-check-update so earlier mergeCodexConfig writes
are not collateral damage.

CR4 finding 2 (MAJOR test) — New TOML-parsed behavioral test for the
legacy-hook upgrade path: pre-install has [[hooks.SessionStart]] (user) +
legacy flat [[hooks]] managed gsd-check-update entry; post-install must
have hooks.SessionStart as Array-of-tables with both user hook and GSD
entry, and no top-level [[hooks]] AoT remaining. Also asserts exactly one
gsd-check-update entry (no duplicates).

CR4 finding 3 (MAJOR test) — parseTomlToObject regression suite: rejects
floats (timeout = 0.5), dates (created = 1979-05-27), trailing garbage
(key = "x" junk), and accepts trailing whitespace + # comment.

* fix(#2760): CR5 — pre-write fatal, TOML duplicate-key/header rejection, namespaced AoT migration

Address all five CodeRabbit round-5 findings on PR #2785:

Finding 1 (MAJOR) — Pre-write failures in the Codex hook configuration
catch (around bin/install.js:7002) used to fall through to console.warn
even though restoreCodexSnapshot() had already run. This produced "Done!"
output with no Codex hooks configured. Now wraps the original error with
a "(pre-write)" prefix and rethrows so install aborts loudly. Same defect
class as CR4 finding 1, different layer.

Finding 2 (MAJOR) — parseTomlToObject silently reused existing tables and
overwrote duplicate keys. Real TOML 1.0 rejects:
  - duplicate scalar key in same table ([a]\nx=1\nx=2)
  - re-declared [a] header (two [a] sections)
  - [[arr]] then [arr] for same path (shape mismatch)
Tracks pathShape, declaredHeaders, and per-table-instance key sets;
throws "duplicate or shape-mismatched table header at <path>" or
"duplicate key <name> in <path>".

Finding 3 (MAJOR) — migrateCodexHooksMapFormat used to emit flat
[[hooks]]\nevent="<TYPE>", which produced mixed flat+namespaced layouts
when the user already had [[hooks.<OTHER>]] entries. Now emits
[[hooks.<TYPE>]] directly (the namespace IS the event); managed-emit
detector hasUserNamespacedAotHooks fires correctly so the install
converges on a single namespaced layout regardless of pre-existing state.

Finding 4 (NIT) — tests/bug-2760-codex-install-defensive.test.cjs
rename-failure test tightened from "throw OR warn acceptable" to
assert.equal(threw, true), locking the contract Finding 1 establishes.

Finding 5 (NIT) — bug-2760 test suite snapshots and restores fs.renameSync
defensively in beforeEach/afterEach (symmetric with fs.writeFileSync),
removing the fragile per-test try/finally. Second test in the same
suite cleaned up to drop its try/finally.

Updates tests/codex-config.test.cjs to assert the new namespaced AoT
migration shape via parseTomlToObject (no source-grep). Existing field-
parity test reframed as shape-parity since both paths now emit
namespaced.

Tests: 5764 pass (+8 new). lint:tests: 0 violations.

* docs(#2760): add CHANGELOG entry for Codex install defensive triple

Adds the [Unreleased] Fixed entry for the Codex install fix landed in this
PR — defensive strip of legacy [agents]/[[agents]] blocks, namespaced AoT
hook detection across all events, atomic write + rollback, strict TOML
validation rejecting duplicate keys/repeated headers/trailing bytes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-27 17:11:59 -04:00
committed by GitHub
parent 936cf26706
commit b44482cf03
4 changed files with 1762 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
CHANGELOG.md
View File

@@ -14,6 +14,19 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
it). Re-run `gsd update` without `--minimal` to expand to the full surface. The
install manifest now records `mode: "minimal" | "full"`. (#2762)
### Fixed
- **Codex install no longer corrupts existing `~/.codex/config.toml`** — the installer
now defensively strips legacy `[agents]` (single-bracket) and `[[agents]]` (sequence)
blocks regardless of GSD marker presence (both invalid in current Codex schema), emits
the GSD-managed hook in the user's preferred shape (`[[hooks.<Event>]]` namespaced AoT
if any user hook uses it, otherwise top-level `[[hooks]]`), migrates legacy
`[hooks.<Event>]` to namespaced AoT, and atomically writes via temp-file +
`renameSync`. A strict TOML parser validates the post-write bytes against the Codex
schema and rejects duplicate keys, repeated table headers, trailing bytes after
values, and unsupported value types. Both pre-write helper failures and write-time
failures restore the pre-install snapshot and abort with a clear error rather than
warn-and-continue. (#2760)
## [1.38.5] - 2026-04-25
### Fixed

674
bin/install.js
View File

@@ -2089,6 +2089,10 @@ function generateCodexConfigBlock(agents, targetDir) {
/**
* Strip any managed GSD agent sections from a TOML string.
*
* Used by the uninstall path (`stripGsdFromCodexConfig`). Removes only what GSD
* owns; user-authored `[agents.<name>]` and `[[agents]]` entries are preserved
* so uninstall returns the file to its pre-GSD shape.
*
* Handles BOTH shapes so reinstall self-heals configs from all GSD versions:
* - Current (#2727): `[agents.gsd-*]` struct tables (Codex 0.120.0+).
* - Legacy (#2645): `[[agents]]` array-of-tables whose `name = "gsd-*"`.
@@ -2815,25 +2819,32 @@ function isLegacyGsdAgentsSection(body) {
}
function stripLeakedGsdCodexSections(content) {
// Defensive precedence (#2760): we own the `agents` namespace under our
// managed `gsd-*` names, and the legacy bare-table and sequence forms
// (`[agents]`, `[[agents]]`) are invalid in the current Codex schema —
// they trigger "invalid type: ..., expected struct AgentsToml" and break
// every Codex CLI invocation. They MUST never coexist with the new
// `[agents.<name>]` struct format we now emit, so install-time always
// purges them regardless of GSD marker presence. Users who had legitimate
// user-authored `[[agents]]` entries before are already broken on Codex
// ≥0.124 — purging is the only path to a loadable config.
const leakedSections = getTomlTableSections(content)
.filter((section) => {
// Legacy [agents.gsd-<name>] map tables (pre-#2645).
if (!section.array && section.path.startsWith('agents.gsd-')) return true;
// Legacy bare [agents] table with only the old max_threads/max_depth keys.
if (
!section.array &&
section.path === 'agents' &&
isLegacyGsdAgentsSection(content.slice(section.headerEnd, section.end))
) return true;
// ANY bare [agents] single-bracket table — invalid in current Codex
// schema, always purged at install time (#2760). Previously gated
// on `isLegacyGsdAgentsSection`, which missed bare tables holding
// arbitrary user keys (`default = "..."`, etc.) that still produce
// the AgentsToml type error.
if (!section.array && section.path === 'agents') return true;
// Current [[agents]] array-of-tables whose name is gsd-*. Preserve
// user-authored [[agents]] entries (other names) untouched.
if (section.array && section.path === 'agents') {
const body = content.slice(section.headerEnd, section.end);
const nameMatch = body.match(/^[ \t]*name[ \t]*=[ \t]*["']([^"']+)["']/m);
if (nameMatch && /^gsd-/.test(nameMatch[1])) return true;
}
// ANY [[agents]] array-of-tables — invalid in current Codex schema,
// always purged at install time (#2760). Previously gated on
// `name = "gsd-..."` which preserved user-authored entries that are
// themselves rejected by Codex 0.124+.
if (section.array && section.path === 'agents') return true;
return false;
});
@@ -2862,15 +2873,17 @@ function stripLeakedGsdCodexSections(content) {
* [hooks.shell]
* command = "..."
*
* to the new array-of-tables format:
* [[hooks]]
* type = "shell"
* to the new array-of-tables format. #2760 CR5 finding 3 — emit the
* namespaced AoT shape directly so a mixed flat + namespaced layout never
* arises post-install:
* [[hooks.shell]]
* command = "..."
*
* This function detects any non-array hooks sections in the config and
* converts them to the [[hooks]] format, preserving all key-value pairs and
* user comments. Bare [hooks] container sections (no key-value content) are
* dropped. User-authored [[hooks]] array entries are left untouched.
* converts them to the namespaced `[[hooks.<TYPE>]]` array-of-tables form,
* preserving all key-value pairs and user comments. Bare [hooks] container
* sections (no key-value content) are dropped. User-authored AoT entries are
* left untouched.
*
* Returns the migrated content, or the original content unchanged if no
* legacy hooks sections were found.
@@ -2901,8 +2914,15 @@ function migrateCodexHooksMapFormat(content) {
const type = section.path.slice('hooks.'.length);
const body = content.slice(section.headerEnd, section.end);
// Build [[hooks]] block: type line + original body lines
const block = `[[hooks]]${eol}type = "${type}"${eol}${body}`;
// #2760 CR5 finding 3 — emit the namespaced AoT form directly:
// `[[hooks.<TYPE>]]` (no synthetic `event` field — the namespace IS the
// event). Previously we emitted flat `[[hooks]]\nevent = "<TYPE>"`,
// which produced mixed flat + namespaced layouts when the user already
// had `[[hooks.<OTHER>]]` entries. With every migration emit using the
// namespaced shape, the managed-emit detector
// (`hasUserNamespacedAotHooks`) fires correctly and the install
// converges on a single hook layout.
const block = `[[hooks.${type}]]${eol}${body}`;
newHooksBlocks.push(block);
}
@@ -2955,6 +2975,446 @@ function migrateCodexHooksMapFormat(content) {
return result;
}
/**
* Detect whether the user already uses the namespaced AoT hooks form
* (`[[hooks.<EVENT>]]`) for the given event in the config. When true,
* the GSD-managed hook block must be emitted in the same shape so it
* coexists cleanly — mixing `[[hooks]]` (flat) with `[[hooks.SessionStart]]`
* (namespaced) in the same file confuses round-trip writers and can
* produce a config that Codex rejects (#2760, defect 3).
*/
function hasUserNamespacedAotHooks(content, event) {
const sections = getTomlTableSections(content);
return sections.some(
(section) => section.array && section.path === `hooks.${event}`
);
}
/**
* Parse a TOML value RHS expression starting at index `i` of `text`.
* Returns { value, end } on success or throws on parse failure.
*
* Supports the value forms GSD emits or that real Codex configs commonly use:
* - basic strings ("…" with simple escapes)
* - literal strings ('…')
* - booleans (true / false)
* - integers (optional sign, decimal digits)
* - inline arrays of the above
* - inline tables { k = v, … }
*
* This is intentionally not a complete TOML implementation — it is the
* minimal value grammar required to validate Codex config structure and to
* back behavioral assertions in tests (#2760).
*/
function parseTomlValue(text, i) {
// Skip leading whitespace.
while (i < text.length && (text[i] === ' ' || text[i] === '\t')) {
i += 1;
}
if (i >= text.length) {
throw new Error('expected value, got end of input');
}
const ch = text[i];
// Basic string
if (ch === '"') {
if (text.startsWith('"""', i)) {
const close = findMultilineBasicStringClose(text, i + 3);
if (close === -1) {
throw new Error('unterminated multi-line basic string');
}
const raw = text.slice(i + 3, close);
return { value: raw.replace(/^\r?\n/, ''), end: close + 3 };
}
let j = i + 1;
let out = '';
while (j < text.length) {
const c = text[j];
if (c === '\\') {
const next = text[j + 1];
if (next === 'n') { out += '\n'; j += 2; continue; }
if (next === 't') { out += '\t'; j += 2; continue; }
if (next === 'r') { out += '\r'; j += 2; continue; }
if (next === '\\') { out += '\\'; j += 2; continue; }
if (next === '"') { out += '"'; j += 2; continue; }
if (next === '/') { out += '/'; j += 2; continue; }
// Pass-through unrecognized escape (Codex/GSD don't use these).
out += next === undefined ? '' : next;
j += 2;
continue;
}
if (c === '"') {
return { value: out, end: j + 1 };
}
out += c;
j += 1;
}
throw new Error('unterminated basic string');
}
// Literal string
if (ch === '\'') {
if (text.startsWith('\'\'\'', i)) {
const close = text.indexOf('\'\'\'', i + 3);
if (close === -1) throw new Error('unterminated multi-line literal string');
return { value: text.slice(i + 3, close).replace(/^\r?\n/, ''), end: close + 3 };
}
const close = text.indexOf('\'', i + 1);
if (close === -1) throw new Error('unterminated literal string');
return { value: text.slice(i + 1, close), end: close + 1 };
}
// Boolean
if (text.startsWith('true', i) && !/[A-Za-z0-9_-]/.test(text[i + 4] || '')) {
return { value: true, end: i + 4 };
}
if (text.startsWith('false', i) && !/[A-Za-z0-9_-]/.test(text[i + 5] || '')) {
return { value: false, end: i + 5 };
}
// Inline array
if (ch === '[') {
const arr = [];
let j = i + 1;
while (true) {
while (j < text.length && /[\s\r\n]/.test(text[j])) j += 1;
if (j >= text.length) throw new Error('unterminated inline array');
if (text[j] === ']') return { value: arr, end: j + 1 };
if (text[j] === '#') {
const nl = text.indexOf('\n', j);
j = nl === -1 ? text.length : nl + 1;
continue;
}
const parsed = parseTomlValue(text, j);
arr.push(parsed.value);
j = parsed.end;
while (j < text.length && /[\s\r\n]/.test(text[j])) j += 1;
if (j < text.length && text[j] === ',') {
j += 1;
continue;
}
while (j < text.length && /[\s\r\n]/.test(text[j])) j += 1;
if (text[j] === ']') return { value: arr, end: j + 1 };
throw new Error(`expected , or ] in inline array at offset ${j}`);
}
}
// Inline table
if (ch === '{') {
const obj = {};
let j = i + 1;
while (true) {
while (j < text.length && /[\s\r\n]/.test(text[j])) j += 1;
if (text[j] === '}') return { value: obj, end: j + 1 };
const keyMatch = text.slice(j).match(/^([A-Za-z0-9_-]+|"[^"]*"|'[^']*')\s*=\s*/);
if (!keyMatch) throw new Error(`expected key in inline table at offset ${j}`);
let rawKey = keyMatch[1];
if ((rawKey.startsWith('"') && rawKey.endsWith('"')) || (rawKey.startsWith('\'') && rawKey.endsWith('\''))) {
rawKey = rawKey.slice(1, -1);
}
j += keyMatch[0].length;
const parsed = parseTomlValue(text, j);
obj[rawKey] = parsed.value;
j = parsed.end;
while (j < text.length && /[\s\r\n]/.test(text[j])) j += 1;
if (text[j] === ',') { j += 1; continue; }
if (text[j] === '}') return { value: obj, end: j + 1 };
throw new Error(`expected , or } in inline table at offset ${j}`);
}
}
// Number (integer with optional sign). Float / date / time / hex / oct / bin
// are NOT supported — we reject them explicitly instead of silently truncating
// an integer prefix off a `0.5` float or `1979-05-27` date. (#2760 CR4 finding 3)
const numMatch = text.slice(i).match(/^[+-]?\d[\d_]*/);
if (numMatch) {
const after = text[i + numMatch[0].length];
// Reject: float (`.`, `e`, `E`), date/time (`-`, `:`, `T`, `Z`), or any
// continuation digit/letter that suggests an unsupported numeric form.
if (after !== undefined && /[.eE:\-TZ]/.test(after)) {
throw new Error(
`unsupported TOML value at offset ${i}: floats, dates, and times are not supported (got ${text.slice(i, i + 20)})`
);
}
const digits = numMatch[0].replace(/_/g, '');
const n = Number(digits);
if (!Number.isFinite(n)) throw new Error(`invalid number: ${numMatch[0]}`);
return { value: n, end: i + numMatch[0].length };
}
throw new Error(`unsupported value at offset ${i}: ${text.slice(i, i + 20)}`);
}
/**
* Parse TOML content into a JavaScript object. Throws on malformed input.
*
* Handles `[table]`, `[[array.of.tables]]`, dotted key paths, and the value
* forms supported by parseTomlValue. Sufficient for validating Codex config
* structure and for behavioral test assertions in #2760 — not a general
* TOML implementation.
*/
function parseTomlToObject(content) {
const root = {};
const records = getTomlLineRecords(content);
// Tracks the *object* (not path) that subsequent key=value lines target.
let currentTable = root;
// #2760 CR5 finding 2 — track shape and definition status of every path so
// we can reject duplicate header redeclarations, shape mismatches, and
// duplicate keys per real TOML 1.0 semantics. Without this, walkPath
// silently reuses existing tables and assignment overwrites existing keys —
// a real TOML parser would refuse the file.
//
// pathShape: dotted path -> 'table' | 'array' | 'inline_parent' | 'key'
// - 'table' — declared via [a.b]
// - 'array' — declared via [[a.b]] (path is the array itself; each
// element is its own implicit table)
// - 'inline_parent' — created implicitly while walking parents
// - 'key' — assigned a scalar value
// declaredHeaders: set of dotted paths explicitly declared via [hdr] (not
// [[arr]]) — used to reject duplicate [a] / [a] sections.
// tableKeys: dotted-path -> Set<string> of keys assigned in that exact
// table instance. For [[arr]] elements we use a per-element marker.
const pathShape = new Map();
const declaredHeaders = new Set();
const tableKeys = new Map();
// currentTableId — string identifier for the current table instance, used
// as the key into tableKeys so that key uniqueness is per-table-instance
// (each [[arr]] element gets its own id).
let currentTableId = '__root__';
pathShape.set('__root__', 'table');
tableKeys.set('__root__', new Set());
function ensureKeySet(id) {
if (!tableKeys.has(id)) tableKeys.set(id, new Set());
return tableKeys.get(id);
}
function walkPath(segments, { creatingArrayElement = false } = {}) {
let node = root;
const parents = segments.slice(0, -1);
const last = segments[segments.length - 1];
for (let p = 0; p < parents.length; p += 1) {
const seg = parents[p];
const partialPath = parents.slice(0, p + 1).join('.');
if (node[seg] === undefined) {
node[seg] = {};
if (!pathShape.has(partialPath)) {
pathShape.set(partialPath, 'inline_parent');
}
} else if (Array.isArray(node[seg])) {
// Walk into the latest element of an array-of-tables.
node = node[seg][node[seg].length - 1];
continue;
} else if (typeof node[seg] !== 'object' || node[seg] === null) {
throw new Error(`path segment ${seg} is not a table`);
}
node = node[seg];
}
const fullPath = segments.join('.');
if (creatingArrayElement) {
const existingShape = pathShape.get(fullPath);
if (node[last] === undefined) {
node[last] = [];
pathShape.set(fullPath, 'array');
} else if (!Array.isArray(node[last])) {
throw new Error(
`duplicate or shape-mismatched table header at ${fullPath}: ` +
`cannot redefine as array of tables (previously seen as ${existingShape || 'table'})`
);
} else if (existingShape && existingShape !== 'array') {
throw new Error(
`duplicate or shape-mismatched table header at ${fullPath}: ` +
`previously seen as ${existingShape}, cannot extend as array of tables`
);
}
const elem = {};
node[last].push(elem);
const elemId = `${fullPath}[${node[last].length - 1}]`;
pathShape.set(elemId, 'array_element');
tableKeys.set(elemId, new Set());
currentTableId = elemId;
return elem;
}
// Plain [table] header.
if (node[last] === undefined) {
node[last] = {};
pathShape.set(fullPath, 'table');
declaredHeaders.add(fullPath);
tableKeys.set(fullPath, new Set());
} else if (Array.isArray(node[last])) {
throw new Error(
`duplicate or shape-mismatched table header at ${fullPath}: ` +
`previously declared as array of tables ([[${fullPath}]]), cannot redeclare as table ([${fullPath}])`
);
} else if (typeof node[last] !== 'object') {
throw new Error(`cannot redefine ${fullPath} as table`);
} else if (declaredHeaders.has(fullPath)) {
throw new Error(
`duplicate or shape-mismatched table header at ${fullPath}: ` +
`[${fullPath}] declared more than once`
);
} else {
// Implicitly created earlier (e.g., as a parent path); first explicit
// declaration is allowed.
pathShape.set(fullPath, 'table');
declaredHeaders.add(fullPath);
if (!tableKeys.has(fullPath)) tableKeys.set(fullPath, new Set());
}
currentTableId = fullPath;
return node[last];
}
for (let idx = 0; idx < records.length; idx += 1) {
const rec = records[idx];
if (rec.startsInMultilineString) continue;
if (rec.tableHeader) {
const segs = rec.tableHeader.segments;
currentTable = walkPath(segs, { creatingArrayElement: rec.tableHeader.array });
continue;
}
const trimmed = rec.text.trim();
if (trimmed === '' || trimmed.startsWith('#')) continue;
const equalsIndex = findTomlAssignmentEquals(rec.text);
if (equalsIndex === -1) continue;
const keyText = rec.text.slice(0, equalsIndex).trim();
const segments = parseTomlKeyPath(keyText);
if (!segments) {
throw new Error(`invalid TOML key on line ${idx + 1}: ${rec.text}`);
}
// Value RHS may span multiple lines (inline arrays, multi-line strings,
// inline tables). Parse from the absolute content offset right after `=`.
const valueStartAbs = rec.start + equalsIndex + 1;
const parsed = parseTomlValue(content, valueStartAbs);
// #2760 CR4 finding 3 — verify the full RHS was consumed. Anything other
// than whitespace + optional # comment between parsed.end and the next
// newline (or EOF) means the parser silently accepted a prefix and
// dropped trailing bytes. Reject so malformed TOML cannot slip past
// "parse before commit" guarantees.
let scan = parsed.end;
while (scan < content.length && (content[scan] === ' ' || content[scan] === '\t')) {
scan += 1;
}
if (scan < content.length && content[scan] !== '\n' && content[scan] !== '\r' && content[scan] !== '#') {
const lineEnd = content.indexOf('\n', scan);
const trailing = content.slice(scan, lineEnd === -1 ? content.length : lineEnd);
throw new Error(
`trailing bytes after value on line ${idx + 1}: ${JSON.stringify(trailing)}`
);
}
// Place value into currentTable under dotted key.
// #2760 CR5 finding 2 — reject duplicate keys per real TOML 1.0. Track
// the dotted key against the current table instance id; an exact repeat
// throws.
let target = currentTable;
for (let s = 0; s < segments.length - 1; s += 1) {
const seg = segments[s];
if (target[seg] === undefined) target[seg] = {};
else if (typeof target[seg] !== 'object' || Array.isArray(target[seg])) {
throw new Error(`cannot descend into non-table key ${seg}`);
}
target = target[seg];
}
const finalKey = segments[segments.length - 1];
const dottedKey = segments.join('.');
const keySet = ensureKeySet(currentTableId);
if (keySet.has(dottedKey) || Object.prototype.hasOwnProperty.call(target, finalKey)) {
throw new Error(
`duplicate key ${dottedKey} in ${currentTableId === '__root__' ? 'root table' : currentTableId}`
);
}
keySet.add(dottedKey);
target[finalKey] = parsed.value;
}
return root;
}
/**
* Validate that the post-install config.toml matches Codex's expected schema
* (#2760, fix 3). Returns { ok: true } on success, or { ok: false, reason }
* with a human-readable explanation of the offending section.
*
* Strategy: parse the bytes into a structured object first — malformed TOML
* fails validation immediately rather than slipping past a header-only scan.
* Then enforce the schema-shape rules against the parsed structure.
*
* Schema rules enforced:
* - File MUST parse as TOML (no syntax errors).
* - `agents` MUST be a struct table (`[agents.<name>]`) — never a bare
* table value or an array of tables.
* - `hooks.<Event>` MUST be an array of tables when present (Codex ≥0.124
* rejects bare `[hooks.<Event>]` single-bracket maps).
*/
function validateCodexConfigSchema(content) {
let parsed;
try {
parsed = parseTomlToObject(content);
} catch (e) {
return {
ok: false,
reason: `TOML parse failed: ${e.message}`,
};
}
// Header-shape check: arrays-of-tables are visible in the parsed structure
// (as Array values) but bare-vs-struct distinction for `[agents]` requires
// looking at section headers too — `[agents]` with `default = "x"` parses
// to `{ agents: { default: 'x' } }`, indistinguishable from
// `[agents.foo]` writing into the same shape. Use header sections to
// disambiguate.
const sections = getTomlTableSections(content);
for (const section of sections) {
if (section.array && section.path === 'agents') {
return {
ok: false,
reason: '[[agents]] sequence form is invalid in current Codex schema (expected [agents.<name>] struct form)',
};
}
if (!section.array && section.path === 'agents') {
return {
ok: false,
reason: 'bare [agents] table is invalid in current Codex schema (expected [agents.<name>] struct form)',
};
}
if (!section.array && section.path.startsWith('hooks.')) {
return {
ok: false,
reason: `bare [${section.path}] table is invalid in current Codex schema (expected [[${section.path}]] array-of-tables)`,
};
}
}
// Structural confirmation against parsed object: any present hooks.<Event>
// must be an array.
if (parsed.hooks && typeof parsed.hooks === 'object' && !Array.isArray(parsed.hooks)) {
for (const [event, value] of Object.entries(parsed.hooks)) {
if (!Array.isArray(value)) {
return {
ok: false,
reason: `hooks.${event} must be an array of tables, got ${typeof value}`,
};
}
}
}
return { ok: true };
}
function normalizeCodexHooksLine(line, key) {
const leadingWhitespace = line.match(/^\s*/)[0];
const commentStart = findTomlCommentStart(line);
@@ -3092,14 +3552,38 @@ function rewriteTomlKeyLines(content, matches, key) {
return rewritten;
}
/**
* Atomic write — write to <target>.tmp-<pid>-<n> first, then renameSync over
* the target. Eliminates the partial-write corruption window: an interrupted
* write leaves the temp file (which we clean up) but never truncates the
* original target. Used for any mutation of Codex config.toml so we cannot
* leave the user with a half-written file (#2760 fix 4).
*/
let __atomicWriteCounter = 0;
function atomicWriteFileSync(target, data, options) {
__atomicWriteCounter += 1;
const tmp = `${target}.tmp-${process.pid}-${__atomicWriteCounter}`;
try {
fs.writeFileSync(tmp, data, options);
fs.renameSync(tmp, target);
} catch (e) {
// Best-effort cleanup of the partial temp file; never mask the real error.
try { fs.rmSync(tmp, { force: true }); } catch (_) { /* ignore */ }
throw e;
}
}
/**
* Merge GSD config block into an existing or new config.toml.
* Three cases: new file, existing with GSD marker, existing without marker.
*
* All writes go through atomicWriteFileSync so a mid-write failure leaves
* the original config.toml untouched (#2760 fix 4).
*/
function mergeCodexConfig(configPath, gsdBlock) {
// Case 1: No config.toml — create fresh
if (!fs.existsSync(configPath)) {
fs.writeFileSync(configPath, gsdBlock + '\n');
atomicWriteFileSync(configPath, gsdBlock + '\n');
return;
}
@@ -3115,9 +3599,9 @@ function mergeCodexConfig(configPath, gsdBlock) {
// Strip any GSD-managed sections that leaked above the marker from previous installs
before = stripLeakedGsdCodexSections(before).trimEnd();
fs.writeFileSync(configPath, before + eol + eol + normalizedGsdBlock + eol);
atomicWriteFileSync(configPath, before + eol + eol + normalizedGsdBlock + eol);
} else {
fs.writeFileSync(configPath, normalizedGsdBlock + eol);
atomicWriteFileSync(configPath, normalizedGsdBlock + eol);
}
return;
}
@@ -3130,7 +3614,7 @@ function mergeCodexConfig(configPath, gsdBlock) {
content = normalizedGsdBlock + eol;
}
fs.writeFileSync(configPath, content);
atomicWriteFileSync(configPath, content);
}
/**
@@ -6412,9 +6896,36 @@ function install(isGlobal, runtime = 'claude') {
}
if (isCodex && !isMinimalMode(installMode)) {
// Generate Codex config.toml and per-agent .toml files.
// Skipped under --minimal — same rationale as filesystem agents above.
const agentCount = installCodexConfig(targetDir, agentsSrc);
// Capture pre-install snapshot of config.toml before ANY GSD mutation
// (#2760 fix 3). On post-write schema-validation failure OR any throw
// during the mutation sequence (write failure, merge throw, etc.) we
// restore these exact bytes so the user is never left with a broken
// Codex CLI (#2760 fix 4 — extends snapshot coverage to write-failure
// paths, paired with atomic temp-file writes in mergeCodexConfig and
// the final hooks-write below).
const codexConfigPathPreInstall = path.join(targetDir, 'config.toml');
const codexConfigPreInstallSnapshot = fs.existsSync(codexConfigPathPreInstall)
? fs.readFileSync(codexConfigPathPreInstall)
: null;
const restoreCodexSnapshot = () => {
if (codexConfigPreInstallSnapshot !== null) {
try { fs.writeFileSync(codexConfigPathPreInstall, codexConfigPreInstallSnapshot); }
catch (_) { /* best-effort restore — surface the original error */ }
} else if (fs.existsSync(codexConfigPathPreInstall)) {
try { fs.rmSync(codexConfigPathPreInstall); } catch (_) { /* best-effort */ }
}
};
let agentCount;
try {
// Generate Codex config.toml and per-agent .toml files.
// Skipped under --minimal — same rationale as filesystem agents above.
agentCount = installCodexConfig(targetDir, agentsSrc);
} catch (e) {
restoreCodexSnapshot();
throw e;
}
console.log(` ${green}✓${reset} Generated config.toml with ${agentCount} agent roles`);
console.log(` ${green}✓${reset} Generated ${agentCount} agent .toml config files`);
@@ -6454,6 +6965,10 @@ function install(isGlobal, runtime = 'claude') {
// Add Codex hooks (SessionStart for update checking) — requires codex_hooks feature flag
const configPath = path.join(targetDir, 'config.toml');
// Use the pre-install snapshot captured before installCodexConfig ran so
// restore returns the file to its true pre-GSD state on validation
// failure (#2760 fix 3) — not to the post-agent-merge state.
const preWriteBackup = codexConfigPreInstallSnapshot;
try {
let configContent = fs.existsSync(configPath) ? fs.readFileSync(configPath, 'utf-8') : '';
const eol = detectLineEnding(configContent);
@@ -6470,13 +6985,22 @@ function install(isGlobal, runtime = 'claude') {
const codexHooksFeature = ensureCodexHooksFeature(configContent);
configContent = setManagedCodexHooksOwnership(codexHooksFeature.content, codexHooksFeature.ownership);
// Add SessionStart hook for update checking
// Add SessionStart hook for update checking. Default to top-level
// `[[hooks]]` AoT with `event` field — the form GSD has emitted since
// the Codex 0.124 migration (#2637). When the user already uses the
// namespaced AoT form `[[hooks.SessionStart]]` for their own hooks,
// emit our managed entry in that same shape so the two forms don't
// collide on round-trip (#2760, defect 3).
const updateCheckScript = path.resolve(targetDir, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/');
const hookBlock =
`${eol}# GSD Hooks${eol}` +
`[[hooks]]${eol}` +
`event = "SessionStart"${eol}` +
`command = "node ${updateCheckScript}"${eol}`;
const useNamespacedAot = hasUserNamespacedAotHooks(configContent, 'SessionStart');
const hookBlock = useNamespacedAot
? `${eol}# GSD Hooks${eol}` +
`[[hooks.SessionStart]]${eol}` +
`command = "node ${updateCheckScript}"${eol}`
: `${eol}# GSD Hooks${eol}` +
`[[hooks]]${eol}` +
`event = "SessionStart"${eol}` +
`command = "node ${updateCheckScript}"${eol}`;
// Migrate legacy gsd-update-check entries from prior installs (#1755 followup)
// Remove stale hook blocks that used the inverted filename or wrong path.
@@ -6488,14 +7012,89 @@ function install(isGlobal, runtime = 'claude') {
);
}
// #2760 CR4 finding 2 — Strip ALL existing managed gsd-check-update
// hook blocks (top-level [[hooks]] AND namespaced [[hooks.SessionStart]])
// BEFORE evaluating the includes guard. Without this, an install that
// already has a legacy flat [[hooks]] block short-circuits the new
// namespaced AoT emit and stays stuck in the mixed layout this fix is
// designed to eliminate. Stripping first means every install converges
// on the right shape regardless of prior state.
if (configContent.includes('gsd-check-update')) {
configContent = configContent.replace(
/(?:\r?\n|^)# GSD Hooks\r?\n\[\[hooks\]\]\r?\nevent = "SessionStart"\r?\ncommand = "node [^\r\n]*gsd-check-update\.js"\r?\n/gm,
(match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''),
);
configContent = configContent.replace(
/(?:\r?\n|^)# GSD Hooks\r?\n\[\[hooks\.SessionStart\]\]\r?\ncommand = "node [^\r\n]*gsd-check-update\.js"\r?\n/gm,
(match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''),
);
}
if (hasEnabledCodexHooksFeature(configContent) && !configContent.includes('gsd-check-update')) {
configContent += hookBlock;
}
fs.writeFileSync(configPath, configContent, 'utf-8');
// #2760 fix 3 — post-write schema validation. Parse the bytes we are
// about to commit and assert they match Codex's expected shape. If
// validation fails we restore the pre-install backup and abort so the
// user is never left with a Codex CLI that won't load.
// Test seam: tests can inject `__codexSchemaValidator` to force the
// validator to fail and exercise the restore-and-abort path.
const validatorFn = (typeof module !== 'undefined' && module.exports && module.exports.__codexSchemaValidator)
? module.exports.__codexSchemaValidator
: validateCodexConfigSchema;
const validation = validatorFn(configContent);
if (!validation.ok) {
restoreCodexSnapshot();
throw new Error(
`post-write Codex schema validation failed: ${validation.reason}. ` +
`Restored ${preWriteBackup !== null ? 'pre-install backup' : 'empty state'}.`
);
}
// Atomic write (#2760 fix 4) — write to a sibling temp file, then
// renameSync over the target. A mid-write failure cannot truncate the
// existing config; the snapshot restore below is a second line of
// defense if even the rename fails.
try {
atomicWriteFileSync(configPath, configContent, 'utf-8');
} catch (writeErr) {
// #2760 CR4 finding 1 — write failure must be loud and fatal. Wrap
// with a `post-write` prefix the outer catch recognises so install
// aborts with a clear error rather than warn-and-continue (which
// produced "Done!" with no Codex agents configured).
restoreCodexSnapshot();
const wrapped = new Error(
`post-write Codex install failed: ${writeErr && writeErr.message ? writeErr.message : String(writeErr)}. ` +
`Restored ${preWriteBackup !== null ? 'pre-install backup' : 'empty state'}.`
);
throw wrapped;
}
console.log(` ${green}✓${reset} Configured Codex hooks (SessionStart)`);
} catch (e) {
console.warn(` ${yellow}⚠${reset} Could not configure Codex hooks: ${e.message}`);
// #2760 — schema-validation and write failures must be loud and fatal
// so the user is never left with a config Codex refuses to load (or no
// Codex agents configured at all). The pre-install snapshot restore has
// already run for write-side throws via the inner catch above and via
// restoreCodexSnapshot in the validation branch.
if (e && typeof e.message === 'string' && e.message.startsWith('post-write')) {
console.error(` ${red}✗${reset} ${e.message}`);
throw e;
}
// #2760 CR5 finding 1 — pre-write failures (migrateCodexHooksMapFormat,
// ensureCodexHooksFeature, config reads, configContent construction,
// etc.) must ALSO be fatal. Previously this branch downgraded to a
// console.warn, leaving the install to print "Done!" with no Codex
// hooks configured — same defect class as finding 1, different layer.
// Restore the pre-install snapshot and rethrow so the outer install
// pipeline aborts.
restoreCodexSnapshot();
const wrapped = new Error(
`Codex hook configuration failed (pre-write): ${e && e.message ? e.message : String(e)}. ` +
`Restored ${preWriteBackup !== null ? 'pre-install backup' : 'empty state'}.`
);
console.error(` ${red}✗${reset} ${wrapped.message}`);
throw wrapped;
}
return { settingsPath: null, settings: null, statuslineCommand: null, runtime, configDir: targetDir };
@@ -7638,6 +8237,9 @@ if (process.env.GSD_TEST_MODE) {
generateCodexConfigBlock,
stripGsdFromCodexConfig,
migrateCodexHooksMapFormat,
hasUserNamespacedAotHooks,
parseTomlToObject,
validateCodexConfigSchema,
mergeCodexConfig,
installCodexConfig,
readGsdRuntimeProfileResolver,

978
tests/bug-2760-codex-install-defensive.test.cjs Normal file
View File

@@ -0,0 +1,978 @@
/**
* Regression: issue #2760 — Codex install path corrupts existing config.toml.
*
* Three defects, three fixes (defensive triple):
*
* Defect 3 (confirmed real) — Hooks AoT downgrade. When the user already has
* `[[hooks.SessionStart]]` (namespaced AoT) entries in their config, GSD
* used to append a `[[hooks]]` (top-level AoT) block that confuses
* round-trip writers and produces a config Codex refuses to load.
* Fix: detect the user's preferred shape and emit GSD's hook in the same
* namespaced form so both coexist cleanly.
*
* Defects 1+2 (defensive) — Strip-step robustness. Pre-existing legacy
* `[agents]` (single-bracket) and `[[agents]]` (sequence) blocks are
* invalid in current Codex schema and break Codex even though GSD now
* emits the correct `[agents.<name>]` struct form. Fix: install-time
* stripping always purges these forms regardless of GSD marker presence
* so reinstall self-heals files where the marker was edited out or never
* existed (third-party tools).
*
* Fix 3 (defensive) — Post-write validation. Parse the bytes we are about
* to commit, assert they match Codex's expected schema (no bare/sequence
* `agents`, no bare `hooks.<Event>`); on failure, restore the pre-install
* backup and abort so the user never gets a broken Codex CLI.
*/
// Scope GSD_TEST_MODE to module load only — restore prior value (or unset) so
// downstream tests in the same node process never see test-only behaviour
// leak through (#2760 CR4 finding 5).
const previousGsdTestMode = process.env.GSD_TEST_MODE;
process.env.GSD_TEST_MODE = '1';
const { test, describe, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const os = require('os');
const {
install,
validateCodexConfigSchema,
hasUserNamespacedAotHooks,
stripGsdFromCodexConfig,
installCodexConfig,
parseTomlToObject,
} = require('../bin/install.js');
if (previousGsdTestMode === undefined) {
delete process.env.GSD_TEST_MODE;
} else {
process.env.GSD_TEST_MODE = previousGsdTestMode;
}
function runCodexInstall(codexHome, cwd = path.join(__dirname, '..')) {
const previousCodeHome = process.env.CODEX_HOME;
const previousCwd = process.cwd();
process.env.CODEX_HOME = codexHome;
try {
process.chdir(cwd);
return install(true, 'codex');
} finally {
process.chdir(previousCwd);
if (previousCodeHome === undefined) {
delete process.env.CODEX_HOME;
} else {
process.env.CODEX_HOME = previousCodeHome;
}
}
}
function readCodexConfig(codexHome) {
return fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8');
}
function writeCodexConfig(codexHome, content) {
fs.mkdirSync(codexHome, { recursive: true });
fs.writeFileSync(path.join(codexHome, 'config.toml'), content, 'utf8');
}
describe('#2760 defect 3 — Hooks AoT preservation across install/uninstall/reinstall', () => {
let tmpDir;
let codexHome;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-d3-'));
codexHome = path.join(tmpDir, 'codex-home');
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('preserves both pre-existing [[hooks.SessionStart]] entries and adds GSD entry in namespaced form', () => {
const userConfig = [
'[[hooks.SessionStart]]',
'command = "echo first user hook"',
'',
'[[hooks.SessionStart]]',
'command = "echo second user hook"',
'',
].join('\n');
writeCodexConfig(codexHome, userConfig);
runCodexInstall(codexHome);
const afterInstall = readCodexConfig(codexHome);
const parsed = parseTomlToObject(afterInstall);
// hooks.SessionStart must be an array-of-tables (namespaced AoT form).
assert.ok(
parsed.hooks && Array.isArray(parsed.hooks.SessionStart),
'hooks.SessionStart must be an array-of-tables, got: '
+ (parsed.hooks ? typeof parsed.hooks.SessionStart : 'no hooks table')
);
const commands = parsed.hooks.SessionStart.map((entry) => entry.command);
// Both pre-existing user hook entries survive in the parsed structure.
assert.ok(
commands.includes('echo first user hook'),
'first user [[hooks.SessionStart]] entry preserved in parsed structure: ' + JSON.stringify(commands)
);
assert.ok(
commands.includes('echo second user hook'),
'second user [[hooks.SessionStart]] entry preserved in parsed structure: ' + JSON.stringify(commands)
);
// GSD's managed entry is emitted in the same namespaced AoT shape so it
// does not collide with the user's preferred form.
assert.ok(
commands.some((cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)),
'GSD entry must appear in hooks.SessionStart array (not top-level [[hooks]]): '
+ JSON.stringify(commands)
);
// Top-level [[hooks]] AoT must not coexist when namespaced form is in use —
// mixing forms is what produces the round-trip break this fix prevents.
assert.ok(
!Array.isArray(parsed.hooks) || parsed.hooks.length === 0,
'no top-level [[hooks]] AoT entries when namespaced form is in use'
);
});
test('selects top-level [[hooks]] form when user has no namespaced hooks (status-quo behavior)', () => {
writeCodexConfig(codexHome, '');
runCodexInstall(codexHome);
const content = readCodexConfig(codexHome);
const parsed = parseTomlToObject(content);
// Top-level hooks must be an array-of-tables; the GSD entry must be one
// of those tables and carry event = "SessionStart".
assert.ok(
Array.isArray(parsed.hooks),
'fresh install must produce top-level [[hooks]] AoT, got: ' + typeof parsed.hooks
);
assert.ok(
parsed.hooks.some((h) => h && h.event === 'SessionStart'),
'top-level [[hooks]] AoT must contain an entry with event = "SessionStart": '
+ JSON.stringify(parsed.hooks)
);
});
});
describe('#2760 fix 2 — Strip purges invalid legacy [agents] / [[agents]] regardless of marker', () => {
let tmpDir;
let codexHome;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-f2-'));
codexHome = path.join(tmpDir, 'codex-home');
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('strips bare [agents] single-bracket block (no GSD marker, arbitrary user keys)', () => {
writeCodexConfig(codexHome, [
'[agents]',
'default = "custom-agent"',
'extra_key = "value"',
'',
'[model]',
'name = "o3"',
'',
].join('\n'));
runCodexInstall(codexHome);
const content = readCodexConfig(codexHome);
const parsed = parseTomlToObject(content);
// Bare [agents] would have left { default, extra_key } as scalar leaves
// on parsed.agents. After strip + struct emit, every key under agents
// must itself be a table (the gsd-* struct form).
assert.ok(
parsed.agents && typeof parsed.agents === 'object' && !Array.isArray(parsed.agents),
'agents must be a table-of-tables in parsed structure, got: ' + typeof parsed.agents
);
assert.equal(parsed.agents.default, undefined, 'bare [agents] default key must be stripped');
assert.equal(parsed.agents.extra_key, undefined, 'bare [agents] extra_key must be stripped');
const gsdAgents = Object.keys(parsed.agents).filter((k) => k.startsWith('gsd-'));
assert.ok(
gsdAgents.length > 0 && gsdAgents.every((k) => typeof parsed.agents[k] === 'object'),
'agents.gsd-* struct form must be present: ' + JSON.stringify(Object.keys(parsed.agents))
);
// User's unrelated [model] section preserved structurally.
assert.ok(
parsed.model && parsed.model.name === 'o3',
'unrelated user [model] section preserved with name = "o3", got: ' + JSON.stringify(parsed.model)
);
});
test('strips [[agents]] sequence-form block without GSD marker (third-party / marker-edited-out)', () => {
writeCodexConfig(codexHome, [
'[[agents]]',
'name = "user-helper"',
'description = "third-party agent"',
'',
'[[agents]]',
'name = "another-helper"',
'description = "second one"',
'',
'[projects."/tmp/x"]',
'trust_level = "trusted"',
'',
].join('\n'));
runCodexInstall(codexHome);
const content = readCodexConfig(codexHome);
const parsed = parseTomlToObject(content);
// [[agents]] sequence form would parse to Array — after strip it must be
// a table-of-tables with gsd-* struct keys.
assert.ok(
parsed.agents && typeof parsed.agents === 'object' && !Array.isArray(parsed.agents),
'agents must be a table-of-tables in parsed structure (sequence form must be stripped), got: '
+ (Array.isArray(parsed.agents) ? 'array' : typeof parsed.agents)
);
const gsdAgents = Object.keys(parsed.agents).filter((k) => k.startsWith('gsd-'));
assert.ok(
gsdAgents.length > 0,
'agents.gsd-* struct form must be present: ' + JSON.stringify(Object.keys(parsed.agents))
);
// User's unrelated [projects."/tmp/x"] section preserved structurally.
assert.ok(
parsed.projects && parsed.projects['/tmp/x'] && parsed.projects['/tmp/x'].trust_level === 'trusted',
'unrelated user [projects."/tmp/x"] section preserved with trust_level = "trusted", got: '
+ JSON.stringify(parsed.projects)
);
});
});
// concurrency: false — the third test mutates installModule.__codexSchemaValidator,
// a module-level test seam. Other tests in this file (and in bug-2153, etc.)
// also call runCodexInstall() and would observe the injected validator if
// node:test ran them in parallel. Serializing this describe block keeps the
// seam mutation invisible to siblings.
describe('#2760 fix 3 — Post-write Codex schema validation', { concurrency: false }, () => {
test('passes a clean config produced by GSD install', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-f3a-'));
try {
const codexHome = path.join(tmpDir, 'codex-home');
runCodexInstall(codexHome);
const content = readCodexConfig(codexHome);
const result = validateCodexConfigSchema(content);
assert.equal(result.ok, true, 'GSD-emitted config passes schema validation');
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
test('rejects bare [agents] and bare [hooks.SessionStart] in arbitrary content', () => {
const bareAgents = [
'[agents]',
'default = "x"',
'',
].join('\n');
const bareHooks = [
'[hooks.SessionStart]',
'command = "x"',
'',
].join('\n');
const sequenceAgents = [
'[[agents]]',
'name = "x"',
'',
].join('\n');
assert.equal(validateCodexConfigSchema(bareAgents).ok, false, 'bare [agents] rejected');
assert.equal(validateCodexConfigSchema(bareHooks).ok, false, 'bare [hooks.SessionStart] rejected');
assert.equal(validateCodexConfigSchema(sequenceAgents).ok, false, '[[agents]] sequence rejected');
});
test('aborts install and restores pre-install backup when post-write validation fails', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-f3b-'));
const installModule = require('../bin/install.js');
try {
const codexHome = path.join(tmpDir, 'codex-home');
// Pre-install file the user wants protected.
const preInstall = [
'# user file',
'[model]',
'name = "o3"',
'',
].join('\n');
writeCodexConfig(codexHome, preInstall);
// Force the post-write validator to fail via the documented test seam.
// This simulates the writer producing legacy-form output that Codex
// would reject — install MUST abort, restore the pre-install bytes,
// and surface a clear error.
installModule.__codexSchemaValidator = () => ({
ok: false,
reason: 'simulated invalid output for test',
});
let threw = false;
try {
runCodexInstall(codexHome);
} catch (e) {
threw = true;
assert.match(
e.message,
/post-write Codex schema validation failed/,
'thrown error names the validation failure'
);
assert.match(e.message, /simulated invalid output for test/, 'thrown error includes reason');
}
assert.equal(threw, true, 'install threw when validator failed');
const afterInstall = fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8');
assert.equal(
afterInstall,
preInstall,
'pre-install file restored verbatim after validation failure'
);
} finally {
delete installModule.__codexSchemaValidator;
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
});
describe('#2760 — hasUserNamespacedAotHooks helper', () => {
test('detects [[hooks.SessionStart]] AoT entries', () => {
const content = [
'[[hooks.SessionStart]]',
'command = "x"',
'',
].join('\n');
assert.equal(hasUserNamespacedAotHooks(content, 'SessionStart'), true);
});
test('returns false when only top-level [[hooks]] entries exist', () => {
const content = [
'[[hooks]]',
'event = "SessionStart"',
'command = "x"',
'',
].join('\n');
assert.equal(hasUserNamespacedAotHooks(content, 'SessionStart'), false);
});
test('returns false when only single-bracket [hooks.SessionStart] exists', () => {
const content = [
'[hooks.SessionStart]',
'command = "x"',
'',
].join('\n');
assert.equal(hasUserNamespacedAotHooks(content, 'SessionStart'), false);
});
});
// concurrency: false — these tests monkey-patch fs.writeFileSync, a global
// shared with every other suite running in parallel. Serializing prevents
// stray writes from sibling tests landing in the stub.
describe('#2760 fix 4 — Write-failure rollback (atomic write + snapshot restore)', { concurrency: false }, () => {
let tmpDir;
let codexHome;
let originalWriteFileSync;
// #2760 CR5 finding 5 — symmetric snapshot/restore for fs.renameSync. The
// first test below monkey-patches renameSync; without a beforeEach/afterEach
// pair, only the local `finally` restores it, which is fragile to future
// edits that add early-return paths.
let originalRenameSync;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-f4-'));
codexHome = path.join(tmpDir, 'codex-home');
originalWriteFileSync = fs.writeFileSync;
originalRenameSync = fs.renameSync;
});
afterEach(() => {
fs.renameSync = originalRenameSync;
fs.writeFileSync = originalWriteFileSync;
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('pre-install config bytes survive when fs.renameSync throws over configPath', () => {
const preInstall = [
'# user file',
'[model]',
'name = "o3"',
'',
].join('\n');
writeCodexConfig(codexHome, preInstall);
// After fs is restored we'll re-read the file. Capture the byte buffer
// exactly so the comparison is bit-for-bit.
const preInstallBytes = fs.readFileSync(path.join(codexHome, 'config.toml'));
const configPath = path.join(codexHome, 'config.toml');
const tempPattern = new RegExp('^' + configPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '\\.tmp-');
// Stub: allow writes to atomic temp files (which renameSync overwrites
// the target, never truncating it directly) but throw on any direct
// write to the canonical configPath. This simulates either:
// (a) an older code path doing a non-atomic write, or
// (b) a downstream module bypassing atomicWriteFileSync.
// Either way the snapshot must be restored. We let the temp write go
// through, then make renameSync throw to simulate the partial write
// never landing.
// #2760 CR5 finding 5 — fs.renameSync is restored by the suite-level
// afterEach; no local finally needed.
fs.renameSync = (src, dst) => {
if (dst === configPath) {
throw new Error('simulated rename failure mid-install');
}
return originalRenameSync(src, dst);
};
let threw = false;
let thrownErr = null;
try {
runCodexInstall(codexHome);
} catch (e) {
threw = true;
thrownErr = e;
assert.ok(/rename failure|simulated|post-write/.test(e.message),
'thrown error must surface the simulated failure or its post-write wrapper: ' + e.message);
}
// #2760 CR5 finding 4 — tighten contract per finding #1: ALL pre-write
// and write failures must be fatal. This test previously accepted either
// throw OR warn — sibling tests already require throw, so lock parity.
assert.equal(threw, true, 'rename failure must be fatal: ' + (thrownErr && thrownErr.message));
const afterBytes = fs.readFileSync(path.join(codexHome, 'config.toml'));
assert.deepStrictEqual(
afterBytes,
preInstallBytes,
'pre-install config.toml bytes must survive a mid-install write/rename failure'
);
// And the parsed structure of the surviving file must still be the
// user's [model] section, not a half-written GSD block.
const parsed = parseTomlToObject(afterBytes.toString('utf8'));
assert.equal(parsed.model && parsed.model.name, 'o3',
'surviving file must still be the user pre-install content');
assert.equal(parsed.agents, undefined,
'no GSD agents block may have leaked into the surviving file');
// No stray .tmp-* siblings left behind in the codex home.
const stray = fs.readdirSync(codexHome).filter((f) => tempPattern.test(path.join(codexHome, f)));
assert.equal(stray.length, 0,
'atomic write must clean up its temp file on failure: ' + stray.join(', '));
});
test('pre-install config bytes survive when fs.writeFileSync throws on the .tmp- target', () => {
const preInstall = [
'# user file',
'[model]',
'name = "o3"',
'',
].join('\n');
writeCodexConfig(codexHome, preInstall);
const preInstallBytes = fs.readFileSync(path.join(codexHome, 'config.toml'));
const configPath = path.join(codexHome, 'config.toml');
const tempPattern = new RegExp('^' + configPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '\\.tmp-');
// Stub: fault writes targeting the atomic temp file (the pre-rename branch
// of atomicWriteFileSync). Other writes (agent .toml files in CODEX_HOME)
// pass through. This exercises the failure path where the temp write itself
// throws, not the rename — the case the prior test left untested.
// #2760 CR5 finding 5 — fs.writeFileSync is restored by the suite-level
// afterEach (via originalWriteFileSync); no local finally needed.
const captured = originalWriteFileSync;
fs.writeFileSync = function patchedWriteFileSync(target, data, options) {
if (typeof target === 'string' && tempPattern.test(target)) {
throw new Error('simulated writeFileSync failure on .tmp- target');
}
return captured.call(this, target, data, options);
};
let threw = false;
try {
runCodexInstall(codexHome);
} catch (e) {
threw = true;
assert.ok(/simulated writeFileSync failure|post-write Codex install failed|pre-write/.test(e.message),
'thrown error must surface the simulated failure or its post-write wrapper: ' + e.message);
}
// Per #2760 CR4 finding 1 / CR5 finding 1, write failures must abort install (not warn).
assert.equal(threw, true, 'install must throw when atomic temp-write fails');
const afterBytes = fs.readFileSync(path.join(codexHome, 'config.toml'));
assert.deepStrictEqual(
afterBytes,
preInstallBytes,
'pre-install config.toml bytes must survive a temp-write failure'
);
const parsed = parseTomlToObject(afterBytes.toString('utf8'));
assert.equal(parsed.model && parsed.model.name, 'o3',
'surviving file must still be the user pre-install content');
assert.equal(parsed.agents, undefined,
'no GSD agents block may have leaked into the surviving file');
const stray = fs.readdirSync(codexHome).filter((f) => tempPattern.test(path.join(codexHome, f)));
assert.equal(stray.length, 0,
'atomic write must clean up its temp file on failure: ' + stray.join(', '));
});
});
// concurrency: false — these tests rely on the same install path and module-
// level pre-install snapshot that the fix-3/fix-4 suites exercise. Serializing
// keeps state mutations from leaking across parallel siblings.
describe('#2760 CR4 finding 2 — Legacy flat [[hooks]] block migrates to namespaced AoT on reinstall', { concurrency: false }, () => {
let tmpDir;
let codexHome;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-cr4-f2-'));
codexHome = path.join(tmpDir, 'codex-home');
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('pre-install legacy flat [[hooks]] gsd-check-update + user namespaced [[hooks.SessionStart]] → post-install converges on namespaced AoT', () => {
// Reproduce the upgrade scenario:
// - User has [[hooks.SessionStart]] entry of their own (signal that GSD
// should emit in the namespaced shape).
// - A previous GSD install left the legacy flat [[hooks]] managed block
// for gsd-check-update. The pre-CR4 strip step would short-circuit
// the namespaced emit and leave the user stuck in the mixed layout.
const userPlusLegacy = [
'[[hooks.SessionStart]]',
'command = "echo user hook"',
'',
'# GSD Hooks',
'[[hooks]]',
'event = "SessionStart"',
'command = "node /old/path/hooks/gsd-check-update.js"',
'',
].join('\n');
writeCodexConfig(codexHome, userPlusLegacy);
runCodexInstall(codexHome);
const afterInstall = readCodexConfig(codexHome);
const parsed = parseTomlToObject(afterInstall);
// After CR4 finding 2: the legacy flat [[hooks]] managed block is stripped
// and the GSD entry is re-emitted in the namespaced AoT shape so the two
// forms do not coexist.
assert.ok(
parsed.hooks && Array.isArray(parsed.hooks.SessionStart),
'hooks.SessionStart must be an array-of-tables, got: '
+ (parsed.hooks ? typeof parsed.hooks.SessionStart : 'no hooks table')
);
const namespacedCommands = parsed.hooks.SessionStart.map((entry) => entry.command);
assert.ok(
namespacedCommands.includes('echo user hook'),
'user [[hooks.SessionStart]] entry preserved: ' + JSON.stringify(namespacedCommands)
);
assert.ok(
namespacedCommands.some((cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)),
'GSD entry must appear in hooks.SessionStart array (namespaced AoT form): '
+ JSON.stringify(namespacedCommands)
);
// The legacy top-level [[hooks]] AoT must NOT coexist with the namespaced
// form after migration. parseTomlToObject distinguishes via Array.isArray.
assert.ok(
!Array.isArray(parsed.hooks) || parsed.hooks.length === 0,
'no top-level [[hooks]] AoT entries may remain after legacy migration: '
+ JSON.stringify(parsed.hooks)
);
// No duplicate gsd-check-update entries — exactly one managed entry.
const gsdEntries = namespacedCommands.filter(
(cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)
);
assert.equal(gsdEntries.length, 1,
'exactly one gsd-check-update entry after migration, got: ' + gsdEntries.length);
});
});
describe('#2760 CR4 finding 3 — parseTomlToObject rejects malformed input that previously slipped through', () => {
test('rejects float values (timeout = 0.5)', () => {
const content = [
'[server]',
'timeout = 0.5',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/unsupported TOML value|trailing bytes/,
'float values must be rejected, not silently truncated to int prefix'
);
});
test('rejects date values (created = 1979-05-27)', () => {
const content = [
'[meta]',
'created = 1979-05-27',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/unsupported TOML value|trailing bytes/,
'date values must be rejected, not silently truncated'
);
});
test('rejects trailing garbage after a string value (key = "x" junk)', () => {
const content = [
'[section]',
'key = "x" junk',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/trailing bytes/,
'trailing bytes after a complete value must be rejected'
);
});
test('accepts trailing whitespace and # comment after a value', () => {
const content = [
'[section]',
'key = "x" # an inline comment',
'flag = true',
'count = 7 ',
'',
].join('\n');
const parsed = parseTomlToObject(content);
assert.equal(parsed.section.key, 'x');
assert.equal(parsed.section.flag, true);
assert.equal(parsed.section.count, 7);
});
});
// concurrency: false — see the fix-3 suite above for the same rationale.
describe('#2760 CR4 finding 1 — atomicWriteFileSync failure aborts install (post-write fatal)', { concurrency: false }, () => {
let tmpDir;
let codexHome;
let originalRenameSync;
let originalConsoleLog;
let consoleOutput;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-cr4-f1-'));
codexHome = path.join(tmpDir, 'codex-home');
originalRenameSync = fs.renameSync;
originalConsoleLog = console.log;
consoleOutput = [];
console.log = (...args) => { consoleOutput.push(args.join(' ')); };
});
afterEach(() => {
fs.renameSync = originalRenameSync;
console.log = originalConsoleLog;
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('install throws and never prints "Done!" when atomicWriteFileSync fails on configPath', () => {
const preInstall = [
'# user file',
'[model]',
'name = "o3"',
'',
].join('\n');
writeCodexConfig(codexHome, preInstall);
const configPath = path.join(codexHome, 'config.toml');
// Only fault the hook-block atomic rename — earlier writes to config.toml
// happen via mergeCodexConfig (agent-block emit). We want to exercise the
// post-write Codex install branch specifically. Detect by reading the temp
// file's contents and only faulting when the hook block is present.
fs.renameSync = (src, dst) => {
if (dst === configPath) {
let isHookWrite = false;
try {
const data = fs.readFileSync(src, 'utf8');
isHookWrite = /gsd-check-update\.js/.test(data);
} catch (_) { /* ignore */ }
if (isHookWrite) {
throw new Error('simulated rename failure');
}
}
return originalRenameSync(src, dst);
};
let threw = false;
let thrownMessage = '';
try {
runCodexInstall(codexHome);
} catch (e) {
threw = true;
thrownMessage = e.message;
}
assert.equal(threw, true, 'install must throw when atomic write fails');
assert.match(
thrownMessage,
/post-write Codex install failed/,
'thrown error must use the post-write prefix so the outer catch treats it as fatal'
);
// Critical: install must NOT have printed any "Done!" success banner.
const printedDone = consoleOutput.some(
(line) => typeof line === 'string' && /Done!/i.test(line)
);
assert.equal(printedDone, false,
'install must NOT print "Done!" after a write failure: ' + JSON.stringify(consoleOutput.filter((l) => /Done|✓/.test(l))));
// And the user's pre-install bytes are intact (snapshot restore).
const after = fs.readFileSync(configPath, 'utf8');
assert.equal(after, preInstall, 'pre-install bytes preserved after fatal abort');
});
});
// concurrency: false — patches module.exports.__codexSchemaValidator, a
// shared test seam. Serializing prevents stray patches from sibling tests.
describe('#2760 CR5 finding 1 — pre-write failures abort install (outer catch fatal)', { concurrency: false }, () => {
let tmpDir;
let codexHome;
let originalConsoleLog;
let consoleOutput;
const installModule = require('../bin/install.js');
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-cr5-f1-'));
codexHome = path.join(tmpDir, 'codex-home');
originalConsoleLog = console.log;
consoleOutput = [];
console.log = (...args) => { consoleOutput.push(args.join(' ')); };
});
afterEach(() => {
console.log = originalConsoleLog;
delete installModule.__codexSchemaValidator;
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('pre-write throw (validator throws, not returns {ok:false}) is fatal and restores snapshot', () => {
// A validator that THROWS (vs returning {ok:false}) bypasses the
// validation branch and exits the inner try via the catch at the outer
// level. Pre-CR5, that catch downgraded to console.warn and let the
// install print "Done!" with no Codex hooks. Post-CR5 it must rethrow.
const preInstall = [
'# user file',
'[model]',
'name = "o3"',
'',
].join('\n');
writeCodexConfig(codexHome, preInstall);
installModule.__codexSchemaValidator = () => {
throw new Error('synthetic validator-throw simulating a pre-write helper failure');
};
let threw = false;
let thrownMsg = '';
try {
runCodexInstall(codexHome);
} catch (e) {
threw = true;
thrownMsg = e.message;
}
assert.equal(threw, true,
'install must rethrow when a pre-write step throws (CR5 finding 1)');
assert.match(thrownMsg, /pre-write|synthetic validator-throw/,
'thrown error must surface the pre-write wrapper or original message: ' + thrownMsg);
const printedDone = consoleOutput.some(
(line) => typeof line === 'string' && /Done!/i.test(line)
);
assert.equal(printedDone, false,
'install must NOT print "Done!" after a pre-write failure: ' +
JSON.stringify(consoleOutput.filter((l) => /Done|✓/.test(l))));
// Pre-install bytes intact (snapshot restored).
const after = fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8');
assert.equal(after, preInstall,
'pre-install bytes must survive a pre-write helper throw');
});
});
describe('#2760 CR5 finding 2 — parseTomlToObject rejects duplicate keys and shape-mismatched headers', () => {
test('rejects duplicate scalar key in same table ([a]\\nx=1\\nx=2)', () => {
const content = [
'[a]',
'x = 1',
'x = 2',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/duplicate key/,
'real TOML 1.0 rejects duplicate keys in the same table'
);
});
test('rejects duplicate scalar key in root table', () => {
const content = [
'x = 1',
'x = 2',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/duplicate key/,
'duplicate root-table keys must be rejected'
);
});
test('rejects re-declared [a] table header ([a] then [a] again)', () => {
const content = [
'[a]',
'x = 1',
'',
'[a]',
'y = 2',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/duplicate or shape-mismatched table header/,
'real TOML 1.0 rejects re-declaring the same [a] header twice'
);
});
test('rejects [[arr]] then [arr] for same path (array-of-tables → table)', () => {
const content = [
'[[arr]]',
'x = 1',
'',
'[arr]',
'y = 2',
'',
].join('\n');
assert.throws(
() => parseTomlToObject(content),
/duplicate or shape-mismatched table header/,
'cannot redeclare an array-of-tables path as a plain table'
);
});
test('accepts repeated [[arr]] (genuine array-of-tables)', () => {
const content = [
'[[arr]]',
'x = 1',
'',
'[[arr]]',
'x = 2',
'',
].join('\n');
const parsed = parseTomlToObject(content);
assert.ok(Array.isArray(parsed.arr));
assert.strictEqual(parsed.arr.length, 2);
assert.strictEqual(parsed.arr[0].x, 1);
assert.strictEqual(parsed.arr[1].x, 2);
});
test('accepts disjoint nested headers (not duplicates)', () => {
const content = [
'[a.b]',
'x = 1',
'',
'[a.c]',
'y = 2',
'',
].join('\n');
const parsed = parseTomlToObject(content);
assert.strictEqual(parsed.a.b.x, 1);
assert.strictEqual(parsed.a.c.y, 2);
});
});
// concurrency: false — drives the same install pipeline as the other f-suites.
describe('#2760 CR5 finding 3 — migration emits namespaced AoT (no flat/namespaced mixing)', { concurrency: false }, () => {
let tmpDir;
let codexHome;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2760-cr5-f3-'));
codexHome = path.join(tmpDir, 'codex-home');
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('user has [[hooks.AfterTool]] AND legacy [hooks.SessionStart] → post-install both namespaced, no flat AoT', () => {
// Reproduces the mixed-form scenario from finding 3:
// - User pre-config has both a namespaced AoT entry [[hooks.AfterTool]]
// AND a legacy single-bracket [hooks.SessionStart].
// - Pre-CR5 migration converts the legacy section to flat [[hooks]]
// with event="SessionStart", leaving a mixed flat+namespaced layout.
// - Post-CR5 migration emits [[hooks.SessionStart]] directly so both
// of the user's hooks coexist in the namespaced shape, and the
// GSD-managed entry converges on namespaced too.
const userPlusLegacy = [
'[[hooks.AfterTool]]',
'command = "x"',
'',
'[hooks.SessionStart]',
'command = "y"',
'',
].join('\n');
writeCodexConfig(codexHome, userPlusLegacy);
runCodexInstall(codexHome);
const after = readCodexConfig(codexHome);
const parsed = parseTomlToObject(after);
// The pre-existing [[hooks.AfterTool]] entry is preserved.
assert.ok(
parsed.hooks && Array.isArray(parsed.hooks.AfterTool),
'pre-existing [[hooks.AfterTool]] must remain a namespaced AoT array'
);
assert.ok(
parsed.hooks.AfterTool.some((entry) => entry.command === 'x'),
'user AfterTool entry must be preserved: ' + JSON.stringify(parsed.hooks.AfterTool)
);
// The migrated SessionStart entry is now namespaced AoT, not flat
// [[hooks]] with event="SessionStart".
assert.ok(
parsed.hooks && Array.isArray(parsed.hooks.SessionStart),
'migrated SessionStart must be namespaced AoT (not flat [[hooks]])'
);
const ssCommands = parsed.hooks.SessionStart.map((e) => e.command);
assert.ok(
ssCommands.includes('y'),
'user SessionStart command "y" must be preserved in namespaced array: ' +
JSON.stringify(ssCommands)
);
// GSD's managed gsd-check-update entry also lives in the namespaced array.
assert.ok(
ssCommands.some((cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)),
'managed gsd-check-update entry must appear in hooks.SessionStart array: ' +
JSON.stringify(ssCommands)
);
// No flat top-level [[hooks]] AoT may remain.
assert.ok(
!Array.isArray(parsed.hooks) || parsed.hooks.length === 0,
'no flat top-level [[hooks]] AoT entries may remain after migration: ' +
JSON.stringify(parsed.hooks)
);
// No synthetic event field on the migrated SessionStart entries — the
// namespace IS the event.
for (const entry of parsed.hooks.SessionStart) {
assert.equal(entry.event, undefined,
'no synthetic event field — namespace [[hooks.SessionStart]] encodes the event: ' +
JSON.stringify(entry));
}
});
});

179
tests/codex-config.test.cjs
View File

@@ -8,11 +8,30 @@
// Enable test exports from install.js (skips main CLI logic)
process.env.GSD_TEST_MODE = '1';
const { test, describe, beforeEach, afterEach } = require('node:test');
const { test, describe, before, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const os = require('os');
const { execFileSync } = require('child_process');
// #2153 follow-up: ensure hooks/dist/ exists before any install integration
// test runs. The Codex install path copies hook files from hooks/dist/, which
// is gitignored and only populated by `npm run build:hooks`. When this file is
// run in isolation (`node --test tests/codex-config.test.cjs`) the build step
// from the npm-test pretest chain does not run, and the "Codex install copies
// hook file" regression silently fails because hooks/dist/ is empty.
// Build on demand so the test passes regardless of runner ordering.
const HOOKS_DIST = path.join(__dirname, '..', 'hooks', 'dist');
const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js');
before(() => {
if (!fs.existsSync(HOOKS_DIST) || fs.readdirSync(HOOKS_DIST).length === 0) {
execFileSync(process.execPath, [BUILD_HOOKS_SCRIPT], {
encoding: 'utf-8',
stdio: 'pipe',
});
}
});
const {
getCodexSkillAdapterHeader,
@@ -26,6 +45,7 @@ const {
install,
GSD_CODEX_MARKER,
CODEX_AGENT_SANDBOX,
parseTomlToObject,
} = require('../bin/install.js');
function runCodexInstall(codexHome, cwd = path.join(__dirname, '..')) {
@@ -575,7 +595,7 @@ describe('migrateCodexHooksMapFormat', () => {
assert.strictEqual(migrateCodexHooksMapFormat(''), '');
});
test('converts [hooks.shell] with command key to [[hooks]] with type = "shell"', () => {
test('converts [hooks.shell] to namespaced AoT [[hooks.shell]] (#2760 CR5 finding 3)', () => {
const content = [
'[features]',
'codex_hooks = true',
@@ -587,34 +607,38 @@ describe('migrateCodexHooksMapFormat', () => {
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
// Old format removed
assert.ok(!result.includes('[hooks.shell]'), 'removes [hooks.shell] map header');
assert.ok(!result.match(/^\[hooks\]$/m), 'removes bare [hooks] container');
// New format present
assert.ok(result.includes('[[hooks]]'), 'adds [[hooks]] array header');
assert.ok(result.includes('type = "shell"'), 'adds type = "shell" key');
assert.ok(result.includes('command = "node /home/.codex/hooks/gsd-check-update.js"'), 'preserves command value');
// User content preserved
assert.ok(result.includes('[features]'), 'preserves [features] section');
assert.ok(result.includes('codex_hooks = true'), 'preserves codex_hooks key');
// Parse structurally — no source-grep on raw bytes.
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.shell),
'hooks.shell must be an array of tables, got: ' + (parsed.hooks ? typeof parsed.hooks.shell : 'no hooks table'));
assert.strictEqual(parsed.hooks.shell.length, 1);
assert.strictEqual(parsed.hooks.shell[0].command, 'node /home/.codex/hooks/gsd-check-update.js');
// No flat top-level [[hooks]] AoT and no synthetic event field.
assert.ok(!Array.isArray(parsed.hooks),
'no top-level [[hooks]] AoT — namespace IS the event in CR5 form');
assert.equal(parsed.hooks.shell[0].event, undefined,
'no synthetic event field — namespace [[hooks.shell]] encodes the event');
// User content preserved.
assert.equal(parsed.features && parsed.features.codex_hooks, true);
});
test('converts [hooks.exec] to [[hooks]] with type = "exec"', () => {
test('converts [hooks.exec] to namespaced AoT [[hooks.exec]] (#2760 CR5 finding 3)', () => {
const content = [
'[hooks.exec]',
'command = "echo hello"',
'event = "SessionStart"',
'extra_key = "preserved"',
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
assert.ok(!result.includes('[hooks.exec]'), 'removes [hooks.exec] map header');
assert.ok(result.includes('[[hooks]]'), 'adds [[hooks]] array header');
assert.ok(result.includes('type = "exec"'), 'adds type = "exec" key');
assert.ok(result.includes('command = "echo hello"'), 'preserves command');
assert.ok(result.includes('event = "SessionStart"'), 'preserves event');
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.exec));
assert.strictEqual(parsed.hooks.exec.length, 1);
assert.strictEqual(parsed.hooks.exec[0].command, 'echo hello');
assert.strictEqual(parsed.hooks.exec[0].extra_key, 'preserved');
assert.equal(parsed.hooks.exec[0].event, undefined);
});
test('converts multiple [hooks.TYPE] sections to separate [[hooks]] blocks', () => {
test('converts multiple [hooks.TYPE] sections to separate namespaced AoT blocks (#2760 CR5 finding 3)', () => {
const content = [
'[hooks.shell]',
'command = "node /home/.codex/hooks/gsd-check-update.js"',
@@ -624,12 +648,13 @@ describe('migrateCodexHooksMapFormat', () => {
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
assert.ok(!result.includes('[hooks.shell]'), 'removes [hooks.shell]');
assert.ok(!result.includes('[hooks.exec]'), 'removes [hooks.exec]');
const hookHeaders = (result.match(/\[\[hooks\]\]/g) || []).length;
assert.strictEqual(hookHeaders, 2, 'produces two [[hooks]] array entries');
assert.ok(result.includes('type = "shell"'), 'first entry has type = "shell"');
assert.ok(result.includes('type = "exec"'), 'second entry has type = "exec"');
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.shell));
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.exec));
assert.strictEqual(parsed.hooks.shell.length, 1);
assert.strictEqual(parsed.hooks.exec.length, 1);
assert.strictEqual(parsed.hooks.shell[0].command, 'node /home/.codex/hooks/gsd-check-update.js');
assert.strictEqual(parsed.hooks.exec[0].command, 'echo done');
});
test('leaves user-authored [[hooks]] array entries untouched when no legacy [hooks] map present', () => {
@@ -642,7 +667,7 @@ describe('migrateCodexHooksMapFormat', () => {
assert.strictEqual(migrateCodexHooksMapFormat(content), content);
});
test('end-to-end: install on config with old [hooks] map format produces [[hooks]] array format (#2637)', () => {
test('end-to-end: install on config with old [hooks] map format produces namespaced AoT (#2637, #2760 CR5)', () => {
// Simulates the exact old GSD config.toml format that broke on Codex 0.124.0
const oldContent = [
'[features]',
@@ -655,18 +680,15 @@ describe('migrateCodexHooksMapFormat', () => {
'',
].join('\n');
const result = migrateCodexHooksMapFormat(oldContent);
// Must not contain any [hooks] or [hooks.*] map-style headers
assert.ok(!result.match(/^\s*\[hooks\]\s*$/m), 'no bare [hooks] map header');
assert.ok(!result.match(/^\s*\[hooks\./m), 'no [hooks.TYPE] map headers');
// Must contain [[hooks]] array format
assert.ok(result.includes('[[hooks]]'), 'has [[hooks]] array-of-tables header');
// type key must be present
assert.ok(result.includes('type = "shell"'), 'has type = "shell" in [[hooks]] entry');
// command is preserved
assert.ok(result.includes('command = "node /home/.codex/hooks/gsd-check-update.js"'), 'command preserved');
// [features] user content preserved
assert.ok(result.includes('[features]'), 'preserves [features]');
assert.ok(result.includes('codex_hooks = true'), 'preserves codex_hooks');
const parsed = parseTomlToObject(result);
// Codex 0.124.0+: must produce array-of-tables form. CR5 finding 3:
// namespaced AoT [[hooks.shell]] (no flat [[hooks]] with synthetic event).
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.shell),
'hooks.shell must be array-of-tables in namespaced form');
assert.strictEqual(parsed.hooks.shell.length, 1);
assert.strictEqual(parsed.hooks.shell[0].command,
'node /home/.codex/hooks/gsd-check-update.js');
assert.equal(parsed.features && parsed.features.codex_hooks, true);
});
test('bare [hooks] section without sub-tables is dropped (no [[hooks]] block added)', () => {
@@ -688,7 +710,7 @@ describe('migrateCodexHooksMapFormat', () => {
assert.ok(result.includes('[model]'), 'preserves [model]');
});
test('CRLF line endings are preserved through migration', () => {
test('CRLF line endings are preserved through migration (#2760 CR5: namespaced AoT)', () => {
const content = [
'[features]',
'codex_hooks = true',
@@ -698,9 +720,58 @@ describe('migrateCodexHooksMapFormat', () => {
'',
].join('\r\n');
const result = migrateCodexHooksMapFormat(content);
assert.ok(result.includes('[[hooks]]\r\n'), 'uses CRLF in [[hooks]] header');
assert.ok(result.includes('type = "shell"\r\n'), 'uses CRLF in type line');
assert.ok(!result.includes('[hooks.shell]'), 'removes legacy [hooks.shell]');
assert.ok(result.includes('[[hooks.shell]]\r\n'),
'uses CRLF in namespaced [[hooks.shell]] header');
// Round-trip parse confirms the structural shape independent of EOL.
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.shell));
assert.strictEqual(parsed.hooks.shell[0].command,
'node /home/.codex/hooks/gsd-check-update.js');
});
});
// ─── shape parity between migration and managed emit (#2760 CR5 finding 3) ──
describe('Codex hooks emit: migration produces namespaced AoT so managed-emit converges', () => {
// After #2760 CR5 finding 3, the legacy migration path
// (migrateCodexHooksMapFormat) emits `[[hooks.<TYPE>]]` directly — the
// namespace IS the event, no synthetic `event = ...` field. The managed
// install path (writes "# GSD Hooks") detects existing namespaced AoT via
// hasUserNamespacedAotHooks and emits its block in the same shape. The two
// paths must therefore both produce a namespaced layout when a legacy
// [hooks.SessionStart] is migrated, eliminating the mixed flat+namespaced
// bug class entirely.
let tmpDir;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-codex-fieldparity-'));
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('migration of legacy [hooks.SessionStart] produces namespaced AoT', () => {
const legacyContent = [
'[features]',
'codex_hooks = true',
'',
'[hooks.SessionStart]',
'command = "node /home/.codex/hooks/gsd-check-update.js"',
'',
].join('\n');
const migrated = migrateCodexHooksMapFormat(legacyContent);
const parsed = parseTomlToObject(migrated);
assert.ok(
parsed.hooks && Array.isArray(parsed.hooks.SessionStart),
'migration must emit [[hooks.SessionStart]] namespaced AoT'
);
assert.equal(parsed.hooks.SessionStart[0].event, undefined,
'migration must NOT emit a synthetic event field — namespace IS the event');
assert.equal(
Array.isArray(parsed.hooks),
false,
'migration must NOT emit a flat top-level [[hooks]] AoT'
);
});
});
@@ -929,7 +1000,7 @@ describe('mergeCodexConfig', () => {
assertUsesOnlyEol(content, '\r\n');
});
test('case 2 preserves user-authored [agents] tables while stripping leaked GSD sections in CRLF files', () => {
test('case 2 strips bare [agents] tables (invalid in current Codex schema, #2760) and removes leaked GSD sections in CRLF files', () => {
const configPath = path.join(tmpDir, 'config.toml');
const brokenContent = [
'[features]',
@@ -958,8 +1029,24 @@ describe('mergeCodexConfig', () => {
const markerIndex = content.indexOf(GSD_CODEX_MARKER);
const beforeMarker = content.slice(0, markerIndex);
assert.ok(beforeMarker.includes('[agents]\r\ndefault = "custom-agent"\r\n'), 'preserves user-authored [agents] table');
assert.strictEqual(countMatches(beforeMarker, /^\[agents\.gsd-executor\]\s*$/gm), 0, 'removes leaked GSD agent section above marker');
// Bare [agents] is invalid under Codex's current schema (rejected with
// "expected struct AgentsToml") so install-time stripping always purges
// it (#2760). User feature keys above the marker are preserved.
// Structural assertion: TOML-parse the pre-marker region and verify the
// bare [agents] block is fully gone — header AND body keys (e.g.,
// `default = "custom-agent"`). A header-only check would miss a
// partial-strip regression that leaves orphan body keys reparented to a
// sibling section.
const parsedBefore = parseTomlToObject(beforeMarker);
assert.equal(
parsedBefore.agents,
undefined,
'bare [agents] block fully purged including body keys (#2760)',
);
assert.ok(
parsedBefore.features && parsedBefore.features.child_agents_md === false,
'preserves user feature keys above marker',
);
// New struct format: exactly one [agents.gsd-executor] in the GSD block (after marker)
assert.strictEqual(countMatches(content, /^\[agents\.gsd-executor\]\s*$/gm), 1, 'exactly one struct agent header in GSD block');
assert.strictEqual(countMatches(content, /name = "gsd-executor"/g), 0, 'no name = field in struct format');