diff --git a/bin/install.js b/bin/install.js index 0381d09f4..7f9d8ded9 100755 --- a/bin/install.js +++ b/bin/install.js @@ -5666,8 +5666,11 @@ function install(isGlobal, runtime = 'claude') { const hasValidateCommitHook = settings.hooks[preToolEvent].some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-validate-commit')) ); - - if (!hasValidateCommitHook) { + // Guard: only register if the .sh file was actually installed. If the npm package + // 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)) { settings.hooks[preToolEvent].push({ matcher: 'Bash', hooks: [ @@ -5679,6 +5682,8 @@ function install(isGlobal, runtime = 'claude') { ] }); 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`); } // Configure session state orientation hook (opt-in) @@ -5688,8 +5693,8 @@ function install(isGlobal, runtime = 'claude') { const hasSessionStateHook = settings.hooks.SessionStart.some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-session-state')) ); - - if (!hasSessionStateHook) { + const sessionStateFile = path.join(targetDir, 'hooks', 'gsd-session-state.sh'); + if (!hasSessionStateHook && fs.existsSync(sessionStateFile)) { settings.hooks.SessionStart.push({ hooks: [ { @@ -5699,6 +5704,8 @@ function install(isGlobal, runtime = 'claude') { ] }); 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`); } // Configure phase boundary detection hook (opt-in) @@ -5708,8 +5715,8 @@ function install(isGlobal, runtime = 'claude') { const hasPhaseBoundaryHook = settings.hooks[postToolEvent].some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-phase-boundary')) ); - - if (!hasPhaseBoundaryHook) { + const phaseBoundaryFile = path.join(targetDir, 'hooks', 'gsd-phase-boundary.sh'); + if (!hasPhaseBoundaryHook && fs.existsSync(phaseBoundaryFile)) { settings.hooks[postToolEvent].push({ matcher: 'Write|Edit', hooks: [ @@ -5721,6 +5728,8 @@ function install(isGlobal, runtime = 'claude') { ] }); 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`); } } diff --git a/tests/bug-1817-sh-hook-guard.test.cjs b/tests/bug-1817-sh-hook-guard.test.cjs new file mode 100644 index 000000000..4378a8923 --- /dev/null +++ b/tests/bug-1817-sh-hook-guard.test.cjs @@ -0,0 +1,63 @@ +/** + * Regression tests for bug #1817 + * + * The installer must NOT register .sh hook entries in settings.json when the + * corresponding .sh file does not exist at the target path. The original bug: + * v1.32.0's npm package omitted the .sh files from hooks/dist/, so the copy + * step produced no files, yet the registration step ran unconditionally — + * leaving users with hook errors on every tool invocation. + * + * Defensive guard: before registering each .sh hook in settings.json, + * install.js must verify the target file exists. If it doesn't, skip + * registration and emit a warning. + */ + +'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', settingsVar: 'validateCommitCommand' }, + { name: 'gsd-session-state.sh', settingsVar: 'sessionStateCommand' }, + { name: 'gsd-phase-boundary.sh', settingsVar: 'phaseBoundaryCommand' }, +]; + +describe('bug #1817: .sh hook registration guards', () => { + let src; + + // Read once — all tests in this suite share the same source snapshot. + try { + src = fs.readFileSync(INSTALL_SRC, 'utf-8'); + } catch { + src = ''; + } + + for (const { name, settingsVar } of SH_HOOKS) { + describe(`${name} registration`, () => { + test(`install.js checks file existence before registering ${name}`, () => { + // Find the block where this .sh hook is registered. + // Each registration block is preceded by the command variable declaration + // and followed by the next hook or end of registration section. + const varIdx = src.indexOf(settingsVar); + assert.ok(varIdx !== -1, `${settingsVar} variable not found in install.js`); + + // Extract ~900 chars around the variable to find the registration block + const blockStart = Math.max(0, varIdx - 50); + const blockEnd = Math.min(src.length, varIdx + 900); + const block = src.slice(blockStart, blockEnd); + + assert.ok( + block.includes('fs.existsSync') || block.includes('existsSync'), + `install.js must call fs.existsSync on the target path before registering ${name} in settings.json. ` + + `Without this guard, hooks are registered even when the .sh file was never copied ` + + `(the root cause of #1817).` + ); + }); + }); + } +}); diff --git a/tests/thinking-model-guidance.test.cjs b/tests/thinking-model-guidance.test.cjs index 989fead55..60960134d 100644 --- a/tests/thinking-model-guidance.test.cjs +++ b/tests/thinking-model-guidance.test.cjs @@ -226,7 +226,7 @@ describe('agent files use inline @-reference wiring at decision points', () => { assert.equal( reqReadingContent.includes(wiring.refFile), false, - `${agent}.md puts ${wiring.refFile} inside a block — should use inline @-reference wiring instead` + `${agent}.md puts ${wiring.refFile} inside a block — thinking-model references must use inline @-reference wiring at decision points, not blocks` ); }); });