* 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
This commit is contained in:
5
.changeset/166-windows-claude-sh-hook-command.md
Normal file
5
.changeset/166-windows-claude-sh-hook-command.md
Normal file
@@ -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.
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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 ─────────────────────────
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user