From 656fb5868c2c333086b248782e763853d6734312 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 15:32:21 -0400 Subject: [PATCH] Fix(hooks): resolve Windows Bash hook runner (#3397) * fix: resolve windows bash hook runner * chore: add changeset for codex bash hooks * fix: avoid hardcoded windows bash paths * fix: skip unavailable bash hook registrations --- .changeset/codex-windows-bash-runner.md | 5 ++ bin/install.js | 59 ++++++++++++++++------ tests/bug-2979-hook-absolute-node.test.cjs | 21 ++++++++ 3 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 .changeset/codex-windows-bash-runner.md diff --git a/.changeset/codex-windows-bash-runner.md b/.changeset/codex-windows-bash-runner.md new file mode 100644 index 000000000..848e05b7f --- /dev/null +++ b/.changeset/codex-windows-bash-runner.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3397 +--- +**Codex Windows Bash-backed hooks now resolve Git Bash instead of assuming bare bash is on PATH** - installs skip those hook registrations when no supported Bash runner is found. diff --git a/bin/install.js b/bin/install.js index 26829d552..af390795c 100755 --- a/bin/install.js +++ b/bin/install.js @@ -613,6 +613,29 @@ function formatManagedHookScriptToken(scriptPath, opts) { return JSON.stringify(scriptPath.replace(/\\/g, '/')); } +function resolveBashRunner(opts) { + const platform = (opts && opts.platform) || process.platform; + if (platform !== 'win32') return 'bash'; + + const env = (opts && opts.env) || process.env; + const exists = (opts && opts.existsSync) || fs.existsSync; + const candidates = []; + if (env.GSD_BASH_PATH) candidates.push(env.GSD_BASH_PATH); + if (env.ProgramFiles) candidates.push(path.win32.join(env.ProgramFiles, 'Git', 'bin', 'bash.exe')); + if (env['ProgramFiles(x86)']) candidates.push(path.win32.join(env['ProgramFiles(x86)'], 'Git', 'bin', 'bash.exe')); + if (env.SystemDrive) { + candidates.push(path.win32.join(env.SystemDrive, 'Program Files', 'Git', 'bin', 'bash.exe')); + candidates.push(path.win32.join(env.SystemDrive, 'Program Files (x86)', 'Git', 'bin', 'bash.exe')); + } + + for (const candidate of candidates) { + if (candidate && exists(candidate)) { + return JSON.stringify(candidate.replace(/\\/g, '/')); + } + } + return null; +} + function rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts) { if (!settings || !settings.hooks || !absoluteRunner) return false; if (!opts) opts = {}; @@ -803,21 +826,19 @@ function rewriteLegacyCodexHookBlock(content, absoluteRunner) { */ function buildHookCommand(configDir, hookName, opts) { if (!opts) opts = {}; - // .sh hooks run under bare `bash` (PATH-resolved). POSIX guarantees - // /bin/sh but not /bin/bash, and distros like NixOS do not ship - // /bin/bash by default — so PATH-resolved `bash` is more portable than - // an absolute /bin/bash. The wrapping `bash ` invocation also - // means the script's own shebang (#!/usr/bin/env bash) is read as a - // comment in this code path; it only matters when the script is run - // directly (e.g. tests or future installer changes). .js hooks still - // need the absolute node path because GUI-launched runtimes start with - // a minimal PATH that does not include nvm/Homebrew/Volta-installed - // node binaries (#2979). + // POSIX .sh hooks run under PATH-resolved `bash`: POSIX guarantees /bin/sh + // but not /bin/bash, and distros like NixOS do not ship /bin/bash by default. + // Windows Codex launches hooks from PowerShell/cmd environments where bare + // `bash` may not be on PATH, so resolve Git Bash explicitly or return null so + // callers skip registration instead of installing a known-broken hook (#3393). + // .js hooks still need the absolute node path because GUI-launched runtimes + // start with a minimal PATH that may not include nvm/Homebrew/Volta node + // binaries (#2979). const nodeRunner = resolveNodeRunner(); - const runner = hookName.endsWith('.sh') ? 'bash' : nodeRunner; - // resolveNodeRunner returns null when process.execPath is unavailable. + const runner = hookName.endsWith('.sh') ? resolveBashRunner(opts) : nodeRunner; + // Runner resolvers return null when the executable path is unavailable. // Fall through with null so callers can skip registration with a warning - // instead of emitting bare `node` (which would recreate the #2979 bug). + // instead of emitting a command that recreates the original hook failure. if (runner === null) return null; if (opts.portableHooks) { @@ -9076,7 +9097,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { // omitted the file (as happened in v1.32.0, bug #1817), registering a missing hook // causes a hook error on every Bash tool invocation. const validateCommitFile = path.join(targetDir, 'hooks', 'gsd-validate-commit.sh'); - if (!hasValidateCommitHook && fs.existsSync(validateCommitFile)) { + if (!hasValidateCommitHook && fs.existsSync(validateCommitFile) && validateCommitCommand) { settings.hooks[preToolEvent].push({ matcher: 'Bash', hooks: [ @@ -9090,6 +9111,8 @@ function install(isGlobal, runtime = 'claude', options = {}) { console.log(` ${green}✓${reset} Configured commit validation hook (opt-in via config)`); } else if (!hasValidateCommitHook && !fs.existsSync(validateCommitFile)) { console.warn(` ${yellow}⚠${reset} Skipped commit validation hook — gsd-validate-commit.sh not found at target`); + } else if (!hasValidateCommitHook && !validateCommitCommand) { + console.warn(` ${yellow}⚠${reset} Skipped commit validation hook — Bash executable path unavailable (#3393)`); } // Configure session state orientation hook (opt-in) @@ -9100,7 +9123,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-session-state')) ); const sessionStateFile = path.join(targetDir, 'hooks', 'gsd-session-state.sh'); - if (!hasSessionStateHook && fs.existsSync(sessionStateFile)) { + if (!hasSessionStateHook && fs.existsSync(sessionStateFile) && sessionStateCommand) { settings.hooks.SessionStart.push({ hooks: [ { @@ -9112,6 +9135,8 @@ function install(isGlobal, runtime = 'claude', options = {}) { console.log(` ${green}✓${reset} Configured session state orientation hook (opt-in via config)`); } else if (!hasSessionStateHook && !fs.existsSync(sessionStateFile)) { console.warn(` ${yellow}⚠${reset} Skipped session state hook — gsd-session-state.sh not found at target`); + } else if (!hasSessionStateHook && !sessionStateCommand) { + console.warn(` ${yellow}⚠${reset} Skipped session state hook — Bash executable path unavailable (#3393)`); } // Configure phase boundary detection hook (opt-in) @@ -9122,7 +9147,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-phase-boundary')) ); const phaseBoundaryFile = path.join(targetDir, 'hooks', 'gsd-phase-boundary.sh'); - if (!hasPhaseBoundaryHook && fs.existsSync(phaseBoundaryFile)) { + if (!hasPhaseBoundaryHook && fs.existsSync(phaseBoundaryFile) && phaseBoundaryCommand) { settings.hooks[postToolEvent].push({ matcher: 'Write|Edit', hooks: [ @@ -9136,6 +9161,8 @@ function install(isGlobal, runtime = 'claude', options = {}) { console.log(` ${green}✓${reset} Configured phase boundary detection hook (opt-in via config)`); } else if (!hasPhaseBoundaryHook && !fs.existsSync(phaseBoundaryFile)) { console.warn(` ${yellow}⚠${reset} Skipped phase boundary hook — gsd-phase-boundary.sh not found at target`); + } else if (!hasPhaseBoundaryHook && !phaseBoundaryCommand) { + console.warn(` ${yellow}⚠${reset} Skipped phase boundary hook — Bash executable path unavailable (#3393)`); } } diff --git a/tests/bug-2979-hook-absolute-node.test.cjs b/tests/bug-2979-hook-absolute-node.test.cjs index 7ae14e913..c260ae35c 100644 --- a/tests/bug-2979-hook-absolute-node.test.cjs +++ b/tests/bug-2979-hook-absolute-node.test.cjs @@ -129,6 +129,27 @@ describe('Bug #2979: buildHookCommand for .sh hooks still uses bare "bash" (POSI const parsed = parseHookCommand(cmd); assert.equal(parsed.runner, 'bash'); }); + + test('Windows .sh hook uses resolved Git Bash path instead of bare bash (#3393)', () => { + const cmd = buildHookCommand('C:/Users/me/.codex', 'gsd-validate-commit.sh', { + platform: 'win32', + env: { ProgramFiles: 'C:\\Program Files' }, + existsSync: (candidate) => candidate === 'C:\\Program Files\\Git\\bin\\bash.exe', + }); + assert.equal( + cmd, + '& "C:/Program Files/Git/bin/bash.exe" "C:/Users/me/.codex/hooks/gsd-validate-commit.sh"', + ); + }); + + test('Windows .sh hook returns null when no supported Bash runner is found (#3393)', () => { + const cmd = buildHookCommand('C:/Users/me/.codex', 'gsd-phase-boundary.sh', { + platform: 'win32', + env: {}, + existsSync: () => false, + }); + assert.equal(cmd, null); + }); }); // ─── #3002 CR follow-up: legacy-bare-node migration ─────────────────────────