From 5c1f902204adc9e6b32584f6045ef2f97bdc2179 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Tue, 7 Apr 2026 05:13:52 -0700 Subject: [PATCH] fix(hooks): handle missing reference files gracefully during fresh install (#1878) Add fs.existsSync() guards to all .js hook registrations in install.js, matching the pattern already used for .sh hooks (#1817). When hooks/dist/ is missing from the npm package, the copy step produces no files but the registration step previously ran unconditionally for .js hooks, causing "PreToolUse:Bash hook error" on every tool invocation. Each .js hook (check-update, context-monitor, prompt-guard, read-guard, workflow-guard) now verifies the target file exists before registering in settings.json, and emits a skip warning when the file is absent. Co-authored-by: Claude Opus 4.6 (1M context) --- bin/install.js | 29 ++++++-- tests/bug-1754-js-hook-guard.test.cjs | 103 ++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 tests/bug-1754-js-hook-guard.test.cjs diff --git a/bin/install.js b/bin/install.js index a3e0e168..2de04b47 100755 --- a/bin/install.js +++ b/bin/install.js @@ -5578,7 +5578,12 @@ function install(isGlobal, runtime = 'claude') { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-check-update')) ); - if (!hasGsdUpdateHook) { + // Guard: only register if the hook file was actually installed (#1754). + // When hooks/dist/ is missing from the npm package (as in v1.32.0), the + // copy step produces no files but the registration step ran unconditionally, + // causing "hook error" on every tool invocation. + const checkUpdateFile = path.join(targetDir, 'hooks', 'gsd-check-update.js'); + if (!hasGsdUpdateHook && fs.existsSync(checkUpdateFile)) { settings.hooks.SessionStart.push({ hooks: [ { @@ -5588,6 +5593,8 @@ function install(isGlobal, runtime = 'claude') { ] }); console.log(` ${green}✓${reset} Configured update check hook`); + } else if (!hasGsdUpdateHook && !fs.existsSync(checkUpdateFile)) { + console.warn(` ${yellow}⚠${reset} Skipped update check hook — gsd-check-update.js not found at target`); } // Configure post-tool hook for context window monitoring @@ -5599,7 +5606,8 @@ function install(isGlobal, runtime = 'claude') { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-context-monitor')) ); - if (!hasContextMonitorHook) { + const contextMonitorFile = path.join(targetDir, 'hooks', 'gsd-context-monitor.js'); + if (!hasContextMonitorHook && fs.existsSync(contextMonitorFile)) { settings.hooks[postToolEvent].push({ matcher: 'Bash|Edit|Write|MultiEdit|Agent|Task', hooks: [ @@ -5611,6 +5619,8 @@ function install(isGlobal, runtime = 'claude') { ] }); console.log(` ${green}✓${reset} Configured context window monitor hook`); + } else if (!hasContextMonitorHook && !fs.existsSync(contextMonitorFile)) { + console.warn(` ${yellow}⚠${reset} Skipped context monitor hook — gsd-context-monitor.js not found at target`); } else { // Migrate existing context monitor hooks: add matcher and timeout if missing for (const entry of settings.hooks[postToolEvent]) { @@ -5644,7 +5654,8 @@ function install(isGlobal, runtime = 'claude') { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-prompt-guard')) ); - if (!hasPromptGuardHook) { + const promptGuardFile = path.join(targetDir, 'hooks', 'gsd-prompt-guard.js'); + if (!hasPromptGuardHook && fs.existsSync(promptGuardFile)) { settings.hooks[preToolEvent].push({ matcher: 'Write|Edit', hooks: [ @@ -5656,6 +5667,8 @@ function install(isGlobal, runtime = 'claude') { ] }); console.log(` ${green}✓${reset} Configured prompt injection guard hook`); + } else if (!hasPromptGuardHook && !fs.existsSync(promptGuardFile)) { + console.warn(` ${yellow}⚠${reset} Skipped prompt guard hook — gsd-prompt-guard.js not found at target`); } // Configure PreToolUse hook for read-before-edit guidance (#1628) @@ -5665,7 +5678,8 @@ function install(isGlobal, runtime = 'claude') { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-read-guard')) ); - if (!hasReadGuardHook) { + const readGuardFile = path.join(targetDir, 'hooks', 'gsd-read-guard.js'); + if (!hasReadGuardHook && fs.existsSync(readGuardFile)) { settings.hooks[preToolEvent].push({ matcher: 'Write|Edit', hooks: [ @@ -5677,6 +5691,8 @@ function install(isGlobal, runtime = 'claude') { ] }); console.log(` ${green}✓${reset} Configured read-before-edit guard hook`); + } else if (!hasReadGuardHook && !fs.existsSync(readGuardFile)) { + console.warn(` ${yellow}⚠${reset} Skipped read guard hook — gsd-read-guard.js not found at target`); } // Community hooks — registered on install but opt-in at runtime. @@ -5694,7 +5710,8 @@ function install(isGlobal, runtime = 'claude') { entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-workflow-guard')) ); - if (!hasWorkflowGuardHook) { + const workflowGuardFile = path.join(targetDir, 'hooks', 'gsd-workflow-guard.js'); + if (!hasWorkflowGuardHook && fs.existsSync(workflowGuardFile)) { settings.hooks[preToolEvent].push({ matcher: 'Write|Edit', hooks: [ @@ -5706,6 +5723,8 @@ function install(isGlobal, runtime = 'claude') { ] }); console.log(` ${green}✓${reset} Configured workflow guard hook (opt-in via hooks.workflow_guard)`); + } else if (!hasWorkflowGuardHook && !fs.existsSync(workflowGuardFile)) { + console.warn(` ${yellow}⚠${reset} Skipped workflow guard hook — gsd-workflow-guard.js not found at target`); } // Configure commit validation hook (Conventional Commits enforcement, opt-in) diff --git a/tests/bug-1754-js-hook-guard.test.cjs b/tests/bug-1754-js-hook-guard.test.cjs new file mode 100644 index 00000000..3910195b --- /dev/null +++ b/tests/bug-1754-js-hook-guard.test.cjs @@ -0,0 +1,103 @@ +/** + * Regression tests for bug #1754 + * + * The installer must NOT register .js hook entries in settings.json when the + * corresponding .js file does not exist at the target path. The original bug: + * on fresh installs where hooks/dist/ was missing from the npm package (as in + * v1.32.0), the hook copy step produced no files, yet the registration step + * ran unconditionally for .js hooks — leaving users with "PreToolUse:Bash + * hook error" on every tool invocation. + * + * The .sh hooks already had fs.existsSync() guards (added in #1817). This + * test verifies the same defensive pattern exists for all .js hooks. + */ + +'use strict'; + +const { describe, test, before } = 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 JS_HOOKS = [ + { name: 'gsd-check-update.js', registrationAnchor: 'hasGsdUpdateHook' }, + { name: 'gsd-context-monitor.js', registrationAnchor: 'hasContextMonitorHook' }, + { name: 'gsd-prompt-guard.js', registrationAnchor: 'hasPromptGuardHook' }, + { name: 'gsd-read-guard.js', registrationAnchor: 'hasReadGuardHook' }, + { name: 'gsd-workflow-guard.js', registrationAnchor: 'hasWorkflowGuardHook' }, +]; + +describe('bug #1754: .js hook registration guards', () => { + let src; + + before(() => { + src = fs.readFileSync(INSTALL_SRC, 'utf-8'); + }); + + for (const { name, registrationAnchor } of JS_HOOKS) { + describe(`${name} registration`, () => { + test(`install.js checks file existence before registering ${name}`, () => { + // Find the registration block by locating the "has...Hook" variable + const anchorIdx = src.indexOf(registrationAnchor); + assert.ok( + anchorIdx !== -1, + `${registrationAnchor} variable not found in install.js` + ); + + // Extract a window around the registration block to find the guard + const blockStart = anchorIdx; + const blockEnd = Math.min(src.length, anchorIdx + 1200); + const block = src.slice(blockStart, blockEnd); + + // The block must contain an fs.existsSync check for the hook file + 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 .js file ` + + `was never copied (the root cause of #1754).` + ); + }); + + test(`install.js emits a warning when ${name} is missing`, () => { + // The hook file name (without extension) should appear in a warning message + const hookBaseName = name.replace('.js', ''); + const warnPattern = `Skipped`; + const anchorIdx = src.indexOf(registrationAnchor); + const block = src.slice(anchorIdx, Math.min(src.length, anchorIdx + 1200)); + + assert.ok( + block.includes(warnPattern) && block.includes(hookBaseName), + `install.js must emit a skip warning when ${name} is not found at the target path` + ); + }); + }); + } + + test('all .js hooks use the same guard pattern as .sh hooks', () => { + // Count existsSync calls in the hook registration section. + // There should be guards for all JS hooks plus the existing SH hooks. + // This test ensures new hooks added in the future follow the same pattern. + const registrationSection = src.slice( + src.indexOf('// Configure SessionStart hook'), + src.indexOf('return { settingsPath, settings, statuslineCommand') + ); + + // Count unique hook file existence checks (pattern: path.join(targetDir, 'hooks', 'gsd-*.js')) + const jsGuards = (registrationSection.match(/gsd-[\w-]+\.js.*not found at target/g) || []); + const shGuards = (registrationSection.match(/gsd-[\w-]+\.sh.*not found at target/g) || []); + + assert.ok( + jsGuards.length >= JS_HOOKS.length, + `Expected at least ${JS_HOOKS.length} .js hook guards, found ${jsGuards.length}. ` + + `Every .js hook registration must check file existence before registering.` + ); + + assert.ok( + shGuards.length >= 3, + `Expected at least 3 .sh hook guards (validate-commit, session-state, phase-boundary), ` + + `found ${shGuards.length}.` + ); + }); +});