* 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>
3c03a153a5