fix(#2773): emit correct Codex 0.124.0+ two-level nested hooks schema (#2809)

* fix(#2773): emit correct Codex 0.124.0+ two-level nested hooks schema

Codex 0.124.0's stable spec requires:

  [[hooks.SessionStart]]          ← event entry (optional matcher)

  [[hooks.SessionStart.hooks]]    ← handler sub-table
  type = "command"
  command = "node ..."

Previous GSD versions wrote the flat [[hooks]] + event = "SessionStart"
form (#2637) or a single-block [[hooks.SessionStart]] without the nested
.hooks sub-table (#2760). Both are rejected by Codex 0.124.0+ at launch.

Changes:

bin/install.js
  - Hook block emission now always writes the two-level nested AoT form.
  - migrateCodexHooksMapFormat extended to also migrate flat [[hooks]]
    array-of-tables entries (event = "..." key → [[hooks.<EVENT>]] form).
    Flat [[hooks]] and [[hooks.<EVENT>]] are mutually exclusive TOML types;
    any pre-existing flat entries must be promoted before GSD appends its
    own namespaced hooks.
  - Migrated flat AoT blocks are inserted BEFORE the GSD marker so they
    stay in the "user" portion of the file and survive stripGsdFromCodexConfig.
  - stripCodexGsd* regexes cover all four historical block shapes.
  - validateCodexConfigSchema no longer rejects flat [[hooks]] at the root
    level (removing the false-positive that blocked install when users had
    their own AfterCommand hooks). The validator still enforces the nested
    [[hooks.<EVENT>.hooks]] shape for entries that have a .hooks sub-table.

tests/
  - bug-2760-codex-install-defensive.test.cjs: 29/29 passing.
    Added 5 new regression cases for fresh install, upgrade from each
    legacy shape, idempotent reinstall, and user hook preservation.
  - codex-config.test.cjs: 106/106 passing.
    All migration tests updated to assert [[hooks.<TYPE>.hooks]] sub-table
    (command now in handler level, not event-entry level).
    New tests: flat [[hooks]] migration (SessionStart, AfterCommand),
    install+uninstall preserves non-GSD AfterCommand hook.

Closes #2773

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address CodeRabbit review + CI regression in bug-2698-crlf-install

CI regression (#2698 tests):
  Strip GSD-managed hook blocks BEFORE running migrateCodexHooksMapFormat.
  The previous order let migration convert the stale [[hooks]] + event =
  "SessionStart" + gsd-update-check.js block to [[hooks.SessionStart]] form
  before Shape 1 strip regex could match it; Shape 1 only matches the flat
  [[hooks]] form, so the stale block survived reinstall. Swapping to
  strip-then-migrate ensures only user-authored hooks reach the migration step.
  Shape 3/4 regexes also extended to match both gsd-check-update.js and the
  legacy gsd-update-check.js filename so no variant slips through.

CodeRabbit actionable (major):
  migrateCodexHooksMapFormat now accepts single-quoted TOML event values
  (event = 'SessionStart') in the flat [[hooks]] filter and event-name
  extractor. TOML spec allows single-quoted literal strings; double-quote-only
  regexes silently skipped them, leaving the block unmigrated and triggering
  the hard-fail validator.

CodeRabbit nitpicks:
  tests/codex-config.test.cjs: replace indexOf('[[hooks.AfterCommand]]')
  ordering check with parseTomlToObject structural assertions (no-source-grep
  rule).
  tests/bug-2760-codex-install-defensive.test.cjs: replace three
  content.match(/…/g).length raw-text counts with parseTomlToObject structural
  assertions for single-handler and single-event-entry invariants.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address CodeRabbit review #2 — extractFlatHookEventName helper + type assertions

- bin/install.js: consolidate TOML_QUOTED_STRING + TOML_EVENT_CAPTURE into a
  single extractFlatHookEventName() helper that rejects empty-string event values
  (event = "" or event = ''); previously two independent regexes had to be kept
  in sync and neither guarded against a blank event name producing a [[hooks.]]
  header

- tests/bug-2760-codex-install-defensive.test.cjs: add comments explaining why
  the e.command fallback is retained in both allSessionStartCommands and
  afterToolCommands collectors — migration only upgrades [hooks.TYPE] map-format
  sections, not existing [[hooks.TYPE]] namespaced AoT entries authored with
  command at event-entry level; removing the fallback causes false failures for
  preserved user entries

- tests/codex-config.test.cjs: add type = "command" assertions to all migration
  tests that verify .command but were missing .type checks; buildNestedBlock
  injects type = "command" when the source body has no explicit type key, so
  every migrated handler must carry it per the Codex 0.124.0+ schema

138 tests pass, 0 fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: CR round 3 + proactive audit — TOML quoting, stale AoT migration, strict validator

Three real issues from CodeRabbit round 3, plus the collateral improvements they
enable:

bin/install.js — tomlBareKey() helper (#2773 CR6a)
  buildNestedBlock interpolated the raw event name into [[hooks.${type}]] and
  [[hooks.${type}.hooks]] headers without TOML escaping. An event name containing
  spaces or punctuation (e.g. "Before Tool") would produce invalid TOML that
  parseTomlToObject would subsequently reject. Added tomlBareKey() — wraps the
  key in double-quoted TOML strings when it contains non-bare-key characters
  ([A-Za-z0-9_-]).

bin/install.js — staleNamespacedAotSections migration path (#2773 CR6b)
  migrateCodexHooksMapFormat handled [hooks.TYPE] (map-format) and flat [[hooks]]
  with event = "..." but ignored [[hooks.TYPE]] AoT entries that carried handler
  fields (command, type, timeout, statusMessage) at event-entry level without a
  nested [[hooks.TYPE.hooks]] sub-table. This is the pre-#2773 single-block shape
  that Codex 0.124.0+ rejects. Added staleNamespacedAotSections as the third
  migration category: detected by STALE_HANDLER_FIELD_PATTERN + absence of a
  [[hooks.TYPE.hooks]] sub-table in the same file; promoted to the two-level
  nested form by buildNestedBlock. Matcher-only entries (no handler fields) are
  intentionally skipped.

bin/install.js — validator now rejects event-level handler fields (#2773 CR6c)
  With migration covering the stale AoT shape, validateCodexConfigSchema can be
  strict: entries that have handler fields at event-entry level but no .hooks
  sub-array return ok: false instead of silently passing. Matcher-only entries
  (no handler fields and no .hooks) remain valid as event filters.

tests/codex-config.test.cjs — four new migration tests + missing type assertion
  Four tests cover the new stale AoT migration path: single-entry promotion,
  already-nested entry is left untouched (no double-wrap), multiple event types,
  and matcher-only entry is skipped. Added the missing type = "command" assertion
  to the CRLF migration test (the one miss from CR round 2).

tests/bug-2760-codex-install-defensive.test.cjs — strict .hooks-only collectors
  With stale AoT entries now migrated, the entry.command fallbacks in
  allSessionStartCommands and afterToolCommands are dead code. Replaced with
  strict entry.hooks-only collection guarded by an every(Array.isArray(e.hooks))
  pre-assertion, so any future regression that leaves handler fields at event
  level produces an explicit test failure rather than silently collecting them.

142 tests pass, 0 fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: CR round 4 — segment-safe quoted-key detection + structural test assertions

bin/install.js — getTomlTableSections now exposes segments (#2773 CR7a)
  The staleNamespacedAotSections filter used section.path.split('.').length > 2
  to skip [[hooks.TYPE.hooks]] sub-table entries. That check misclassifies quoted
  event names containing dots: [[hooks."before.tool"]] has path hooks.before.tool
  (3 dot-parts) but only 2 true parsed segments, so it was incorrectly excluded
  from migration. Fixed by adding segments to the getTomlTableSections return
  shape (already available on record.tableHeader.segments) and replacing the
  split-based check with section.segments.length !== 2, which uses the true
  parsed key count regardless of dots inside quoted names.

tests/codex-config.test.cjs — replace raw-equality assertions (#2773 CR7b)
  The two new no-op migration tests (already-nested and matcher-only) used
  assert.strictEqual(result, content) — raw string equality that conflicts with
  the repo no-source-grep testing standard. Replaced with structural assertions
  using parseTomlToObject: the already-nested test verifies the handler stays
  under .hooks[0] and no double-wrap occurs; the matcher-only test verifies the
  matcher key is preserved and no .hooks sub-array is added.

142 tests pass, 0 fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: CR round 5 — parseHooksBody key parser, empty-handler guard, segment-safe legacyMap filter, stronger test assertions

- parseHooksBody: replace /^([\w.]+)\s*=/ regex with parseTomlKey() so
  hyphenated keys (status-message) and quoted keys are not silently dropped
- buildNestedBlock: guard against handlerEntries.length === 0 — do not
  synthesise [[hooks.TYPE.hooks]] with type="command" but no command for
  matcher-only or otherwise handler-empty stale sections
- legacyMapSections filter: use section.segments.length === 2 (same fix
  applied to staleNamespacedAotSections in round 4) to prevent [hooks.X.Y]
  3-segment tables from being misclassified as event entries
- tests: add regression test for [[hooks."before.tool"]] quoted-dot event
  names; strengthen command path assertion to exact absolute path comparison

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-28 18:28:53 -04:00
committed by GitHub
parent c0730fffde
commit 3c03a153a5
3 changed files with 678 additions and 165 deletions

View File

@@ -2628,6 +2628,11 @@ function getTomlTableSections(content) {
return headerLines.map((record, index) => ({
path: record.tableHeader.path,
// segments preserves the true parsed key count so callers that need to
// distinguish a 2-segment path like hooks."before.tool" from a 3-segment
// path like hooks.SessionStart.hooks can do so without splitting on dots
// (which misclassifies quoted key names that contain dot characters).
segments: record.tableHeader.segments,
array: record.tableHeader.array,
start: record.start,
headerEnd: record.end + record.eol.length,
@@ -2891,40 +2896,133 @@ function stripLeakedGsdCodexSections(content) {
function migrateCodexHooksMapFormat(content) {
const sections = getTomlTableSections(content);
// Find all non-array hooks sections: [hooks] or [hooks.TYPE]
const legacyHooksSections = sections.filter(
(section) => !section.array && (section.path === 'hooks' || section.path.startsWith('hooks.'))
// Find all non-array hooks sections: bare [hooks] container or [hooks.TYPE] event tables.
// Use section.segments (parsed key count) rather than section.path.startsWith() so that
// nested handler tables like [hooks.SessionStart.hooks] (3 segments) are not mistakenly
// included and re-emitted as an event named "SessionStart.hooks".
const legacyMapSections = sections.filter(
(section) => !section.array && (
section.path === 'hooks' ||
(section.path.startsWith('hooks.') && section.segments.length === 2)
)
);
if (legacyHooksSections.length === 0) {
// Find flat [[hooks]] array-of-tables entries (path === 'hooks', array === true).
// These are incompatible with [[hooks.<EVENT>]] namespaced form — both cannot
// coexist in the same TOML file because `hooks` cannot be simultaneously an
// array and a table. Migrate each flat entry to [[hooks.<EVENT>]] form using
// the `event` key as the event name.
const flatAotSections = sections.filter(
(section) => section.array && section.path === 'hooks'
);
// Find [[hooks.TYPE]] namespaced AoT entries that carry handler fields
// (command, type, timeout, statusMessage) at event-entry level but have no
// [[hooks.TYPE.hooks]] sub-table. This is the pre-#2773 single-block shape
// that Codex 0.124.0+ rejects. Promote them to the two-level nested form.
// Entries that already have a [[hooks.TYPE.hooks]] sub-table are left untouched.
// Matcher-only entries (no handler fields) are intentionally valid and skipped.
const STALE_HANDLER_FIELD_PATTERN = /^\s*(?:command|type|timeout|statusMessage)\s*=/m;
const staleNamespacedAotSections = sections.filter((section) => {
if (!section.array) return false;
if (!section.path.startsWith('hooks.')) return false;
// [[hooks.TYPE.hooks]] sub-tables have 3 parsed segments — skip them.
// Use section.segments (true parsed key count) rather than splitting
// section.path on '.', which misclassifies quoted event names that contain
// dots (e.g. [[hooks."before.tool"]] has segments ['hooks','before.tool']
// but path 'hooks.before.tool' would split into 3 parts).
if (section.segments.length !== 2) return false;
// Must carry at least one handler field at event-entry level.
const body = content.slice(section.headerEnd, section.end);
if (!STALE_HANDLER_FIELD_PATTERN.test(body)) return false;
// Don't migrate when the nested [[hooks.TYPE.hooks]] sub-table already exists.
const subPath = section.path + '.hooks';
return !sections.some((s) => s.array && s.path === subPath);
});
if (legacyMapSections.length === 0 && flatAotSections.length === 0 && staleNamespacedAotSections.length === 0) {
return content;
}
const eol = detectLineEnding(content);
// Build [[hooks]] blocks for each [hooks.TYPE] section (skipping bare [hooks])
const newHooksBlocks = [];
for (const section of legacyHooksSections) {
if (section.path === 'hooks') {
// Bare [hooks] container — drop it (no key-value content to convert)
continue;
// Helper: parse a hooks body into event-level and handler-level entries,
// returning { eventEntries, handlerEntries, hasExplicitType }.
// Event-level keys: matcher. Everything else is handler-level.
// The `event` key (used in flat [[hooks]] blocks) is consumed as the type
// name and excluded from both levels.
const EVENT_LEVEL_KEYS = new Set(['matcher']);
function parseHooksBody(body, skipKeys = new Set()) {
const bodyLines = body.split(/\r?\n/);
const eventEntries = [];
const handlerEntries = [];
let hasExplicitType = false;
for (const line of bodyLines) {
const trimmed = line.trim();
if (!trimmed || trimmed.startsWith('#')) continue;
// Use parseTomlKey so hyphenated keys (e.g. status-message) and quoted
// keys are recognised — the old /^([\w.]+)\s*=/ regex silently dropped them.
const parsed = parseTomlKey(trimmed);
if (!parsed) continue;
// Hook body keys are always single-segment; use segments[0] for the name.
const key = parsed.segments[0];
if (skipKeys.has(key)) continue;
if (key === 'type') {
hasExplicitType = true;
handlerEntries.push(trimmed);
} else if (EVENT_LEVEL_KEYS.has(key)) {
eventEntries.push(trimmed);
} else {
handlerEntries.push(trimmed);
}
}
return { eventEntries, handlerEntries, hasExplicitType };
}
// Extract the type from the path: "hooks.shell" → "shell"
const type = section.path.slice('hooks.'.length);
// TOML key quoting: bare keys may only contain [A-Za-z0-9_-]. Event names
// containing spaces, dots, or other punctuation must be wrapped in double-
// quoted TOML strings with backslash and double-quote characters escaped.
// Using raw event names in [[hooks.${type}]] headers produces invalid TOML
// for any non-bare-key character (e.g. "Before Tool" → [[hooks.Before Tool]]).
function tomlBareKey(key) {
if (/^[A-Za-z0-9_-]+$/.test(key)) return key;
return '"' + key.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '"';
}
function buildNestedBlock(type, body, skipKeys = new Set()) {
const quotedType = tomlBareKey(type);
const { eventEntries, handlerEntries, hasExplicitType } = parseHooksBody(body, skipKeys);
const eventBody = eventEntries.length > 0 ? eventEntries.join(eol) + eol : '';
// If no handler fields were found (e.g. matcher-only entry), do not synthesise
// an empty [[hooks.TYPE.hooks]] block — that would produce structurally valid
// TOML but semantically broken output (a handler entry with no command).
if (handlerEntries.length === 0) {
return `[[hooks.${quotedType}]]${eol}${eventBody}`;
}
if (!hasExplicitType) handlerEntries.unshift('type = "command"');
const handlerBody = handlerEntries.join(eol) + eol;
return `[[hooks.${quotedType}]]${eol}${eventBody}${eol}[[hooks.${quotedType}.hooks]]${eol}${handlerBody}`;
}
// Extract the event name from a flat [[hooks]] section body.
// Returns null if no `event` key is found, if the value is an empty string, or if
// the quoting is unrecognised. Both TOML double-quoted ("...") and single-quoted
// ('...') strings are accepted. An empty event string (event = "" or event = '')
// is explicitly rejected — it cannot be meaningfully namespaced and is left untouched.
function extractFlatHookEventName(body) {
const TOML_EVENT_CAPTURE = /^\s*event\s*=\s*(?:"((?:[^"\\]|\\.)*)"|'([^']*)')/m;
const m = body.match(TOML_EVENT_CAPTURE);
if (!m) return null;
const name = (m[1] ?? m[2] ?? '').trim();
return name || null;
}
const migratedFlatAotSections = flatAotSections.filter((section) => {
const body = content.slice(section.headerEnd, section.end);
return extractFlatHookEventName(body) !== null;
});
// #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);
}
const legacyHooksSections = [...legacyMapSections, ...migratedFlatAotSections, ...staleNamespacedAotSections];
// Remove all legacy hooks sections from the content
let result = removeContentRanges(
@@ -2933,28 +3031,43 @@ function migrateCodexHooksMapFormat(content) {
);
result = collapseTomlBlankLines(result);
// Insert new [[hooks]] blocks at the position of the first legacy section
// (adjusted for removed content), or append if nothing remains before EOF.
if (newHooksBlocks.length > 0) {
const insertionText = newHooksBlocks.join('');
// Find a good insertion point: before the first remaining table section
// that came after our removed hooks, or just append.
const remainingSections = getTomlTableSections(result);
const firstHooksSection = legacyHooksSections[0];
// Find the first remaining section whose original start was after the legacy hooks block
const anchorSection = remainingSections.find((s) => {
// Use content position in the result string as a heuristic
// We insert before the first non-hooks section if any exists
return s.start > 0;
// Map-format blocks ([hooks.TYPE]) are inserted at the position of the first
// remaining table section (preserving their relative placement in the file).
// Flat AoT blocks ([[hooks]] with event = "...") are always APPENDED because
// flat [[hooks]] entries only appear at the END of a TOML file (AoT cannot
// precede a regular table), and inserting before the first table would push
// them above [features] / [model] etc., corrupting relative ordering.
const mapOnlyBlocks = legacyMapSections
.filter((s) => s.path !== 'hooks') // skip bare [hooks] container
.map((s) => {
const type = s.path.slice('hooks.'.length);
const body = content.slice(s.headerEnd, s.end);
return buildNestedBlock(type, body);
});
// Prefer to insert the new blocks right before the first remaining table
// that was originally positioned after the legacy hooks area, but since
// positions shift after removal, we simply append before the first table
// header or at end-of-file.
// Stale namespaced AoT blocks: [[hooks.TYPE]] entries with handler fields at
// event-entry level (no .hooks sub-table). Treated like map-format blocks —
// inserted before the first remaining table section.
const staleNamespacedAotBlocks = staleNamespacedAotSections.map((s) => {
const type = s.path.slice('hooks.'.length);
const body = content.slice(s.headerEnd, s.end);
return buildNestedBlock(type, body);
});
const flatAotBlocks = migratedFlatAotSections.map((s) => {
const body = content.slice(s.headerEnd, s.end);
const eventName = extractFlatHookEventName(body);
if (!eventName) return '';
return buildNestedBlock(eventName, body, new Set(['event']));
}).filter(Boolean);
// Insert map-format and stale-namespaced-AoT conversions before the first
// remaining table section (both share the same placement strategy).
const allMapStyleBlocks = [...mapOnlyBlocks, ...staleNamespacedAotBlocks];
if (allMapStyleBlocks.length > 0) {
const insertionText = allMapStyleBlocks.join('');
const remainingSections = getTomlTableSections(result);
if (remainingSections.length > 0) {
// Find where to insert: after any leading top-level keys, before first table
const firstTable = remainingSections[0];
const before = result.slice(0, firstTable.start);
const after = result.slice(firstTable.start);
@@ -2966,7 +3079,23 @@ function migrateCodexHooksMapFormat(content) {
(needsTrailingGap ? eol : '') +
after;
} else {
// No remaining sections — append
const needsGap = result.length > 0 && !result.endsWith(eol + eol);
result = result + (needsGap ? eol : '') + insertionText;
}
}
// Insert flat-AoT conversions before the GSD managed marker (if present) so
// the migrated user hooks stay in the "user" portion of the file and are not
// swept away when stripGsdFromCodexConfig strips from the marker to EOF.
// If no marker exists, append at the end of the file.
if (flatAotBlocks.length > 0) {
const insertionText = flatAotBlocks.join('');
const markerIdx = result.indexOf(GSD_CODEX_MARKER);
if (markerIdx !== -1) {
const before = result.slice(0, markerIdx).trimEnd();
const after = result.slice(markerIdx);
result = before + eol + eol + insertionText + eol + after;
} else {
const needsGap = result.length > 0 && !result.endsWith(eol + eol);
result = result + (needsGap ? eol : '') + insertionText;
}
@@ -3400,15 +3529,64 @@ function validateCodexConfigSchema(content) {
}
// Structural confirmation against parsed object: any present hooks.<Event>
// must be an array.
if (parsed.hooks && typeof parsed.hooks === 'object' && !Array.isArray(parsed.hooks)) {
// must be an array, and flat top-level [[hooks]] (parsed as Array on root)
// is rejected — Codex 0.124.0+ requires [[hooks.<Event>]] namespaced form.
if (parsed.hooks !== undefined) {
if (Array.isArray(parsed.hooks)) {
return {
ok: false,
reason: 'flat [[hooks]] array-of-tables is invalid in Codex 0.124.0+ (expected [[hooks.<Event>]] namespaced form)',
};
}
if (typeof parsed.hooks === 'object' && parsed.hooks !== null) {
for (const [event, value] of Object.entries(parsed.hooks)) {
// Skip the nested .hooks sub-array — it lives under hooks.<Event>[n].hooks
// and is validated separately below.
if (!Array.isArray(value)) {
return {
ok: false,
reason: `hooks.${event} must be an array of tables, got ${typeof value}`,
};
}
// Each entry in hooks.<Event> must either be a matcher-only filter (no
// handler fields) or carry a .hooks sub-array of handler tables.
// Entries with handler fields (command, type, timeout, statusMessage) at
// event-entry level but without a .hooks sub-table are the pre-#2773
// single-block shape that Codex 0.124.0+ rejects. migrateCodexHooksMapFormat
// converts these before validation runs; their presence here means migration
// failed to cover this entry — fail loudly rather than pass a broken config.
const HANDLER_FIELD_NAMES = new Set(['command', 'type', 'timeout', 'statusMessage']);
for (const entry of value) {
if (!entry || typeof entry !== 'object') continue;
if (entry.hooks === undefined) {
const strayKey = Object.keys(entry).find((k) => HANDLER_FIELD_NAMES.has(k));
if (strayKey) {
return {
ok: false,
reason: `hooks.${event}[] entry has handler field "${strayKey}" at event-entry level; ` +
`Codex 0.124.0+ requires handler fields nested under [[hooks.${event}.hooks]]`,
};
}
continue;
}
if (!Array.isArray(entry.hooks)) {
return {
ok: false,
reason: `hooks.${event}[].hooks must be an array of handler tables, got ${typeof entry.hooks}`,
};
}
for (const handler of entry.hooks) {
if (handler && typeof handler === 'object' && handler.type !== undefined) {
if (handler.type !== 'command') {
return {
ok: false,
reason: `hooks.${event}[].hooks[].type must be "command", got "${handler.type}"`,
};
}
}
}
}
}
}
}
@@ -6973,63 +7151,65 @@ function install(isGlobal, runtime = 'claude') {
let configContent = fs.existsSync(configPath) ? fs.readFileSync(configPath, 'utf-8') : '';
const eol = detectLineEnding(configContent);
// Migrate legacy [hooks] map format to [[hooks]] array-of-tables (#2637).
// Codex 0.124.0 requires [[hooks]] array-of-tables; old GSD installs wrote
// [hooks.shell] map tables which now cause a startup parse error.
// Strip ALL prior GSD-managed hook blocks BEFORE migration so the migration
// only touches user-authored hooks, not GSD-owned stale entries. Running
// strip after migration causes Shape 1 (legacy gsd-update-check filename)
// to be converted by migration before the strip regex can match it (#2698).
//
// Historical shapes stripped, in order:
// Shape 1 — legacy gsd-update-check filename (pre-#1755): flat [[hooks]] + event
// Shape 2 — flat [[hooks]] + event = "SessionStart" (#2637 era, never correct)
// Shape 4 — correct two-block nested (strip before shape 3 to avoid orphaned header)
// Shape 3 — single-block [[hooks.SessionStart]] without nested .hooks (#2760 era)
if (configContent.includes('gsd-update-check') || configContent.includes('gsd-check-update')) {
// Shape 1
configContent = configContent.replace(
/(?:\r?\n|^)# GSD Hooks\r?\n\[\[hooks\]\]\r?\nevent = "SessionStart"\r?\ncommand = "node [^\r\n]*gsd-update-check\.js"\r?\n/gm,
(match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''),
);
// Shape 2
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' : ''),
);
// Shape 4 — strip before shape 3 to avoid orphaned header
configContent = configContent.replace(
/(?:\r?\n|^)# GSD Hooks\r?\n\[\[hooks\.SessionStart\]\]\r?\n\r?\n\[\[hooks\.SessionStart\.hooks\]\]\r?\ntype = "command"\r?\ncommand = "node [^\r\n]*(?:gsd-check-update|gsd-update-check)\.js"\r?\n/gm,
(match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''),
);
// Shape 3
configContent = configContent.replace(
/(?:\r?\n|^)# GSD Hooks\r?\n\[\[hooks\.SessionStart\]\]\r?\ncommand = "node [^\r\n]*(?:gsd-check-update|gsd-update-check)\.js"\r?\n/gm,
(match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''),
);
}
// Migrate legacy [hooks] map format and flat [[hooks]] AoT entries to the
// namespaced [[hooks.<EVENT>]] form after stripping GSD-managed stale blocks.
// Running migration after strip ensures only user-authored hooks are migrated
// (#2698 regression: migration before strip converts stale GSD blocks before
// the strip regexes can match their original shape).
const migratedContent = migrateCodexHooksMapFormat(configContent);
if (migratedContent !== configContent) {
configContent = migratedContent;
console.log(` ${green}✓${reset} Migrated legacy Codex [hooks] map format to [[hooks]] array-of-tables`);
console.log(` ${green}✓${reset} Migrated legacy Codex [hooks] format to two-level nested AoT`);
}
const codexHooksFeature = ensureCodexHooksFeature(configContent);
configContent = setManagedCodexHooksOwnership(codexHooksFeature.content, codexHooksFeature.ownership);
// 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).
// Add SessionStart hook for update checking. Codex 0.124.0+ requires the
// two-level nested AoT schema: [[hooks.SessionStart]] for the event entry
// (holds optional matcher) and [[hooks.SessionStart.hooks]] for the handler
// (holds type, command, statusMessage, timeout). (#2637, #2760, #2773)
const updateCheckScript = path.resolve(targetDir, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/');
const useNamespacedAot = hasUserNamespacedAotHooks(configContent, 'SessionStart');
const hookBlock = useNamespacedAot
? `${eol}# GSD Hooks${eol}` +
const hookBlock = `${eol}# GSD Hooks${eol}` +
`[[hooks.SessionStart]]${eol}` +
`command = "node ${updateCheckScript}"${eol}`
: `${eol}# GSD Hooks${eol}` +
`[[hooks]]${eol}` +
`event = "SessionStart"${eol}` +
`${eol}` +
`[[hooks.SessionStart.hooks]]${eol}` +
`type = "command"${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.
// Single \r?\n-aware regex handles LF, CRLF, and block-at-file-start (#2698).
if (configContent.includes('gsd-update-check')) {
configContent = configContent.replace(
/(?:\r?\n|^)# GSD Hooks\r?\n\[\[hooks\]\]\r?\nevent = "SessionStart"\r?\ncommand = "node [^\r\n]*gsd-update-check\.js"\r?\n/gm,
(match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''),
);
}
// #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;
}

View File

@@ -90,12 +90,59 @@ describe('#2760 defect 3 — Hooks AoT preservation across install/uninstall/rei
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('preserves both pre-existing [[hooks.SessionStart]] entries and adds GSD entry in namespaced form', () => {
test('fresh install emits the two-level nested AoT schema (#2773)', () => {
// Codex 0.124.0+ requires [[hooks.SessionStart]] + [[hooks.SessionStart.hooks]]
// with type = "command". Neither the flat [[hooks]] + event field form nor
// the single-block [[hooks.SessionStart]] form without .hooks is accepted.
writeCodexConfig(codexHome, '');
runCodexInstall(codexHome);
const content = readCodexConfig(codexHome);
const parsed = parseTomlToObject(content);
// hooks must be an object (namespaced), NOT a flat array.
assert.ok(
parsed.hooks && !Array.isArray(parsed.hooks) && typeof parsed.hooks === 'object',
'hooks must be a namespaced object, not a flat array: got ' + JSON.stringify(parsed.hooks)
);
// hooks.SessionStart must be an array-of-tables.
assert.ok(
Array.isArray(parsed.hooks.SessionStart),
'hooks.SessionStart must be array-of-tables: got ' + typeof parsed.hooks.SessionStart
);
// Each event entry must have a .hooks sub-array.
const eventEntry = parsed.hooks.SessionStart[0];
assert.ok(
eventEntry && Array.isArray(eventEntry.hooks),
'hooks.SessionStart[0].hooks must be an array of handlers: got ' + JSON.stringify(eventEntry)
);
// The handler must have type = "command" and reference gsd-check-update.js.
const handler = eventEntry.hooks[0];
assert.strictEqual(handler.type, 'command', 'handler type must be "command"');
assert.ok(
typeof handler.command === 'string' && /gsd-check-update\.js/.test(handler.command),
'handler command must reference gsd-check-update.js: got ' + handler.command
);
// No flat [[hooks]] entries must exist alongside the namespaced form.
assert.ok(
!Array.isArray(parsed.hooks),
'flat [[hooks]] AoT must not coexist with namespaced [[hooks.SessionStart]]'
);
});
test('preserves user [[hooks.SessionStart]] entries and adds GSD nested handler', () => {
// Users may have their own [[hooks.SessionStart]] entries using the new schema.
// GSD must append its own two-level block without disturbing theirs.
const userConfig = [
'[[hooks.SessionStart]]',
'',
'[[hooks.SessionStart.hooks]]',
'type = "command"',
'command = "echo first user hook"',
'',
'[[hooks.SessionStart]]',
'',
'[[hooks.SessionStart.hooks]]',
'type = "command"',
'command = "echo second user hook"',
'',
].join('\n');
@@ -105,59 +152,111 @@ describe('#2760 defect 3 — Hooks AoT preservation across install/uninstall/rei
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')
'hooks.SessionStart must remain an array-of-tables after install'
);
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)
// Collect all handler commands across all event entries.
const allCommands = parsed.hooks.SessionStart.flatMap((entry) =>
Array.isArray(entry.hooks) ? entry.hooks.map((h) => h.command) : []
);
// 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)
allCommands.includes('echo first user hook'),
'first user hook preserved: ' + JSON.stringify(allCommands)
);
// 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'
allCommands.includes('echo second user hook'),
'second user hook preserved: ' + JSON.stringify(allCommands)
);
assert.ok(
allCommands.some((cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)),
'GSD handler must appear in hooks.SessionStart[].hooks: ' + JSON.stringify(allCommands)
);
assert.ok(!Array.isArray(parsed.hooks), 'no flat [[hooks]] entries');
});
test('selects top-level [[hooks]] form when user has no namespaced hooks (status-quo behavior)', () => {
writeCodexConfig(codexHome, '');
test('reinstall replaces flat [[hooks]] + event form with nested schema', () => {
// Upgrade path: user has a config written by GSD 1.38.x (flat [[hooks]] form).
const legacyConfig = [
'[features]',
'codex_hooks = true',
'',
'# GSD Hooks',
'[[hooks]]',
'event = "SessionStart"',
'command = "node /old/path/to/gsd-check-update.js"',
'',
].join('\n');
writeCodexConfig(codexHome, legacyConfig);
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
// Old flat form must be gone.
assert.ok(!Array.isArray(parsed.hooks), 'flat [[hooks]] must be stripped on upgrade');
// New nested form must be present.
assert.ok(Array.isArray(parsed.hooks && parsed.hooks.SessionStart), 'new [[hooks.SessionStart]] must be present');
const handler = parsed.hooks.SessionStart[0].hooks[0];
assert.strictEqual(handler.type, 'command');
assert.ok(/gsd-check-update\.js/.test(handler.command));
// Only one GSD hook entry must exist (no duplication).
const sessionStart = parsed.hooks?.SessionStart ?? [];
const gsdHandlers = sessionStart.flatMap((entry) =>
Array.isArray(entry.hooks) ? entry.hooks : []
).filter((h) => typeof h?.command === 'string' && /gsd-check-update\.js/.test(h.command));
assert.strictEqual(gsdHandlers.length, 1, 'exactly one managed handler after upgrade');
});
test('reinstall replaces single-block [[hooks.SessionStart]] (no .hooks sub-table) with nested schema', () => {
// Upgrade path: user has a config written by the PR #2802 shape —
// [[hooks.SessionStart]] without a nested [[hooks.SessionStart.hooks]] sub-table.
const prBranchConfig = [
'[features]',
'codex_hooks = true',
'',
'# GSD Hooks',
'[[hooks.SessionStart]]',
'command = "node /old/path/to/gsd-check-update.js"',
'',
].join('\n');
writeCodexConfig(codexHome, prBranchConfig);
runCodexInstall(codexHome);
const content = readCodexConfig(codexHome);
const parsed = parseTomlToObject(content);
assert.ok(Array.isArray(parsed.hooks && parsed.hooks.SessionStart), '[[hooks.SessionStart]] must be present');
const eventEntry = parsed.hooks.SessionStart[0];
assert.ok(Array.isArray(eventEntry.hooks), '[[hooks.SessionStart.hooks]] sub-table must be present');
const handler = eventEntry.hooks[0];
assert.strictEqual(handler.type, 'command');
assert.ok(/gsd-check-update\.js/.test(handler.command));
const handlers = (parsed.hooks?.SessionStart ?? []).flatMap((entry) =>
Array.isArray(entry.hooks) ? entry.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)
assert.strictEqual(
handlers.filter((h) => typeof h?.command === 'string' && /gsd-check-update\.js/.test(h.command)).length,
1,
'exactly one managed handler after upgrade from PR-#2802-shape'
);
});
test('reinstall is idempotent: correct nested schema is stripped and re-emitted cleanly', () => {
writeCodexConfig(codexHome, '');
runCodexInstall(codexHome);
runCodexInstall(codexHome); // second install
const content = readCodexConfig(codexHome);
const parsed = parseTomlToObject(content);
const sessionStart = parsed.hooks?.SessionStart ?? [];
assert.strictEqual(sessionStart.length, 1, 'exactly one SessionStart event entry after double install');
assert.ok(Array.isArray(sessionStart[0].hooks), 'SessionStart event has nested handlers array');
assert.strictEqual(sessionStart[0].hooks.length, 1, 'exactly one handler in SessionStart after double install');
assert.ok(/gsd-check-update\.js/.test(sessionStart[0].hooks[0].command), 'managed handler command preserved');
});
});
describe('#2760 fix 2 — Strip purges invalid legacy [agents] / [[agents]] regardless of marker', () => {
@@ -572,15 +671,25 @@ describe('#2760 CR4 finding 2 — Legacy flat [[hooks]] block migrates to namesp
+ (parsed.hooks ? typeof parsed.hooks.SessionStart : 'no hooks table')
);
const namespacedCommands = parsed.hooks.SessionStart.map((entry) => entry.command);
// Migration now handles stale [[hooks.SessionStart]] entries with handler
// fields at event-entry level (pre-#2773 shape), promoting them to the
// two-level nested form. Every entry must carry a .hooks sub-array after
// migration, so collect from nested handlers only.
assert.ok(
namespacedCommands.includes('echo user hook'),
'user [[hooks.SessionStart]] entry preserved: ' + JSON.stringify(namespacedCommands)
parsed.hooks.SessionStart.every((entry) => Array.isArray(entry.hooks)),
'every hooks.SessionStart entry must use nested [[hooks.SessionStart.hooks]] handlers after migration'
);
const allSessionStartCommands = parsed.hooks.SessionStart.flatMap((entry) =>
entry.hooks.map((h) => h.command).filter(Boolean)
);
assert.ok(
namespacedCommands.some((cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)),
allSessionStartCommands.includes('echo user hook'),
'user [[hooks.SessionStart]] entry preserved: ' + JSON.stringify(allSessionStartCommands)
);
assert.ok(
allSessionStartCommands.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)
+ JSON.stringify(allSessionStartCommands)
);
// The legacy top-level [[hooks]] AoT must NOT coexist with the namespaced
@@ -592,7 +701,7 @@ describe('#2760 CR4 finding 2 — Legacy flat [[hooks]] block migrates to namesp
);
// No duplicate gsd-check-update entries — exactly one managed entry.
const gsdEntries = namespacedCommands.filter(
const gsdEntries = allSessionStartCommands.filter(
(cmd) => typeof cmd === 'string' && /gsd-check-update\.js/.test(cmd)
);
assert.equal(gsdEntries.length, 1,
@@ -936,18 +1045,35 @@ describe('#2760 CR5 finding 3 — migration emits namespaced AoT (no flat/namesp
parsed.hooks && Array.isArray(parsed.hooks.AfterTool),
'pre-existing [[hooks.AfterTool]] must remain a namespaced AoT array'
);
// AfterTool was in [[hooks.AfterTool]] with command at event-entry level
// (pre-#2773 stale namespaced AoT shape). Migration now promotes these to
// the two-level nested form, so every entry must have a .hooks sub-array.
assert.ok(
parsed.hooks.AfterTool.some((entry) => entry.command === 'x'),
'user AfterTool entry must be preserved: ' + JSON.stringify(parsed.hooks.AfterTool)
parsed.hooks.AfterTool.every((e) => Array.isArray(e.hooks)),
'every AfterTool entry must use nested [[hooks.AfterTool.hooks]] handlers after migration'
);
const afterToolCommands = parsed.hooks.AfterTool.flatMap((e) =>
e.hooks.map((h) => h.command).filter(Boolean)
);
assert.ok(
afterToolCommands.includes('x'),
'user AfterTool entry must be preserved: ' + JSON.stringify(afterToolCommands)
);
// The migrated SessionStart entry is now namespaced AoT, not flat
// [[hooks]] with event="SessionStart".
// The migrated SessionStart entry is now namespaced AoT with nested .hooks sub-table.
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);
// After migration, [hooks.SessionStart] map-format is promoted to nested AoT.
// Command lives in [[hooks.SessionStart.hooks]][0].command (nested schema).
assert.ok(
parsed.hooks.SessionStart.every((e) => Array.isArray(e.hooks)),
'every SessionStart entry must use nested [[hooks.SessionStart.hooks]] handlers after migration'
);
const ssCommands = parsed.hooks.SessionStart.flatMap((e) =>
e.hooks.map((h) => h.command).filter(Boolean)
);
assert.ok(
ssCommands.includes('y'),
'user SessionStart command "y" must be preserved in namespaced array: ' +

View File

@@ -578,7 +578,9 @@ describe('stripGsdFromCodexConfig', () => {
// ─── migrateCodexHooksMapFormat ─────────────────────────────────────────────────
describe('migrateCodexHooksMapFormat', () => {
test('returns content unchanged when no legacy [hooks] map sections present', () => {
test('migrates flat [[hooks]] with event key to namespaced [[hooks.<EVENT>]] form', () => {
// Flat [[hooks]] + event = "..." is TOML-incompatible with [[hooks.SessionStart]],
// so migrateCodexHooksMapFormat now converts it to the nested namespaced form.
const content = [
'[features]',
'codex_hooks = true',
@@ -588,7 +590,21 @@ describe('migrateCodexHooksMapFormat', () => {
'command = "node /home/.codex/hooks/gsd-check-update.js"',
'',
].join('\n');
assert.strictEqual(migrateCodexHooksMapFormat(content), content);
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.SessionStart),
'flat [[hooks]] event=SessionStart must be promoted to [[hooks.SessionStart]] AoT');
assert.strictEqual(parsed.hooks.SessionStart.length, 1);
assert.ok(Array.isArray(parsed.hooks.SessionStart[0].hooks),
'must emit [[hooks.SessionStart.hooks]] sub-table');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].command,
'node /home/.codex/hooks/gsd-check-update.js');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].type, 'command',
'migrated handler must carry type = "command" per Codex 0.124.0+ schema');
assert.equal(parsed.hooks.SessionStart[0].event, undefined,
'event key consumed as namespace — must not appear in emitted block');
assert.ok(!Array.isArray(parsed.hooks), 'hooks must be a table, not a flat array');
assert.equal(parsed.features && parsed.features.codex_hooks, true);
});
test('returns content unchanged for empty string', () => {
@@ -612,7 +628,10 @@ describe('migrateCodexHooksMapFormat', () => {
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');
// #2773: command now lives in [[hooks.shell.hooks]] sub-table, not at event-entry level
assert.ok(Array.isArray(parsed.hooks.shell[0].hooks), 'must emit [[hooks.shell.hooks]] sub-table');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].command, 'node /home/.codex/hooks/gsd-check-update.js');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].type, 'command');
// 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');
@@ -633,8 +652,12 @@ describe('migrateCodexHooksMapFormat', () => {
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');
// #2773: command and extra keys now live in [[hooks.exec.hooks]] sub-table
assert.ok(Array.isArray(parsed.hooks.exec[0].hooks), 'must emit [[hooks.exec.hooks]] sub-table');
assert.strictEqual(parsed.hooks.exec[0].hooks[0].command, 'echo hello');
assert.strictEqual(parsed.hooks.exec[0].hooks[0].type, 'command',
'migrated handler must carry type = "command" per Codex 0.124.0+ schema');
assert.strictEqual(parsed.hooks.exec[0].hooks[0].extra_key, 'preserved');
assert.equal(parsed.hooks.exec[0].event, undefined);
});
@@ -653,18 +676,37 @@ describe('migrateCodexHooksMapFormat', () => {
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');
// #2773: commands now live in the [[hooks.<TYPE>.hooks]] sub-table
assert.strictEqual(parsed.hooks.shell[0].hooks[0].command, 'node /home/.codex/hooks/gsd-check-update.js');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].type, 'command',
'migrated shell handler must carry type = "command"');
assert.strictEqual(parsed.hooks.exec[0].hooks[0].command, 'echo done');
assert.strictEqual(parsed.hooks.exec[0].hooks[0].type, 'command',
'migrated exec handler must carry type = "command"');
});
test('leaves user-authored [[hooks]] array entries untouched when no legacy [hooks] map present', () => {
test('migrates flat [[hooks]] with event=AfterCommand to [[hooks.AfterCommand]] namespaced form', () => {
// Flat [[hooks]] + event = "..." is incompatible with [[hooks.<EVENT>]] AoT in the same
// file — TOML cannot have hooks be both an array and a table. Migration promotes it.
const content = [
'[[hooks]]',
'event = "AfterCommand"',
'command = "echo custom"',
'',
].join('\n');
assert.strictEqual(migrateCodexHooksMapFormat(content), content);
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.AfterCommand),
'flat [[hooks]] event=AfterCommand must become [[hooks.AfterCommand]] AoT');
assert.strictEqual(parsed.hooks.AfterCommand.length, 1);
assert.ok(Array.isArray(parsed.hooks.AfterCommand[0].hooks),
'must emit [[hooks.AfterCommand.hooks]] sub-table');
assert.strictEqual(parsed.hooks.AfterCommand[0].hooks[0].command, 'echo custom');
assert.strictEqual(parsed.hooks.AfterCommand[0].hooks[0].type, 'command',
'migrated AfterCommand handler must carry type = "command" per Codex 0.124.0+ schema');
assert.equal(parsed.hooks.AfterCommand[0].event, undefined,
'event key consumed as namespace — must not appear in emitted block');
assert.ok(!Array.isArray(parsed.hooks), 'hooks must be a table, not a flat array');
});
test('end-to-end: install on config with old [hooks] map format produces namespaced AoT (#2637, #2760 CR5)', () => {
@@ -686,8 +728,12 @@ describe('migrateCodexHooksMapFormat', () => {
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,
// #2773: command lives in [[hooks.shell.hooks]] sub-table
assert.ok(Array.isArray(parsed.hooks.shell[0].hooks), 'must emit [[hooks.shell.hooks]] sub-table');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].command,
'node /home/.codex/hooks/gsd-check-update.js');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].type, 'command',
'migrated shell handler must carry type = "command" per Codex 0.124.0+ schema');
assert.equal(parsed.features && parsed.features.codex_hooks, true);
});
@@ -710,6 +756,132 @@ describe('migrateCodexHooksMapFormat', () => {
assert.ok(result.includes('[model]'), 'preserves [model]');
});
test('upgrades stale [[hooks.SessionStart]] with event-level command to nested schema (#2773 CR6)', () => {
// Pre-#2773 single-block format: handler fields live directly under
// [[hooks.SessionStart]] rather than under [[hooks.SessionStart.hooks]].
// Codex 0.124.0+ rejects this shape. Migration must promote it.
const content = [
'[features]',
'codex_hooks = true',
'',
'[[hooks.SessionStart]]',
'command = "echo stale-user-hook"',
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.SessionStart),
'stale [[hooks.SessionStart]] must remain a namespaced AoT');
assert.strictEqual(parsed.hooks.SessionStart.length, 1);
assert.ok(Array.isArray(parsed.hooks.SessionStart[0].hooks),
'must emit [[hooks.SessionStart.hooks]] sub-table');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].command, 'echo stale-user-hook');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].type, 'command',
'must inject type = "command" when source body has no explicit type');
assert.equal(parsed.hooks.SessionStart[0].command, undefined,
'command must not remain at event-entry level after promotion');
assert.equal(parsed.features && parsed.features.codex_hooks, true);
});
test('leaves [[hooks.SessionStart]] + [[hooks.SessionStart.hooks]] untouched (already nested)', () => {
// Properly-nested schema: handler lives under [[hooks.SessionStart.hooks]].
// Migration must NOT create a double-wrapped [[hooks.SessionStart.hooks.hooks]] shape.
const content = [
'[[hooks.SessionStart]]',
'',
'[[hooks.SessionStart.hooks]]',
'type = "command"',
'command = "echo already-nested"',
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
assert.ok(Array.isArray(parsed.hooks?.SessionStart),
'SessionStart must remain a namespaced AoT after no-op migration');
assert.strictEqual(parsed.hooks.SessionStart.length, 1,
'must not duplicate the event entry');
assert.ok(Array.isArray(parsed.hooks.SessionStart[0].hooks),
'nested [[hooks.SessionStart.hooks]] sub-table must still be present');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks.length, 1,
'must not create a double-wrapped [[hooks.SessionStart.hooks.hooks]]');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].type, 'command');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].command, 'echo already-nested');
assert.equal(parsed.hooks.SessionStart[0].command, undefined,
'command must not appear at event-entry level');
});
test('promotes multiple stale [[hooks.TYPE]] entries from different event types', () => {
const content = [
'[[hooks.SessionStart]]',
'command = "echo session"',
'',
'[[hooks.AfterCommand]]',
'command = "echo after-cmd"',
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.SessionStart));
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.AfterCommand));
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].command, 'echo session');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].type, 'command');
assert.strictEqual(parsed.hooks.AfterCommand[0].hooks[0].command, 'echo after-cmd');
assert.strictEqual(parsed.hooks.AfterCommand[0].hooks[0].type, 'command');
assert.equal(parsed.hooks.SessionStart[0].command, undefined);
assert.equal(parsed.hooks.AfterCommand[0].command, undefined);
});
test('matcher-only [[hooks.SessionStart]] (no handler fields) is left untouched', () => {
// A [[hooks.SessionStart]] entry with only a `matcher` key is a valid
// event filter — no handler fields → not a stale single-block entry.
const content = [
'[[hooks.SessionStart]]',
'matcher = "some-tool"',
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
assert.ok(Array.isArray(parsed.hooks?.SessionStart),
'matcher-only SessionStart must remain a namespaced AoT');
assert.strictEqual(parsed.hooks.SessionStart.length, 1);
assert.strictEqual(parsed.hooks.SessionStart[0].matcher, 'some-tool',
'matcher key must be preserved');
assert.equal(parsed.hooks.SessionStart[0].hooks, undefined,
'matcher-only entry must not gain a .hooks sub-array');
assert.equal(parsed.hooks.SessionStart[0].command, undefined,
'no spurious command key must appear');
});
test('quoted event name with dot ([[hooks."before.tool"]]) is treated as single 2-segment namespace', () => {
// Regression for the split('.') bug: "before.tool" contains a dot, but the
// key is quoted so it is ONE segment — [[hooks."before.tool"]] has exactly
// two path segments and must be classified the same as [[hooks.SessionStart]].
// It should NOT be treated as a 3-level path (hooks / before / tool).
const content = [
'[[hooks."before.tool"]]',
'command = "echo hi"',
'',
].join('\n');
const result = migrateCodexHooksMapFormat(content);
const parsed = parseTomlToObject(result);
// The key in the parsed object is the unquoted event name "before.tool".
assert.ok(
parsed.hooks && Array.isArray(parsed.hooks['before.tool']),
'[[hooks."before.tool"]] must be a namespaced AoT — not split on the inner dot'
);
assert.ok(
Array.isArray(parsed.hooks['before.tool'][0].hooks),
'must emit [[hooks."before.tool".hooks]] sub-table'
);
assert.strictEqual(
parsed.hooks['before.tool'][0].hooks[0].command,
'echo hi',
'command must be preserved in the nested handler sub-table'
);
// Ensure no spurious "before" or "tool" top-level hook keys appeared.
assert.equal(parsed.hooks?.before, undefined, 'must not split quoted key on dot');
});
test('CRLF line endings are preserved through migration (#2760 CR5: namespaced AoT)', () => {
const content = [
'[features]',
@@ -725,8 +897,12 @@ describe('migrateCodexHooksMapFormat', () => {
// 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,
// #2773: command lives in [[hooks.shell.hooks]] sub-table
assert.ok(Array.isArray(parsed.hooks.shell[0].hooks), 'must emit [[hooks.shell.hooks]] sub-table');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].command,
'node /home/.codex/hooks/gsd-check-update.js');
assert.strictEqual(parsed.hooks.shell[0].hooks[0].type, 'command',
'migrated shell handler must carry type = "command" per Codex 0.124.0+ schema');
});
});
@@ -750,7 +926,7 @@ describe('Codex hooks emit: migration produces namespaced AoT so managed-emit co
fs.rmSync(tmpDir, { recursive: true, force: true });
});
test('migration of legacy [hooks.SessionStart] produces namespaced AoT', () => {
test('migration of legacy [hooks.SessionStart] produces two-level nested AoT (#2773)', () => {
const legacyContent = [
'[features]',
'codex_hooks = true',
@@ -761,16 +937,27 @@ describe('Codex hooks emit: migration produces namespaced AoT so managed-emit co
].join('\n');
const migrated = migrateCodexHooksMapFormat(legacyContent);
const parsed = parseTomlToObject(migrated);
// Outer event entry
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'
assert.equal(Array.isArray(parsed.hooks), false,
'migration must NOT emit a flat top-level [[hooks]] AoT');
// Inner handler sub-table
assert.ok(
Array.isArray(parsed.hooks.SessionStart[0].hooks),
'migration must emit [[hooks.SessionStart.hooks]] sub-table'
);
const handler = parsed.hooks.SessionStart[0].hooks[0];
assert.strictEqual(handler.type, 'command',
'migration must inject type = "command" in handler sub-table');
assert.strictEqual(
handler.command,
'node /home/.codex/hooks/gsd-check-update.js',
'migration must preserve original command value in handler sub-table'
);
});
});
@@ -1237,7 +1424,17 @@ describe('Codex install hook configuration (e2e)', () => {
const content = readCodexConfig(codexHome);
assert.ok(content.includes('[features]\ncodex_hooks = true\n'), 'writes codex_hooks feature');
assert.ok(content.includes('# GSD Hooks\n[[hooks]]\nevent = "SessionStart"\n'), 'writes GSD SessionStart hook block');
// Codex 0.124.0+ nested schema: [[hooks.SessionStart]] + [[hooks.SessionStart.hooks]]
const parsed = parseTomlToObject(content);
assert.ok(parsed.hooks && Array.isArray(parsed.hooks.SessionStart), 'writes [[hooks.SessionStart]] AoT');
assert.ok(Array.isArray(parsed.hooks.SessionStart[0].hooks), 'writes [[hooks.SessionStart.hooks]] sub-table');
assert.strictEqual(parsed.hooks.SessionStart[0].hooks[0].type, 'command', 'handler type is "command"');
assert.strictEqual(
parsed.hooks.SessionStart[0].hooks[0].command,
'node ' + path.join(codexHome, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/'),
'handler command must be the exact absolute path to gsd-check-update.js'
);
assert.ok(!Array.isArray(parsed.hooks), 'no flat [[hooks]] AoT emitted');
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'writes one codex_hooks key');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'writes one GSD update hook');
assertNoDraftRootKeys(content);
@@ -1667,7 +1864,14 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('notes = \'\'\'\n[model]\ncodex_hooks = false\n\'\'\''), 'preserves multiline string content');
assert.strictEqual(countMatches(content, /^codex_hooks = false$/gm), 1, 'does not rewrite codex_hooks text inside multiline string');
assert.ok(content.indexOf('codex_hooks = true') > content.indexOf('other_feature = true'), 'does not stop the features section at multiline string content');
assert.ok(content.indexOf('codex_hooks = true') < content.indexOf('[[hooks]]'), 'inserts the real codex_hooks key before the next table');
// Parse structurally — verify codex_hooks and migrated AfterCommand hook via parsed object
const parsed = parseTomlToObject(content);
assert.equal(parsed.features?.codex_hooks, true, 'writes a real codex_hooks boolean key');
assert.ok(Array.isArray(parsed.hooks?.AfterCommand), 'AfterCommand flat [[hooks]] migrated to namespaced AoT');
const afterCmds = parsed.hooks.AfterCommand.flatMap((entry) =>
Array.isArray(entry.hooks) ? entry.hooks.map((h) => h.command).filter(Boolean) : []
);
assert.ok(afterCmds.includes('echo custom-after-command'), 'preserves AfterCommand user hook command');
assertNoDraftRootKeys(content);
});
@@ -1846,7 +2050,10 @@ describe('Codex install hook configuration (e2e)', () => {
// [features] is inserted after top-level lines, before [model] — not prepended
assert.ok(content.includes('# first line wins\n\n[features]\ncodex_hooks = true\n'), 'inserts features after top-level lines using first newline style');
assert.ok(content.includes(`# GSD Agent Configuration — managed by get-shit-done installer\n`), 'writes the managed agent block using the first newline style');
assert.ok(content.includes('# GSD Hooks\n[[hooks]]\nevent = "SessionStart"\n'), 'writes the GSD hook block using the first newline style');
// Structural check: nested schema must be present regardless of mixed EOL
const parsedMixed = parseTomlToObject(content);
assert.ok(parsedMixed.hooks && Array.isArray(parsedMixed.hooks.SessionStart), 'writes [[hooks.SessionStart]] AoT with first-newline style');
assert.ok(Array.isArray(parsedMixed.hooks.SessionStart[0].hooks), 'writes [[hooks.SessionStart.hooks]] sub-table');
assert.ok(content.includes('[model]\r\nname = "o3"'), 'preserves the existing CRLF model lines');
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'remains idempotent on repeated installs');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'does not duplicate the GSD hook block');