From 9b77320580adc296aed017edd0550163ece72abb Mon Sep 17 00:00:00 2001
From: Carlos Cativo <cativo23.kt@gmail.com>
Date: Wed, 2 Sep 2026 06:04:37 -0600
Subject: [PATCH] fix(#4076): add missing gsd-hook-version header to
 gsd-node-runner.sh (#4092)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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>
---
 .changeset/graceful-finches-wake.md |  5 +++
 hooks/gsd-node-runner.sh            |  1 +
 scripts/build-hooks.js              | 13 +++++---
 tests/managed-hooks.test.cjs        | 51 +++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/graceful-finches-wake.md

diff --git a/.changeset/graceful-finches-wake.md b/.changeset/graceful-finches-wake.md
new file mode 100644
index 000000000..0a3643eac
--- /dev/null
+++ b/.changeset/graceful-finches-wake.md
@@ -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.
diff --git a/hooks/gsd-node-runner.sh b/hooks/gsd-node-runner.sh
index 77910efaa..787fae064 100755
--- a/hooks/gsd-node-runner.sh
+++ b/hooks/gsd-node-runner.sh
@@ -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:
diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js
index 228d9aab9..59e4a97e3 100644
--- a/scripts/build-hooks.js
+++ b/scripts/build-hooks.js
@@ -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.
diff --git a/tests/managed-hooks.test.cjs b/tests/managed-hooks.test.cjs
index 233e781d2..95e0eda83 100644
--- a/tests/managed-hooks.test.cjs
+++ b/tests/managed-hooks.test.cjs
@@ -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)