mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
* 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>
89 lines
3.8 KiB
JavaScript
89 lines
3.8 KiB
JavaScript
/**
|
|
* Regression test for #1750: orphaned hook files from removed features
|
|
* (e.g., gsd-intel-*.js) should NOT be flagged as stale by gsd-check-update.js.
|
|
*
|
|
* The stale hooks scanner should only check hooks that are part of the current
|
|
* distribution, not every gsd-*.js file in the hooks directory.
|
|
*/
|
|
|
|
const { test, describe } = require('node:test');
|
|
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(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.
|
|
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('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]*?)\]/);
|
|
assert.ok(hooksArrayMatch, 'should find HOOKS_TO_COPY array');
|
|
|
|
const jsHooks = [];
|
|
const hookEntries = hooksArrayMatch[1].matchAll(/'([^']+\.js)'/g);
|
|
for (const m of hookEntries) {
|
|
jsHooks.push(m[1]);
|
|
}
|
|
assert.ok(jsHooks.length >= 5, `expected at least 5 JS hooks in HOOKS_TO_COPY, got ${jsHooks.length}`);
|
|
|
|
// 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(
|
|
workerContent.includes(hook),
|
|
`MANAGED_HOOKS in worker should include '${hook}' from HOOKS_TO_COPY`
|
|
);
|
|
}
|
|
});
|
|
|
|
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 = [
|
|
'gsd-intel-index.js',
|
|
'gsd-intel-prune.js',
|
|
'gsd-intel-session.js',
|
|
];
|
|
|
|
for (const orphan of orphanedHooks) {
|
|
assert.ok(
|
|
!workerContent.includes(orphan),
|
|
`orphaned hook '${orphan}' must NOT be in the MANAGED_HOOKS list`
|
|
);
|
|
}
|
|
});
|
|
});
|