From 0dc3fd604c276b9321dfdb2ef9fc8d6cdc0a2a82 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 24 May 2026 14:16:16 -0400 Subject: [PATCH] fix(#166): omit bash.exe wrapper for windows claude sh hooks (#203) * fix(#166): omit bash.exe wrapper for windows claude sh hooks * chore(#166): add changeset for windows claude sh hook fix * fix(#166): reset exitCode in sdk bridge integration test --- .../166-windows-claude-sh-hook-command.md | 5 ++++ bin/install.js | 27 ++++++++++++++++--- tests/bug-2979-hook-absolute-node.test.cjs | 14 ++++++++++ tests/cjs-sdk-bridge-integration.test.cjs | 7 ++++- 4 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 .changeset/166-windows-claude-sh-hook-command.md diff --git a/.changeset/166-windows-claude-sh-hook-command.md b/.changeset/166-windows-claude-sh-hook-command.md new file mode 100644 index 000000000..d7468ba44 --- /dev/null +++ b/.changeset/166-windows-claude-sh-hook-command.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 166 +--- +**Windows Claude `.sh` hook command serialization** — installer-managed Claude hooks now emit script-only command entries (no explicit `bash.exe` wrapper) for Windows settings hooks, preventing the `bash.exe: ... cannot execute binary file` failure mode reported in SessionStart/PreToolUse hook execution. diff --git a/bin/install.js b/bin/install.js index fa442e12d..ba0b68a4f 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1137,6 +1137,25 @@ function removeCodexHooksJsonSessionStart(targetDir) { */ function buildHookCommand(configDir, hookName, opts) { if (!opts) opts = {}; + const platform = opts.platform || process.platform; + const runtime = opts.runtime || 'generic'; + const isShellHook = hookName.endsWith('.sh'); + + // #166: Claude Code executes these hook commands inside a bash context on + // Windows, so wrapping `.sh` hooks with an explicit `bash.exe` path can + // trigger `bash.exe: ... cannot execute binary file`. Emit only the quoted + // script path for Claude on Windows. + if (platform === 'win32' && runtime === 'claude' && isShellHook) { + if (opts.portableHooks) { + const portableBaseDir = projectPortableHookBaseDir({ + configDir, + homeDir: os.homedir(), + }); + return JSON.stringify(`${portableBaseDir}/hooks/${hookName}`); + } + return JSON.stringify(configDir.replace(/\\/g, '/') + '/hooks/' + hookName); + } + // 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 @@ -1146,7 +1165,7 @@ function buildHookCommand(configDir, hookName, opts) { // start with a minimal PATH that may not include nvm/Homebrew/Volta node // binaries (#2979). const nodeRunner = resolveNodeRunner(); - const runner = hookName.endsWith('.sh') ? resolveBashRunner(opts) : nodeRunner; + const runner = isShellHook ? 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 a command that recreates the original hook failure. @@ -1161,7 +1180,7 @@ function buildHookCommand(configDir, hookName, opts) { absoluteRunner: runner, scriptPath: `${portableBaseDir}/hooks/${hookName}`, runtime: opts.runtime || 'generic', - platform: opts.platform || process.platform, + platform, }); } @@ -1170,8 +1189,8 @@ function buildHookCommand(configDir, hookName, opts) { return projectManagedHookCommand({ absoluteRunner: runner, scriptPath: hooksPath, - runtime: opts.runtime || 'generic', - platform: opts.platform || process.platform, + runtime, + platform, }); } diff --git a/tests/bug-2979-hook-absolute-node.test.cjs b/tests/bug-2979-hook-absolute-node.test.cjs index 53b498d36..d872de1d3 100644 --- a/tests/bug-2979-hook-absolute-node.test.cjs +++ b/tests/bug-2979-hook-absolute-node.test.cjs @@ -178,6 +178,20 @@ describe('Bug #2979: buildHookCommand for .sh hooks still uses bare "bash" (POSI }); assert.equal(cmd, null); }); + + test('Windows Claude .sh hook omits explicit bash.exe wrapper (#166)', () => { + const cmd = buildHookCommand('C:/Users/me/.claude', 'gsd-session-state.sh', { + platform: 'win32', + runtime: 'claude', + env: { ProgramFiles: 'C:\\Program Files' }, + existsSync: (candidate) => candidate === 'C:\\Program Files\\Git\\bin\\bash.exe', + }); + assert.equal( + cmd, + '"C:/Users/me/.claude/hooks/gsd-session-state.sh"', + 'Claude win32 .sh hooks should serialize as script-only commands' + ); + }); }); // ─── #3002 CR follow-up: legacy-bare-node migration ───────────────────────── diff --git a/tests/cjs-sdk-bridge-integration.test.cjs b/tests/cjs-sdk-bridge-integration.test.cjs index ad282edb5..799eb7ce6 100644 --- a/tests/cjs-sdk-bridge-integration.test.cjs +++ b/tests/cjs-sdk-bridge-integration.test.cjs @@ -28,11 +28,16 @@ * surface the cause directly rather than silently masking under fallback. */ -const { test, describe } = require('node:test'); +const { test, describe, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const path = require('node:path'); const BRIDGE_PATH = path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'cjs-sdk-bridge.cjs'); +const initialExitCode = process.exitCode; + +afterEach(() => { + process.exitCode = initialExitCode; +}); describe('cjs-sdk-bridge: SDK runtime bridge integration', () => { test('tryLoadSdk() resolves the bundled SDK on the current checkout', () => {