From 589318ce0a28aad9cc27f69e4d7d6f7736a2a926 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 12 May 2026 18:16:36 -0400 Subject: [PATCH] feat: route Codex hook commands through projection seam (#3444) * feat: route Codex hook commands through projection seam * docs: add changeset for Codex hook projection slice * docs: set changeset pr for #3444 * fix(codex): decode toml-escaped legacy hook path * test(codex): use structured assertion for hook projection --- .changeset/fair-hawks-play.md | 6 +++ bin/install.js | 45 ++++++++++++------- .../bin/lib/shell-command-projection.cjs | 26 +++++++++++ ...bug-3017-codex-hook-absolute-node.test.cjs | 44 ++++++++++++++++++ 4 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 .changeset/fair-hawks-play.md diff --git a/.changeset/fair-hawks-play.md b/.changeset/fair-hawks-play.md new file mode 100644 index 000000000..d3c253878 --- /dev/null +++ b/.changeset/fair-hawks-play.md @@ -0,0 +1,6 @@ +--- +type: Added +pr: 3444 +--- + +**Codex managed hook commands now project through the shared shell-command seam** — the installer reuses the same runtime-aware command rendering policy for `config.toml` hook blocks and Codex reinstall rewrites, reducing shell-format drift between runtime surfaces. Closes #3440. diff --git a/bin/install.js b/bin/install.js index 2f0142411..e899ea4b8 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8,6 +8,7 @@ const crypto = require('crypto'); const { formatHookCommandForRuntime: formatHookCommandForShell, formatManagedHookScriptToken, + projectCodexHookTomlCommand, } = require('../get-shit-done/bin/lib/shell-command-projection.cjs'); // Colors @@ -750,19 +751,19 @@ function buildCodexHookBlock(targetDir, opts) { const absoluteRunner = opts && opts.absoluteRunner; if (!absoluteRunner) return null; const eol = (opts && opts.eol) || '\n'; - const updateCheckScript = path.resolve(targetDir, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/'); - // toml requires escaped interior quotes (\"). The runner is already a - // JSON-stringified token (with literal " around the absolute path); we - // need to escape those quotes so the toml parser sees them as part of - // the string value, not as the closing quote of the command field. - const runnerEscaped = absoluteRunner.replace(/"/g, '\\"'); - const hookPathEscaped = updateCheckScript.replace(/"/g, '\\"'); + const platform = (opts && opts.platform) || process.platform; + const updateCheckScript = path.resolve(targetDir, 'hooks', 'gsd-check-update.js'); + const commandValue = projectCodexHookTomlCommand({ + absoluteRunner, + scriptPath: updateCheckScript, + platform, + }); return `${eol}# GSD Hooks${eol}` + `[[hooks.SessionStart]]${eol}` + `${eol}` + `[[hooks.SessionStart.hooks]]${eol}` + `type = "command"${eol}` + - `command = "${runnerEscaped} \\"${hookPathEscaped}\\""${eol}`; + `command = "${commandValue}"${eol}`; } /** @@ -778,8 +779,9 @@ function buildCodexHookBlock(targetDir, opts) { * @param {string|null} absoluteRunner - Result of resolveNodeRunner(). * @returns {{ content: string, changed: boolean }} */ -function rewriteLegacyCodexHookBlock(content, absoluteRunner) { +function rewriteLegacyCodexHookBlock(content, absoluteRunner, opts) { if (!content || !absoluteRunner) return { content, changed: false }; + const platform = (opts && opts.platform) || process.platform; let changed = false; // Match `command = "node "` lines where scriptToken is // either an unquoted path (no spaces) or a toml-escaped quoted path. @@ -794,16 +796,27 @@ function rewriteLegacyCodexHookBlock(content, absoluteRunner) { /^(command\s*=\s*")node\s+((?:\\"[^"]+\\"|\S+))("\s*)$/gm, (full, prefix, scriptToken, suffix) => { // Extract the underlying script path from the captured token — - // either the bare token or the inner content of \"...\". - const quoted = scriptToken.match(/^\\"(.+)\\"$/); - const scriptPath = quoted ? quoted[1] : scriptToken; + // either the bare token or the decoded inner content of \"...\". + const quoted = scriptToken.match(/^\\"([\s\S]+)\\"$/); + let scriptPath = scriptToken; + if (quoted) { + try { + scriptPath = String(parseTomlValue(`"${quoted[1]}"`, 0).value); + } catch { + scriptPath = quoted[1]; + } + } const base = scriptPath.split(/[\\/]/).pop() || ''; if (!CODEX_MANAGED_HOOK_BASENAMES.has(base)) return full; + const desiredCommand = projectCodexHookTomlCommand({ + absoluteRunner, + scriptPath, + platform, + }); + const currentCommand = `${prefix}${scriptToken}${suffix}`.replace(/^(command\s*=\s*")|("\s*)$/g, ''); + if (currentCommand === desiredCommand) return full; changed = true; - const runnerEscaped = absoluteRunner.replace(/"/g, '\\"'); - // Always re-quote the path on output for consistency with the new - // builder's shape. - return `${prefix}${runnerEscaped} \\"${scriptPath}\\"${suffix}`; + return `${prefix}${desiredCommand}${suffix}`; }, ); return { content: updated, changed }; diff --git a/get-shit-done/bin/lib/shell-command-projection.cjs b/get-shit-done/bin/lib/shell-command-projection.cjs index 0a01d2251..94c516123 100644 --- a/get-shit-done/bin/lib/shell-command-projection.cjs +++ b/get-shit-done/bin/lib/shell-command-projection.cjs @@ -45,8 +45,34 @@ function formatManagedHookScriptToken(scriptPath, opts = {}) { return JSON.stringify(scriptPath.replace(/\\/g, '/')); } +function projectManagedHookCommand({ absoluteRunner, scriptPath, runtime = 'generic', platform = process.platform }) { + if (!absoluteRunner || !scriptPath) return null; + const normalizedScriptPath = platform === 'win32' ? scriptPath.replace(/\\/g, '/') : scriptPath; + return formatHookCommandForRuntime(`${absoluteRunner} ${JSON.stringify(normalizedScriptPath)}`, { + platform, + runtime, + }); +} + +function escapeTomlDoubleQuotedString(value) { + return String(value).replace(/\\/g, '\\\\').replace(/"/g, '\\"'); +} + +function projectCodexHookTomlCommand({ absoluteRunner, scriptPath, platform = process.platform }) { + const command = projectManagedHookCommand({ + absoluteRunner, + scriptPath, + runtime: 'codex', + platform, + }); + return command === null ? null : escapeTomlDoubleQuotedString(command); +} + module.exports = { hookCommandNeedsPowerShellCallOperator, formatHookCommandForRuntime, formatManagedHookScriptToken, + projectManagedHookCommand, + escapeTomlDoubleQuotedString, + projectCodexHookTomlCommand, }; diff --git a/tests/bug-3017-codex-hook-absolute-node.test.cjs b/tests/bug-3017-codex-hook-absolute-node.test.cjs index 0d9b55c8d..933fbe39c 100644 --- a/tests/bug-3017-codex-hook-absolute-node.test.cjs +++ b/tests/bug-3017-codex-hook-absolute-node.test.cjs @@ -37,7 +37,9 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const INSTALL = require(path.join(__dirname, '..', 'bin', 'install.js')); +const projection = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'shell-command-projection.cjs')); const { buildCodexHookBlock, rewriteLegacyCodexHookBlock, resolveNodeRunner } = INSTALL; +const { projectCodexHookTomlCommand } = projection; /** * Parse the toml hook block into a typed record so tests can assert on @@ -86,6 +88,20 @@ function unescapeRunner(token) { return t; } +describe('Bug #3017 / #3440: Codex hook projection seam', () => { + test('projectCodexHookTomlCommand renders escaped command value from shared projection module', () => { + const commandValue = projectCodexHookTomlCommand({ + absoluteRunner: '"/usr/local/bin/node"', + scriptPath: '/tmp/codex-test/.codex/hooks/gsd-check-update.js', + platform: 'linux', + }); + assert.equal( + commandValue, + '\\"/usr/local/bin/node\\" \\"/tmp/codex-test/.codex/hooks/gsd-check-update.js\\"', + ); + }); +}); + describe('Bug #3017: buildCodexHookBlock emits absolute node runner', () => { test('exported as a function', () => { assert.equal(typeof buildCodexHookBlock, 'function'); @@ -169,6 +185,34 @@ describe('Bug #3017: rewriteLegacyCodexHookBlock migrates bare-node on reinstall assert.ok(result.content.includes('name = "o3"')); }); + test('decodes TOML-escaped quoted script paths before projection', () => { + const before = [ + '# GSD Hooks', + '[[hooks.SessionStart]]', + '', + '[[hooks.SessionStart.hooks]]', + 'type = "command"', + 'command = "node \\"C:\\\\Users\\\\x\\\\.codex\\\\hooks\\\\gsd-check-update.js\\""', + '', + ].join('\n'); + const runner = '"/usr/local/bin/node"'; + const result = rewriteLegacyCodexHookBlock(before, runner, { platform: 'win32' }); + assert.equal(result.changed, true); + const parsed = parseCodexHookBlock(result.content); + assert.equal(parsed.ok, true, 'hook block must parse correctly'); + const expected = projectCodexHookTomlCommand({ + absoluteRunner: runner, + scriptPath: 'C:\\Users\\x\\.codex\\hooks\\gsd-check-update.js', + platform: 'win32', + }); + assert.equal(parsed.command, expected, + 'rewritten command must project from decoded Windows path (not TOML-escaped token text)'); + assert.equal(unescapeRunner(parsed.runner), '/usr/local/bin/node', + 'runner must equal supplied absolute path'); + assert.equal(parsed.hookPath, 'C:/Users/x/.codex/hooks/gsd-check-update.js', + 'hook path must equal decoded Windows path after projection normalization'); + }); + test('does NOT touch a managed-hook entry that already uses an absolute runner', () => { const already = [ '# GSD Hooks',