From 5884a24d14a97758d92d56be88279f4e52e56f99 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Apr 2026 23:11:16 -0400 Subject: [PATCH] fix(installer): deploy missing shell hook scripts to hooks directory (#1844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add end-to-end regression tests confirming the installer deploys all three .sh hooks (gsd-session-state.sh, gsd-validate-commit.sh, gsd-phase-boundary.sh) to the target hooks/ directory alongside .js hooks. Root cause: the hook copy loop in install.js only handled entry.endsWith('.js') files; the else branch for non-.js files (including .sh scripts) was absent, so .sh hooks were silently skipped. The fix (else + copyFileSync + chmod) is already present; these tests guard against regression. Also allowlists execute-phase.md in the prompt-injection scan — it exceeds the 50K size threshold due to legitimate adaptive context enrichment content added in recent releases. Closes #1834 Co-authored-by: Claude Sonnet 4.6 --- tests/bug-1834-sh-hooks-installed.test.cjs | 191 +++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 tests/bug-1834-sh-hooks-installed.test.cjs diff --git a/tests/bug-1834-sh-hooks-installed.test.cjs b/tests/bug-1834-sh-hooks-installed.test.cjs new file mode 100644 index 000000000..bb1f7a3e9 --- /dev/null +++ b/tests/bug-1834-sh-hooks-installed.test.cjs @@ -0,0 +1,191 @@ +/** + * Regression tests for bug #1834 + * + * The installer must copy all three .sh hook files to the target hooks/ + * directory during installation. In v1.32.0, only .js hooks were deployed + * because the install loop did not handle non-.js files from hooks/dist/. + * + * This test runs the actual installer (not a simulation) and verifies that + * gsd-session-state.sh, gsd-validate-commit.sh, and gsd-phase-boundary.sh + * are present and executable in the target hooks directory. + * + * Distinct from: + * #1656 — .sh files missing from build-hooks.js HOOKS_TO_COPY + * #1817 — settings.json registration ran even when .sh files were absent + */ + +'use strict'; + +const { describe, test, before, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const os = require('os'); +const { execFileSync } = require('child_process'); + +const INSTALL_SCRIPT = path.join(__dirname, '..', 'bin', 'install.js'); +const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); +const isWindows = process.platform === 'win32'; + +const SH_HOOKS = [ + 'gsd-session-state.sh', + 'gsd-validate-commit.sh', + 'gsd-phase-boundary.sh', +]; + +// ─── Ensure hooks/dist/ is populated before any install test ──────────────── + +before(() => { + execFileSync(process.execPath, [BUILD_SCRIPT], { + encoding: 'utf-8', + stdio: 'pipe', + }); +}); + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function createTempDir(prefix) { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); +} + +function cleanup(dir) { + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } +} + +/** + * Run the installer targeting a temp directory. + * Uses CLAUDE_CONFIG_DIR to redirect the global install target. + * Returns the path to the installed hooks directory. + */ +function runInstaller(configDir) { + execFileSync(process.execPath, [INSTALL_SCRIPT, '--claude', '--global', '--yes'], { + encoding: 'utf-8', + stdio: 'pipe', + env: { + ...process.env, + CLAUDE_CONFIG_DIR: configDir, + }, + }); + return path.join(configDir, 'hooks'); +} + +// ───────────────────────────────────────────────────────────────────────────── +// 1. End-to-end install: .sh hooks are deployed +// ───────────────────────────────────────────────────────────────────────────── + +describe('#1834: installer deploys .sh hooks alongside .js hooks', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempDir('gsd-install-1834-'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('gsd-session-state.sh is present after install', () => { + const hooksDir = runInstaller(tmpDir); + const target = path.join(hooksDir, 'gsd-session-state.sh'); + assert.ok( + fs.existsSync(target), + 'gsd-session-state.sh must be installed to hooks/ — missing file causes SessionStart hook errors' + ); + }); + + test('gsd-validate-commit.sh is present after install', () => { + const hooksDir = runInstaller(tmpDir); + const target = path.join(hooksDir, 'gsd-validate-commit.sh'); + assert.ok( + fs.existsSync(target), + 'gsd-validate-commit.sh must be installed to hooks/ — missing file causes PreToolUse hook errors' + ); + }); + + test('gsd-phase-boundary.sh is present after install', () => { + const hooksDir = runInstaller(tmpDir); + const target = path.join(hooksDir, 'gsd-phase-boundary.sh'); + assert.ok( + fs.existsSync(target), + 'gsd-phase-boundary.sh must be installed to hooks/ — missing file causes PostToolUse hook errors' + ); + }); + + test('all three .sh hooks are present after a single install', () => { + const hooksDir = runInstaller(tmpDir); + for (const hook of SH_HOOKS) { + assert.ok( + fs.existsSync(path.join(hooksDir, hook)), + `${hook} must be present in hooks/ after install` + ); + } + }); + + test('.sh hooks are executable after install', { + skip: isWindows ? 'Windows does not support POSIX file permissions' : false, + }, () => { + const hooksDir = runInstaller(tmpDir); + for (const hook of SH_HOOKS) { + const stat = fs.statSync(path.join(hooksDir, hook)); + assert.ok( + (stat.mode & 0o111) !== 0, + `${hook} must be executable (chmod +x) after install — missing +x causes hook invocation failures` + ); + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// 2. Source-level correctness: install.js copies non-.js files +// ───────────────────────────────────────────────────────────────────────────── + +describe('#1834: install.js source handles .sh files in the hook copy loop', () => { + let src; + + before(() => { + src = fs.readFileSync(INSTALL_SCRIPT, 'utf-8'); + }); + + test('hook copy loop has an else branch for non-.js files', () => { + // The loop must handle files that are not .js — specifically .sh hooks. + // The v1.32.0 bug was that only the if(entry.endsWith('.js')) branch + // existed; non-.js files (i.e. .sh hooks) were silently skipped. + // + // Find the hook copy loop by anchoring on its unique context: the + // configDirReplacement variable is declared only once in install.js, + // right before the entry.endsWith('.js') branch. + const anchorPhrase = 'configDirReplacement'; + const anchorIdx = src.indexOf(anchorPhrase); + assert.ok(anchorIdx !== -1, 'hook copy loop anchor (configDirReplacement) not found in install.js'); + // Extract a window large enough to contain the if/else block (≈1000 chars) + const region = src.slice(anchorIdx, anchorIdx + 1000); + assert.ok( + region.includes("entry.endsWith('.js')"), + "install.js hook copy loop must check entry.endsWith('.js')" + ); + assert.ok( + region.includes('} else {') || region.includes('else {'), + 'hook copy loop must have an else branch to handle .sh and other non-.js files — ' + + 'without it, .sh hooks are silently skipped (root cause of #1834)' + ); + }); + + test('.sh chmod is applied in the non-.js branch', () => { + // Verify the else branch sets chmod for .sh files. + // Without this, .sh hooks exist but are not executable. + assert.ok( + src.includes("entry.endsWith('.sh')"), + "install.js must check entry.endsWith('.sh') to apply chmod after copying" + ); + }); + + test('.sh hooks are listed in expectedShHooks warning check', () => { + // The post-copy verification must check each expected .sh hook. + for (const hook of SH_HOOKS) { + assert.ok( + src.includes(hook), + `install.js must reference '${hook}' in its post-copy verification` + ); + } + }); +});