fix(3426): Codex Windows hooks use .cmd shim to avoid POSIX exec fail (#3768)

* test(#3426): add RED test for Codex Windows hooks .cmd shim requirement

Drive buildCodexHookWindowsShimIR (typed IR) + ensureCodexHooksJsonSessionStart
integration against mocked win32 platform. Counter-tests confirm darwin/linux
paths remain unchanged.

NOTE: Windows wall-clock verification depends on Docker matrix Windows
runners. Local test exercises the generator IR shape only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#3426): Codex Windows hooks use .cmd shim to avoid bash.exe POSIX-exec failure

Root cause: Codex on Windows runs hook commands from PowerShell/cmd. The previous
hooks.json command format was `"node.exe" "script.js"`. Codex's hook-dispatch shell
(Git Bash / MSYS) tried to POSIX-exec node.exe (a Windows PE binary) via execvp(),
which fails with ENOEXEC — reported as `bash.exe: cannot execute binary file`.

Fix: `ensureCodexHooksJsonSessionStart` now calls `buildCodexHookWindowsShimIR` on
win32 to write a .cmd shim alongside the .js hook file. cmd.exe executes .cmd files
natively via CreateProcess, bypassing the POSIX exec layer entirely. Non-Windows
paths (darwin, linux) are unchanged: they continue to use the node-runner command.

Also adds `gsd-check-update.cmd` to the codex-hooks-json managed-basename set so
reconcileCodexHooksJsonSessionStart correctly replaces stale node-runner entries on
reinstall.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(3426): update changeset to reference PR #3768

* fix(3426): fail-loud on Codex Windows shim-write failure instead of silently restoring broken command

Replace the silent fallback to `projectManagedHookCommand` (the old
`node.exe script.js` form) with an explicit warn-and-skip path.

When `atomicWriteFileSync` fails to write the `.cmd` shim, the previous
code silently called `reconcileCodexHooksJsonSessionStart` with the
legacy node-runner command. That command triggers the exact
`bash.exe: cannot execute binary file` POSIX-exec failure that #3426
exists to fix — so a successful-looking install was secretly restoring
the original bug.

New behaviour:
- Emit `console.warn` with the failure reason and a remediation hint,
  matching the `${yellow}⚠${reset}  Skipped …` idiom used at line 9098.
- Return `{ changed: false, wrote: false }` to skip registration for
  this runtime entirely, so the outer caller can surface "NOT installed"
  instead of "installed (but broken)".

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(3426): typed-IR assertions on .cmd shim eol/quoting/passthrough + IR extension

Extend `buildCodexHookWindowsShimIR` to expose two new typed fields on
the returned IR object (CONTRIBUTING.md L558-L565 IR-first discipline):

  eol: { cmd: '\r\n' }   — CRLF is canonical for cmd.exe .cmd files
  passthroughArgs: true  — shim forwards all args via %*

Add a new describe block (Step 2b) with three IR-level assertions:

1. `eol.cmd === '\r\n'` — prevents silent EOL regression that could
   break parsing on Windows versions that require CRLF.
2. `invocation.target` is the raw unquoted path (no shell-metachar
   leakage) — quoting happens only at render time.
3. `passthroughArgs === true` — the %* forwarding contract is
   explicitly typed so regressions fail before the text is rendered.

All assertions operate on the typed IR returned by the generator, NOT
on the rendered `.cmd` file content — text-matching is the anti-pattern
CONTRIBUTING.md L522-582 prohibits.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(3426): fix Windows CI failures — update hook-command filter patterns

Four test files filtered for managed hooks in hooks.json using the
literal string `gsd-check-update.js`.  On Windows the PR-introduced
.cmd shim changes the hooks.json command to
`"path/gsd-check-update.cmd"` (no node prefix, .cmd extension), so
those filters matched 0 entries and 24 Windows subtests failed.

Fixes:
- bug-2760-codex-install-defensive.test.cjs (7 filters): change
  `/gsd-check-update\.js/` → `/gsd-check-update/` to match both
  .js (POSIX) and .cmd (Windows) commands.
- bug-3357-codex-legacy-hooks-json-migration.test.cjs (3 filters):
  same `.js` → no-extension change.
- bug-3427-3433-codex-install-shape.test.cjs (2 filters): same fix;
  add explanatory comment to uninstall assertion.
- codex-config.test.cjs (9 filters + 1 exact-command assertion):
  bulk-replace all `hooksJsonCommands.filter(cmd => cmd.includes('gsd-check-update.js'))`
  with `gsd-check-update`; make the `fresh CODEX_HOME` test platform-
  aware — on win32 assert `.cmd` shim path, on POSIX assert the
  existing `"runner" "script.js"` form (#3017).

All four suites pass locally (macOS / darwin).  Windows subtests
verified against the Windows CI failure log patterns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(3426): address pr-review-toolkit + codex review findings

- fix(uninstall): add gsd-check-update.cmd to gsdHooks cleanup list so
  the .cmd shim is removed from disk on Windows uninstall (was left as
  orphan artifact — silent failure post-uninstall)
- test(3426): add uninstall test asserting gsd-check-update.cmd is
  deleted from hooks dir after `uninstall(true, 'codex')` (no coverage existed)
- fix(comment): correct JSDoc on buildCodexHookWindowsShimIR — shim
  content is three-line @ECHO OFF/@SETLOCAL/@runner snippet, not bare
  `@node "script.js" %*` as the old comment claimed
- fix(comment): update stale assertion message in codex-config.test.cjs
  L1457 — said "config.toml references it" but the hook is in hooks.json

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
2026-05-20 23:13:50 -04:00
committed by GitHub
parent a7abc6df2f
commit 2d3027767b
8 changed files with 625 additions and 46 deletions

View File

@@ -1446,15 +1446,15 @@ describe('Codex install hook configuration (e2e)', () => {
);
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
assert.equal(
hooksJsonCommands.some((cmd) => cmd.includes('gsd-check-update.js')),
hooksJsonCommands.some((cmd) => cmd.includes('gsd-check-update')),
true,
'hooks.json references gsd-check-update.js'
'hooks.json references gsd-check-update (.js on POSIX, .cmd on Windows)'
);
// 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 (directly on POSIX, via .cmd shim on Windows) but file was not installed`
);
});
@@ -1465,19 +1465,24 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('[features]\nhooks = true\n'), 'writes codex_hooks feature');
const parsed = parseTomlToObject(content);
assert.ok(!parsed.hooks || !Array.isArray(parsed.hooks.SessionStart), 'config.toml does not carry managed SessionStart hooks');
// #3017: handler command now uses the absolute Node binary path so
// GUI/minimal-PATH runtimes can resolve it. The shape is
// "<absolute-node-path>" "<hook-path>"
// where <absolute-node-path> is the normalized runner selected by
// resolveNodeRunner() and the hook path is also quoted. Homebrew Cellar
// execPath values intentionally normalize to stable Homebrew symlinks.
const expectedRunner = JSON.parse(resolveNodeRunner());
const expectedHookPath = path.join(codexHome, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/');
const expectedCommand = `"${expectedRunner}" "${expectedHookPath}"`;
// #3017 / #3426: on POSIX the handler command uses the absolute Node binary path
// "<absolute-node-path>" "<hook-path.js>"
// On Windows (#3426) a .cmd shim is written instead; the command in hooks.json
// is the quoted .cmd path (no node runner prefix — cmd.exe executes .cmd natively).
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdCommands = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdCommands = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdCommands.length, 1, 'writes one GSD update hook in hooks.json');
assert.strictEqual(gsdCommands[0], expectedCommand, 'handler command must use absolute node runner pointing at gsd-check-update.js (#3017)');
if (process.platform === 'win32') {
// On Windows, the command is the .cmd shim path (quoted).
const expectedCmdPath = path.join(codexHome, 'hooks', 'gsd-check-update.cmd').replace(/\\/g, '/');
assert.strictEqual(gsdCommands[0], JSON.stringify(expectedCmdPath), 'win32: handler command must be the .cmd shim path (#3426)');
} else {
// On POSIX, the command is the node runner + .js hook path.
const expectedRunner = JSON.parse(resolveNodeRunner());
const expectedHookPath = path.join(codexHome, 'hooks', 'gsd-check-update.js').replace(/\\/g, '/');
const expectedCommand = `"${expectedRunner}" "${expectedHookPath}"`;
assert.strictEqual(gsdCommands[0], expectedCommand, 'handler command must use absolute node runner pointing at gsd-check-update.js (#3017)');
}
assert.strictEqual(countMatches(content, /^hooks = true$/gm), 1, 'writes one codex_hooks key');
assertNoDraftRootKeys(content);
assertUsesOnlyEol(content, '\n');
@@ -1561,7 +1566,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('[model]\nname = "o3"'), 'preserves model section');
assert.ok(content.includes('command = "echo custom"'), 'preserves custom hook');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'adds one GSD update hook in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -1700,7 +1705,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('other_feature = true'), 'preserves other feature keys');
assert.ok(content.includes('command = "echo custom"'), 'preserves custom hook');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'does not duplicate GSD update hook in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -1744,7 +1749,7 @@ describe('Codex install hook configuration (e2e)', () => {
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');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'keeps one GSD update hook in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -1767,7 +1772,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 1, 'adds one real top-level features table');
assert.strictEqual(countMatches(content, /^hooks = true$/gm), 1, 'adds one codex_hooks key');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'remains idempotent for the GSD hook block in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -1789,7 +1794,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^features\.hooks = true$/gm), 1, 'adds one dotted codex_hooks key');
assert.ok(content.includes('features.other_feature = true'), 'preserves existing dotted features key');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'adds one GSD update hook for dotted codex_hooks and remains idempotent');
assertNoDraftRootKeys(content);
});
@@ -1855,7 +1860,7 @@ describe('Codex install hook configuration (e2e)', () => {
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');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'adds one GSD update hook for quoted dotted codex_hooks and remains idempotent');
assertNoDraftRootKeys(content);
});
@@ -1972,7 +1977,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(!content.includes('multiline-basic-sentinel'), 'removes multiline basic-string continuation lines');
assert.ok(content.includes('other_feature = true'), 'preserves following feature keys');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'remains idempotent for the GSD hook block in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -1999,7 +2004,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(!content.includes('multiline-literal-sentinel'), 'removes multiline literal-string continuation lines');
assert.ok(content.includes('other_feature = true'), 'preserves following feature keys');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'remains idempotent for the GSD hook block in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -2027,7 +2032,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(!content.includes('array-sentinel-2'), 'removes multiline array continuation lines');
assert.ok(content.includes('other_feature = true'), 'preserves following feature keys');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'remains idempotent for the GSD hook block in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -2074,7 +2079,7 @@ describe('Codex install hook configuration (e2e)', () => {
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');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'keeps one GSD update hook in hooks.json');
assertUsesOnlyEol(content, '\r\n');
assertNoDraftRootKeys(content);
@@ -2099,7 +2104,7 @@ describe('Codex install hook configuration (e2e)', () => {
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');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'adds the GSD update hook once in hooks.json');
assertNoDraftRootKeys(content);
});
@@ -2118,7 +2123,7 @@ describe('Codex install hook configuration (e2e)', () => {
const parsedMixed = parseTomlToObject(content);
assert.ok(!parsedMixed.hooks || !Array.isArray(parsedMixed.hooks.SessionStart), 'does not write managed SessionStart hooks to config.toml');
const hooksJsonCommands = readHooksSessionStartCommands(codexHome);
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update.js'));
const gsdEntries = hooksJsonCommands.filter((cmd) => cmd.includes('gsd-check-update'));
assert.strictEqual(gsdEntries.length, 1, 'writes one managed SessionStart hook to hooks.json');
assert.ok(content.includes('[model]\r\nname = "o3"'), 'preserves the existing CRLF model lines');
assert.strictEqual(countMatches(content, /^hooks = true$/gm), 1, 'remains idempotent on repeated installs');