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