From 9263fa1e46ca7b0fc2645cf56fb59884120a83cd Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 2 Jun 2026 13:00:29 -0400 Subject: [PATCH] =?UTF-8?q?test(#606):=20regression=20guard=20=E2=80=94=20?= =?UTF-8?q?every=20same-dir=20require()=20target=20of=20a=20shipped=20hook?= =?UTF-8?q?=20must=20itself=20ship=20(#613)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#606): guard that every same-dir require() target of a shipped hook is shipped #606: gsd-check-update-worker.js require()'d its sibling managed-hooks-registry.cjs, but the file was missing from HOOKS_TO_COPY, so the installer never placed it next to the worker and the background update worker crashed silently with "Cannot find module". The fix shipped in #611 (added the file to HOOKS_TO_COPY); this adds the regression guard that was the issue's third acceptance criterion. The new test scans every JS/CJS hook in HOOKS_TO_COPY for same-directory relative requires — require('./x') and require('./subdir/x') — and asserts each target is itself shipped: './x' is in HOOKS_TO_COPY, or './subdir/...' lives under a dir in HOOKS_SUBDIRS_TO_COPY. require('../...') targets that escape the hooks/ dir are out of scope (their shipping is governed by package.json "files"). Exports HOOKS_SUBDIRS_TO_COPY from scripts/build-hooks.js so the test reads the real subdir allowlist instead of hardcoding ['lib']. Co-Authored-By: Claude Opus 4.8 * chore(#606): add changeset crediting installer registry fix Fixed-type fragment so the #606 installer fix (managed-hooks-registry.cjs now shipped) is credited in the release notes. The behavioral change landed in #611; this records it for the changelog and credits the reporter. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/fierce-finches-munch.md | 5 ++++ scripts/build-hooks.js | 2 +- tests/orphaned-hooks.test.cjs | 44 +++++++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 .changeset/fierce-finches-munch.md diff --git a/.changeset/fierce-finches-munch.md b/.changeset/fierce-finches-munch.md new file mode 100644 index 000000000..d6e9a8b16 --- /dev/null +++ b/.changeset/fierce-finches-munch.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 613 +--- +**Fresh installs now ship `managed-hooks-registry.cjs` next to `gsd-check-update-worker.js`, so the background update checker no longer crashes silently (#606)** — the worker `require()`s that sibling, but it had been missing from the hooks copy allowlist, so on a clean install the worker threw `Cannot find module` in the background and the update cache was never written. The file is now in the allowlist, and a regression guard asserts that every same-directory `require()` target of a shipped hook is itself shipped. Thanks @baksohyeon for the clean-room reproduction. diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js index 550b4a053..3072a5e50 100644 --- a/scripts/build-hooks.js +++ b/scripts/build-hooks.js @@ -237,4 +237,4 @@ if (require.main === module) { build(); } -module.exports = { HOOKS_TO_COPY }; +module.exports = { HOOKS_TO_COPY, HOOKS_SUBDIRS_TO_COPY }; diff --git a/tests/orphaned-hooks.test.cjs b/tests/orphaned-hooks.test.cjs index 4bd41d4a3..277e678af 100644 --- a/tests/orphaned-hooks.test.cjs +++ b/tests/orphaned-hooks.test.cjs @@ -26,7 +26,7 @@ const HOOKS_DIR = path.join(__dirname, '..', 'hooks'); // Typed imports — no source-grep needed (#455) const { MANAGED_HOOKS } = require(path.join(HOOKS_DIR, 'managed-hooks-registry.cjs')); -const { HOOKS_TO_COPY } = require(path.join(__dirname, '..', 'scripts', 'build-hooks.js')); +const { HOOKS_TO_COPY, HOOKS_SUBDIRS_TO_COPY } = require(path.join(__dirname, '..', 'scripts', 'build-hooks.js')); describe('orphaned hooks stale detection (#1750)', () => { test('MANAGED_HOOKS is an array and does not use a broad gsd-* wildcard', () => { @@ -111,4 +111,46 @@ describe('orphaned hooks stale detection (#1750)', () => { ); } }); + + test('every same-dir require() target of a shipped hook is itself shipped (#606)', () => { + // Regression guard for #606: gsd-check-update-worker.js does + // require('./managed-hooks-registry.cjs'), but that sibling was missing from + // HOOKS_TO_COPY, so the installer never placed it next to the worker and the + // background worker crashed at runtime with "Cannot find module". The fix added + // the file to HOOKS_TO_COPY; this guard fails if any shipped hook ever again + // requires a same-directory file that the installer would not ship. + // + // Scope: only *same-directory* relative requires — require('./x') and + // require('./subdir/x'). A require('../...') target reaches out of the hooks/ + // dir into sibling package dirs (e.g. get-shit-done/) whose shipping is governed + // by package.json "files", not by this allowlist, so it is out of scope here. + assert.ok(Array.isArray(HOOKS_SUBDIRS_TO_COPY), 'HOOKS_SUBDIRS_TO_COPY must be exported as an array'); + + const SAME_DIR_REQUIRE = /require\(\s*['"](\.\/[^'"]+)['"]\s*\)/g; + const jsHooks = HOOKS_TO_COPY.filter(h => /\.c?js$/.test(h)); + assert.ok(jsHooks.length >= 5, `expected at least 5 JS/CJS hooks in HOOKS_TO_COPY, got ${jsHooks.length}`); + + for (const hook of jsHooks) { + const source = fs.readFileSync(path.join(HOOKS_DIR, hook), 'utf8'); + for (const match of source.matchAll(SAME_DIR_REQUIRE)) { + const rel = match[1].slice(2); // strip leading './' + const slash = rel.indexOf('/'); + if (slash === -1) { + assert.ok( + HOOKS_TO_COPY.includes(rel), + `${hook} requires './${rel}', but '${rel}' is not in HOOKS_TO_COPY — the installer ` + + `would not place it next to ${hook}, so the require would throw at runtime (the #606 class of bug). ` + + `Add '${rel}' to HOOKS_TO_COPY in scripts/build-hooks.js.` + ); + } else { + const subdir = rel.slice(0, slash); + assert.ok( + HOOKS_SUBDIRS_TO_COPY.includes(subdir), + `${hook} requires './${rel}', but subdirectory '${subdir}' is not in HOOKS_SUBDIRS_TO_COPY — ` + + `the installer would not ship it. Add '${subdir}' to HOOKS_SUBDIRS_TO_COPY in scripts/build-hooks.js.` + ); + } + } + } + }); });