diff --git a/.changeset/1821-kilo-zcode-dead-hooks.md b/.changeset/1821-kilo-zcode-dead-hooks.md index 2840e7640..30058bea0 100644 --- a/.changeset/1821-kilo-zcode-dead-hooks.md +++ b/.changeset/1821-kilo-zcode-dead-hooks.md @@ -2,4 +2,4 @@ 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. +**The installer no longer copies dead lifecycle hook scripts for ZCode** — it declares `hooksSurface: 'none'` and has no plugin surface, so the staged `hooks/*.js`, `hooks/*.sh`, `hooks/lib/` and the CommonJS `package.json` marker were dead weight in `~/.zcode/`. The hook-copy guards in `install.js` now exclude 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. (This fix originally excluded Kilo too, on the premise that it had no plugin surface; that premise was wrong — Kilo's native plugin spawns the staged guard hooks, exactly like OpenCode's — and #2327 reverses the Kilo half.) diff --git a/.changeset/loud-guard-hooks.md b/.changeset/loud-guard-hooks.md new file mode 100644 index 000000000..896759953 --- /dev/null +++ b/.changeset/loud-guard-hooks.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2327 +--- +**Kilo installs now stage the shared PreToolUse guard hooks the native plugin spawns** — Kilo's capability descriptor declared both a `nativePlugin` (which spawns `gsd-prompt-guard`, `gsd-read-guard`, and `gsd-worktree-path-guard` as subprocesses) and `skipSharedHooksInstall: true` (which suppressed staging those scripts into the Kilo config dir), so every guard silently no-opped on every Kilo install. The skip flag is removed (Kilo now stages the same hooks bundle as OpenCode, whose byte-identical plugin was unaffected), and the plugin's `runHook` now warns loudly — once per hook file — when a guard script is missing instead of treating the absence as a silent allow. Resolves #2305. diff --git a/.kilo/plugins/gsd-core.js b/.kilo/plugins/gsd-core.js index 039277b7b..18539b9d3 100644 --- a/.kilo/plugins/gsd-core.js +++ b/.kilo/plugins/gsd-core.js @@ -200,9 +200,23 @@ function mapToolInput(args) { * @param {string} [opts.cwd] working directory for the child * @returns {{ stdout: string, exitCode: number, timedOut: boolean }} */ +const warnedMissingHooks = new Set(); + function runHook(hookFile, payload, opts = {}) { const hookPath = path.join(HOOKS_DIR, hookFile); if (!fs.existsSync(hookPath)) { + // A missing guard script means the guard is silently NOT enforced — the + // exact failure mode of #2305 (plugin staged, hooks bundle not). Never + // break the tool call (the adapter's design contract), but never be + // silent about it either: warn loudly, once per hook file. + if (!warnedMissingHooks.has(hookFile)) { + warnedMissingHooks.add(hookFile); + console.error( + `[gsd-core] hook script missing: ${hookPath} — ${hookFile} is NOT ` + + "enforced. The GSD install may be incomplete; reinstall (or run " + + "/gsd-update) to restage the hooks/ bundle.", + ); + } return { stdout: "", exitCode: 0, timedOut: false }; } const timeout = opts.timeout ?? 8000; diff --git a/.opencode/plugins/gsd-core.js b/.opencode/plugins/gsd-core.js index 039277b7b..18539b9d3 100644 --- a/.opencode/plugins/gsd-core.js +++ b/.opencode/plugins/gsd-core.js @@ -200,9 +200,23 @@ function mapToolInput(args) { * @param {string} [opts.cwd] working directory for the child * @returns {{ stdout: string, exitCode: number, timedOut: boolean }} */ +const warnedMissingHooks = new Set(); + function runHook(hookFile, payload, opts = {}) { const hookPath = path.join(HOOKS_DIR, hookFile); if (!fs.existsSync(hookPath)) { + // A missing guard script means the guard is silently NOT enforced — the + // exact failure mode of #2305 (plugin staged, hooks bundle not). Never + // break the tool call (the adapter's design contract), but never be + // silent about it either: warn loudly, once per hook file. + if (!warnedMissingHooks.has(hookFile)) { + warnedMissingHooks.add(hookFile); + console.error( + `[gsd-core] hook script missing: ${hookPath} — ${hookFile} is NOT ` + + "enforced. The GSD install may be incomplete; reinstall (or run " + + "/gsd-update) to restage the hooks/ bundle.", + ); + } return { stdout: "", exitCode: 0, timedOut: false }; } const timeout = opts.timeout ?? 8000; diff --git a/bin/install.js b/bin/install.js index e22236a10..9fde53ced 100755 --- a/bin/install.js +++ b/bin/install.js @@ -11059,13 +11059,15 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } // Gate hooks/lib/ install on the same set of runtimes that receive hooks/. - // Codex/Copilot/Cursor/Windsurf/Trae/Cline/Kilo do not use the shared + // Codex/Copilot/Cursor/Windsurf/Trae/Cline do not use the shared // hooks/lib/ helpers (Cursor uses standalone .js hook scripts registered // via hooks.json — gated descriptor-driven via - // hostBehaviors.skipSharedHooksInstall, #2089; Cline likewise #2090; Kilo - // likewise #2093; Trae likewise #2094; Codex uses hooks.json directly; - // the others skip hooks entirely); Kilo and ZCode also skip hooks entirely - // (hooksSurface:'none' with no plugin surface — #1821). None of the + // hostBehaviors.skipSharedHooksInstall, #2089; Cline likewise #2090; + // Trae likewise #2094; Codex uses hooks.json directly; + // the others skip hooks entirely); ZCode also skips hooks entirely + // (hooksSurface:'none' with no plugin surface — #1821). Kilo is NOT + // excluded since #2305: its native plugin adapter (#2093) spawns the + // staged hooks/*.js scripts, same as OpenCode. 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. (Gating lives at the call sites @@ -11081,18 +11083,21 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { return hooksOk; } - // #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. + // #1821: ZCode declares hooksSurface:'none' AND has no plugin surface, + // so the staged hook scripts are dead weight for it — excluded 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. + // them and the CommonJS package.json marker written below. Kilo is the same + // shape since #2093 (a nativePlugin spawning the staged hooks), so it must + // NOT skip either — declaring skipSharedHooksInstall:true alongside a + // nativePlugin left every guard the plugin spawns a silent no-op (#2305). // #2089: Cursor's exclusion is now descriptor-driven via // hostBehaviors.skipSharedHooksInstall (was hardcoded !isCursor). // #2090: Cline's exclusion is likewise descriptor-driven (cline declares // skipSharedHooksInstall:true) — the redundant `&& !isCline` was removed. - // #2093: Kilo's exclusion is likewise descriptor-driven (kilo declares - // skipSharedHooksInstall:true) — the redundant `&& !isKilo` was removed. + // #2093/#2305: Kilo's former exclusion (descriptor-driven via + // skipSharedHooksInstall:true) was removed in #2305 — see above. // #2094: Trae's exclusion is likewise descriptor-driven (trae declares // skipSharedHooksInstall:true) — the redundant `&& !isTrae` was removed. // #2101: ZCode's exclusion is likewise descriptor-driven (zcode declares diff --git a/capabilities/kilo/capability.json b/capabilities/kilo/capability.json index fe8b82e58..89cb31522 100644 --- a/capabilities/kilo/capability.json +++ b/capabilities/kilo/capability.json @@ -101,8 +101,7 @@ "file": "gsd-core.js", "source": ".kilo/plugins/gsd-core.js" }, - "skipUpdateBannerCommand": true, - "skipSharedHooksInstall": true + "skipUpdateBannerCommand": true } } } diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index ea7d2de2a..29ee310ce 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -1603,8 +1603,7 @@ const capabilities = { "file": "gsd-core.js", "source": ".kilo/plugins/gsd-core.js" }, - "skipUpdateBannerCommand": true, - "skipSharedHooksInstall": true + "skipUpdateBannerCommand": true } } }, @@ -4889,8 +4888,7 @@ const runtimes = { "file": "gsd-core.js", "source": ".kilo/plugins/gsd-core.js" }, - "skipUpdateBannerCommand": true, - "skipSharedHooksInstall": true + "skipUpdateBannerCommand": true } } }, diff --git a/tests/fixtures/golden-install-parity/kilo.json b/tests/fixtures/golden-install-parity/kilo.json index 55a43199a..21081a03d 100644 --- a/tests/fixtures/golden-install-parity/kilo.json +++ b/tests/fixtures/golden-install-parity/kilo.json @@ -385,8 +385,36 @@ "gsd-core/workflows/validate-phase.md": "52b9fa8a9533b444", "gsd-core/workflows/verify-phase.md": "b321551a82bc7c2f", "gsd-core/workflows/verify-work.md": "9fc717845828c6e3", + "hooks/gsd-check-update-worker.js": "c992bbad91d0e994", + "hooks/gsd-check-update.js": "fdd77abe7ef26a2d", + "hooks/gsd-config-reload.js": "96546e0e8bb47904", + "hooks/gsd-context-monitor.js": "6578822a701b2a94", + "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", + "hooks/gsd-cursor-pre-tool.js": "873998b25e308c29", + "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", + "hooks/gsd-cursor-stop.js": "bfaaf60f419e3238", + "hooks/gsd-cursor-subagent-start.js": "06d77fde5c1372b6", + "hooks/gsd-cursor-subagent-stop.js": "4bbf22917da4d389", + "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": "69a706d0d86f0c8d", + "hooks/gsd-update-banner.js": "55143a25f978f301", + "hooks/gsd-validate-commit.sh": "bf5dd61d33cb3a38", + "hooks/gsd-windsurf-pre-command.js": "948be1c6d14c79cd", + "hooks/gsd-windsurf-pre-write.js": "92d4dbfbc36ab0cf", + "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": "b63a879b8b3436bf", "kilo.json": "13151e97ff23c1aa", - "plugins/gsd-core.js": "931ca839dc9eb7f1", + "package.json": "dbf8353f77358bc1", + "plugins/gsd-core.js": "eb1ac14ffd7abfae", "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/opencode.json b/tests/fixtures/golden-install-parity/opencode.json index 9a6fb216b..b93a4abf8 100644 --- a/tests/fixtures/golden-install-parity/opencode.json +++ b/tests/fixtures/golden-install-parity/opencode.json @@ -414,7 +414,7 @@ "hooks/managed-hooks-registry.cjs": "bd57cc72f482a14f", "opencode.json": "2c12c446a88f2f36", "package.json": "dbf8353f77358bc1", - "plugins/gsd-core.js": "931ca839dc9eb7f1", + "plugins/gsd-core.js": "eb1ac14ffd7abfae", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", "scripts/changeset/github-release-notes.cjs": "795677f0c009b132", diff --git a/tests/fixtures/install-tree/kilo.json b/tests/fixtures/install-tree/kilo.json index 01d454ef8..3d19558cd 100644 --- a/tests/fixtures/install-tree/kilo.json +++ b/tests/fixtures/install-tree/kilo.json @@ -385,7 +385,35 @@ "gsd-core/workflows/validate-phase.md", "gsd-core/workflows/verify-phase.md", "gsd-core/workflows/verify-work.md", + "hooks/gsd-check-update-worker.js", + "hooks/gsd-check-update.js", + "hooks/gsd-config-reload.js", + "hooks/gsd-context-monitor.js", + "hooks/gsd-cursor-post-tool.js", + "hooks/gsd-cursor-pre-tool.js", + "hooks/gsd-cursor-session-start.js", + "hooks/gsd-cursor-stop.js", + "hooks/gsd-cursor-subagent-start.js", + "hooks/gsd-cursor-subagent-stop.js", + "hooks/gsd-ensure-canonical-path.js", + "hooks/gsd-graphify-update.sh", + "hooks/gsd-phase-boundary.sh", + "hooks/gsd-prompt-guard.js", + "hooks/gsd-read-guard.js", + "hooks/gsd-read-injection-scanner.js", + "hooks/gsd-session-state.sh", + "hooks/gsd-statusline.js", + "hooks/gsd-update-banner.js", + "hooks/gsd-validate-commit.sh", + "hooks/gsd-windsurf-pre-command.js", + "hooks/gsd-windsurf-pre-write.js", + "hooks/gsd-workflow-guard.js", + "hooks/gsd-worktree-path-guard.js", + "hooks/lib/git-cmd.js", + "hooks/lib/gsd-graphify-rebuild.sh", + "hooks/managed-hooks-registry.cjs", "kilo.json", + "package.json", "plugins/gsd-core.js", "scripts/changeset/README.md", "scripts/changeset/cli.cjs", diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index 73d1dea3e..cccc896e0 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -671,19 +671,23 @@ describe('#1755: .sh hooks are copied and executable after install', () => { }); }); -// ─── #1821: Kilo/ZCode (hooksSurface:none, no plugin) receive no dead hooks ──── +// ─── #1821/#2305: hooks staged iff a surface consumes them ───────────────────── // // #1821 reported dead hook scripts staged for runtimes with hooksSurface:'none'. -// OpenCode and pi ALSO declare hooksSurface:'none', but each has a native plugin -// adapter that spawns the staged hooks/*.js scripts as subprocesses (OpenCode's -// #1914 plugins/gsd-core.js via OpenCode's event bus; pi's #2102 Stage 2 -// pi/gsd.cjs → extensions/gsd.cjs via pi.on(...) bridges) — so for both, the -// hooks are LIVE and must keep being copied. Kilo and ZCode have no plugin -// surface at all, 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/pi (and Claude) still do. +// OpenCode, pi — and, corrected by #2305, Kilo — ALSO declare +// hooksSurface:'none', but each has a native plugin adapter that spawns the +// staged hooks/*.js scripts as subprocesses (OpenCode's #1914 +// plugins/gsd-core.js via OpenCode's event bus; pi's #2102 Stage 2 pi/gsd.cjs +// → extensions/gsd.cjs via pi.on(...) bridges; Kilo's plugins/gsd-core.js, +// byte-identical to OpenCode's) — so for all three, the hooks are LIVE and +// must keep being copied. ZCode has no plugin surface at all, so its staged +// hooks are genuinely dead: that is the case #1821's fix removes. (#1821 +// originally excluded Kilo too, on the false premise that it had no plugin +// surface — #2305 reversed that: the skip flag silently no-opped every guard +// hook Kilo's plugin spawns.) These tests assert the split: ZCode gets no +// hooks; Kilo/OpenCode/pi (and Claude) do. -describe('#1821: Kilo/ZCode receive no dead hook files; OpenCode/Claude keep their hooks', () => { +describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude keep their hooks', () => { function gsdHookFilesUnder(configDir) { const hooksDir = path.join(configDir, 'hooks'); if (!fs.existsSync(hooksDir)) return []; @@ -716,10 +720,11 @@ describe('#1821: Kilo/ZCode receive no dead hook files; OpenCode/Claude keep the } } - // Kilo and ZCode both declare hooksSurface:'none' with no plugin surface, so - // their staged hooks are genuinely dead weight (#1821) — this is the case - // the fix removes. - for (const runtime of ['kilo', 'zcode']) { + // ZCode declares hooksSurface:'none' with no plugin surface, so its staged + // hooks are genuinely dead weight (#1821) — this is the case the fix + // removes. (Kilo was originally in this loop; #2305 moved it to the + // OpenCode group below — its native plugin spawns the staged guard hooks.) + for (const runtime of ['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(', ')}`); @@ -727,6 +732,26 @@ describe('#1821: Kilo/ZCode receive no dead hook files; OpenCode/Claude keep the }); } + // #2305: Kilo's native plugin (plugins/gsd-core.js, byte-identical to + // OpenCode's) spawns the staged PreToolUse guard hooks as subprocesses, so + // Kilo must receive the shared hooks bundle as a sibling of gsd-core/ — + // the shape the plugin's resolveRepoRoot walk requires. #1821 excluded + // Kilo on the false premise that it had no plugin surface; with the skip + // flag set, every guard silently no-opped on every Kilo install. + test('kilo --global install stages the guard hooks its plugin spawns, hooks/lib, and the plugin', () => { + const { hookFiles, hooksLibExists, gitCmdExists, pluginExists } = installAndCollect('kilo'); + const basenames = hookFiles.map((f) => path.basename(f)); + for (const expected of ['gsd-prompt-guard.js', 'gsd-read-guard.js', 'gsd-worktree-path-guard.js']) { + assert.ok( + basenames.includes(expected), + `kilo install must copy ${expected} (spawned by the plugin's runHook), found: ${basenames.join(', ')}`, + ); + } + assert.ok(hooksLibExists, 'kilo install must create hooks/lib/'); + assert.ok(gitCmdExists, 'kilo install must copy hooks/lib/git-cmd.js (required by the shared hooks)'); + assert.ok(pluginExists, 'kilo install must install plugins/gsd-core.js (the hook-spawning plugin)'); + }); + // 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. diff --git a/tests/installer-migration-install.integration.test.cjs b/tests/installer-migration-install.integration.test.cjs index 0428bbf40..57667c291 100644 --- a/tests/installer-migration-install.integration.test.cjs +++ b/tests/installer-migration-install.integration.test.cjs @@ -35,9 +35,10 @@ 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 }, - // #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 }, + // #2305: Kilo's native plugin spawns the staged guard hooks, so it receives + // the shared hooks bundle + the CommonJS package.json marker, like OpenCode. + // (#1821 excluded Kilo on the false premise that it had no plugin surface.) + kilo: { surface: 'flat-command', settings: false, packageJson: true }, // #2329: OpenCode discovers commands from the PLURAL `commands/` dir — the // singular `command/` (still correct for Kilo) made all /gsd-* commands // invisible to OpenCode. commandDirName overrides the flat-command default. @@ -52,9 +53,9 @@ const RUNTIME_INSTALL_CONTRACTS = { // compact bridges) and its /gsd tokenizer requires hooks/lib/git-cmd.js, so // `hostBehaviors.skipSharedHooksInstall` was removed — pi now receives // hooks/ + hooks/lib/ + the {"type":"commonjs"} package.json marker, exactly - // like OpenCode (architecturally identical: hooksSurface:'none' + a native - // plugin that spawns the staged hooks), NOT like Kilo/ZCode (no plugin - // surface, where the same hooks are genuinely dead weight). + // like OpenCode and (since #2305) Kilo — architecturally identical: + // hooksSurface:'none' + a native plugin that spawns the staged hooks — NOT + // like ZCode (no plugin surface, where the same hooks are dead weight). pi: { surface: 'plugin-only', settings: false, packageJson: true }, qwen: { surface: 'flat-skills', settings: true, packageJson: true }, trae: { surface: 'flat-skills', settings: false, packageJson: false }, diff --git a/tests/kilo-imperative-reference.test.cjs b/tests/kilo-imperative-reference.test.cjs index 3f6da94f9..4d6410576 100644 --- a/tests/kilo-imperative-reference.test.cjs +++ b/tests/kilo-imperative-reference.test.cjs @@ -131,7 +131,11 @@ test('kilo descriptor declares runtime.hostBehaviors (the folded-in behaviors)', assert.equal(hb.combinedFamilyInstall, true); assert.equal(hb.frontmatterDialect, 'kilo'); assert.equal(hb.skipUpdateBannerCommand, true); - assert.equal(hb.skipSharedHooksInstall, true); + // #2305: Kilo must NOT declare skipSharedHooksInstall — its nativePlugin + // (below) spawns the shared hooks/*.js guard scripts, so the install has to + // stage them (same shape as OpenCode). Declaring both was the descriptor + // contradiction that silently no-opped all four PreToolUse guards on Kilo. + assert.equal(hb.skipSharedHooksInstall, undefined); assert.ok(hb.nativePlugin && typeof hb.nativePlugin === 'object'); assert.equal(hb.nativePlugin.dir, 'plugins'); assert.equal(hb.nativePlugin.file, 'gsd-core.js'); diff --git a/tests/kilo-upgrades.test.cjs b/tests/kilo-upgrades.test.cjs index fa1e182ad..c96a79bcf 100644 --- a/tests/kilo-upgrades.test.cjs +++ b/tests/kilo-upgrades.test.cjs @@ -26,13 +26,14 @@ * block — the slug Kilo's Task tool dispatches by. */ -const { test } = require('node:test'); +const { test, before } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); +const os = require('node:os'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); -const { runMinimalInstall } = require('./helpers/install-shared.cjs'); +const { runMinimalInstall, BUILD_SCRIPT } = require('./helpers/install-shared.cjs'); const { cleanup } = require('./helpers.cjs'); const { listAgentFiles } = require('./helpers/agent-roster.cjs'); const { convertClaudeToKiloFrontmatter } = require('../bin/install.js'); @@ -293,3 +294,127 @@ test('capabilities/kilo/capability.json extendedHookEvents is exactly [] (hooksS assert.equal(KILO_CAP.runtime.hooksSurface, 'none'); assert.equal(KILO_CAP.runtime.hostIntegration.dispatch.subagentToolkit, 'undocumented'); }); + +// --------------------------------------------------------------------------- +// #2305: the shared guard hooks Kilo's native plugin spawns must be STAGED. +// +// Kilo's capability descriptor used to declare BOTH hostBehaviors.nativePlugin +// (a plugin that spawns the shared PreToolUse guard scripts as subprocesses) +// AND hostBehaviors.skipSharedHooksInstall:true (which suppresses staging of +// hooks/*.js into the config dir). The plugin's runHook treats an absent hook +// script as a silent allow, so every guard it spawned no-opped on a normal +// Kilo install. OpenCode (same plugin, hooks staged) is the reference shape. +// --------------------------------------------------------------------------- + +// hooks/dist is gitignored and built; the scoped CI lane does not run +// build:hooks, so a real install there would stage no hooks/ dir. Build it +// idempotently (mirrors golden-install-parity + install-minimal-hooks). +before(() => { + const build = spawnSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf8' }); + assert.equal(build.status, 0, `build:hooks failed: ${build.stderr}`); +}); + +// The three PreToolUse guards the plugin spawns that ship today. When a new +// guard lands on the plugin's dispatch path, add it here. +const PLUGIN_GUARD_HOOKS = [ + 'gsd-prompt-guard.js', + 'gsd-read-guard.js', + 'gsd-worktree-path-guard.js', + 'gsd-workflow-guard.js', +]; + +for (const scope of ['global', 'local']) { + test(`kilo --${scope}: stages the guard hook scripts where the native plugin resolves them (#2305)`, (t) => { + const { manifest, configDir, root } = runMinimalInstall({ runtime: 'kilo', scope }); + t.after(() => cleanup(root)); + + // The shared hooks bundle lands in the config dir, next to gsd-core/. + for (const hook of PLUGIN_GUARD_HOOKS) { + const hookPath = path.join(configDir, 'hooks', hook); + assert.ok(fs.existsSync(hookPath), `${hookPath} must be staged by the install`); + } + // The CommonJS marker installSharedHooksBundle writes alongside hooks/. + const marker = path.join(configDir, 'package.json'); + assert.ok(fs.existsSync(marker), 'CommonJS package.json marker must be staged'); + assert.equal(JSON.parse(fs.readFileSync(marker, 'utf8')).type, 'commonjs'); + + // Staged hooks are tracked in the manifest (drift/uninstall accounting). + assert.ok(manifest && manifest.files['hooks/gsd-prompt-guard.js'], + 'manifest must track the staged guard hooks'); + + // The installed plugin's own walk-up resolution (hooks/ + gsd-core/ both + // present) lands on the config dir — i.e. HOOKS_DIR points at the staged + // scripts, closing the resolveRepoRoot fallback miss from #2305. + const installedPlugin = path.join(configDir, 'plugins', 'gsd-core.js'); + assert.ok(fs.existsSync(installedPlugin), 'native plugin must be staged'); + delete require.cache[require.resolve(installedPlugin)]; + const mod = require(installedPlugin); + assert.equal(mod.server._internals.REPO_ROOT, fs.realpathSync(configDir), + 'plugin REPO_ROOT must resolve to the config dir (hooks/ + gsd-core/ siblings)'); + }); +} + +test('kilo: a disallowed write through the REAL installed tree is rejected by the worktree-path guard (#2305)', async (t) => { + const { configDir, root } = runMinimalInstall({ runtime: 'kilo', scope: 'global' }); + t.after(() => cleanup(root)); + + // Build a GSD-shaped executor worktree: gsd-worktree-path-guard hard-blocks + // only when cwd is a linked worktree on a worktree-agent-* branch and the + // write targets an absolute path outside that worktree's toplevel. + const scratch = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-kilo-2305-'))); + t.after(() => cleanup(scratch)); + const mainRepo = path.join(scratch, 'main'); + fs.mkdirSync(mainRepo, { recursive: true }); + const git = (args, cwd) => { + const r = spawnSync('git', ['-c', 'user.email=t@t', '-c', 'user.name=t', ...args], { cwd, encoding: 'utf8' }); + assert.equal(r.status, 0, `git ${args.join(' ')} failed: ${r.stderr}`); + return r; + }; + git(['init', '-q'], mainRepo); + fs.writeFileSync(path.join(mainRepo, 'seed.md'), 'seed'); + git(['add', 'seed.md'], mainRepo); + git(['commit', '-q', '-m', 'seed'], mainRepo); + const wt = path.join(scratch, 'wt'); + git(['worktree', 'add', '-q', '-b', 'worktree-agent-2305', wt], mainRepo); + + // Load the plugin exactly as installed and pin its cwd to the worktree. + const installedPlugin = path.join(configDir, 'plugins', 'gsd-core.js'); + delete require.cache[require.resolve(installedPlugin)]; + const mod = require(installedPlugin); + const handlers = await mod.server({ directory: wt }); + + // A write escaping the worktree back into the main repo must be BLOCKED — + // pre-#2305 no hook script was staged, so this silently resolved (allow). + await assert.rejects( + () => handlers['tool.execute.before']( + { tool: 'write' }, + { args: { filePath: path.join(mainRepo, 'escape.md'), content: 'x' } }, + ), + /./, + 'guard must reject the out-of-worktree write through the installed Kilo tree', + ); + + // Control: the same write kept inside the worktree passes. + await handlers['tool.execute.before']( + { tool: 'write' }, + { args: { filePath: path.join(wt, 'inside.md'), content: 'x' } }, + ); +}); + +// Regression guard for the descriptor-contradiction CLASS, not just Kilo: a +// runtime whose nativePlugin spawns the shared hooks while its descriptor +// suppresses staging them re-creates #2305 for that runtime. +test('no capability declares BOTH hostBehaviors.nativePlugin and skipSharedHooksInstall:true (#2305)', () => { + const capsDir = path.join(__dirname, '..', 'capabilities'); + for (const entry of fs.readdirSync(capsDir)) { + const capPath = path.join(capsDir, entry, 'capability.json'); + if (!fs.existsSync(capPath)) continue; + const cap = JSON.parse(fs.readFileSync(capPath, 'utf8')); + const hb = cap.runtime && cap.runtime.hostBehaviors; + if (!hb || !hb.nativePlugin) continue; + assert.notEqual(hb.skipSharedHooksInstall, true, + `${entry}: declares a nativePlugin (which spawns the shared hooks) while ` + + 'also declaring skipSharedHooksInstall:true — the hooks it depends on ' + + 'would never be staged (#2305)'); + } +}); diff --git a/tests/opencode-plugin-adapter.test.cjs b/tests/opencode-plugin-adapter.test.cjs index 7bef3e940..623ff6b75 100644 --- a/tests/opencode-plugin-adapter.test.cjs +++ b/tests/opencode-plugin-adapter.test.cjs @@ -15,7 +15,9 @@ * * Cross-platform note: filesystem-failure paths are not exercised here; the * adapter's own error handling swallows spawn failures by design (a broken hook - * must never break a tool call), which the "missing hook" case covers. + * must never break a tool call). A MISSING hook script likewise never breaks + * the tool call, but since #2305 it warns loudly (once per hook file) — a + * silently-absent guard script was exactly how every Kilo guard no-opped. */ const { test } = require('node:test'); @@ -279,9 +281,15 @@ test('tool.execute.after: Read content rewriting maps ~/.claude/gsd-core paths', ); }); -test('missing hook script is a silent allow (never breaks the tool call)', async (t) => { - // No hook stubs written at all → every runHook finds no file → silent allow. +test('missing hook script warns loudly but still allows (never breaks the tool call, #2305)', async (t) => { + // No hook stubs written at all → every runHook finds no file → allow, but + // each absent guard script must be warned about (once per hook file): a + // silently-missing guard is how #2305 no-opped every Kilo guard. const { mod } = buildInstalledLayout(t, {}); + const warnings = []; + const realConsoleError = console.error; + t.after(() => { console.error = realConsoleError; }); + console.error = (...args) => { warnings.push(args.join(' ')); }; const handlers = await mod.server({ directory: process.cwd() }); await assert.doesNotReject(() => handlers['tool.execute.before']( @@ -289,6 +297,21 @@ test('missing hook script is a silent allow (never breaks the tool call)', async { args: { filePath: '/proj/a.md', old_string: 'a', new_string: 'b' } }, ), ); + const missingWarnings = warnings.filter((w) => w.includes('hook script missing')); + assert.ok(missingWarnings.length > 0, 'a missing guard script must be warned about'); + // Warn-once: driving a second identical tool call must not re-warn. + const warnedOnce = missingWarnings.length; + await assert.doesNotReject(() => + handlers['tool.execute.before']( + { tool: 'edit' }, + { args: { filePath: '/proj/a.md', old_string: 'a', new_string: 'b' } }, + ), + ); + assert.equal( + warnings.filter((w) => w.includes('hook script missing')).length, + warnedOnce, + 'the missing-hook warning fires once per hook file, not once per tool call', + ); }); test('config hook is a no-op in installed (non-package) layout', async (t) => {