fix(hooks): complete stale-hooks false-positive fix — stamp .sh version headers + fix detector regex (#2224)
* fix(hooks): stamp gsd-hook-version in .sh hooks and fix stale detection regex (#2136, #2206) Three-part fix for the persistent "⚠ stale hooks — run /gsd-update" false positive that appeared on every session after a fresh install. Root cause: the stale-hook detector (gsd-check-update.js) could only match the JS comment syntax // in its version regex — never the bash # syntax used in .sh hooks. And the bash hooks had no version header at all, so they always landed in the "unknown / stale" branch regardless. Neither partial fix (PR #2207 regex only, PR #2215 install stamping only) was sufficient alone: - Regex fix without install stamping: hooks install with literal "{{GSD_VERSION}}", the {{-guard silently skips them, bash hook staleness permanently undetectable after future updates. - Install stamping without regex fix: hooks are stamped correctly with "# gsd-hook-version: 1.36.0" but the detector's // regex can't read it; still falls to the unknown/stale branch on every session. Fix: 1. Add "# gsd-hook-version: {{GSD_VERSION}}" header to gsd-phase-boundary.sh, gsd-session-state.sh, gsd-validate-commit.sh 2. Extend install.js (both bundled and Codex paths) to substitute {{GSD_VERSION}} in .sh files at install time (same as .js hooks) 3. Extend gsd-check-update.js versionMatch regex to handle bash "#" comment syntax: /(?:\/\/|#) gsd-hook-version:\s*(.+)/ Tests: 11 new assertions across 5 describe blocks covering all three fix parts independently plus an E2E install+detect round-trip. 3885/3885 pass. Approach credit: PR #2207 (j2h4u / Maxim Brashenko) for the regex fix; PR #2215 (nitsan2dots) for the install.js substitution approach. Closes #2136, #2206, #2209, #2210, #2212 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(hooks): extract check-update worker to dedicated file, eliminating template-literal regex escaping Move stale-hook detection logic from inline `node -e '<template literal>'` subprocess to a standalone gsd-check-update-worker.js. Benefits: - Regex is plain JS with no double-escaping (root cause of the (?:\\/\\/|#) confusion) - Worker is independently testable and can be read directly by tests - Uses execFileSync (array args) to satisfy security hook that blocks execSync - MANAGED_HOOKS now includes gsd-check-update-worker.js itself Update tests to read worker file instead of main hook for regex/configDir assertions. All 3886 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -6,6 +6,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- **Shell hooks falsely flagged as stale on every session** — `gsd-phase-boundary.sh`, `gsd-session-state.sh`, and `gsd-validate-commit.sh` now ship with a `# gsd-hook-version: {{GSD_VERSION}}` header; the installer substitutes `{{GSD_VERSION}}` in `.sh` hooks the same way it does for `.js` hooks; and the stale-hook detector in `gsd-check-update.js` now matches bash `#` comment syntax in addition to JS `//` syntax. All three changes are required together — neither the regex fix alone nor the install fix alone is sufficient to resolve the false positive (#2136, #2206, #2209, #2210, #2212)
|
||||
|
||||
## [1.36.0] - 2026-04-14
|
||||
|
||||
### Added
|
||||
|
||||
@@ -5761,10 +5761,15 @@ function install(isGlobal, runtime = 'claude') {
|
||||
// Ensure hook files are executable (fixes #1162 — missing +x permission)
|
||||
try { fs.chmodSync(destFile, 0o755); } catch (e) { /* Windows doesn't support chmod */ }
|
||||
} else {
|
||||
fs.copyFileSync(srcFile, destFile);
|
||||
// Ensure .sh hook files are executable (mirrors chmod in build-hooks.js)
|
||||
// .sh hooks carry a gsd-hook-version header so gsd-check-update.js can
|
||||
// detect staleness after updates — stamp the version just like .js hooks.
|
||||
if (entry.endsWith('.sh')) {
|
||||
let content = fs.readFileSync(srcFile, 'utf8');
|
||||
content = content.replace(/\{\{GSD_VERSION\}\}/g, pkg.version);
|
||||
fs.writeFileSync(destFile, content);
|
||||
try { fs.chmodSync(destFile, 0o755); } catch (e) { /* Windows doesn't support chmod */ }
|
||||
} else {
|
||||
fs.copyFileSync(srcFile, destFile);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -5876,9 +5881,13 @@ function install(isGlobal, runtime = 'claude') {
|
||||
fs.writeFileSync(destFile, content);
|
||||
try { fs.chmodSync(destFile, 0o755); } catch (e) { /* Windows */ }
|
||||
} else {
|
||||
fs.copyFileSync(srcFile, destFile);
|
||||
if (entry.endsWith('.sh')) {
|
||||
let content = fs.readFileSync(srcFile, 'utf8');
|
||||
content = content.replace(/\{\{GSD_VERSION\}\}/g, pkg.version);
|
||||
fs.writeFileSync(destFile, content);
|
||||
try { fs.chmodSync(destFile, 0o755); } catch (e) { /* Windows */ }
|
||||
} else {
|
||||
fs.copyFileSync(srcFile, destFile);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
107
hooks/gsd-check-update-worker.js
Normal file
107
hooks/gsd-check-update-worker.js
Normal file
@@ -0,0 +1,107 @@
|
||||
#!/usr/bin/env node
|
||||
// gsd-hook-version: {{GSD_VERSION}}
|
||||
// Background worker spawned by gsd-check-update.js (SessionStart hook).
|
||||
// Checks for GSD updates and stale hooks, writes result to cache file.
|
||||
// Receives paths via environment variables set by the parent hook.
|
||||
//
|
||||
// Using a separate file (rather than node -e '<inline code>') avoids the
|
||||
// template-literal regex-escaping problem: regex source is plain JS here.
|
||||
|
||||
'use strict';
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { execFileSync } = require('child_process');
|
||||
|
||||
const cacheFile = process.env.GSD_CACHE_FILE;
|
||||
const projectVersionFile = process.env.GSD_PROJECT_VERSION_FILE;
|
||||
const globalVersionFile = process.env.GSD_GLOBAL_VERSION_FILE;
|
||||
|
||||
// Compare semver: true if a > b (a is strictly newer than b)
|
||||
// Strips pre-release suffixes (e.g. '3-beta.1' → '3') to avoid NaN from Number()
|
||||
function isNewer(a, b) {
|
||||
const pa = (a || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0);
|
||||
const pb = (b || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0);
|
||||
for (let i = 0; i < 3; i++) {
|
||||
if (pa[i] > pb[i]) return true;
|
||||
if (pa[i] < pb[i]) return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check project directory first (local install), then global
|
||||
let installed = '0.0.0';
|
||||
let configDir = '';
|
||||
try {
|
||||
if (fs.existsSync(projectVersionFile)) {
|
||||
installed = fs.readFileSync(projectVersionFile, 'utf8').trim();
|
||||
configDir = path.dirname(path.dirname(projectVersionFile));
|
||||
} else if (fs.existsSync(globalVersionFile)) {
|
||||
installed = fs.readFileSync(globalVersionFile, 'utf8').trim();
|
||||
configDir = path.dirname(path.dirname(globalVersionFile));
|
||||
}
|
||||
} catch (e) {}
|
||||
|
||||
// Check for stale hooks — compare hook version headers against installed VERSION
|
||||
// Hooks are installed at configDir/hooks/ (e.g. ~/.claude/hooks/) (#1421)
|
||||
// Only check hooks that GSD currently ships — orphaned files from removed features
|
||||
// (e.g., gsd-intel-*.js) must be ignored to avoid permanent stale warnings (#1750)
|
||||
const MANAGED_HOOKS = [
|
||||
'gsd-check-update-worker.js',
|
||||
'gsd-check-update.js',
|
||||
'gsd-context-monitor.js',
|
||||
'gsd-phase-boundary.sh',
|
||||
'gsd-prompt-guard.js',
|
||||
'gsd-read-guard.js',
|
||||
'gsd-session-state.sh',
|
||||
'gsd-statusline.js',
|
||||
'gsd-validate-commit.sh',
|
||||
'gsd-workflow-guard.js',
|
||||
];
|
||||
|
||||
let staleHooks = [];
|
||||
if (configDir) {
|
||||
const hooksDir = path.join(configDir, 'hooks');
|
||||
try {
|
||||
if (fs.existsSync(hooksDir)) {
|
||||
const hookFiles = fs.readdirSync(hooksDir).filter(f => MANAGED_HOOKS.includes(f));
|
||||
for (const hookFile of hookFiles) {
|
||||
try {
|
||||
const content = fs.readFileSync(path.join(hooksDir, hookFile), 'utf8');
|
||||
// Match both JS (//) and bash (#) comment styles
|
||||
const versionMatch = content.match(/(?:\/\/|#) gsd-hook-version:\s*(.+)/);
|
||||
if (versionMatch) {
|
||||
const hookVersion = versionMatch[1].trim();
|
||||
if (isNewer(installed, hookVersion) && !hookVersion.includes('{{')) {
|
||||
staleHooks.push({ file: hookFile, hookVersion, installedVersion: installed });
|
||||
}
|
||||
} else {
|
||||
// No version header at all — definitely stale (pre-version-tracking)
|
||||
staleHooks.push({ file: hookFile, hookVersion: 'unknown', installedVersion: installed });
|
||||
}
|
||||
} catch (e) {}
|
||||
}
|
||||
}
|
||||
} catch (e) {}
|
||||
}
|
||||
|
||||
let latest = null;
|
||||
try {
|
||||
latest = execFileSync('npm', ['view', 'get-shit-done-cc', 'version'], {
|
||||
encoding: 'utf8',
|
||||
timeout: 10000,
|
||||
windowsHide: true,
|
||||
}).trim();
|
||||
} catch (e) {}
|
||||
|
||||
const result = {
|
||||
update_available: latest && isNewer(latest, installed),
|
||||
installed,
|
||||
latest: latest || 'unknown',
|
||||
checked: Math.floor(Date.now() / 1000),
|
||||
stale_hooks: staleHooks.length > 0 ? staleHooks : undefined,
|
||||
};
|
||||
|
||||
if (cacheFile) {
|
||||
try { fs.writeFileSync(cacheFile, JSON.stringify(result)); } catch (e) {}
|
||||
}
|
||||
@@ -44,99 +44,21 @@ if (!fs.existsSync(cacheDir)) {
|
||||
fs.mkdirSync(cacheDir, { recursive: true });
|
||||
}
|
||||
|
||||
// Run check in background (spawn background process, windowsHide prevents console flash)
|
||||
const child = spawn(process.execPath, ['-e', `
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { execSync } = require('child_process');
|
||||
|
||||
// Compare semver: true if a > b (a is strictly newer than b)
|
||||
// Strips pre-release suffixes (e.g. '3-beta.1' → '3') to avoid NaN from Number()
|
||||
function isNewer(a, b) {
|
||||
const pa = (a || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0);
|
||||
const pb = (b || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0);
|
||||
for (let i = 0; i < 3; i++) {
|
||||
if (pa[i] > pb[i]) return true;
|
||||
if (pa[i] < pb[i]) return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
const cacheFile = ${JSON.stringify(cacheFile)};
|
||||
const projectVersionFile = ${JSON.stringify(projectVersionFile)};
|
||||
const globalVersionFile = ${JSON.stringify(globalVersionFile)};
|
||||
|
||||
// Check project directory first (local install), then global
|
||||
let installed = '0.0.0';
|
||||
let configDir = '';
|
||||
try {
|
||||
if (fs.existsSync(projectVersionFile)) {
|
||||
installed = fs.readFileSync(projectVersionFile, 'utf8').trim();
|
||||
configDir = path.dirname(path.dirname(projectVersionFile));
|
||||
} else if (fs.existsSync(globalVersionFile)) {
|
||||
installed = fs.readFileSync(globalVersionFile, 'utf8').trim();
|
||||
configDir = path.dirname(path.dirname(globalVersionFile));
|
||||
}
|
||||
} catch (e) {}
|
||||
|
||||
// Check for stale hooks — compare hook version headers against installed VERSION
|
||||
// Hooks are installed at configDir/hooks/ (e.g. ~/.claude/hooks/) (#1421)
|
||||
// Only check hooks that GSD currently ships — orphaned files from removed features
|
||||
// (e.g., gsd-intel-*.js) must be ignored to avoid permanent stale warnings (#1750)
|
||||
const MANAGED_HOOKS = [
|
||||
'gsd-check-update.js',
|
||||
'gsd-context-monitor.js',
|
||||
'gsd-phase-boundary.sh',
|
||||
'gsd-prompt-guard.js',
|
||||
'gsd-read-guard.js',
|
||||
'gsd-session-state.sh',
|
||||
'gsd-statusline.js',
|
||||
'gsd-validate-commit.sh',
|
||||
'gsd-workflow-guard.js',
|
||||
];
|
||||
let staleHooks = [];
|
||||
if (configDir) {
|
||||
const hooksDir = path.join(configDir, 'hooks');
|
||||
try {
|
||||
if (fs.existsSync(hooksDir)) {
|
||||
const hookFiles = fs.readdirSync(hooksDir).filter(f => MANAGED_HOOKS.includes(f));
|
||||
for (const hookFile of hookFiles) {
|
||||
try {
|
||||
const content = fs.readFileSync(path.join(hooksDir, hookFile), 'utf8');
|
||||
const versionMatch = content.match(/\\/\\/ gsd-hook-version:\\s*(.+)/);
|
||||
if (versionMatch) {
|
||||
const hookVersion = versionMatch[1].trim();
|
||||
if (isNewer(installed, hookVersion) && !hookVersion.includes('{{')) {
|
||||
staleHooks.push({ file: hookFile, hookVersion, installedVersion: installed });
|
||||
}
|
||||
} else {
|
||||
// No version header at all — definitely stale (pre-version-tracking)
|
||||
staleHooks.push({ file: hookFile, hookVersion: 'unknown', installedVersion: installed });
|
||||
}
|
||||
} catch (e) {}
|
||||
}
|
||||
}
|
||||
} catch (e) {}
|
||||
}
|
||||
|
||||
let latest = null;
|
||||
try {
|
||||
latest = execSync('npm view get-shit-done-cc version', { encoding: 'utf8', timeout: 10000, windowsHide: true }).trim();
|
||||
} catch (e) {}
|
||||
|
||||
const result = {
|
||||
update_available: latest && isNewer(latest, installed),
|
||||
installed,
|
||||
latest: latest || 'unknown',
|
||||
checked: Math.floor(Date.now() / 1000),
|
||||
stale_hooks: staleHooks.length > 0 ? staleHooks : undefined
|
||||
};
|
||||
|
||||
fs.writeFileSync(cacheFile, JSON.stringify(result));
|
||||
`], {
|
||||
// Run check in background via a dedicated worker script.
|
||||
// Spawning a file (rather than node -e '<inline code>') keeps the worker logic
|
||||
// in plain JS with no template-literal regex-escaping concerns, and makes the
|
||||
// worker independently testable.
|
||||
const workerPath = path.join(__dirname, 'gsd-check-update-worker.js');
|
||||
const child = spawn(process.execPath, [workerPath], {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true,
|
||||
detached: true // Required on Windows for proper process detachment
|
||||
detached: true, // Required on Windows for proper process detachment
|
||||
env: {
|
||||
...process.env,
|
||||
GSD_CACHE_FILE: cacheFile,
|
||||
GSD_PROJECT_VERSION_FILE: projectVersionFile,
|
||||
GSD_GLOBAL_VERSION_FILE: globalVersionFile,
|
||||
},
|
||||
});
|
||||
|
||||
child.unref();
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
#!/bin/bash
|
||||
# gsd-hook-version: {{GSD_VERSION}}
|
||||
# gsd-phase-boundary.sh — PostToolUse hook: detect .planning/ file writes
|
||||
# Outputs a reminder when planning files are modified outside normal workflow.
|
||||
# Uses Node.js for JSON parsing (always available in GSD projects, no jq dependency).
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
#!/bin/bash
|
||||
# gsd-hook-version: {{GSD_VERSION}}
|
||||
# gsd-session-state.sh — SessionStart hook: inject project state reminder
|
||||
# Outputs STATE.md head on every session start for orientation.
|
||||
#
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
#!/bin/bash
|
||||
# gsd-hook-version: {{GSD_VERSION}}
|
||||
# gsd-validate-commit.sh — PreToolUse hook: enforce Conventional Commits format
|
||||
# Blocks git commit commands with non-conforming messages (exit 2).
|
||||
# Allows conforming messages and all non-commit commands (exit 0).
|
||||
|
||||
@@ -15,6 +15,7 @@ const DIST_DIR = path.join(HOOKS_DIR, 'dist');
|
||||
|
||||
// Hooks to copy (pure Node.js, no bundling needed)
|
||||
const HOOKS_TO_COPY = [
|
||||
'gsd-check-update-worker.js',
|
||||
'gsd-check-update.js',
|
||||
'gsd-context-monitor.js',
|
||||
'gsd-prompt-guard.js',
|
||||
|
||||
359
tests/bug-2136-sh-hook-version.test.cjs
Normal file
359
tests/bug-2136-sh-hook-version.test.cjs
Normal file
@@ -0,0 +1,359 @@
|
||||
/**
|
||||
* Regression tests for bug #2136 / #2206
|
||||
*
|
||||
* Root cause: three bash hooks (gsd-phase-boundary.sh, gsd-session-state.sh,
|
||||
* gsd-validate-commit.sh) shipped without a gsd-hook-version header, and the
|
||||
* stale-hook detector in gsd-check-update.js only matched JavaScript comment
|
||||
* syntax (//) — not bash comment syntax (#).
|
||||
*
|
||||
* Result: every session showed "⚠ stale hooks — run /gsd-update" immediately
|
||||
* after a fresh install, because the detector saw hookVersion: 'unknown' for
|
||||
* all three bash hooks.
|
||||
*
|
||||
* This fix requires THREE parts working in concert:
|
||||
* 1. Bash hooks ship with "# gsd-hook-version: {{GSD_VERSION}}"
|
||||
* 2. install.js substitutes {{GSD_VERSION}} in .sh files at install time
|
||||
* 3. gsd-check-update.js regex matches both "//" and "#" comment styles
|
||||
*
|
||||
* Neither fix alone is sufficient:
|
||||
* - Headers + regex fix only (no install.js fix): installed hooks contain
|
||||
* literal "{{GSD_VERSION}}" — the {{-guard silently skips them, making
|
||||
* bash hook staleness permanently undetectable after future updates.
|
||||
* - Headers + install.js fix only (no regex fix): installed hooks are
|
||||
* stamped correctly but the detector still can't read bash "#" comments,
|
||||
* so they still land in the "unknown / stale" branch on every session.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
// NOTE: Do NOT set GSD_TEST_MODE here — the E2E install tests spawn the
|
||||
// real installer subprocess, which skips all install logic when GSD_TEST_MODE=1.
|
||||
|
||||
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 HOOKS_DIR = path.join(__dirname, '..', 'hooks');
|
||||
const CHECK_UPDATE_FILE = path.join(HOOKS_DIR, 'gsd-check-update.js');
|
||||
const WORKER_FILE = path.join(HOOKS_DIR, 'gsd-check-update-worker.js');
|
||||
const INSTALL_SCRIPT = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js');
|
||||
|
||||
const SH_HOOKS = [
|
||||
'gsd-phase-boundary.sh',
|
||||
'gsd-session-state.sh',
|
||||
'gsd-validate-commit.sh',
|
||||
];
|
||||
|
||||
// ─── Ensure hooks/dist/ is populated before install tests ────────────────────
|
||||
|
||||
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 */ }
|
||||
}
|
||||
|
||||
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');
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Part 1: Bash hook sources carry the version header placeholder
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('bug #2136 part 1: bash hook sources carry gsd-hook-version placeholder', () => {
|
||||
for (const sh of SH_HOOKS) {
|
||||
test(`${sh} contains "# gsd-hook-version: {{GSD_VERSION}}"`, () => {
|
||||
const content = fs.readFileSync(path.join(HOOKS_DIR, sh), 'utf8');
|
||||
assert.ok(
|
||||
content.includes('# gsd-hook-version: {{GSD_VERSION}}'),
|
||||
`${sh} must include "# gsd-hook-version: {{GSD_VERSION}}" so the ` +
|
||||
`installer can stamp it and gsd-check-update.js can detect staleness`
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
test('version header is on line 2 (immediately after shebang)', () => {
|
||||
// Placing the header immediately after #!/bin/bash ensures it is always
|
||||
// found regardless of how much of the file is read.
|
||||
for (const sh of SH_HOOKS) {
|
||||
const lines = fs.readFileSync(path.join(HOOKS_DIR, sh), 'utf8').split('\n');
|
||||
assert.strictEqual(lines[0], '#!/bin/bash', `${sh} line 1 must be #!/bin/bash`);
|
||||
assert.ok(
|
||||
lines[1].startsWith('# gsd-hook-version:'),
|
||||
`${sh} line 2 must be the gsd-hook-version header (got: "${lines[1]}")`
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Part 2: gsd-check-update-worker.js regex handles bash "#" comment syntax
|
||||
// (Logic moved from inline -e template literal to dedicated worker file)
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('bug #2136 part 2: stale-hook detector handles bash comment syntax', () => {
|
||||
let src;
|
||||
|
||||
before(() => {
|
||||
src = fs.readFileSync(WORKER_FILE, 'utf8');
|
||||
});
|
||||
|
||||
test('version regex in source matches "#" comment syntax in addition to "//"', () => {
|
||||
// The regex string in the source must contain the alternation for "#".
|
||||
// The worker uses plain JS (no template-literal escaping), so the form is
|
||||
// "(?:\/\/|#)" directly in source.
|
||||
const hasBashAlternative =
|
||||
src.includes('(?:\\/\\/|#)') || // escaped form (old template-literal style)
|
||||
src.includes('(?:\/\/|#)'); // direct form in plain JS worker
|
||||
assert.ok(
|
||||
hasBashAlternative,
|
||||
'gsd-check-update-worker.js version regex must include an alternative for bash "#" comments. ' +
|
||||
'Expected to find (?:\\/\\/|#) or (?:\/\/|#) in the source. ' +
|
||||
'The original "//" only regex causes bash hooks to always report hookVersion: "unknown"'
|
||||
);
|
||||
});
|
||||
|
||||
test('version regex does not use the old JS-only form as the sole pattern', () => {
|
||||
// The old regex inside the template literal was the string:
|
||||
// /\\/\\/ gsd-hook-version:\\s*(.+)/
|
||||
// which, when evaluated in the subprocess, produced: /\/\/ gsd-hook-version:\s*(.+)/
|
||||
// That only matched JS "//" comments — never bash "#".
|
||||
// We verify that the old exact string no longer appears.
|
||||
assert.ok(
|
||||
!src.includes('\\/\\/ gsd-hook-version'),
|
||||
'gsd-check-update-worker.js must not use the old JS-only (\\/\\/ gsd-hook-version) ' +
|
||||
'escape form as the sole version matcher — it cannot match bash "#" comments'
|
||||
);
|
||||
});
|
||||
|
||||
test('version regex correctly matches both bash and JS hook version headers', () => {
|
||||
// Verify that the versionMatch line in the source uses a regex that matches
|
||||
// both bash "#" and JS "//" comment styles. We check the source contains the
|
||||
// expected alternation, then directly test the known required pattern.
|
||||
//
|
||||
// We do NOT try to extract and evaluate the regex from source (it contains ")"
|
||||
// which breaks simple extraction), so instead we confirm the source matches
|
||||
// our expectation and run the regex itself.
|
||||
assert.ok(
|
||||
src.includes('gsd-hook-version'),
|
||||
'gsd-check-update-worker.js must contain a gsd-hook-version version check'
|
||||
);
|
||||
|
||||
// The fixed regex that must be present: matches both comment styles
|
||||
const fixedRegex = /(?:\/\/|#) gsd-hook-version:\s*(.+)/;
|
||||
|
||||
assert.ok(
|
||||
fixedRegex.test('# gsd-hook-version: 1.36.0'),
|
||||
'bash-style "# gsd-hook-version: X" must be matchable by the required regex'
|
||||
);
|
||||
assert.ok(
|
||||
fixedRegex.test('// gsd-hook-version: 1.36.0'),
|
||||
'JS-style "// gsd-hook-version: X" must still match (no regression)'
|
||||
);
|
||||
assert.ok(
|
||||
!fixedRegex.test('gsd-hook-version: 1.36.0'),
|
||||
'line without a comment prefix must not match (prevents false positives)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Part 3a: install.js bundled path substitutes {{GSD_VERSION}} in .sh hooks
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('bug #2136 part 3a: install.js bundled path substitutes {{GSD_VERSION}} in .sh hooks', () => {
|
||||
let src;
|
||||
|
||||
before(() => {
|
||||
src = fs.readFileSync(INSTALL_SCRIPT, 'utf8');
|
||||
});
|
||||
|
||||
test('.sh branch in bundled hook copy loop reads file and substitutes GSD_VERSION', () => {
|
||||
// Anchor on configDirReplacement — unique to the bundled-hooks path.
|
||||
const anchorIdx = src.indexOf('configDirReplacement');
|
||||
assert.ok(anchorIdx !== -1, 'bundled hook copy loop anchor (configDirReplacement) not found');
|
||||
|
||||
// Window large enough for the if/else block
|
||||
const region = src.slice(anchorIdx, anchorIdx + 2000);
|
||||
|
||||
assert.ok(
|
||||
region.includes("entry.endsWith('.sh')"),
|
||||
"bundled hook copy loop must check entry.endsWith('.sh')"
|
||||
);
|
||||
assert.ok(
|
||||
region.includes('GSD_VERSION'),
|
||||
'bundled .sh branch must reference GSD_VERSION substitution. Without this, ' +
|
||||
'installed .sh hooks contain the literal "{{GSD_VERSION}}" placeholder and ' +
|
||||
'bash hook staleness becomes permanently undetectable after future updates'
|
||||
);
|
||||
// copyFileSync on a .sh file would skip substitution — ensure we read+write instead
|
||||
const shBranchIdx = region.indexOf("entry.endsWith('.sh')");
|
||||
const shBranchRegion = region.slice(shBranchIdx, shBranchIdx + 400);
|
||||
assert.ok(
|
||||
shBranchRegion.includes('readFileSync') || shBranchRegion.includes('writeFileSync'),
|
||||
'bundled .sh branch must read the file (readFileSync) to perform substitution, ' +
|
||||
'not copyFileSync directly (which skips template expansion)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Part 3b: install.js Codex path also substitutes {{GSD_VERSION}} in .sh hooks
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('bug #2136 part 3b: install.js Codex path substitutes {{GSD_VERSION}} in .sh hooks', () => {
|
||||
let src;
|
||||
|
||||
before(() => {
|
||||
src = fs.readFileSync(INSTALL_SCRIPT, 'utf8');
|
||||
});
|
||||
|
||||
test('.sh branch in Codex hook copy block substitutes GSD_VERSION', () => {
|
||||
// Anchor on codexHooksSrc — unique to the Codex path.
|
||||
const anchorIdx = src.indexOf('codexHooksSrc');
|
||||
assert.ok(anchorIdx !== -1, 'Codex hook copy block anchor (codexHooksSrc) not found');
|
||||
|
||||
const region = src.slice(anchorIdx, anchorIdx + 2000);
|
||||
|
||||
assert.ok(
|
||||
region.includes("entry.endsWith('.sh')"),
|
||||
"Codex hook copy block must check entry.endsWith('.sh')"
|
||||
);
|
||||
assert.ok(
|
||||
region.includes('GSD_VERSION'),
|
||||
'Codex .sh branch must substitute {{GSD_VERSION}}. The bundled path was fixed ' +
|
||||
'but Codex installs a separate copy of the hooks from hooks/dist that also needs stamping'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Part 4: End-to-end — installed .sh hooks have stamped version, not placeholder
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('bug #2136 part 4: installed .sh hooks contain stamped concrete version', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempDir('gsd-2136-install-');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('installed .sh hooks contain a concrete version string, not the template placeholder', () => {
|
||||
const hooksDir = runInstaller(tmpDir);
|
||||
|
||||
for (const sh of SH_HOOKS) {
|
||||
const hookPath = path.join(hooksDir, sh);
|
||||
assert.ok(fs.existsSync(hookPath), `${sh} must be installed`);
|
||||
|
||||
const content = fs.readFileSync(hookPath, 'utf8');
|
||||
|
||||
assert.ok(
|
||||
content.includes('# gsd-hook-version:'),
|
||||
`installed ${sh} must contain a "# gsd-hook-version:" header`
|
||||
);
|
||||
assert.ok(
|
||||
!content.includes('{{GSD_VERSION}}'),
|
||||
`installed ${sh} must not contain literal "{{GSD_VERSION}}" — ` +
|
||||
`install.js must substitute it with the concrete package version`
|
||||
);
|
||||
|
||||
const versionMatch = content.match(/# gsd-hook-version:\s*(\S+)/);
|
||||
assert.ok(versionMatch, `installed ${sh} version header must have a version value`);
|
||||
assert.match(
|
||||
versionMatch[1],
|
||||
/^\d+\.\d+\.\d+/,
|
||||
`installed ${sh} version "${versionMatch[1]}" must be a semver-like string`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('stale-hook detector reports zero stale bash hooks immediately after fresh install', () => {
|
||||
// This is the definitive end-to-end proof: after install, run the actual
|
||||
// version-check logic (extracted from gsd-check-update.js) against the
|
||||
// installed hooks and verify none are flagged stale.
|
||||
const hooksDir = runInstaller(tmpDir);
|
||||
const pkg = require(path.join(__dirname, '..', 'package.json'));
|
||||
const installedVersion = pkg.version;
|
||||
|
||||
// Build a subprocess that runs the staleness check logic in isolation.
|
||||
// We pass the installed version, hooks dir, and hook filenames as JSON
|
||||
// to avoid any injection risk.
|
||||
const checkScript = `
|
||||
'use strict';
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
function isNewer(a, b) {
|
||||
const pa = (a || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0);
|
||||
const pb = (b || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0);
|
||||
for (let i = 0; i < 3; i++) {
|
||||
if (pa[i] > pb[i]) return true;
|
||||
if (pa[i] < pb[i]) return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
const hooksDir = ${JSON.stringify(hooksDir)};
|
||||
const installed = ${JSON.stringify(installedVersion)};
|
||||
const shHooks = ${JSON.stringify(SH_HOOKS)};
|
||||
// Use the same regex that the fixed gsd-check-update.js uses
|
||||
const versionRe = /(?:\\/\\/|#) gsd-hook-version:\\s*(.+)/;
|
||||
|
||||
const staleHooks = [];
|
||||
for (const hookFile of shHooks) {
|
||||
const hookPath = path.join(hooksDir, hookFile);
|
||||
if (!fs.existsSync(hookPath)) {
|
||||
staleHooks.push({ file: hookFile, hookVersion: 'missing' });
|
||||
continue;
|
||||
}
|
||||
const content = fs.readFileSync(hookPath, 'utf8');
|
||||
const m = content.match(versionRe);
|
||||
if (m) {
|
||||
const hookVersion = m[1].trim();
|
||||
if (isNewer(installed, hookVersion) && !hookVersion.includes('{{')) {
|
||||
staleHooks.push({ file: hookFile, hookVersion, installedVersion: installed });
|
||||
}
|
||||
} else {
|
||||
staleHooks.push({ file: hookFile, hookVersion: 'unknown', installedVersion: installed });
|
||||
}
|
||||
}
|
||||
process.stdout.write(JSON.stringify(staleHooks));
|
||||
`;
|
||||
|
||||
const result = execFileSync(process.execPath, ['-e', checkScript], { encoding: 'utf8' });
|
||||
const staleHooks = JSON.parse(result);
|
||||
|
||||
assert.deepStrictEqual(
|
||||
staleHooks,
|
||||
[],
|
||||
`Fresh install must produce zero stale bash hooks.\n` +
|
||||
`Got: ${JSON.stringify(staleHooks, null, 2)}\n` +
|
||||
`This indicates either the version header was not stamped by install.js, ` +
|
||||
`or the detector regex cannot match bash "#" comment syntax.`
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1071,8 +1071,10 @@ describe('stale hook filter', () => {
|
||||
|
||||
describe('stale hook path', () => {
|
||||
test('gsd-check-update.js checks configDir/hooks/ where hooks are actually installed (#1421)', () => {
|
||||
// The stale-hook scan logic lives in the worker (moved from inline -e template literal).
|
||||
// The worker receives configDir via env and constructs the hooksDir path.
|
||||
const content = fs.readFileSync(
|
||||
path.join(__dirname, '..', 'hooks', 'gsd-check-update.js'), 'utf-8'
|
||||
path.join(__dirname, '..', 'hooks', 'gsd-check-update-worker.js'), 'utf-8'
|
||||
);
|
||||
// Hooks are installed at configDir/hooks/ (e.g. ~/.claude/hooks/),
|
||||
// not configDir/get-shit-done/hooks/ which doesn't exist (#1421)
|
||||
|
||||
@@ -18,7 +18,9 @@ const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const HOOKS_DIR = path.join(__dirname, '..', 'hooks');
|
||||
const CHECK_UPDATE_FILE = path.join(HOOKS_DIR, 'gsd-check-update.js');
|
||||
// MANAGED_HOOKS now lives in the worker script (extracted from inline -e code
|
||||
// to avoid template-literal regex-escaping concerns). The test reads the worker.
|
||||
const MANAGED_HOOKS_FILE = path.join(HOOKS_DIR, 'gsd-check-update-worker.js');
|
||||
|
||||
describe('bug #2136: MANAGED_HOOKS must include all shipped hook files', () => {
|
||||
let src;
|
||||
@@ -26,12 +28,12 @@ describe('bug #2136: MANAGED_HOOKS must include all shipped hook files', () => {
|
||||
let shippedHooks;
|
||||
|
||||
// Read once — all tests share the same source snapshot
|
||||
src = fs.readFileSync(CHECK_UPDATE_FILE, 'utf-8');
|
||||
src = fs.readFileSync(MANAGED_HOOKS_FILE, 'utf-8');
|
||||
|
||||
// Extract the MANAGED_HOOKS array entries from the source
|
||||
// The array is defined as a multi-line array literal of quoted strings
|
||||
const match = src.match(/const MANAGED_HOOKS\s*=\s*\[([\s\S]*?)\]/);
|
||||
assert.ok(match, 'MANAGED_HOOKS array not found in gsd-check-update.js');
|
||||
assert.ok(match, 'MANAGED_HOOKS array not found in gsd-check-update-worker.js');
|
||||
|
||||
managedHooks = match[1]
|
||||
.split('\n')
|
||||
@@ -47,7 +49,7 @@ describe('bug #2136: MANAGED_HOOKS must include all shipped hook files', () => {
|
||||
for (const hookFile of jsHooks) {
|
||||
assert.ok(
|
||||
managedHooks.includes(hookFile),
|
||||
`${hookFile} is shipped in hooks/ but missing from MANAGED_HOOKS in gsd-check-update.js`
|
||||
`${hookFile} is shipped in hooks/ but missing from MANAGED_HOOKS in gsd-check-update-worker.js`
|
||||
);
|
||||
}
|
||||
});
|
||||
@@ -57,7 +59,7 @@ describe('bug #2136: MANAGED_HOOKS must include all shipped hook files', () => {
|
||||
for (const hookFile of shHooks) {
|
||||
assert.ok(
|
||||
managedHooks.includes(hookFile),
|
||||
`${hookFile} is shipped in hooks/ but missing from MANAGED_HOOKS in gsd-check-update.js`
|
||||
`${hookFile} is shipped in hooks/ but missing from MANAGED_HOOKS in gsd-check-update-worker.js`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -11,31 +11,41 @@ const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
// MANAGED_HOOKS lives in the worker file (extracted from inline -e code to eliminate
|
||||
// template-literal regex-escaping concerns). Tests read the worker directly.
|
||||
const CHECK_UPDATE_PATH = path.join(__dirname, '..', 'hooks', 'gsd-check-update.js');
|
||||
const WORKER_PATH = path.join(__dirname, '..', 'hooks', 'gsd-check-update-worker.js');
|
||||
const BUILD_HOOKS_PATH = path.join(__dirname, '..', 'scripts', 'build-hooks.js');
|
||||
|
||||
describe('orphaned hooks stale detection (#1750)', () => {
|
||||
test('stale hook scanner uses an allowlist of managed hooks, not a wildcard', () => {
|
||||
const content = fs.readFileSync(CHECK_UPDATE_PATH, 'utf8');
|
||||
const content = fs.readFileSync(WORKER_PATH, 'utf8');
|
||||
|
||||
// The scanner MUST NOT use a broad `startsWith('gsd-')` filter that catches
|
||||
// orphaned files from removed features (gsd-intel-index.js, gsd-intel-prune.js, etc.)
|
||||
// Instead, it should reference a known set of managed hook filenames.
|
||||
|
||||
// Extract the spawned child script (everything between the template literal backticks)
|
||||
const childScriptMatch = content.match(/spawn\(process\.execPath,\s*\['-e',\s*`([\s\S]*?)`\]/);
|
||||
assert.ok(childScriptMatch, 'should find the spawned child script');
|
||||
const childScript = childScriptMatch[1];
|
||||
|
||||
// The child script must NOT have a broad gsd-*.js wildcard filter
|
||||
const hasBroadFilter = /readdirSync\([^)]+\)\.filter\([^)]*startsWith\('gsd-'\)\s*&&[^)]*endsWith\('\.js'\)/s.test(childScript);
|
||||
const hasBroadFilter = /readdirSync\([^)]+\)\.filter\([^)]*startsWith\('gsd-'\)\s*&&[^)]*endsWith\('\.js'\)/s.test(content);
|
||||
assert.ok(!hasBroadFilter,
|
||||
'scanner must NOT use broad startsWith("gsd-") && endsWith(".js") filter — ' +
|
||||
'this catches orphaned hooks from removed features (e.g., gsd-intel-index.js). ' +
|
||||
'Use a MANAGED_HOOKS allowlist instead.');
|
||||
});
|
||||
|
||||
test('managed hooks list in check-update matches build-hooks HOOKS_TO_COPY JS entries', () => {
|
||||
test('gsd-check-update.js spawns the worker by file path (not inline -e code)', () => {
|
||||
// After the worker extraction, the main hook must spawn the worker file
|
||||
// rather than embedding all logic in a template literal.
|
||||
const content = fs.readFileSync(CHECK_UPDATE_PATH, 'utf8');
|
||||
assert.ok(
|
||||
content.includes('gsd-check-update-worker.js'),
|
||||
'gsd-check-update.js must reference gsd-check-update-worker.js as the spawn target'
|
||||
);
|
||||
assert.ok(
|
||||
!content.includes("'-e'"),
|
||||
'gsd-check-update.js must not use node -e inline code (logic moved to worker file)'
|
||||
);
|
||||
});
|
||||
|
||||
test('managed hooks list in worker matches build-hooks HOOKS_TO_COPY JS entries', () => {
|
||||
// Extract JS hooks from build-hooks.js HOOKS_TO_COPY
|
||||
const buildContent = fs.readFileSync(BUILD_HOOKS_PATH, 'utf8');
|
||||
const hooksArrayMatch = buildContent.match(/HOOKS_TO_COPY\s*=\s*\[([\s\S]*?)\]/);
|
||||
@@ -48,25 +58,18 @@ describe('orphaned hooks stale detection (#1750)', () => {
|
||||
}
|
||||
assert.ok(jsHooks.length >= 5, `expected at least 5 JS hooks in HOOKS_TO_COPY, got ${jsHooks.length}`);
|
||||
|
||||
// The check-update hook should define its own managed hooks list
|
||||
// that matches the JS entries from HOOKS_TO_COPY
|
||||
const checkContent = fs.readFileSync(CHECK_UPDATE_PATH, 'utf8');
|
||||
const childScriptMatch = checkContent.match(/spawn\(process\.execPath,\s*\['-e',\s*`([\s\S]*?)`\]/);
|
||||
const childScript = childScriptMatch[1];
|
||||
|
||||
// Verify each JS hook from HOOKS_TO_COPY is referenced in the managed list
|
||||
// MANAGED_HOOKS in the worker must include each JS hook from HOOKS_TO_COPY
|
||||
const workerContent = fs.readFileSync(WORKER_PATH, 'utf8');
|
||||
for (const hook of jsHooks) {
|
||||
assert.ok(
|
||||
childScript.includes(hook),
|
||||
`managed hooks in check-update should include '${hook}' from HOOKS_TO_COPY`
|
||||
workerContent.includes(hook),
|
||||
`MANAGED_HOOKS in worker should include '${hook}' from HOOKS_TO_COPY`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('orphaned hook filenames would NOT match the managed hooks list', () => {
|
||||
const checkContent = fs.readFileSync(CHECK_UPDATE_PATH, 'utf8');
|
||||
const childScriptMatch = checkContent.match(/spawn\(process\.execPath,\s*\['-e',\s*`([\s\S]*?)`\]/);
|
||||
const childScript = childScriptMatch[1];
|
||||
test('orphaned hook filenames are NOT in the MANAGED_HOOKS list', () => {
|
||||
const workerContent = fs.readFileSync(WORKER_PATH, 'utf8');
|
||||
|
||||
// These are real orphaned hooks from the removed intel feature
|
||||
const orphanedHooks = [
|
||||
@@ -77,8 +80,8 @@ describe('orphaned hooks stale detection (#1750)', () => {
|
||||
|
||||
for (const orphan of orphanedHooks) {
|
||||
assert.ok(
|
||||
!childScript.includes(orphan),
|
||||
`orphaned hook '${orphan}' must NOT be in the managed hooks list`
|
||||
!workerContent.includes(orphan),
|
||||
`orphaned hook '${orphan}' must NOT be in the MANAGED_HOOKS list`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user