* fix(#4076): add missing gsd-hook-version header to gsd-node-runner.sh gsd-node-runner.sh was registered in MANAGED_HOOKS but shipped without a gsd-hook-version header, so gsd-check-update-worker.js always classified it as 'definitely stale' (a missing header is indistinguishable from a pre-version-tracking file). Every install on an otherwise up-to-date version showed a permanent, unclearable '⚠ stale hooks — run /gsd-update' warning naming this one file. Root cause: the build-hooks.js comment claimed the file is 'not a registered hook' and 'staged verbatim — no templating', but it IS in MANAGED_HOOKS (managed-hooks-registry.cjs:34) and install.js already stamps {{GSD_VERSION}} into every .sh hook unconditionally, gsd-node-runner.sh included. The comment contradicted both the registry and the installer's actual behavior, and the header line itself was simply never added. Fix: add the header (matching every other managed .sh hook's format) and correct the comment so it no longer asserts the opposite of what the registry and installer actually do. Adds a regression test that iterates every MANAGED_HOOKS entry and asserts it carries a header matching the worker's own detection regex, so a future hook added to the registry without one fails CI instead of shipping silently. Fixes #4076 * chore(#4076): add changeset fragment for PR #4092 * fix(#4076): address review nits — drop unneeded exemption, fix blank line Per @trek-e's review on #4092: - tests/managed-hooks.test.cjs:96: the readFileSync call uses a loop variable (entry-derived hookPath), not a literal path, so local/no-source-grep's static literal-path detector never flags it — the allow-test-rule exemption comment was unnecessary. Replaced with a plain note explaining the source-read rationale. - tests/managed-hooks.test.cjs:121-122: dropped a stray extra blank line before the bug #2136 section divider. --------- Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
5
.changeset/graceful-finches-wake.md
Normal file
5
.changeset/graceful-finches-wake.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4092
|
||||
---
|
||||
**`gsd-node-runner.sh` no longer triggers a permanent, unclearable "⚠ stale hooks" warning** — it was registered in `MANAGED_HOOKS` but shipped without its `gsd-hook-version` header, so up-to-date installs always flagged it as stale.
|
||||
@@ -1,4 +1,5 @@
|
||||
#!/bin/sh
|
||||
# gsd-hook-version: {{GSD_VERSION}}
|
||||
# gsd-node-runner.sh — GSD portable node resolver (#3662).
|
||||
#
|
||||
# Managed JS hook commands under --portable-hooks route through this script:
|
||||
|
||||
@@ -71,10 +71,15 @@ const HOOKS_TO_COPY = [
|
||||
'gsd-session-state.sh',
|
||||
'gsd-validate-commit.sh',
|
||||
'gsd-phase-boundary.sh',
|
||||
// Portable node resolver (#3662). Not a registered hook itself: managed JS
|
||||
// hook commands under --portable-hooks route through it (bash <resolver>
|
||||
// <baked-node> <script>) so node resolves at hook-fire time in every
|
||||
// environment sharing the config root. Staged verbatim — no templating.
|
||||
// Portable node resolver (#3662). Managed JS hook commands under
|
||||
// --portable-hooks route through it (bash <resolver> <baked-node>
|
||||
// <script>) so node resolves at hook-fire time in every environment
|
||||
// sharing the config root. It IS registered in MANAGED_HOOKS
|
||||
// (managed-hooks-registry.cjs) for staleness tracking like every other
|
||||
// shipped .sh hook, and install.js stamps its {{GSD_VERSION}} header the
|
||||
// same way (#4076 — the prior comment here claimed the opposite on both
|
||||
// counts, which is why the header was missing and staleness detection was
|
||||
// permanently broken for this file).
|
||||
'gsd-node-runner.sh',
|
||||
// Graphify auto-update hook (#3347 / PR #3557 / #3579). Opt-in via
|
||||
// .planning/config.json graphify.auto_update; off by default.
|
||||
|
||||
@@ -67,6 +67,57 @@ describe('bug #2136: MANAGED_HOOKS must include all shipped hook files', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Regression test for bug #4076
|
||||
*
|
||||
* gsd-node-runner.sh was registered in MANAGED_HOOKS but shipped without a
|
||||
* "gsd-hook-version" header. gsd-check-update-worker.js treats a hook with
|
||||
* no header as "definitely stale" (there is no way to tell it apart from a
|
||||
* pre-version-tracking file), so every install on an otherwise up-to-date
|
||||
* version showed a permanent, unclearable "⚠ stale hooks — run /gsd-update"
|
||||
* warning naming that one file.
|
||||
*
|
||||
* This is the same *class* as bug #2136 (bash hooks missing the header
|
||||
* entirely) but from the opposite direction: #2136 covered hooks that were
|
||||
* *missing from MANAGED_HOOKS*; this covers a hook that *is* in
|
||||
* MANAGED_HOOKS but never got the header line added to its source. The test
|
||||
* below closes the whole class by iterating every MANAGED_HOOKS entry —
|
||||
* rather than a hardcoded list of hook filenames — so a future hook added to
|
||||
* the registry without a header fails CI instead of shipping silently.
|
||||
*/
|
||||
describe('bug #4076: every MANAGED_HOOKS entry carries a gsd-hook-version header', () => {
|
||||
// Mirrors the exact regex gsd-check-update-worker.js uses at runtime to
|
||||
// detect the header (both "//" JS-style and "#" bash-style comments).
|
||||
const VERSION_HEADER_RE = /(?:\/\/|#) gsd-hook-version:\s*(.+)/;
|
||||
|
||||
for (const entry of MANAGED_HOOKS) {
|
||||
test(`${entry} has a gsd-hook-version header matching the worker's detection regex`, () => {
|
||||
const hookPath = path.join(HOOKS_DIR, entry);
|
||||
// Note: `entry` is a loop variable, not a literal path, so
|
||||
// local/no-source-grep's static literal-path detector does not flag
|
||||
// this read — no allow-test-rule exemption needed. The header is
|
||||
// still a source-text invariant the stale-hook detector reads via
|
||||
// string matching (not a module export), so a source read is the
|
||||
// correct way to observe it, same rationale as the sibling bug #2136
|
||||
// checks below (folded bug-2136-sh-hook-version.test.cjs).
|
||||
const content = fs.readFileSync(hookPath, 'utf8');
|
||||
const match = content.match(VERSION_HEADER_RE);
|
||||
assert.ok(
|
||||
match,
|
||||
`${entry} is listed in MANAGED_HOOKS but has no "# gsd-hook-version:" / ` +
|
||||
`"// gsd-hook-version:" header — gsd-check-update-worker.js treats a ` +
|
||||
`missing header as "definitely stale", producing a permanent, ` +
|
||||
`unclearable "⚠ stale hooks" warning on every up-to-date install (#4076)`
|
||||
);
|
||||
assert.ok(
|
||||
match[1].trim() === '{{GSD_VERSION}}' || /^\d+\.\d+\.\d+/.test(match[1].trim()),
|
||||
`${entry}'s gsd-hook-version header must be either the unstamped ` +
|
||||
`"{{GSD_VERSION}}" placeholder (source tree) or a concrete semver ` +
|
||||
`string (installed tree) — got "${match[1].trim()}"`
|
||||
);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
// Folded from tests/bug-2136-sh-hook-version.test.cjs — consolidation epic #1969 (B6 #1975)
|
||||
|
||||
Reference in New Issue
Block a user