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.` + ); + } + } + } + }); });