* fix(#2305): stage shared guard hooks for Kilo — drop skipSharedHooksInstall Kilo's capability descriptor declared 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 Kilo config dir. The plugin's runHook treats an absent hook script as a silent allow, so every guard it spawned (gsd-prompt-guard, gsd-read-guard, gsd-worktree-path-guard) no-opped on every Kilo install. OpenCode uses the byte-identical plugin with hook staging on and is unaffected — it is the reference shape. The skip flag predates Kilo's plugin surface: it dates to #1821 (hooks were dead weight for a runtime with no hook consumer), and #2093 added the hooks-dependent nativePlugin without revisiting it. - capabilities/kilo/capability.json: remove skipSharedHooksInstall (regenerated gsd-core/bin/lib/capability-registry.cjs accordingly) - bin/install.js: correct the stale #1821 comments claiming Kilo has no plugin surface - tests/kilo-upgrades.test.cjs: install-fixture tests (global + local) asserting the guard scripts land where the plugin's walk-up resolves them; an end-to-end test driving a disallowed out-of-worktree write through the REAL installed Kilo tree and asserting the guard rejects it; a cross-runtime descriptor invariant (nativePlugin and skipSharedHooksInstall:true must never coexist) - tests/kilo-imperative-reference.test.cjs: flip the pinned assertion - golden fixtures regenerated (kilo now stages the 24 hook files, same set as OpenCode) Fixes #2305 * fix(#2305): warn loudly when a guard hook script is missing (runHook) runHook's absent-file branch returned a silent exit-0 allow — the mechanism that let #2305 ship undetected: with the hooks bundle never staged on Kilo, every PreToolUse guard the plugin spawned resolved to "file not found → allow" with zero signal anywhere. Keep the adapter's design contract (a missing hook must never break the tool call — pinned by the existing adapter test) but make the absence loud: console.error once per hook file, naming the unresolved path and the remediation. Applied identically to .kilo/ and .opencode/ plugin copies (byte-parity guard). Golden parity fixtures regenerated (the installed plugin file's hash changed). Fixes #2305 * chore(#2305): add changeset fragment * test(#2305): include gsd-workflow-guard.js in the staged-guards regression list The native plugin spawns four guards on write-like tool calls — the regression test's PLUGIN_GUARD_HOOKS list covered three. Staging itself was already asserted via the golden fixtures (the full bundle), but the named per-guard assertion should cover every guard the plugin actually dispatches. Surfaced by cross-AI review of PR #2327. * test(#2305): update the #1821 tests that encoded Kilo's false no-plugin premise The #1821 hook-copy test asserted Kilo must receive no staged hooks — the exact behavior this PR reverses (and the cause of all 8 CI failures). Kilo moves from the ZCode "no dead hooks" loop to the OpenCode group, with positive assertions on the new contract: the three guard hooks the plugin spawns, hooks/lib/git-cmd.js, and plugins/gsd-core.js all staged. The integration runtime contract flips kilo packageJson to true (the CommonJS marker ships with the bundle), and the pi contract comment no longer cites Kilo as a no-plugin runtime. * chore(#2305): scope the queued #1821 changeset fragment to ZCode only The fragment still claimed the installer skips hooks for Kilo — rendering both it and this PR's fragment into the same release would ship two contradictory statements about Kilo's install behavior. It now claims ZCode only and notes that #2327 reverses the Kilo half. * chore(#2305): rename changeset fragment to the generator naming convention 2305-kilo-stage-guard-hooks.md -> loud-guard-hooks.md, matching the <adjective>-<noun>-<noun> shape npm run changeset generates (review nit). --------- Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
0xdhx
@@ -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.)
|
||||
|
||||
5
.changeset/loud-guard-hooks.md
Normal file
5
.changeset/loud-guard-hooks.md
Normal file
@@ -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.
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -101,8 +101,7 @@
|
||||
"file": "gsd-core.js",
|
||||
"source": ".kilo/plugins/gsd-core.js"
|
||||
},
|
||||
"skipUpdateBannerCommand": true,
|
||||
"skipSharedHooksInstall": true
|
||||
"skipUpdateBannerCommand": true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
30
tests/fixtures/golden-install-parity/kilo.json
vendored
30
tests/fixtures/golden-install-parity/kilo.json
vendored
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
28
tests/fixtures/install-tree/kilo.json
vendored
28
tests/fixtures/install-tree/kilo.json
vendored
@@ -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",
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 },
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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)');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
Reference in New Issue
Block a user