From 2b806904e225641671e9f5f1f6c8e75fa2dab60b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 7 Jul 2026 08:28:15 -0400 Subject: [PATCH] fix(#1821): stop copying dead hook scripts for Kilo and ZCode (hooksSurface:none) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kilo and ZCode both declare `hooksSurface: 'none'` and have no plugin surface, so the GSD installer staged lifecycle hook scripts (hooks/*.js, hooks/*.sh, hooks/lib/) plus a `{"type":"commonjs"}` package.json marker into their config dirs where nothing ever invokes them — dead weight (#1821). The installer's two hook-copy guards at bin/install.js were still on the legacy hardcoded runtime-name list and never excluded Kilo or ZCode. Add `&& !isKilo && !isZcode` to both (isZcode added to the install() runtimeFlags destructure). OpenCode — which #1821 also named — is deliberately NOT excluded: since the issue was filed, #1914 shipped a native OpenCode plugin (plugins/gsd-core.js) that spawns those exact staged hooks via OpenCode's event bus and requires both the hook scripts and the CommonJS package.json marker. Excluding OpenCode would regress #1914, so its hooks stay live. The genuinely-dead cases are Kilo & ZCode. - bin/install.js: add `&& !isKilo && !isZcode` to the hooks/dist copy guard and the hooks/lib copy guard; document the OpenCode-vs-Kilo/ZCode split. - tests/install-minimal-hooks.test.cjs: regression test asserting Kilo and ZCode receive no gsd-*.js/.sh hooks or hooks/lib, while OpenCode keeps its hooks + #1914 plugin and Claude keeps its hooks (over-exclusion guard). - tests/fixtures/golden-install-parity/{kilo,zcode}.json: drop the 21 hooks/* entries and the package.json marker they no longer receive (opencode unchanged). Co-Authored-By: Claude Opus 4.8 --- .changeset/1821-kilo-zcode-dead-hooks.md | 5 ++ bin/install.js | 21 +++-- .../fixtures/golden-install-parity/kilo.json | 22 ------ .../fixtures/golden-install-parity/zcode.json | 22 ------ tests/install-minimal-hooks.test.cjs | 77 +++++++++++++++++++ ...ler-migration-install.integration.test.cjs | 8 +- 6 files changed, 103 insertions(+), 52 deletions(-) create mode 100644 .changeset/1821-kilo-zcode-dead-hooks.md diff --git a/.changeset/1821-kilo-zcode-dead-hooks.md b/.changeset/1821-kilo-zcode-dead-hooks.md new file mode 100644 index 000000000..2840e7640 --- /dev/null +++ b/.changeset/1821-kilo-zcode-dead-hooks.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2057 +--- +**The installer no longer copies dead lifecycle hook scripts for Kilo and ZCode** — both declare `hooksSurface: 'none'` and have no plugin surface, so the staged `hooks/*.js`, `hooks/*.sh`, `hooks/lib/` and the CommonJS `package.json` marker were dead weight in `~/.kilo/` and `~/.zcode/`. The two hook-copy guards in `install.js` now exclude Kilo and ZCode alongside the other no-hook runtimes. OpenCode, which also declares `hooksSurface: 'none'`, is deliberately kept: its native plugin adapter (#1914) spawns those staged hooks via OpenCode's event bus and needs both them and the marker. diff --git a/bin/install.js b/bin/install.js index 589ca0218..bd148d748 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8161,7 +8161,7 @@ function reportInstallerMigrationResult(result) { } function install(isGlobal, runtime = 'claude', options = {}) { - const { isOpencode, isKilo, isCodex, isCopilot, isAntigravity, isCursor, isWindsurf, isAugment, isTrae, isQwen, isHermes, isCodebuddy, isCline, isKimi } = runtimeFlags(runtime); + const { isOpencode, isKilo, isZcode, isCodex, isCopilot, isAntigravity, isCursor, isWindsurf, isAugment, isTrae, isQwen, isHermes, isCodebuddy, isCline, isKimi } = runtimeFlags(runtime); const plan = resolveInstallPlan(runtime); const dirName = getDirName(runtime); const src = path.join(__dirname, '..'); @@ -9163,7 +9163,13 @@ function install(isGlobal, runtime = 'claude', options = {}) { failures.push('VERSION'); } - if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && !isKimi) { + // #1821: Kilo and ZCode declare hooksSurface:'none' AND have no plugin surface, + // so the staged hook scripts are dead weight for them — exclude both here. + // OpenCode also declares hooksSurface:'none' but is deliberately NOT excluded: + // its native plugin adapter (#1914, installed above under plugins/gsd-core.js) + // spawns the staged hooks/*.js scripts via OpenCode's event bus and needs both + // them and the CommonJS package.json marker written below. + if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && !isKimi && !isKilo && !isZcode) { // Write package.json to force CommonJS mode for GSD scripts // Prevents "require is not defined" errors when project has "type": "module" // Node.js walks up looking for package.json - this stops inheritance from project @@ -9255,11 +9261,14 @@ function install(isGlobal, runtime = 'claude', options = {}) { // Gate hooks/lib/ install on the same runtimes that receive hooks (see line ~8702). // Codex/Copilot/Cursor/Windsurf/Trae/Cline do not use the shared hooks/lib/ helpers // (Cursor uses standalone .js hook scripts registered via hooks.json; Codex uses - // hooks.json directly; the others skip hooks entirely), so they must not receive - // the hooks/lib/ helpers — otherwise the Codex comment downstream - // ("we deliberately do *not* copy hooks/lib/ for Codex") is contradicted in practice. + // hooks.json directly; the others skip hooks entirely); Kilo and ZCode also skip + // hooks entirely (hooksSurface:'none' with no plugin surface — #1821). OpenCode + // is NOT excluded: its #1914 plugin adapter spawns the staged hooks and requires + // hooks/lib/ helpers. None of the excluded runtimes must receive the hooks/lib/ + // helpers — otherwise the Codex comment downstream ("we deliberately do *not* + // copy hooks/lib/ for Codex") is contradicted in practice. const hooksLibSrc = path.join(src, 'hooks', 'lib'); - if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && !isKimi && fs.existsSync(hooksLibSrc)) { + if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && !isKimi && !isKilo && !isZcode && fs.existsSync(hooksLibSrc)) { const hooksLibDest = path.join(targetDir, 'hooks', 'lib'); fs.mkdirSync(hooksLibDest, { recursive: true }); copyLibDir(hooksLibSrc, hooksLibDest, GSD_HOOK_LIB_FILES); diff --git a/tests/fixtures/golden-install-parity/kilo.json b/tests/fixtures/golden-install-parity/kilo.json index 7e737fb76..b44343f3c 100644 --- a/tests/fixtures/golden-install-parity/kilo.json +++ b/tests/fixtures/golden-install-parity/kilo.json @@ -379,29 +379,7 @@ "gsd-core/workflows/validate-phase.md": "557e3251e3b9349a", "gsd-core/workflows/verify-phase.md": "0d4ffabc1caa473a", "gsd-core/workflows/verify-work.md": "b68ac37f6301a3b5", - "hooks/gsd-check-update-worker.js": "c992bbad91d0e994", - "hooks/gsd-check-update.js": "fdd77abe7ef26a2d", - "hooks/gsd-config-reload.js": "96546e0e8bb47904", - "hooks/gsd-context-monitor.js": "2caaaf96d39fe742", - "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", - "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", - "hooks/gsd-ensure-canonical-path.js": "3ce09b366839d324", - "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", - "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", - "hooks/gsd-prompt-guard.js": "a749b8cb2c5248de", - "hooks/gsd-read-guard.js": "9e423cd03e2d1b16", - "hooks/gsd-read-injection-scanner.js": "7792c420f1d72f11", - "hooks/gsd-session-state.sh": "e54379ba86bf1b6d", - "hooks/gsd-statusline.js": "a055020378003805", - "hooks/gsd-update-banner.js": "55143a25f978f301", - "hooks/gsd-validate-commit.sh": "bf5dd61d33cb3a38", - "hooks/gsd-workflow-guard.js": "91ae24a15d2bca6f", - "hooks/gsd-worktree-path-guard.js": "cb8b86e39a5d49e9", - "hooks/lib/git-cmd.js": "268ba15992ca0b23", - "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "8a9f7633c6fe0ea0", "kilo.json": "13151e97ff23c1aa", - "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", "scripts/changeset/github-release-notes.cjs": "795677f0c009b132", diff --git a/tests/fixtures/golden-install-parity/zcode.json b/tests/fixtures/golden-install-parity/zcode.json index 36a418191..928e469d5 100644 --- a/tests/fixtures/golden-install-parity/zcode.json +++ b/tests/fixtures/golden-install-parity/zcode.json @@ -379,28 +379,6 @@ "gsd-core/workflows/validate-phase.md": "2c6d7671fcaabcaa", "gsd-core/workflows/verify-phase.md": "98a995046bdb6c3c", "gsd-core/workflows/verify-work.md": "34e980a6950cd83c", - "hooks/gsd-check-update-worker.js": "6660bcf03ed849a0", - "hooks/gsd-check-update.js": "901ea3bd75fd5bbc", - "hooks/gsd-config-reload.js": "96546e0e8bb47904", - "hooks/gsd-context-monitor.js": "9f969c172d1614a9", - "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", - "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", - "hooks/gsd-ensure-canonical-path.js": "84e6b584da9805c4", - "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", - "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", - "hooks/gsd-prompt-guard.js": "a749b8cb2c5248de", - "hooks/gsd-read-guard.js": "9e423cd03e2d1b16", - "hooks/gsd-read-injection-scanner.js": "7d0344c1ef6dd0ab", - "hooks/gsd-session-state.sh": "e54379ba86bf1b6d", - "hooks/gsd-statusline.js": "c1056568acf6dfd2", - "hooks/gsd-update-banner.js": "55143a25f978f301", - "hooks/gsd-validate-commit.sh": "bf5dd61d33cb3a38", - "hooks/gsd-workflow-guard.js": "91ae24a15d2bca6f", - "hooks/gsd-worktree-path-guard.js": "635cbdc5cde5277b", - "hooks/lib/git-cmd.js": "268ba15992ca0b23", - "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "49f1e3ea9e332097", - "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", "scripts/changeset/github-release-notes.cjs": "795677f0c009b132", diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index a5be6f0c3..a94ec8d6c 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -54,6 +54,7 @@ const { EXPECTED_SH_HOOKS, EXPECTED_ALL_HOOKS, SKILL_RUNTIMES, + walk, simulateHookCopy, installerEnv, runMinimalInstall, @@ -640,6 +641,82 @@ describe('#1755: .sh hooks are copied and executable after install', () => { }); }); +// ─── #1821: Kilo/ZCode (hooksSurface:none, no plugin) receive no dead hooks ──── +// +// #1821 reported dead hook scripts staged for runtimes with hooksSurface:'none'. +// OpenCode ALSO declares hooksSurface:'none', but its #1914 native plugin adapter +// (plugins/gsd-core.js) spawns the staged hooks/*.js via OpenCode's event bus — +// so for OpenCode the hooks are LIVE and must keep being copied. Kilo and ZCode +// have no plugin surface, so their staged hooks are genuinely dead: this is the +// case the fix removes. These tests assert the split: Kilo/ZCode get no hooks; +// OpenCode (and Claude) still do. + +describe('#1821: Kilo/ZCode receive no dead hook files; OpenCode/Claude keep their hooks', () => { + function gsdHookFilesUnder(configDir) { + const hooksDir = path.join(configDir, 'hooks'); + if (!fs.existsSync(hooksDir)) return []; + return walk(hooksDir).filter((f) => { + const base = path.basename(f); + return /^gsd-.*\.(js|sh)$/.test(base); + }); + } + + function installAndCollect(runtime) { + const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-1821-${runtime}-`)); + try { + const result = spawnSync( + process.execPath, + [INSTALL_SCRIPT, `--${runtime}`, '--global', '--config-dir', targetDir], + { encoding: 'utf8', env: installerEnv() }, + ); + assert.strictEqual(result.status, 0, + `installer exited with status ${result.status} for --${runtime} --global\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); + // Collect results while targetDir still exists — cleanup() below removes it. + return { + hookFiles: gsdHookFilesUnder(targetDir), + hooksLibExists: fs.existsSync(path.join(targetDir, 'hooks', 'lib')), + pluginExists: fs.existsSync(path.join(targetDir, 'plugins', 'gsd-core.js')), + }; + } finally { + cleanup(targetDir); + } + } + + // Kilo and ZCode both declare hooksSurface:'none' with no plugin surface, so + // their staged hooks are dead weight (#1821). + for (const runtime of ['kilo', 'zcode']) { + test(`${runtime} --global install creates no gsd-*.js/.sh hook files or hooks/lib`, () => { + const { hookFiles, hooksLibExists } = installAndCollect(runtime); + assert.deepStrictEqual(hookFiles, [], `${runtime} install must not copy any gsd-*.js/.sh hook files, found: ${hookFiles.join(', ')}`); + assert.ok(!hooksLibExists, `${runtime} install must not create hooks/lib/`); + }); + } + + // Regression guard for #1914: OpenCode's plugin adapter spawns the staged + // hooks, so excluding OpenCode from the hook copy would break it. OpenCode + // must KEEP its hooks and receive the plugin. + test('opencode --global install still copies hooks and installs the #1914 plugin', () => { + const { hookFiles, pluginExists } = installAndCollect('opencode'); + const basenames = hookFiles.map((f) => path.basename(f)); + assert.ok( + basenames.includes('gsd-context-monitor.js'), + `opencode install must still copy gsd-*.js hooks (spawned by the #1914 plugin), found: ${basenames.join(', ')}`, + ); + assert.ok(pluginExists, 'opencode install must install plugins/gsd-core.js (#1914 hook bridge)'); + }); + + // Positive control: guards against over-exclusion breaking runtimes that + // legitimately need hooks (hooksSurface !== 'none'). + test('claude --global install still copies gsd-*.js hooks', () => { + const { hookFiles } = installAndCollect('claude'); + const basenames = hookFiles.map((f) => path.basename(f)); + assert.ok( + basenames.includes('gsd-context-monitor.js'), + `claude install must still copy gsd-context-monitor.js, found: ${basenames.join(', ')}`, + ); + }); +}); + // Migrated (#455): uses typed export GSD_UNINSTALL_HOOKS instead of // source-grep assertions on bin/install.js for the uninstall hook list tests. describe('install.js uninstall hooks registry (typed assertions)', () => { diff --git a/tests/installer-migration-install.integration.test.cjs b/tests/installer-migration-install.integration.test.cjs index 92b3dafd2..220cd63b1 100644 --- a/tests/installer-migration-install.integration.test.cjs +++ b/tests/installer-migration-install.integration.test.cjs @@ -35,12 +35,16 @@ const RUNTIME_INSTALL_CONTRACTS = { gemini: { surface: 'commands-gsd', settings: true, packageJson: true }, hermes: { surface: 'hermes-skills', settings: true, packageJson: true }, kimi: { surface: 'kimi-skills-agents', settings: false, packageJson: false }, - kilo: { surface: 'flat-command', settings: false, packageJson: true }, + // #1821: Kilo (hooksSurface:none, no plugin surface) no longer receives the + // dead hook scripts or the CommonJS package.json marker. + kilo: { surface: 'flat-command', settings: false, packageJson: false }, opencode: { surface: 'flat-command', settings: true, packageJson: true }, qwen: { surface: 'flat-skills', settings: true, packageJson: true }, trae: { surface: 'flat-skills', settings: false, packageJson: false }, windsurf: { surface: 'global-artifacts-noop', settings: false, packageJson: false }, - zcode: { surface: 'flat-skills', settings: false, packageJson: true }, + // #1821: ZCode (hooksSurface:none, no plugin surface) no longer receives the + // dead hook scripts or the CommonJS package.json marker. + zcode: { surface: 'flat-skills', settings: false, packageJson: false }, }; function sha256(content) {