mirror of
https://github.com/zernie/vigiles.git
synced 2026-09-14 20:53:57 +08:00
fix(hook): the subagent-delivery caveat is stale, and now a test says so (#172)
* fix(hook): the subagent-delivery caveat is stale, and now a test says so Claude Code #34692 — a PreToolUse hook not firing for a subagent's tool calls — was closed not-planned and quoted across these docs for months as a standing limit on what a gate can promise. It is fixed. Measured against a stock @anthropic-ai/claude-code@2.1.241 installed from the registry, not the container's binary: a subagent's own Bash reaches the hook and an exit-2 deny stops it, with the parent's identical command in the same run as the control and the marker files on disk as ground truth. The event also carries agent_type, naming which subagent made the call. The claim is about someone else's product, so prose cannot keep it honest — a doc cannot notice that the platform moved. src/subagent-delivery.test.ts pins both directions and goes red on a regression; removing the deny from its hook fails it on its own assertion. What did NOT change is stated at every site that was edited: a model can still route around a tool entirely, so a gate remains a strong default and is never unbypassable. The measurement's scope is stated too — headless only, with interactive sessions unmeasured and depth-2 nesting absent there. Gates run, in order: vitest (3282 passed), lint (0 errors), prettier --check, docs:check, internal:check, api:check, build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: regenerate the committed project-file types for the new test CI's generated-types:committed gate compares a fresh `generate types` run against the committed file, and adding a source file changes that list. Caught by CI rather than locally because I ran the individual commands instead of `npm run check`, which is the aggregate CI actually runs — the same shortcut this repo has already written down as a recurring miss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Vendored
+3
-1
@@ -101,7 +101,7 @@ declare module "vigiles/generated" {
|
||||
| "internal:check"
|
||||
| "docs:api";
|
||||
|
||||
/** 392 project files. */
|
||||
/** 393 project files. */
|
||||
export type ProjectFile =
|
||||
| "src/CLAUDE.md"
|
||||
| "src/CLAUDE.md.spec.ts"
|
||||
@@ -482,6 +482,7 @@ declare module "vigiles/generated" {
|
||||
| "src/skill-test.ts"
|
||||
| "src/stats.test.ts"
|
||||
| "src/stats.ts"
|
||||
| "src/subagent-delivery.test.ts"
|
||||
| "src/test-coverage-files.ts"
|
||||
| "src/test-coverage.test.ts"
|
||||
| "src/test-coverage.ts"
|
||||
@@ -946,6 +947,7 @@ declare module "vigiles/spec" {
|
||||
| "src/skill-test.ts"
|
||||
| "src/stats.test.ts"
|
||||
| "src/stats.ts"
|
||||
| "src/subagent-delivery.test.ts"
|
||||
| "src/test-coverage-files.ts"
|
||||
| "src/test-coverage.test.ts"
|
||||
| "src/test-coverage.ts"
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
+6
-2
@@ -47,8 +47,12 @@ A breaking change to any of the above is signalled with a Conventional-Commit
|
||||
internal modules under `dist/core/…` reached by deep import) may change in a
|
||||
minor release. Import from the published subpaths, not deep paths.
|
||||
- **`vigiles/hook`** (compiled hooks) is exported and usable but **not yet
|
||||
frozen** — it carries a known delivery caveat
|
||||
([#34692](https://github.com/anthropics/claude-code/issues/34692)).
|
||||
frozen**. Its long-standing delivery caveat
|
||||
([#34692](https://github.com/anthropics/claude-code/issues/34692) — a subagent's
|
||||
tool calls never reaching `PreToolUse`) is **fixed** as of Claude Code 2.1.241,
|
||||
measured on a stock install and pinned by a test that goes red if it regresses.
|
||||
A gate is still a strong default rather than an unbypassable wall, because a
|
||||
model can route around a tool entirely.
|
||||
|
||||
## Experimental — no stability promise
|
||||
|
||||
|
||||
+5
-2
@@ -329,8 +329,11 @@ cmd)`, or a **registered** provider. `compile` also discovers
|
||||
The merged block points at the `hook-runtime run-program` entrypoint (below).
|
||||
|
||||
Honest scope: this fixes the hook's authoring + logic, not the harness's
|
||||
delivery — a subagent's tool calls still bypass any PreToolUse hook
|
||||
([#34692](https://github.com/anthropics/claude-code/issues/34692)).
|
||||
delivery. The delivery floor moved —
|
||||
[#34692](https://github.com/anthropics/claude-code/issues/34692) (a subagent's
|
||||
tool calls bypassing `PreToolUse`) is fixed as of Claude Code 2.1.241. A gate is
|
||||
still a strong default rather than an unbypassable wall, because a model can
|
||||
route around a tool entirely.
|
||||
|
||||
### `hook-runtime <kind>` — runtime entrypoints (not typed by hand)
|
||||
|
||||
|
||||
@@ -417,7 +417,14 @@ The honest other side: stateful guards, broad I/O, and delivery (#34692) are NOT
|
||||
|
||||
Compiled hooks are neither free nor magic. The honest downsides:
|
||||
|
||||
- ❌ **Delivery floor — a gate is a strong default, not an unbypassable wall.** Compiling fixes a hook's _authoring_ and _logic_, **not** how the harness _delivers_ events to it. Claude Code's [#34692](https://github.com/anthropics/claude-code/issues/34692) (closed not-planned) means a `PreToolUse` hook **does not fire for a subagent's tool calls**. The model can also route around a tool entirely ([#45427](https://github.com/anthropics/claude-code/issues/45427) / [#32376](https://github.com/anthropics/claude-code/issues/32376) — e.g. a Bash heredoc instead of `Write`). A compiled hook removes the bugs that are _yours_; it can't remove the harness's. The most robust claim is about **logic**, not live enforcement. [Guardrail verification](harness-testing.md) (prove the decision blocks the disaster battery) is the companion that survives this bug. **Never call a gate "unbypassable."**
|
||||
- ❌ **Delivery floor — a gate is a strong default, not an unbypassable wall.** Compiling fixes a hook's _authoring_ and _logic_, **not** how the harness _delivers_ events to it.
|
||||
|
||||
**What changed (2026-08-24).** Claude Code's [#34692](https://github.com/anthropics/claude-code/issues/34692) — a `PreToolUse` hook not firing for a subagent's tool calls — was closed not-planned and quoted here for months as a standing limit. It is **fixed**: measured against a stock `@anthropic-ai/claude-code@2.1.241` from the registry, a subagent's own `Bash` reaches the hook and an exit-2 deny stops it (the parent's identical command in the same run is the control). The event even carries `agent_type`, naming which subagent made the call. `src/subagent-delivery.test.ts` pins this in both directions, so a regression goes red instead of quietly returning to the old behaviour.
|
||||
|
||||
**What did NOT change.** A model can still route around a tool entirely ([#45427](https://github.com/anthropics/claude-code/issues/45427) / [#32376](https://github.com/anthropics/claude-code/issues/32376) — a Bash heredoc instead of `Write`), and a compiled hook removes the bugs that are _yours_, not the harness's. The most robust claim remains about **logic**, not live enforcement, and [guardrail verification](harness-testing.md) is still the companion that survives any delivery gap. **Never call a gate "unbypassable."**
|
||||
|
||||
Scope of the measurement, stated plainly: it drives `claude -p` (headless). Interactive sessions are unmeasured, and subagent nesting (depth 2) does not occur there at all.
|
||||
|
||||
- ⚠️ **Runtime cost.** Every matching event spawns `node` and dynamic-imports your program — tens to hundreds of ms per call. Fine for a `PreToolUse` gate. Think twice before a hot-path `PostToolUse` react that fires on every edit.
|
||||
- ⚠️ **Buy-in.** It's a dependency plus a build step, and you author in JS/TS, not a 3-line inline `bash` hook. For a trivial one-liner the compiled path is heavier — the payoff is on the guards that actually have to be _correct_.
|
||||
- ⚠️ **A bounded vocabulary is a ceiling, by design** — but be precise about which bound. (1) What a hook can _do_: `checkHookImports` forbids any import but `vigiles/hook` (no `fs`/`net`/`child_process`), so a hook that must _call a service, read a file, or hold cross-invocation state_ to decide can't be expressed. That is the **deliberate** ceiling — it _is_ the safety guarantee, and such hooks stay hand-written (keep a plain shell hook and verify it with the disaster battery). (2) What a hook can _see_: the AST matchers (`runs`/`touches`/`pipesToShell`/`under`) are a **soft, extensible** limit, not a fundamental one — if you need to match a shape they don't expose yet, the fix is a new matcher, not a redesign.
|
||||
|
||||
@@ -24,9 +24,11 @@
|
||||
* OpenCode via the HookProtocol port later — OpenCode hooks ARE in-process TS).
|
||||
*
|
||||
* Pure core, harness-neutral. HONEST SCOPE (kept in every doc): compile/verify fix
|
||||
* the hook's AUTHORING + LOGIC, not DELIVERY — CC's subagent-bypass (#34692) means
|
||||
* a PreToolUse hook does not fire for a subagent's tool calls, so a gate is a strong
|
||||
* default, never an unbypassable wall. Limits (buy-in, node-startup latency) +
|
||||
* the hook's AUTHORING + LOGIC, not the harness's DELIVERY. #34692 (a subagent's
|
||||
* calls never reaching PreToolUse) is FIXED as of CC 2.1.241 — measured on a stock
|
||||
* install, pinned by src/subagent-delivery.test.ts. A gate is STILL a strong default
|
||||
* rather than an unbypassable wall, because a model can route around a tool
|
||||
* entirely (#45427 / #32376). Limits (buy-in, node-startup latency) +
|
||||
* full record in research/hook-pain-points.md.
|
||||
*/
|
||||
import {
|
||||
|
||||
@@ -14,7 +14,9 @@
|
||||
* the hook via {@link runHook} and check the normalized decision is BLOCK. No model,
|
||||
* no API key, runs in CI, works on a hand-written hook with NO vigiles spec — it
|
||||
* verifies the hook's decision LOGIC, so it sidesteps CC's runtime delivery bugs
|
||||
* (subagent-bypass #34692 etc.) which it deliberately does NOT claim to fix.
|
||||
* (the model routing around a tool entirely, #45427 / #32376) which it deliberately
|
||||
* does NOT claim to fix. (#34692, the old subagent-delivery gap, is fixed as of CC
|
||||
* 2.1.241 — see src/subagent-delivery.test.ts.)
|
||||
*
|
||||
* Pure-ish (wraps the existing runHook tier). The catalog is harness-neutral data;
|
||||
* the scaffold-test generator emits a test that calls these, and the same engine
|
||||
|
||||
+7
-5
@@ -39,11 +39,13 @@
|
||||
* inline `provide(name, cmd)` (read-only) or `dangerously(name, cmd)` (the loud
|
||||
* escape) right in `needs`. See `research/hook-context-providers.md`.
|
||||
*
|
||||
* ⚠️ Honest scope: compile/verify fix the hook's AUTHORING + LOGIC. They do NOT
|
||||
* change DELIVERY — Claude Code's own subagent-bypass (#34692) means a
|
||||
* PreToolUse hook (compiled or hand-written) does not fire for a subagent's
|
||||
* tool calls. A gate is a strong default, never an unbypassable wall. See
|
||||
* `docs/compiled-hooks.md`.
|
||||
* ⚠️ Honest scope: compile/verify fix the hook's AUTHORING + LOGIC, not the
|
||||
* harness's DELIVERY. The delivery floor MOVED — #34692 (a subagent's tool calls
|
||||
* never reaching PreToolUse) is FIXED as of Claude Code 2.1.241, measured against
|
||||
* a stock registry install and pinned by src/subagent-delivery.test.ts, which goes
|
||||
* red if it regresses. What has NOT changed: a model can still route around a tool
|
||||
* entirely (#45427 / #32376 — a Bash heredoc instead of `Write`), so a gate is a
|
||||
* strong default and is NEVER an unbypassable wall. See `docs/compiled-hooks.md`.
|
||||
*/
|
||||
export {
|
||||
// ── the six ENTRY POINTS carry the experimental marking ─────────────────────
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/**
|
||||
* Does the harness DELIVER a subagent's tool calls to `PreToolUse` — and does a
|
||||
* deny actually stop them?
|
||||
*
|
||||
* This is a claim about somebody else's product, and we had it wrong for months.
|
||||
* Claude Code issue #34692 ("PreToolUse does not fire for a subagent's tool
|
||||
* calls", closed not-planned) was quoted across our docs as a standing limit on
|
||||
* what a gate can promise. Measured 2026-08-24 against a stock
|
||||
* `@anthropic-ai/claude-code@2.1.241` installed straight from the registry: the
|
||||
* subagent's own `Bash` DOES reach the hook, and an exit-2 deny DOES stop it.
|
||||
*
|
||||
* So this file exists to keep the claim honest in BOTH directions. Prose in a
|
||||
* doc cannot notice that the platform moved; a test can. If Claude Code ever
|
||||
* reverts to the #34692 behaviour, this goes red and names what changed —
|
||||
* nobody has to remember to re-check.
|
||||
*
|
||||
* WHAT THIS DOES NOT SAY. Delivery is not invulnerability, and the surrounding
|
||||
* caveat survives untouched: a model can still route around a tool entirely
|
||||
* (#45427 / #32376 — a Bash heredoc instead of `Write`), so a gate remains a
|
||||
* strong default and is never "unbypassable". Only the delivery half changed.
|
||||
*
|
||||
* SCOPE, stated because it is the honest limit: this drives `claude -p`
|
||||
* (headless), which is what `runHarnessTest` can reach. Interactive sessions
|
||||
* are unmeasured, and subagent NESTING (depth 2) does not occur here at all —
|
||||
* an outer subagent given the spawn tool runs the work itself rather than
|
||||
* dispatching its own.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { runHarnessTest, scriptModel } from "./harness-test.js";
|
||||
import { onPathClaudeVersion } from "./dialect-drift.js";
|
||||
|
||||
/** A subagent whose whole job is one observable side effect. */
|
||||
const ECHOER = `---
|
||||
name: echoer
|
||||
description: Runs one harmless echo command and reports back.
|
||||
tools: Bash
|
||||
---
|
||||
Run \\\`echo SUBAGENT_RAN > subagent-ran.txt\\\` then reply "echoer done".
|
||||
`;
|
||||
|
||||
/**
|
||||
* A PreToolUse hook that denies ONLY a subagent's call. The discriminator is
|
||||
* `agent_id`, which the harness sets on an event originating inside a subagent
|
||||
* and omits on the parent's own calls — so the parent's IDENTICAL command is
|
||||
* the in-run control: if the parent's echo lands and the subagent's does not,
|
||||
* the block is attributable to the hook's decision, not to a broken subagent.
|
||||
*/
|
||||
const DENY_SUBAGENT_HOOK = `
|
||||
const chunks = [];
|
||||
process.stdin.on("data", (c) => chunks.push(c));
|
||||
process.stdin.on("end", () => {
|
||||
let e = {};
|
||||
try { e = JSON.parse(Buffer.concat(chunks).toString("utf-8")); } catch {}
|
||||
require("node:fs").appendFileSync(process.argv[2], JSON.stringify({
|
||||
agent_id: e.agent_id ?? null, agent_type: e.agent_type ?? null, tool: e.tool_name ?? null,
|
||||
}) + "\\n");
|
||||
if (e.agent_id) { console.error("denied: subagent tool call"); process.exit(2); }
|
||||
process.exit(0);
|
||||
});
|
||||
`;
|
||||
|
||||
describe("subagent tool calls reach PreToolUse (the #34692 claim)", () => {
|
||||
// Loud skip, never a silent pass: the alarm only means something where the
|
||||
// real binary is present.
|
||||
const gate = onPathClaudeVersion() ? it : it.skip;
|
||||
|
||||
if (!onPathClaudeVersion()) {
|
||||
it.skip("delivery check skipped — the claude binary is not on PATH", () => {
|
||||
/* gated above */
|
||||
});
|
||||
}
|
||||
|
||||
gate(
|
||||
"a subagent's Bash is delivered AND blocked, while the parent's identical Bash runs",
|
||||
async () => {
|
||||
const r = await runHarnessTest({
|
||||
files: {
|
||||
".claude/agents/echoer.md": ECHOER,
|
||||
"deny-subagent.cjs": DENY_SUBAGENT_HOOK,
|
||||
},
|
||||
settings: {
|
||||
hooks: {
|
||||
PreToolUse: [
|
||||
{
|
||||
matcher: "*",
|
||||
hooks: [
|
||||
{
|
||||
type: "command",
|
||||
command:
|
||||
"node {cwd}/deny-subagent.cjs {cwd}/hook-log.ndjson",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
model: scriptModel([
|
||||
{
|
||||
tool: "Agent",
|
||||
input: {
|
||||
subagent_type: "echoer",
|
||||
description: "run echoer",
|
||||
prompt: "Run your one Bash command now.",
|
||||
run_in_background: false,
|
||||
},
|
||||
},
|
||||
{
|
||||
tool: "Bash",
|
||||
input: { command: "echo SUBAGENT_RAN > subagent-ran.txt" },
|
||||
},
|
||||
{ text: "echoer done" },
|
||||
{
|
||||
tool: "Bash",
|
||||
input: { command: "echo PARENT_RAN > parent-ran.txt" },
|
||||
},
|
||||
{ text: "parent done" },
|
||||
]),
|
||||
prompt: "Dispatch the echoer subagent, then run your own echo.",
|
||||
allowedTools: ["Read", "Edit", "Write", "Bash", "Agent"],
|
||||
timeoutMs: 180_000,
|
||||
sandbox: false,
|
||||
});
|
||||
|
||||
const events = (r.file("hook-log.ndjson") ?? "")
|
||||
.split("\n")
|
||||
.filter(Boolean)
|
||||
.map((l) => JSON.parse(l) as Record<string, unknown>);
|
||||
|
||||
// DELIVERY — the half that #34692 said was impossible.
|
||||
const fromSubagent = events.filter((e) => e.agent_id !== null);
|
||||
expect(
|
||||
fromSubagent.map((e) => e.tool),
|
||||
"a subagent's own Bash must reach PreToolUse",
|
||||
).toContain("Bash");
|
||||
|
||||
// The event carries WHICH subagent — the field that makes a per-agent
|
||||
// contract enforceable without tracking dispatches ourselves.
|
||||
expect(fromSubagent.map((e) => e.agent_type)).toContain("echoer");
|
||||
|
||||
// ENFORCEMENT, by ground truth on disk rather than by the trace: the
|
||||
// denied command left no file, the allowed identical one did.
|
||||
expect(
|
||||
r.file("subagent-ran.txt"),
|
||||
"the denied subagent command must not have run",
|
||||
).toBeNull();
|
||||
expect(
|
||||
r.file("parent-ran.txt"),
|
||||
"the parent's identical command is the control and must have run",
|
||||
).toBe("PARENT_RAN\n");
|
||||
},
|
||||
200_000,
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user