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
This commit is contained in:
5
.changeset/codex-windows-bash-runner.md
Normal file
5
.changeset/codex-windows-bash-runner.md
Normal file
@@ -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.
|
||||
@@ -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 <path>` 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)`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 ─────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user