diff --git a/CHANGELOG.md b/CHANGELOG.md index f0b169c354..6d5611d39c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). it). Re-run `gsd update` without `--minimal` to expand to the full surface. The install manifest now records `mode: "minimal" | "full"`. (#2762) +### Fixed +- **Codex hook configuration moved to `hooks.json` (#2637).** The installer no longer writes an inline `[[hooks]]` block to `~/.codex/config.toml`; Codex 0.124.0+ rejects that shape with `invalid type: map, expected a sequence in 'hooks'`. The managed `SessionStart` hook now lives in `hooks.json` (Codex's canonical location, per https://developers.openai.com/codex/hooks). Re-installing migrates legacy `[[hooks]]` blocks (both `gsd-check-update.js` and the older `gsd-update-check.js` filename, LF/CRLF/mixed EOLs). User-authored hooks in either file are preserved. Uninstall removes only the managed GSD entry. + ## [1.38.5] - 2026-04-25 ### Fixed diff --git a/bin/install.js b/bin/install.js index f18c25c06d..42ab1c8d01 100755 --- a/bin/install.js +++ b/bin/install.js @@ -2785,6 +2785,111 @@ function setManagedCodexHooksOwnership(content, ownership) { remainder; } +// Substring marker used to identify the GSD-managed hook in hooks.json. +// Matching by command-substring stays stable across install locations. +const GSD_CODEX_HOOK_MARKER = 'gsd-check-update'; +const GSD_CODEX_HOOK_EVENT = 'SessionStart'; + +function isManagedCodexHookCommand(command) { + return typeof command === 'string' && command.includes(GSD_CODEX_HOOK_MARKER); +} + +/** + * Insert (or update) the managed GSD SessionStart hook in Codex's hooks.json. + * Schema: https://developers.openai.com/codex/hooks. User-authored entries are + * preserved; unsupported root/`hooks` shapes return false without rewriting. + */ +function upsertManagedCodexHookInHooksJson(hooksJsonPath, command) { + let data = { hooks: {} }; + if (fs.existsSync(hooksJsonPath)) { + try { + const parsed = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf-8')); + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return false; + data = parsed; + } catch (e) { + return false; + } + } + if (data.hooks === undefined) { + data.hooks = {}; + } else if (!data.hooks || typeof data.hooks !== 'object' || Array.isArray(data.hooks)) { + return false; + } + if (data.hooks[GSD_CODEX_HOOK_EVENT] === undefined) { + data.hooks[GSD_CODEX_HOOK_EVENT] = []; + } else if (!Array.isArray(data.hooks[GSD_CODEX_HOOK_EVENT])) { + return false; + } + + let updated = false; + for (const entry of data.hooks[GSD_CODEX_HOOK_EVENT]) { + if (!entry || !Array.isArray(entry.hooks)) continue; + for (const h of entry.hooks) { + if (h && isManagedCodexHookCommand(h.command)) { + h.command = command; + updated = true; + } + } + } + if (!updated) { + data.hooks[GSD_CODEX_HOOK_EVENT].push({ hooks: [{ type: 'command', command }] }); + } + + fs.writeFileSync(hooksJsonPath, JSON.stringify(data, null, 2) + '\n'); + return true; +} + +/** + * Remove the managed GSD hook from Codex's hooks.json. User entries stay. + * Deletes the file only when the whole root object is empty after cleanup. + */ +function removeManagedCodexHookFromHooksJson(hooksJsonPath) { + if (!fs.existsSync(hooksJsonPath)) return false; + let data; + try { + data = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf-8')); + } catch (e) { + return false; + } + if (!data || typeof data !== 'object' || Array.isArray(data) || + !data.hooks || typeof data.hooks !== 'object' || Array.isArray(data.hooks)) { + // Unsupported root or `hooks` shape — leave the user's file alone. + return false; + } + + let changed = false; + for (const event of Object.keys(data.hooks)) { + if (!Array.isArray(data.hooks[event])) continue; + const filtered = data.hooks[event] + .map((entry) => { + if (!entry || !Array.isArray(entry.hooks)) return entry; + const before = entry.hooks.length; + entry.hooks = entry.hooks.filter((h) => !(h && isManagedCodexHookCommand(h.command))); + if (entry.hooks.length !== before) changed = true; + return entry.hooks.length > 0 ? entry : null; + }) + .filter(Boolean); + if (filtered.length !== data.hooks[event].length) changed = true; + if (filtered.length === 0) delete data.hooks[event]; + else data.hooks[event] = filtered; + } + + if (!changed) return false; + if (Object.keys(data.hooks).length === 0) { + // Only delete the file when the entire root is empty — otherwise the user's + // sibling top-level keys (e.g. version markers, custom metadata) survive. + delete data.hooks; + if (Object.keys(data).length === 0) { + fs.unlinkSync(hooksJsonPath); + } else { + fs.writeFileSync(hooksJsonPath, JSON.stringify(data, null, 2) + '\n'); + } + } else { + fs.writeFileSync(hooksJsonPath, JSON.stringify(data, null, 2) + '\n'); + } + return true; +} + function isLegacyGsdAgentsSection(body) { const lineRecords = getTomlLineRecords(body); const legacyKeys = new Set(['max_threads', 'max_depth']); @@ -4971,6 +5076,13 @@ function uninstall(isGlobal, runtime = 'claude') { console.log(` ${green}✓${reset} Cleaned GSD sections from config.toml`); } } + + // Codex: remove the managed GSD hook from hooks.json (preserves user hooks). + const hooksJsonPath = path.join(targetDir, 'hooks.json'); + if (removeManagedCodexHookFromHooksJson(hooksJsonPath)) { + removedCount++; + console.log(` ${green}✓${reset} Removed GSD hook from hooks.json`); + } } } else if (isCopilot) { // Copilot: remove skills/gsd-*/ directories (same layout as Codex skills) @@ -6420,48 +6532,35 @@ function install(isGlobal, runtime = 'claude') { console.log(` ${green}✓${reset} Installed hooks`); } - // Add Codex hooks (SessionStart for update checking) — requires codex_hooks feature flag + // SessionStart hook lives in hooks.json (https://developers.openai.com/codex/hooks); + // older GSD installs wrote it inline in config.toml — strip that on migration. const configPath = path.join(targetDir, 'config.toml'); + const hooksJsonPath = path.join(targetDir, 'hooks.json'); try { 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. - const migratedContent = migrateCodexHooksMapFormat(configContent); - if (migratedContent !== configContent) { - configContent = migratedContent; - console.log(` ${green}✓${reset} Migrated legacy Codex [hooks] map format to [[hooks]] array-of-tables`); - } const codexHooksFeature = ensureCodexHooksFeature(configContent); configContent = setManagedCodexHooksOwnership(codexHooksFeature.content, codexHooksFeature.ownership); - // Add SessionStart hook for update checking - const updateCheckScript = path.resolve(targetDir, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/'); - const hookBlock = - `${eol}# GSD Hooks${eol}` + - `[[hooks]]${eol}` + - `event = "SessionStart"${eol}` + - `command = "node ${updateCheckScript}"${eol}`; - - // 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' : ''), - ); - } - - if (hasEnabledCodexHooksFeature(configContent) && !configContent.includes('gsd-check-update')) { - configContent += hookBlock; - } + // Single \r?\n-aware regex covers LF, CRLF, mixed EOLs, and both filenames + // (gsd-check-update.js and the older gsd-update-check.js). + configContent = configContent.replace( + /(?:\r?\n|^)# GSD Hooks(?:\r?\n)\[\[hooks\]\](?:\r?\n)event = "SessionStart"(?:\r?\n)command = "node [^\r\n]*gsd-(?:check-update|update-check)\.js"(?:\r?\n)/g, + (match) => (match.startsWith('\r\n') ? '\r\n' : match.startsWith('\n') ? '\n' : ''), + ); fs.writeFileSync(configPath, configContent, 'utf-8'); - console.log(` ${green}✓${reset} Configured Codex hooks (SessionStart)`); + + if (hasEnabledCodexHooksFeature(configContent)) { + const command = buildHookCommand(targetDir, 'gsd-check-update.js'); + if (upsertManagedCodexHookInHooksJson(hooksJsonPath, command)) { + console.log(` ${green}✓${reset} Configured Codex hooks (SessionStart)`); + } else { + console.warn(` ${yellow}⚠${reset} Skipped hooks.json update — file is malformed or has an unsupported root shape`); + } + } else { + console.warn(` ${yellow}⚠${reset} Skipped Codex hooks — could not enable codex_hooks feature flag in config.toml`); + } } catch (e) { console.warn(` ${yellow}⚠${reset} Could not configure Codex hooks: ${e.message}`); } @@ -7486,6 +7585,7 @@ if (process.env.GSD_TEST_MODE) { configureOpencodePermissions, neutralizeAgentReferences, GSD_CODEX_MARKER, + GSD_CODEX_HOOK_MARKER, CODEX_AGENT_SANDBOX, getDirName, getGlobalDir, diff --git a/tests/bug-2698-crlf-install.test.cjs b/tests/bug-2698-crlf-install.test.cjs index cc24f98f19..5774e43410 100644 --- a/tests/bug-2698-crlf-install.test.cjs +++ b/tests/bug-2698-crlf-install.test.cjs @@ -37,7 +37,7 @@ const { execFileSync } = require('child_process'); const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); -const { install, GSD_CODEX_MARKER } = require(INSTALL_SRC); +const { install, GSD_CODEX_MARKER, GSD_CODEX_HOOK_MARKER } = require(INSTALL_SRC); // Ensure hooks/dist/ is populated before install tests before(() => { @@ -47,6 +47,16 @@ before(() => { }); }); +function hasManagedGsdHookInHooksJson(codexDir) { + const p = path.join(codexDir, 'hooks.json'); + if (!fs.existsSync(p)) return false; + const data = JSON.parse(fs.readFileSync(p, 'utf-8')); + const entries = data && data.hooks && data.hooks.SessionStart; + if (!Array.isArray(entries)) return false; + return entries.some((entry) => Array.isArray(entry.hooks) && + entry.hooks.some((h) => typeof h.command === 'string' && h.command.includes(GSD_CODEX_HOOK_MARKER))); +} + describe('#2698: CRLF stale gsd-update-check block is removed on Codex reinstall', () => { let tmpDir; @@ -103,8 +113,8 @@ describe('#2698: CRLF stale gsd-update-check block is removed on Codex reinstall 'Stale gsd-update-check entry must be removed from LF config.toml (#2698)' ); assert.ok( - content.includes('gsd-check-update'), - 'New gsd-check-update hook must appear after reinstall' + hasManagedGsdHookInHooksJson(path.join(tmpDir, '.codex')), + 'New gsd-check-update hook must appear in hooks.json after reinstall' ); }); @@ -124,8 +134,8 @@ describe('#2698: CRLF stale gsd-update-check block is removed on Codex reinstall 'Stale gsd-update-check entry must be removed from CRLF config.toml (#2698)' ); assert.ok( - content.includes('gsd-check-update'), - 'New gsd-check-update hook must appear after reinstall' + hasManagedGsdHookInHooksJson(path.join(tmpDir, '.codex')), + 'New gsd-check-update hook must appear in hooks.json after reinstall' ); }); diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index f7d1b603c0..f40c481874 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -24,7 +24,9 @@ const { migrateCodexHooksMapFormat, mergeCodexConfig, install, + uninstall, GSD_CODEX_MARKER, + GSD_CODEX_HOOK_MARKER, CODEX_AGENT_SANDBOX, } = require('../bin/install.js'); @@ -46,6 +48,20 @@ function runCodexInstall(codexHome, cwd = path.join(__dirname, '..')) { } } +function runCodexUninstall(codexHome, cwd = path.join(__dirname, '..')) { + const previousCodeHome = process.env.CODEX_HOME; + const previousCwd = process.cwd(); + process.env.CODEX_HOME = codexHome; + try { + process.chdir(cwd); + return uninstall(true, 'codex'); + } finally { + process.chdir(previousCwd); + if (previousCodeHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodeHome; + } +} + function readCodexConfig(codexHome) { return fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8'); } @@ -55,6 +71,28 @@ function writeCodexConfig(codexHome, content) { fs.writeFileSync(path.join(codexHome, 'config.toml'), content, 'utf8'); } +function readCodexHooksJson(codexHome) { + const p = path.join(codexHome, 'hooks.json'); + if (!fs.existsSync(p)) return null; + return JSON.parse(fs.readFileSync(p, 'utf8')); +} + +function countCodexGsdHooks(codexHome) { + const data = readCodexHooksJson(codexHome); + if (!data || !data.hooks || typeof data.hooks !== 'object') return 0; + let count = 0; + for (const event of Object.keys(data.hooks)) { + if (!Array.isArray(data.hooks[event])) continue; + for (const entry of data.hooks[event]) { + if (!entry || !Array.isArray(entry.hooks)) continue; + for (const h of entry.hooks) { + if (h && typeof h.command === 'string' && h.command.includes(GSD_CODEX_HOOK_MARKER)) count++; + } + } + } + return count; +} + function countMatches(content, pattern) { return (content.match(pattern) || []).length; } @@ -1129,19 +1167,18 @@ describe('Codex install hook configuration (e2e)', () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); - test('Codex install copies hook file that is referenced in config.toml (#2153)', () => { + test('Codex install copies hook file that is referenced in hooks.json (#2153)', () => { // Regression test: Codex install writes gsd-check-update hook reference into - // config.toml but must also copy the hook file to ~/$CODEX_HOME/hooks/ + // hooks.json but must also copy the hook file to ~/$CODEX_HOME/hooks/ runCodexInstall(codexHome); - const configContent = readCodexConfig(codexHome); - // config.toml must reference the hook - assert.ok(configContent.includes('gsd-check-update.js'), 'config.toml references gsd-check-update.js'); + // hooks.json must reference the hook + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'hooks.json references gsd-check-update.js'); // The hook file must physically exist at the referenced path const hookFile = path.join(codexHome, 'hooks', 'gsd-check-update.js'); assert.ok( fs.existsSync(hookFile), - `gsd-check-update.js must exist at ${hookFile} — config.toml references it but file was not installed` + `gsd-check-update.js must exist at ${hookFile} — hooks.json references it but file was not installed` ); }); @@ -1150,9 +1187,11 @@ 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'); 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'); + assert.ok(!content.includes('[[hooks]]'), 'does not write [[hooks]] block in config.toml'); + assert.ok(!content.includes('gsd-check-update'), 'does not reference GSD hook in config.toml'); + // GSD SessionStart hook lives in hooks.json (Codex's canonical location). + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'writes one GSD update hook into hooks.json'); assertNoDraftRootKeys(content); assertUsesOnlyEol(content, '\n'); }); @@ -1234,7 +1273,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(content.includes('# user comment'), 'preserves user comment'); assert.ok(content.includes('[model]\nname = "o3"'), 'preserves model section'); assert.ok(content.includes('command = "echo custom"'), 'preserves custom hook'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds one GSD update hook'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'adds one GSD update hook to hooks.json'); assertNoDraftRootKeys(content); }); @@ -1371,7 +1410,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(!content.includes('codex_hooks = false'), 'removes false codex_hooks value'); assert.ok(content.includes('other_feature = true'), 'preserves other feature keys'); assert.ok(content.includes('command = "echo custom"'), 'preserves custom hook'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'does not duplicate GSD update hook'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'does not duplicate GSD update hook'); assertNoDraftRootKeys(content); }); @@ -1413,7 +1452,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^"codex_hooks" = true$/gm), 1, 'normalizes the quoted codex_hooks key to true'); assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a second bare features table'); assert.ok(content.includes('other_feature = true'), 'preserves existing feature keys'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'keeps one GSD update hook'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'keeps one GSD update hook'); assertNoDraftRootKeys(content); }); @@ -1434,7 +1473,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(content.includes('[features."a#b"]\nenabled = true'), 'preserves the quoted nested features table'); assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 1, 'adds one real top-level features table'); assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'adds one codex_hooks key'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'remains idempotent for the GSD hook block'); assertNoDraftRootKeys(content); }); @@ -1454,7 +1493,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not add a [features] table'); assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 1, 'adds one dotted codex_hooks key'); assert.ok(content.includes('features.other_feature = true'), 'preserves existing dotted features key'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds one GSD update hook for dotted codex_hooks and remains idempotent'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'adds one GSD update hook for dotted codex_hooks and remains idempotent'); assertNoDraftRootKeys(content); }); @@ -1474,7 +1513,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(content.includes('features = { other_feature = true }'), 'preserves the root inline-table assignment'); assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append an invalid dotted codex_hooks key'); assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a features table'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely'); + assert.strictEqual(countCodexGsdHooks(codexHome), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely'); assert.ok(content.includes('[agents.gsd-executor]'), 'still installs the managed agent block in struct format'); assertNoDraftRootKeys(content); }); @@ -1495,7 +1534,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(content.includes('features = "disabled"'), 'preserves the root scalar assignment'); assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append an invalid dotted codex_hooks key'); assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a features table'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely'); + assert.strictEqual(countCodexGsdHooks(codexHome), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely'); assert.ok(content.includes('[agents.gsd-executor]'), 'still installs the managed agent block in struct format'); assertNoDraftRootKeys(content); }); @@ -1518,7 +1557,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^features\."codex_hooks" = true$/gm), 1, 'normalizes the quoted dotted key to true'); assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append a bare dotted duplicate'); assert.ok(content.includes('features.other_feature = true'), 'preserves other dotted features keys'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds one GSD update hook for quoted dotted codex_hooks and remains idempotent'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'adds one GSD update hook for quoted dotted codex_hooks and remains idempotent'); assertNoDraftRootKeys(content); }); @@ -1626,7 +1665,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'replaces the multiline basic-string assignment with one true value'); assert.ok(!content.includes('multiline-basic-sentinel'), 'removes multiline basic-string continuation lines'); assert.ok(content.includes('other_feature = true'), 'preserves following feature keys'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'remains idempotent for the GSD hook block'); assertNoDraftRootKeys(content); }); @@ -1651,7 +1690,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'replaces the multiline literal-string assignment with one true value'); assert.ok(!content.includes('multiline-literal-sentinel'), 'removes multiline literal-string continuation lines'); assert.ok(content.includes('other_feature = true'), 'preserves following feature keys'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'remains idempotent for the GSD hook block'); assertNoDraftRootKeys(content); }); @@ -1677,7 +1716,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(!content.includes('array-sentinel-1'), 'removes multiline array continuation lines'); assert.ok(!content.includes('array-sentinel-2'), 'removes multiline array continuation lines'); assert.ok(content.includes('other_feature = true'), 'preserves following feature keys'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'remains idempotent for the GSD hook block'); assertNoDraftRootKeys(content); }); @@ -1722,7 +1761,7 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'keeps one codex_hooks = true'); assert.ok(content.includes('other_feature = true'), 'preserves other feature keys'); assert.strictEqual(countMatches(content, /echo custom-after-command/g), 1, 'preserves non-GSD hook exactly once'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'keeps one GSD update hook'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'keeps one GSD update hook'); assertUsesOnlyEol(content, '\r\n'); assertNoDraftRootKeys(content); }); @@ -1745,10 +1784,189 @@ describe('Codex install hook configuration (e2e)', () => { assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 1, 'keeps one [features] section'); assert.strictEqual(countMatches(content, /^codex_hooks = true # keep me$/gm), 1, 'preserves the commented true value'); assert.ok(content.includes('other_feature = true'), 'preserves other feature keys'); - assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds the GSD update hook once'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'adds the GSD update hook once'); assertNoDraftRootKeys(content); }); + test('hooks.json: GSD hook is registered in canonical schema (#2637)', () => { + runCodexInstall(codexHome); + + const data = readCodexHooksJson(codexHome); + assert.ok(data, 'hooks.json exists'); + assert.ok(data.hooks && Array.isArray(data.hooks.SessionStart), 'has hooks.SessionStart array'); + assert.strictEqual(data.hooks.SessionStart.length, 1, 'exactly one SessionStart entry'); + const inner = data.hooks.SessionStart[0].hooks; + assert.ok(Array.isArray(inner) && inner.length === 1, 'one inner hook'); + assert.strictEqual(inner[0].type, 'command', 'inner hook is type=command'); + const expectedPath = path.resolve(codexHome, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/'); + assert.strictEqual(inner[0].command, `node "${expectedPath}"`, 'command points at gsd-check-update.js with absolute, shell-quoted path'); + }); + + test('hooks.json: merges into pre-existing user hooks without overwriting them', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const userHooksPath = path.join(codexHome, 'hooks.json'); + fs.writeFileSync(userHooksPath, JSON.stringify({ + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: 'echo user-session-start' }] }, + ], + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: 'echo user-prompt' }] }, + ], + }, + }, null, 2) + '\n'); + + runCodexInstall(codexHome); + + const data = readCodexHooksJson(codexHome); + // User hooks preserved + assert.strictEqual(data.hooks.UserPromptSubmit.length, 1, 'UserPromptSubmit kept'); + assert.strictEqual( + data.hooks.UserPromptSubmit[0].hooks[0].command, + 'echo user-prompt', + 'UserPromptSubmit command kept verbatim' + ); + // SessionStart now has both user entry and GSD entry + const cmds = data.hooks.SessionStart.flatMap((e) => e.hooks.map((h) => h.command)); + assert.ok(cmds.includes('echo user-session-start'), 'preserves user SessionStart'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'adds exactly one GSD hook'); + + // Idempotent + runCodexInstall(codexHome); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'second install does not duplicate'); + const data2 = readCodexHooksJson(codexHome); + assert.strictEqual(data2.hooks.UserPromptSubmit.length, 1, 'UserPromptSubmit still kept'); + }); + + test('hooks.json: install does NOT write [[hooks]] block in config.toml (#2637)', () => { + runCodexInstall(codexHome); + const content = readCodexConfig(codexHome); + assert.ok(!content.includes('[[hooks]]'), 'config.toml has no [[hooks]] block'); + assert.ok(!content.includes('# GSD Hooks'), 'config.toml has no # GSD Hooks marker'); + assert.ok(!content.includes('gsd-check-update'), 'config.toml does not reference gsd-check-update'); + }); + + test('hooks.json: array-rooted user hooks.json is left untouched', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const userHooksPath = path.join(codexHome, 'hooks.json'); + const arrayRoot = '[]'; + fs.writeFileSync(userHooksPath, arrayRoot); + + runCodexInstall(codexHome); + + // Array root is unsupported — file untouched, no GSD hook written. + assert.strictEqual(fs.readFileSync(userHooksPath, 'utf8'), arrayRoot, 'array-rooted file preserved'); + assert.strictEqual(countCodexGsdHooks(codexHome), 0, 'no GSD hook written into array-rooted file'); + }); + + test('hooks.json: uninstall preserves sibling top-level keys when hooks become empty', () => { + fs.mkdirSync(codexHome, { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'hooks.json'), JSON.stringify({ + version: '1.0', + metadata: { author: 'user' }, + hooks: {}, + }, null, 2) + '\n'); + + runCodexInstall(codexHome); + runCodexUninstall(codexHome); + + // hooks.json must still exist because of sibling keys; only hooks-side is cleaned. + const data = readCodexHooksJson(codexHome); + assert.ok(data, 'hooks.json preserved (had sibling top-level keys)'); + assert.strictEqual(data.version, '1.0', 'sibling version key preserved'); + assert.deepStrictEqual(data.metadata, { author: 'user' }, 'sibling metadata preserved'); + assert.ok(!('hooks' in data), 'empty hooks object removed (no orphan key)'); + }); + + test('hooks.json: unsupported hooks shape is left untouched', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const userHooksPath = path.join(codexHome, 'hooks.json'); + // User has `hooks` as a string (unsupported but parseable JSON). Don't normalize it. + const noncanonical = '{"hooks":"opt-out"}'; + fs.writeFileSync(userHooksPath, noncanonical); + + runCodexInstall(codexHome); + + assert.strictEqual(fs.readFileSync(userHooksPath, 'utf8'), noncanonical, 'noncanonical hooks payload preserved verbatim'); + assert.strictEqual(countCodexGsdHooks(codexHome), 0, 'no GSD hook injected into noncanonical file'); + }); + + test('hooks.json: unsupported event-array shape is left untouched', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const userHooksPath = path.join(codexHome, 'hooks.json'); + // User has `hooks.SessionStart` as an object instead of an array. Don't overwrite it. + const noncanonical = '{"hooks":{"SessionStart":{"opt-out":true}}}'; + fs.writeFileSync(userHooksPath, noncanonical); + + runCodexInstall(codexHome); + + assert.strictEqual(fs.readFileSync(userHooksPath, 'utf8'), noncanonical, 'noncanonical SessionStart shape preserved'); + assert.strictEqual(countCodexGsdHooks(codexHome), 0, 'no GSD hook injected when event slot has wrong shape'); + }); + + test('hooks.json: command path is shell-quoted so spaces in config dir work', () => { + runCodexInstall(codexHome); + + const data = readCodexHooksJson(codexHome); + const cmd = data.hooks.SessionStart[0].hooks[0].command; + assert.match(cmd, /^node "[^"]*gsd-check-update\.js"$/, 'command quotes the script path'); + }); + + test('hooks.json: uninstall leaves array-shaped user hooks payload untouched', () => { + fs.mkdirSync(codexHome, { recursive: true }); + // Pathological user file: `hooks` is an array. Codex's actual schema is an + // object keyed by event name, but we must not mutate user data even if the + // shape is unsupported. + const noncanonical = '{"hooks":[]}'; + const hooksPath = path.join(codexHome, 'hooks.json'); + fs.writeFileSync(hooksPath, noncanonical); + + runCodexInstall(codexHome); + runCodexUninstall(codexHome); + + assert.strictEqual(fs.readFileSync(hooksPath, 'utf8'), noncanonical, 'array-shaped hooks payload preserved verbatim'); + }); + + test('hooks.json: malformed user hooks.json is left untouched', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const userHooksPath = path.join(codexHome, 'hooks.json'); + const malformed = '{ this is not valid json'; + fs.writeFileSync(userHooksPath, malformed); + + runCodexInstall(codexHome); + + // File untouched — we don't overwrite user content we can't parse. + assert.strictEqual(fs.readFileSync(userHooksPath, 'utf8'), malformed, 'malformed file preserved'); + }); + + test('hooks.json: uninstall removes only the GSD hook and preserves user hooks', () => { + fs.mkdirSync(codexHome, { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'hooks.json'), JSON.stringify({ + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: 'echo user-session-start' }] }, + ], + }, + }, null, 2) + '\n'); + + runCodexInstall(codexHome); + runCodexUninstall(codexHome); + + const data = readCodexHooksJson(codexHome); + assert.ok(data, 'hooks.json still exists (had user content)'); + assert.strictEqual(countCodexGsdHooks(codexHome), 0, 'GSD hook removed'); + const cmds = (data.hooks.SessionStart || []).flatMap((e) => e.hooks.map((h) => h.command)); + assert.ok(cmds.includes('echo user-session-start'), 'user hook preserved'); + }); + + test('hooks.json: uninstall deletes hooks.json when GSD was its only entry', () => { + runCodexInstall(codexHome); + assert.ok(fs.existsSync(path.join(codexHome, 'hooks.json')), 'hooks.json created on install'); + + runCodexUninstall(codexHome); + assert.ok(!fs.existsSync(path.join(codexHome, 'hooks.json')), 'hooks.json removed when empty'); + }); + test('mixed-EOL configs use the first newline style for inserted Codex content', () => { writeCodexConfig(codexHome, '# first line wins\n[model]\r\nname = "o3"\r\n'); @@ -1759,10 +1977,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'); + assert.ok(!content.includes('[[hooks]]'), 'does not write [[hooks]] block in config.toml'); 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'); + assert.strictEqual(countCodexGsdHooks(codexHome), 1, 'does not duplicate the GSD hook block'); assertNoDraftRootKeys(content); }); });