* test: codex hooks.state.<key> tables must validate as regular tables (#3285 RED) Drive validateCodexConfigSchema with a fixture containing both [hooks.state] and [[hooks.SessionStart]] entries. Expect the state tables to pass as regular tables. Currently fails — validator over-classifies every hooks.* path as AoT. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(install): treat codex hooks.state as regular table not AoT (#3285) validateCodexConfigSchema was over-classifying every hooks.* section header as an event-handler array-of-tables, rejecting the hooks.state.* namespace that Codex CLI 0.130.0+ uses for per-hook trust persistence. Fix: 1. Section-header check: carve out `hooks.state` and `hooks.state.*` from the AoT-required rule — only paths that are neither of those still require double-bracket form. 2. Parsed-object check: skip the `state` key when iterating Object.entries (parsed.hooks) so the "must be array" guard does not fire for the trust namespace object. All other hooks.<EVENT> validation (SessionStart AoT, handler-field placement) is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * changeset: pr=3289 for #3285 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(install): codex hooks.state must be regular-table, reject AoT/scalar (CR finding 1) - migrateCodexHooksMapFormat: exclude hooks.state and hooks.state.* from legacy-map detection so [hooks.state] is never promoted to [[hooks.state]] AoT - validateCodexConfigSchema: reject [[hooks.state]] / [[hooks.state.*]] AoT at section level; reject Array/scalar values at parsed-object level - Accept only plain-object shape for hooks.state and hooks.state.* (Codex CLI 0.130.0+ trust-persistence namespace) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: assert codex hooks.state trust entry preserved with original values (CR finding 2) Strengthen post-install preservation assertion to verify the actual trust entry key and its enabled/trusted_hash values survive — not just that hooks.state is an object. Add two validator-reject tests for [[hooks.state]] and [[hooks.state.foo]] AoT forms. 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
5
.changeset/mellow-lynx-forage.md
Normal file
5
.changeset/mellow-lynx-forage.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3289
|
||||
---
|
||||
**`get-shit-done-cc --codex` no longer rejects valid Codex `hooks.state` trust-persistence entries** — the schema validator was over-classifying every `hooks.*` table as an event-handler array-of-tables, breaking installs against Codex CLI 0.130.0+ where `hooks.state.<project>/...` stores per-hook trust state. Regular-table shape is now accepted for `hooks.state.*` while `hooks.<EVENT>` still requires AoT.
|
||||
@@ -3336,10 +3336,13 @@ function migrateCodexHooksMapFormat(content) {
|
||||
// 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".
|
||||
// Exclude hooks.state and hooks.state.* — these are Codex's persistent hook-trust
|
||||
// namespace (Codex CLI 0.130.0+) and use regular-table shape, never AoT.
|
||||
const legacyMapSections = sections.filter(
|
||||
(section) => !section.array && (
|
||||
section.path === 'hooks' ||
|
||||
(section.path.startsWith('hooks.') && section.segments.length === 2)
|
||||
(section.path.startsWith('hooks.') && section.segments.length === 2 &&
|
||||
section.path !== 'hooks.state' && !section.path.startsWith('hooks.state.'))
|
||||
)
|
||||
);
|
||||
|
||||
@@ -3977,7 +3980,20 @@ function validateCodexConfigSchema(content) {
|
||||
};
|
||||
}
|
||||
|
||||
if (!section.array && section.path.startsWith('hooks.')) {
|
||||
// hooks.state.* is Codex's persistent hook-trust namespace (added in
|
||||
// Codex CLI 0.130.0). It uses regular-table shape, NOT array-of-tables.
|
||||
// [[hooks.state]] or [[hooks.state.<key>]] (AoT) is invalid; reject it.
|
||||
if (section.array && (section.path === 'hooks.state' || section.path.startsWith('hooks.state.'))) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: `[[${section.path}]] is invalid; hooks.state namespace must use regular tables`,
|
||||
};
|
||||
}
|
||||
|
||||
// All other hooks.* paths (event handlers like hooks.SessionStart) require
|
||||
// AoT shape — bare [hooks.<Event>] (single-bracket) is invalid.
|
||||
if (!section.array && section.path.startsWith('hooks.') &&
|
||||
section.path !== 'hooks.state' && !section.path.startsWith('hooks.state.')) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: `bare [${section.path}] table is invalid in current Codex schema (expected [[${section.path}]] array-of-tables)`,
|
||||
@@ -3997,6 +4013,24 @@ function validateCodexConfigSchema(content) {
|
||||
}
|
||||
if (typeof parsed.hooks === 'object' && parsed.hooks !== null) {
|
||||
for (const [event, value] of Object.entries(parsed.hooks)) {
|
||||
// hooks.state is Codex's persistent hook-trust namespace — a regular
|
||||
// object (table), not an array of event-handler tables.
|
||||
// Reject AoT shape (Array) and scalar forms; only plain objects are valid.
|
||||
if (event === 'state') {
|
||||
if (Array.isArray(value)) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: `hooks.state must be a regular table/object, got array-of-tables`,
|
||||
};
|
||||
}
|
||||
if (typeof value !== 'object' || value === null) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: `hooks.state must be a regular table/object, got ${typeof value}`,
|
||||
};
|
||||
}
|
||||
continue;
|
||||
}
|
||||
// Skip the nested .hooks sub-array — it lives under hooks.<Event>[n].hooks
|
||||
// and is validated separately below.
|
||||
if (!Array.isArray(value)) {
|
||||
|
||||
282
tests/bug-3285-codex-hooks-state-allowed.test.cjs
Normal file
282
tests/bug-3285-codex-hooks-state-allowed.test.cjs
Normal file
@@ -0,0 +1,282 @@
|
||||
/**
|
||||
* Regression: issue #3285 — Codex install fails when config.toml contains
|
||||
* hooks.state entries.
|
||||
*
|
||||
* Root cause: validateCodexConfigSchema walks every `hooks.*` table section
|
||||
* and asserts array-of-tables (AoT) shape, without distinguishing the
|
||||
* `hooks.state.*` namespace (Codex-managed per-hook trust persistence, a
|
||||
* regular table) from `hooks.<EVENT>` (event handlers like SessionStart,
|
||||
* which DO require AoT shape via [[hooks.SessionStart]]).
|
||||
*
|
||||
* Fix: add a carve-out so that any table whose path starts with `hooks.state`
|
||||
* is validated as a regular table (not AoT). All `hooks.<EVENT>` paths still
|
||||
* require AoT.
|
||||
*/
|
||||
|
||||
// GSD_TEST_MODE must be set before require('../bin/install.js') so the module
|
||||
// skips the main CLI entry point and exports its internals.
|
||||
const previousGsdTestMode = process.env.GSD_TEST_MODE;
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
const { test, describe } = 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');
|
||||
|
||||
const { validateCodexConfigSchema, install } = require('../bin/install.js');
|
||||
const installModule = require('../bin/install.js');
|
||||
|
||||
if (previousGsdTestMode === undefined) {
|
||||
delete process.env.GSD_TEST_MODE;
|
||||
} else {
|
||||
process.env.GSD_TEST_MODE = previousGsdTestMode;
|
||||
}
|
||||
|
||||
// Ensure hooks/dist/ is populated — mirrors the pattern used by codex-config.test.cjs.
|
||||
const { before, beforeEach, afterEach } = require('node:test');
|
||||
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' });
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Validator unit tests (no install, just validateCodexConfigSchema)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('#3285 — validateCodexConfigSchema: hooks.state is a regular table (not AoT)', () => {
|
||||
test('bare [hooks.state] table header passes validation', () => {
|
||||
const content = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, true,
|
||||
'bare [hooks.state] must be allowed (regular-table namespace): ' + result.reason);
|
||||
});
|
||||
|
||||
test('bare [hooks.state.<project-key>] table header passes validation', () => {
|
||||
// Mirrors the exact shape Codex CLI 0.130.0+ writes for per-hook trust entries.
|
||||
// The key contains slashes and colons — must be quoted in TOML.
|
||||
const content = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
"[hooks.state.'/home/user/.codex/hooks.json:pre_tool_use:0:0']",
|
||||
'enabled = true',
|
||||
'trusted_hash = "sha256:abc123"',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, true,
|
||||
'bare [hooks.state.<key>] with trust fields must be allowed: ' + result.reason);
|
||||
});
|
||||
|
||||
test('hooks.state alongside [[hooks.SessionStart]] AoT both pass', () => {
|
||||
// The real-world fixture: user has both Codex trust state AND GSD-managed
|
||||
// event hooks in the same config.toml.
|
||||
const content = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
"[hooks.state.'/home/user/.codex/hooks.json:pre_tool_use:0:0']",
|
||||
'enabled = true',
|
||||
'trusted_hash = "sha256:abc123"',
|
||||
'',
|
||||
'[[hooks.SessionStart]]',
|
||||
'',
|
||||
'[[hooks.SessionStart.hooks]]',
|
||||
'type = "command"',
|
||||
'command = "/usr/local/bin/gsd-check-update"',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, true,
|
||||
'mixed hooks.state (regular table) + [[hooks.SessionStart]] (AoT) must pass: ' + result.reason);
|
||||
});
|
||||
|
||||
test('[[hooks.SessionStart]] AoT still requires array-of-tables shape', () => {
|
||||
// Regression guard: the fix must NOT relax AoT requirements for event hooks.
|
||||
// [hooks.SessionStart] (single-bracket) must still fail.
|
||||
const content = [
|
||||
'[hooks.SessionStart]',
|
||||
'type = "command"',
|
||||
'command = "/some/command"',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, false,
|
||||
'[hooks.SessionStart] bare table (not AoT) must still be rejected');
|
||||
assert.ok(
|
||||
result.reason.includes('hooks.SessionStart'),
|
||||
'rejection reason must mention hooks.SessionStart, got: ' + result.reason
|
||||
);
|
||||
});
|
||||
|
||||
test('hooks.state object in parsed structure does not trigger non-array rejection', () => {
|
||||
// The parsed-object check loops over Object.entries(parsed.hooks) and
|
||||
// asserts !Array.isArray(value) → error. hooks.state is an object, not
|
||||
// an array. The fix must skip hooks.state in that loop too.
|
||||
const content = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
"[hooks.state.'some-key']",
|
||||
'enabled = true',
|
||||
'trusted_hash = "sha256:deadbeef"',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, true,
|
||||
'parsed hooks.state object must not trigger "hooks.state must be an array" rejection: ' + result.reason);
|
||||
});
|
||||
|
||||
test('multiple hooks.state sub-keys all pass validation', () => {
|
||||
const content = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
"[hooks.state.'/project/a/.codex/hooks.json:pre_tool_use:0:0']",
|
||||
'enabled = true',
|
||||
'trusted_hash = "sha256:aaa"',
|
||||
'',
|
||||
"[hooks.state.'/project/b/.codex/hooks.json:pre_tool_use:0:0']",
|
||||
'enabled = false',
|
||||
'trusted_hash = "sha256:bbb"',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, true,
|
||||
'multiple hooks.state sub-keys must all pass: ' + result.reason);
|
||||
});
|
||||
|
||||
test('[[hooks.state]] AoT form is rejected', () => {
|
||||
// hooks.state must be a regular table — array-of-tables shape is invalid.
|
||||
const content = [
|
||||
'[[hooks.state]]',
|
||||
'enabled = true',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, false,
|
||||
'[[hooks.state]] (AoT) must be rejected');
|
||||
assert.ok(
|
||||
result.reason.includes('hooks.state'),
|
||||
'rejection reason must mention hooks.state, got: ' + result.reason
|
||||
);
|
||||
});
|
||||
|
||||
test('[[hooks.state.foo]] AoT sub-key form is rejected', () => {
|
||||
// hooks.state.* sub-keys must be regular tables — AoT sub-key shape is invalid.
|
||||
const content = [
|
||||
'[[hooks.state.foo]]',
|
||||
'enabled = true',
|
||||
'',
|
||||
].join('\n');
|
||||
const result = validateCodexConfigSchema(content);
|
||||
assert.strictEqual(result.ok, false,
|
||||
'[[hooks.state.foo]] (AoT sub-key) must be rejected');
|
||||
assert.ok(
|
||||
result.reason.includes('hooks.state'),
|
||||
'rejection reason must mention hooks.state, got: ' + result.reason
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Full install integration test
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('#3285 — install succeeds when config.toml contains hooks.state entries', { concurrency: false }, () => {
|
||||
let tmpDir;
|
||||
let codexHome;
|
||||
|
||||
function writeCodexConfig(content) {
|
||||
fs.mkdirSync(codexHome, { recursive: true });
|
||||
fs.writeFileSync(path.join(codexHome, 'config.toml'), content, 'utf8');
|
||||
}
|
||||
|
||||
function runCodexInstall() {
|
||||
const previousCodexHome = process.env.CODEX_HOME;
|
||||
const previousCwd = process.cwd();
|
||||
process.env.CODEX_HOME = codexHome;
|
||||
try {
|
||||
process.chdir(path.join(__dirname, '..'));
|
||||
return install(true, 'codex');
|
||||
} finally {
|
||||
process.chdir(previousCwd);
|
||||
if (previousCodexHome === undefined) {
|
||||
delete process.env.CODEX_HOME;
|
||||
} else {
|
||||
process.env.CODEX_HOME = previousCodexHome;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3285-'));
|
||||
codexHome = path.join(tmpDir, 'codex-home');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('install does not throw when config.toml contains hooks.state trust entries', () => {
|
||||
// This is the exact failure scenario reported in #3285.
|
||||
const preInstall = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
"[hooks.state.'/home/user/.codex/hooks.json:pre_tool_use:0:0']",
|
||||
'enabled = true',
|
||||
'trusted_hash = "sha256:abc123def456"',
|
||||
'',
|
||||
].join('\n');
|
||||
writeCodexConfig(preInstall);
|
||||
|
||||
assert.doesNotThrow(
|
||||
() => runCodexInstall(),
|
||||
'install must not throw when config.toml contains hooks.state trust entries'
|
||||
);
|
||||
});
|
||||
|
||||
test('hooks.state entries are preserved in post-install config.toml', () => {
|
||||
const preInstall = [
|
||||
'[hooks.state]',
|
||||
'',
|
||||
"[hooks.state.'/home/user/.codex/hooks.json:pre_tool_use:0:0']",
|
||||
'enabled = true',
|
||||
'trusted_hash = "sha256:abc123def456"',
|
||||
'',
|
||||
].join('\n');
|
||||
writeCodexConfig(preInstall);
|
||||
|
||||
runCodexInstall();
|
||||
|
||||
const after = fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8');
|
||||
// Verify structurally: the trust hash key must survive the install.
|
||||
// Do NOT grep for the literal string — parse the TOML structure.
|
||||
const { parseTomlToObject } = require('../bin/install.js');
|
||||
const parsed = parseTomlToObject(after);
|
||||
assert.ok(
|
||||
parsed.hooks && typeof parsed.hooks.state === 'object' && parsed.hooks.state !== null,
|
||||
'post-install config.toml must have hooks.state as an object'
|
||||
);
|
||||
// Verify the actual trust entry survives — not just that hooks.state is an object.
|
||||
const trustKey = "/home/user/.codex/hooks.json:pre_tool_use:0:0";
|
||||
assert.ok(
|
||||
parsed.hooks.state[trustKey] != null,
|
||||
`post-install must preserve the original trust entry for key: ${trustKey}`
|
||||
);
|
||||
assert.strictEqual(
|
||||
parsed.hooks.state[trustKey].enabled,
|
||||
true,
|
||||
'preserved trust entry must have enabled = true'
|
||||
);
|
||||
assert.strictEqual(
|
||||
parsed.hooks.state[trustKey].trusted_hash,
|
||||
'sha256:abc123def456',
|
||||
'preserved trust entry must have the original trusted_hash'
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user