From 50537e5f672a639b39ca58139f4cd144d2f6a937 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 10 Apr 2026 13:55:27 -0400 Subject: [PATCH] =?UTF-8?q?fix(install):=20extend=20buildHookCommand=20to?= =?UTF-8?q?=20.sh=20hooks=20=E2=80=94=20absolute=20quoted=20paths=20(#2049?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(autonomous): add Agent to allowed-tools in gsd-autonomous skill Closes #2043 Co-Authored-By: Claude Sonnet 4.6 * fix(install): extend buildHookCommand to .sh hooks — absolute quoted paths - Extend buildHookCommand() to branch on .sh suffix, using 'bash' runner instead of 'node', so all hook paths go through the same quoted-path construction: bash "/absolute/path/hooks/gsd-*.sh" - Replace three manual 'bash ' + targetDir + '...' concatenations for gsd-validate-commit.sh, gsd-session-state.sh, gsd-phase-boundary.sh with buildHookCommand(targetDir, hookName) for the global-install branch - Global .sh hook paths are now double-quoted, fixing invocation failure when the config dir path contains spaces (Windows usernames, #2045) - Adds regression tests in tests/sh-hook-paths.test.cjs Closes #2045 Closes #2046 Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- bin/install.js | 14 ++- tests/sh-hook-paths.test.cjs | 179 +++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 tests/sh-hook-paths.test.cjs diff --git a/bin/install.js b/bin/install.js index 3d10c3620..063752c1a 100755 --- a/bin/install.js +++ b/bin/install.js @@ -443,7 +443,13 @@ function expandTilde(filePath) { function buildHookCommand(configDir, hookName) { // Use forward slashes for Node.js compatibility on all platforms const hooksPath = configDir.replace(/\\/g, '/') + '/hooks/' + hookName; - return `node "${hooksPath}"`; + // .sh hooks use bash; .js hooks use node. Both wrap the path in double quotes + // so that paths with spaces (e.g. Windows "C:/Users/First Last/") work correctly + // (fixes #2045). Routing .sh hooks through this function also ensures they always + // receive an absolute path rather than the bare relative string that the old manual + // concatenation produced (fixes #2046). + const runner = hookName.endsWith('.sh') ? 'bash' : 'node'; + return `${runner} "${hooksPath}"`; } /** @@ -6038,7 +6044,7 @@ function install(isGlobal, runtime = 'claude') { // Configure commit validation hook (Conventional Commits enforcement, opt-in) const validateCommitCommand = isGlobal - ? 'bash ' + targetDir.replace(/\\/g, '/') + '/hooks/gsd-validate-commit.sh' + ? buildHookCommand(targetDir, 'gsd-validate-commit.sh') : 'bash ' + localPrefix + '/hooks/gsd-validate-commit.sh'; const hasValidateCommitHook = settings.hooks[preToolEvent].some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-validate-commit')) @@ -6065,7 +6071,7 @@ function install(isGlobal, runtime = 'claude') { // Configure session state orientation hook (opt-in) const sessionStateCommand = isGlobal - ? 'bash ' + targetDir.replace(/\\/g, '/') + '/hooks/gsd-session-state.sh' + ? buildHookCommand(targetDir, 'gsd-session-state.sh') : 'bash ' + localPrefix + '/hooks/gsd-session-state.sh'; const hasSessionStateHook = settings.hooks.SessionStart.some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-session-state')) @@ -6087,7 +6093,7 @@ function install(isGlobal, runtime = 'claude') { // Configure phase boundary detection hook (opt-in) const phaseBoundaryCommand = isGlobal - ? 'bash ' + targetDir.replace(/\\/g, '/') + '/hooks/gsd-phase-boundary.sh' + ? buildHookCommand(targetDir, 'gsd-phase-boundary.sh') : 'bash ' + localPrefix + '/hooks/gsd-phase-boundary.sh'; const hasPhaseBoundaryHook = settings.hooks[postToolEvent].some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-phase-boundary')) diff --git a/tests/sh-hook-paths.test.cjs b/tests/sh-hook-paths.test.cjs new file mode 100644 index 000000000..9ff81b93a --- /dev/null +++ b/tests/sh-hook-paths.test.cjs @@ -0,0 +1,179 @@ +/** + * Regression tests for bugs #2045 and #2046 + * + * #2046 (macOS/Linux): The three .sh hooks (gsd-validate-commit.sh, + * gsd-session-state.sh, gsd-phase-boundary.sh) were registered in + * settings.json with RELATIVE paths (bash .claude/hooks/...) for local + * installs, causing "No such file or directory" when Claude Code's cwd + * is not the project root. + * + * #2045 (Windows): The same three .sh hooks were registered WITHOUT quotes + * around the path, so usernames with spaces (e.g. C:/Users/First Last/) + * break bash invocation with a syntax error. + * + * Root cause: buildHookCommand() only handled .js files. The .sh hooks were + * built via manual string concatenation without quoting, and local installs + * used localPrefix (.claude/...) instead of the $CLAUDE_PROJECT_DIR-anchored + * form that .js local hooks use. + * + * Fix: extend buildHookCommand() to handle .sh files (uses 'bash' instead of + * 'node') so that all paths go through the same quoted-path construction. + */ + +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); + +const SH_HOOKS = [ + { name: 'gsd-validate-commit.sh', commandVar: 'validateCommitCommand' }, + { name: 'gsd-session-state.sh', commandVar: 'sessionStateCommand' }, + { name: 'gsd-phase-boundary.sh', commandVar: 'phaseBoundaryCommand' }, +]; + +describe('bugs #2045 #2046: .sh hook paths must be absolute and quoted', () => { + let src; + + try { + src = fs.readFileSync(INSTALL_SRC, 'utf-8'); + } catch { + src = ''; + } + + // ── Test 1: buildHookCommand supports .sh files ────────────────────────── + describe('buildHookCommand', () => { + test('returns a bash command for .sh hookName', () => { + // Extract buildHookCommand from source and verify it branches on .sh + const fnStart = src.indexOf('function buildHookCommand('); + assert.ok(fnStart !== -1, 'buildHookCommand function not found in install.js'); + + // Find the closing brace of the function (scan for the balanced brace) + let depth = 0; + let fnEnd = fnStart; + for (let i = fnStart; i < src.length; i++) { + if (src[i] === '{') depth++; + else if (src[i] === '}') { + depth--; + if (depth === 0) { fnEnd = i + 1; break; } + } + } + const fnBody = src.slice(fnStart, fnEnd); + + assert.ok( + fnBody.includes('.sh') || fnBody.includes('bash'), + 'buildHookCommand must handle .sh files by using "bash" as the runner. ' + + 'The function body must contain ".sh" or "bash" for branching logic.' + ); + }); + + test('buildHookCommand produces bash runner for .sh and node runner for .js', () => { + const fnStart = src.indexOf('function buildHookCommand('); + assert.ok(fnStart !== -1, 'buildHookCommand function not found in install.js'); + + let depth = 0; + let fnEnd = fnStart; + for (let i = fnStart; i < src.length; i++) { + if (src[i] === '{') depth++; + else if (src[i] === '}') { + depth--; + if (depth === 0) { fnEnd = i + 1; break; } + } + } + const fnBody = src.slice(fnStart, fnEnd); + + // Must still produce "node" for .js (existing behavior) + assert.ok( + fnBody.includes('node'), + 'buildHookCommand must still produce a "node" command for .js hooks' + ); + + // Must produce "bash" for .sh + assert.ok( + fnBody.includes('bash'), + 'buildHookCommand must produce a "bash" command for .sh hooks' + ); + }); + }); + + // ── Test 2: each .sh command variable uses a quoted path ───────────────── + for (const { name, commandVar } of SH_HOOKS) { + describe(`${name} command`, () => { + test(`${commandVar} uses double-quoted path (fixes #2045 Windows spaces)`, () => { + const varIdx = src.indexOf(commandVar); + assert.ok(varIdx !== -1, `${commandVar} not found in install.js`); + + // Extract the assignment block (~300 chars should cover a single declaration) + const blockEnd = Math.min(src.length, varIdx + 400); + const block = src.slice(varIdx, blockEnd); + + // The command string for the global branch must contain a quoted path: + // bash "..." — the path must be wrapped in double quotes. + assert.ok( + block.includes('bash "') || block.includes("bash '") || block.includes('buildHookCommand'), + `${commandVar} must use buildHookCommand() (which quotes the path) or manually ` + + `quote the path. Found: ${block.slice(0, 200)}` + ); + }); + + test(`${commandVar} does not use bare localPrefix without quoting (fixes #2046 relative path)`, () => { + const varIdx = src.indexOf(commandVar); + assert.ok(varIdx !== -1, `${commandVar} not found in install.js`); + + const blockEnd = Math.min(src.length, varIdx + 400); + const block = src.slice(varIdx, blockEnd); + + // The old bad pattern was: 'bash ' + localPrefix + '/hooks/...' + // where localPrefix === '.claude' (relative, no quotes). + // The fix routes through buildHookCommand which emits bash "absolutePath". + // So the raw string '.claude/hooks' must NOT appear unquoted in this block. + const hasBareRelativePath = /bash ['"]?\.claude\/hooks/.test(block); + assert.ok( + !hasBareRelativePath, + `${commandVar} must not use a bare relative path ".claude/hooks". ` + + `Use buildHookCommand() so the path is absolute and quoted.` + ); + }); + }); + } + + // ── Test 3: global .sh hooks must not use unquoted manual concatenation ─── + test('global .sh hook commands use buildHookCommand, not unquoted string concat', () => { + // Old bad pattern for global installs: + // 'bash ' + targetDir.replace(/\\/g, '/') + '/hooks/gsd-*.sh' + // This left the absolute path unquoted, breaking paths with spaces (#2045). + // The fix routes all global .sh hooks through buildHookCommand() which + // wraps the path in double quotes: bash "/absolute/path/hooks/gsd-*.sh" + const oldGlobalPattern = /'bash ' \+ targetDir/g; + const globalMatches = src.match(oldGlobalPattern) || []; + + assert.strictEqual( + globalMatches.length, 0, + `Found ${globalMatches.length} occurrence(s) of unquoted global .sh path construction ` + + `('bash ' + targetDir). Use buildHookCommand(targetDir, 'gsd-*.sh') instead.` + ); + }); + + // ── Test 4: global .sh hook commands contain double-quoted absolute paths ─ + test('global .sh hook commands in source use bash with double-quoted path', () => { + // After the fix, buildHookCommand produces: bash "/abs/path/hooks/gsd-*.sh" + // Verify each hook's command variable is assigned via buildHookCommand for the global branch. + for (const { commandVar } of SH_HOOKS) { + const varIdx = src.indexOf(commandVar); + assert.ok(varIdx !== -1, `${commandVar} not found in install.js`); + + // The ternary assignment: const xCommand = isGlobal ? buildHookCommand(...) : ... + const blockEnd = Math.min(src.length, varIdx + 300); + const block = src.slice(varIdx, blockEnd); + + assert.ok( + block.includes('buildHookCommand'), + `${commandVar} global branch must use buildHookCommand() to produce a quoted absolute path. ` + + `Found: ${block.slice(0, 150)}` + ); + } + }); +});