From d55f636f83a59924f4c02aa1dbfd7dabed53de00 Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Sat, 10 Oct 2026 23:28:56 -0700 Subject: [PATCH 01/10] Route restated existing rules to already-covered, not automate-me. Reflect was writing a new preference whenever someone reminded the agent of a skill it had already skipped. Add lesson_route.py so that signal stays an intervention for ordering, and the lesson bucket is already-covered unless no skill states the preference yet. Co-authored-by: Cursor Change-Id: Ibb2d3e90a7cf01987aeb14b71e9413d7cec536c3 --- .../llm-judge/phrases/restated-rule.json | 2 +- engine/skills/reflect/SKILL.md | 47 ++- .../skills/reflect/references/cost-audit.md | 4 +- engine/skills/reflect/references/lenses.md | 2 +- engine/skills/reflect/scripts/lesson_route.py | 389 ++++++++++++++++++ .../reflect/scripts/tests/fixtures/README.md | 2 +- .../scripts/tests/test_lesson_route.py | 139 +++++++ .../tests/test_skill_prose_grounding.py | 31 +- 8 files changed, 601 insertions(+), 15 deletions(-) create mode 100644 engine/skills/reflect/scripts/lesson_route.py create mode 100644 engine/skills/reflect/scripts/tests/test_lesson_route.py diff --git a/engine/hooks/llm-judge/phrases/restated-rule.json b/engine/hooks/llm-judge/phrases/restated-rule.json index 4d758763b..0f0acdbe1 100644 --- a/engine/hooks/llm-judge/phrases/restated-rule.json +++ b/engine/hooks/llm-judge/phrases/restated-rule.json @@ -19,5 +19,5 @@ "we don't have a rule for this yet, can you add one?", "1. yes 2. hold 3. all 4. yes. 5. how much tokens is this all costing me so far?" ], - "on_hit": "restated-rule: the person had to restate a rule or skill that already exists. Count it as an intervention; a restated rule the agent should already follow is automate-me evidence." + "on_hit": "restated-rule: the person had to restate a rule or skill that already exists. Count it as an intervention signal that orders the reflect pass. Default lesson bucket is already-covered (execution miss on an existing skill) via engine/skills/reflect/scripts/lesson_route.py — do not invent a new preference or call automate-me for this hit alone." } diff --git a/engine/skills/reflect/SKILL.md b/engine/skills/reflect/SKILL.md index a7bbf10fd..834327b6c 100644 --- a/engine/skills/reflect/SKILL.md +++ b/engine/skills/reflect/SKILL.md @@ -1,6 +1,6 @@ --- name: reflect -description: Mine a conversation transcript — and the commit history of the files it touched — for durable learnings, then route Accepted items into concrete skill/code edits via an auto-spawned worktree in the repo that owns the named skill + PR (never merge). Working-style preferences route to the sibling skill automate-me, not a task skill edit. User involvement, forced restatement, "you fucked up/messed up", or the same type of complaint twice is a FAILURE and must route to automate-me. Use when the user says reflect, after a complex multi-step task lands cleanly and the recipe is worth keeping, when the agent hit dead ends before finding a working path, or when the user corrected the agent's approach mid-task. +description: Mine a conversation transcript — and the commit history of the files it touched — for durable learnings, then route Accepted items into concrete skill/code edits via an auto-spawned worktree in the repo that owns the named skill + PR (never merge). Working-style preferences route to the sibling skill automate-me, not a task skill edit. User involvement, forced restatement, "you fucked up/messed up", or the same type of complaint twice is a FAILURE that orders the pass; lesson_route.py decides the bucket — a restated existing rule is already-covered, not a new preference. Use when the user says reflect, after a complex multi-step task lands cleanly and the recipe is worth keeping, when the agent hit dead ends before finding a working path, or when the user corrected the agent's approach mid-task. disable-model-invocation: true --- @@ -60,7 +60,7 @@ whole context. - The user said "you fucked up", "you messed up", "I told you", "you're ignoring me", or equivalent agent-blame. Treat this reflect pass as FAIL. The class is *ignored named constraint*, not the swear word. Product-blame ("the UI is messed up") is not this class. - The reply conceded the user's suspicion — "your instinct was right", "your hunch was right", "good catch" followed by a correction. Conceding means the user found what the agent's own checks did not, so the miss is the finding, not the concession. Treat as FAIL; the `wrong-check-reflect` detector fires on these shapes. - The user had to keep iterating, restate requirements, or change product direction because the agent missed something already named. FAIL, then `automate-me`. A genuine mind-change (user learned new facts, then redirected) is not failure. A forced restatement of an already-named constraint is. -- The same *type* of complaint appears in 2+ turns or 2+ sessions (repro-then-fix, UI proof before done, e2e before claiming pass, obey the named verb). That class is a bug. **Must** invoke `automate-me` — not optional, do not wait for the user to say "automate me." `token_audit.py`'s `intervention-must-automate` flag is the mechanical catch; human-message only, never tool_result / skill-injection / `/loop` polls. Run it with `--judge` so a restated existing rule is judged; without it the flag reads `unchecked`, never a clean `no`. +- The same *type* of complaint appears in 2+ turns or 2+ sessions (repro-then-fix, UI proof before done, e2e before claiming pass, obey the named verb). That class is a bug. Run `lesson_route.py` before treating it as a new preference: a restated existing rule is `already-covered` (execution miss), not `automate-me`. Only a working preference that no skill already states routes to `automate-me` — not optional once that bucket wins, do not wait for the user to say "automate me." `token_audit.py`'s `intervention-must-automate` flag is the mechanical catch that *orders* the pass; human-message only, never tool_result / skill-injection / `/loop` polls. Run it with `--judge` so a restated existing rule is judged; without it the flag reads `unchecked`, never a clean `no`. The flag does not by itself create an automate-me row. - It's been a while since the corpus-wide pass (`top_sessions.py` + this skill's lenses across the worst offenders) last ran. No fixed cadence and no cron — just periodically worth doing by hand. - The user asks *why does X keep happening* across a span of time or across machines — that's **multi-conversation mode**; read [references/corpus-scan.md](references/corpus-scan.md). - The invocation is itself an automated `reflect-ci-*` task with no human in the loop: run the step-3 sibling check unconditionally — concurrent automated dispatches produce exactly the duplicate-work burst a human would otherwise be there to notice. @@ -116,13 +116,44 @@ When the budget gate reduced step 3, synthesis must include the exact reduced status line above before findings. It must name every omitted lens, even when all required lenses returned. -- **Accepted** — real, durable, worth acting on. Apply the elimination hierarchy from step 3 before slotting a finding here as a skill edit: if a reviewer proposed a skill/rule fix but a categorical or lint/test fix was actually available, bump it to Backlog with the stronger fix named instead, or split it. +**Lesson-route gate (runs before any Accepted or automate-me row).** For each +candidate finding, run +`python3 engine/skills/reflect/scripts/lesson_route.py` with the incident +quote, the assistant reply, the target skill text (if any), whether the +transcript opened that skill, and the kind (`restated-rule` / +`preference` / durable lesson). Its stdout `bucket` wins over the model's +sort. Buckets: + +- Restated existing rule, or an admitted skip of a rule that already + exists: Rejected. Do not open preference capture. "No known prior art" + does not override. (bucket `already-covered`, for example after + "It is; I skipped it.") +- Named skill never opened in the transcript: Rejected, or tune its + description when the miss was a trigger miss. (bucket `skill-not-used`) +- When the person states how they like future sessions to behave and no + skill already says that: hand it to the preference-capture sibling skill + rather than editing a task skill. (bucket `automate-me`) +- Durable and decision-changing new lesson: Accepted. Frustration and the + intervention flag order the pass; it does not by itself create an automate-me row or select Accepted. (bucket `accepted`) + +- **Accepted** — real, durable, worth acting on, and `lesson_route.py` + returned `accepted`. Apply the elimination hierarchy from step 3 before + slotting a finding here as a skill edit: if a reviewer proposed a + skill/rule fix but a categorical or lint/test fix was actually available, + bump it to Backlog with the stronger fix named instead, or split it. - **Backlog** — real, but the right fix is higher up the hierarchy than a skill edit. Note which tier (1: categorical, 2: lint/test, 3: hook) each backlog item is. -- **Grounding gate for skill prose.** Before an Accepted item becomes skill prose, name the established principle it instantiates — author, title, year, and a checkable URL — or write "no known prior art". An incident-shaped rule with neither goes to Backlog for grounding, not to Accepted. A rule that restates one session's bug in fresh words reads as invented and drifts into an incident log; the field's own name for it (fail fast, invariant, completeness check, reconciliation) is what the skill should say. -- **Rejected** — one-offs, already covered, or too speculative. A rejection +- **Grounding gate for skill prose.** Before an Accepted item becomes skill prose, name the established principle it instantiates — author, title, year, and a checkable URL — or write "no known prior art". An incident-shaped rule with neither goes to Backlog for grounding, not to Accepted. A rule that restates one session's bug in fresh words reads as invented and drifts into an incident log; the field's own name for it (fail fast, invariant, completeness check, reconciliation) is what the skill should say. Grounding never overrides an `already-covered` lesson-route bucket. +- **Rejected** — one-offs, already covered (`lesson_route.py` bucket + `already-covered` or `skill-not-used`), or too speculative. A rejection that says already covered by X names X at file and line; a sentence a hook challenged gets evidence or a tag, never deletion. -- **Route to `automate-me`** — real, but it's about how *this user* likes to work rather than a lesson about the code or task. Don't inline these as edits to a task-specific skill; hand the finding to `automate-me`. Same-type complaints (2+ turns or 2+ sessions) and forced iteration / product-direction change after an agent miss are **mandatory** here, not optional. Invoke `automate-me` in the same turn if the user already asked to capture the preference, or name it as the first follow-up with evidence; do not wait for them to re-prompt. +- **Route to `automate-me`** — only when `lesson_route.py` returned + `automate-me`: a working preference that no existing skill states. Don't + inline these as edits to a task-specific skill; hand the finding to + `automate-me`. A restated existing rule is **not** this bucket. Invoke + `automate-me` in the same turn when that bucket wins and the user already + asked to capture the preference, or name it as the first follow-up with + evidence; do not wait for them to re-prompt. **Three required parts per finding.** Every Accepted, Backlog, and Route-to-`automate-me` finding states all three. A finding missing one is not ready to sort. @@ -144,13 +175,13 @@ Present the full Accepted / Backlog / Route-to-automate-me / Rejected list to th 3. That agent commits, opens a PR via `draft-pr` / `make-pr` (title prefix optional `[auto]` when unattended) **in the owning repo**, and **never merges** — external apply is never-merge in the owning checkout only, same as catstack. 4. Tell the user each worktree's path + branch + owning repo + that a PR is opening; they can veto in chat (“drop item 2”, “abort reflect apply”) — if the PR is not open yet, stop the agent and remove the worktree; if it is open, close/leave the PR per their instruction. -Chat approval is no longer the gate that *starts* Accepted application — the PR is. Backlog wait is for process, agents, and workers (durable automation) — not already-named execution, which dispatches immediately: Invoker if MCP/CLI is installed unless the user said not to, else a separate git worktree + PR stack. Do not auto-apply every Backlog row. Same-type / `intervention-must-automate` still invokes `automate-me` in the same turn; other Route-to-automate-me items still wait for an explicit yes. Never silently edit skills in the parent worktree while the reflect-apply worktree is the intended home for those edits. +Chat approval is no longer the gate that *starts* Accepted application — the PR is. Backlog wait is for process, agents, and workers (durable automation) — not already-named execution, which dispatches immediately: Invoker if MCP/CLI is installed unless the user said not to, else a separate git worktree + PR stack. Do not auto-apply every Backlog row. When `lesson_route.py` returned `automate-me`, invoke `automate-me` in the same turn; other Route-to-automate-me items still wait for an explicit yes. An `intervention-must-automate: yes` flag without that bucket only orders the investigation. Never silently edit skills in the parent worktree while the reflect-apply worktree is the intended home for those edits. If the session is an open product incident and synthesis already names a concrete product change, that change is the first Accepted item the worktree implements. Process hooks stay parallel backlog — do not offer only the hook or a proof plan when the named one-liner is what stops the live defect. Skip the auto-worktree only when the user said reflect-only / “don’t apply” / Accepted is empty / every Accepted item is already landed on an open PR. -If `token_audit.py` flagged `intervention-must-automate: yes`, or synthesis found the same complaint type twice, the first offered action is invoking `automate-me` (alongside any product one-liner). That is not a style note. +If `token_audit.py` flagged `intervention-must-automate: yes`, or synthesis found the same complaint type twice, run `lesson_route.py` on each candidate first. Present Rejected `already-covered` rows for restated existing rules; invoke `automate-me` only for rows whose bucket is `automate-me` (alongside any product one-liner). The flag orders the pass; it does not by itself create an automate-me row. ### 5b. Headless / session-mine mode diff --git a/engine/skills/reflect/references/cost-audit.md b/engine/skills/reflect/references/cost-audit.md index 8af99ec29..58e63892c 100644 --- a/engine/skills/reflect/references/cost-audit.md +++ b/engine/skills/reflect/references/cost-audit.md @@ -46,9 +46,9 @@ They also emit `brevity-hook-blocks`: how many replies the brevity checker itsel They also emit `brevity-follow-ups`: how many times the user had to ask for a shorter reply, counting `/diu` invocations (read from the command-name field, not matched out of prose) plus messages whose whole text is `eli5` / `eli 5`. This is the feed for judging whether the always-on brevity rule is working: a session where the user asked twice is a session where the default reply was too long twice. Codex, OMP, and Cursor transcripts carry no slash-command field, so those modes count the `eli5`-only messages and report `unchecked` when that count is zero — a zero there is not a clean count. `/diu` mentioned inside a longer sentence is not a request; only the typed command is. -They also emit `intervention-must-automate`: yes when a verbatim re-send fired, any intervention kind (`told-you`, `accusation`, `agent-blame`) appears ≥2 times, or ≥2 distinct intervention kinds appear in the session. One "I told you" is frustration only; the same class twice is FAIL and must route to `automate-me`. `/loop` polls and Stop-hook injection text are not the human complaining. +They also emit `intervention-must-automate`: yes when a verbatim re-send fired, any intervention kind (`told-you`, `accusation`, `agent-blame`) appears ≥2 times, or ≥2 distinct intervention kinds appear in the session. One "I told you" is frustration only; the same class twice is FAIL and *orders* the reflect pass. Run `lesson_route.py` before offering a lesson: a restated existing rule is `already-covered`, not a new preference. Only a `lesson_route.py` bucket of `automate-me` may invoke `automate-me`. `/loop` polls and Stop-hook injection text are not the human complaining. -Two more kinds count, both read from human messages only. `restated-after-rejection` is the person's next message after a tool call they rejected; the rejection comes from the harness's typed denial fields (`toolDenialKind: user-rejected`, `toolUseResult: User rejected tool use`), never from tool_result prose. `restated-rule` is a message that restates a rule, skill, or standing decision that already exists; its meaning is decided by the llm-judge against the `engine/hooks/llm-judge/phrases/restated-rule.json` dictionary, and only under `--judge`. Grow that dictionary from real misses (`phrase-judge`), never a regex. Without `--judge`, or when no judge runner answers, `intervention-must-automate` reports `unchecked` unless other evidence already makes it `yes`. +Two more kinds count, both read from human messages only. `restated-after-rejection` is the person's next message after a tool call they rejected; the rejection comes from the harness's typed denial fields (`toolDenialKind: user-rejected`, `toolUseResult: User rejected tool use`), never from tool_result prose. `restated-rule` is a message that restates a rule, skill, or standing decision that already exists; its meaning is decided by the llm-judge against the `engine/hooks/llm-judge/phrases/restated-rule.json` dictionary, and only under `--judge`. Grow that dictionary from real misses (`phrase-judge`), never a regex. Without `--judge`, or when no judge runner answers, `intervention-must-automate` reports `unchecked` unless other evidence already makes it `yes`. The phrase file's `on_hit` names `already-covered` as the default lesson bucket for that signal. To re-run the reality check after tuning the detector, use the shared runner `scripts/test/backtest_detector.py` from the repo root: diff --git a/engine/skills/reflect/references/lenses.md b/engine/skills/reflect/references/lenses.md index aa99bfc89..1b4d114a4 100644 --- a/engine/skills/reflect/references/lenses.md +++ b/engine/skills/reflect/references/lenses.md @@ -25,7 +25,7 @@ One message, parallel `Agent` calls (`subagent_type: general-purpose`), each giv | Cost | Given the `token_audit.py` output (not the raw transcript): which flagged items were genuinely avoidable thrash vs. required work (e.g. a Read immediately followed by an Edit on the same file is required by tool semantics, not thrash — a Read repeated with nothing changed in between is). Recommends concrete fixes: delegate a flagged lookup-only turn to a cheaper model or a fork, batch a run of small Edits into fewer passes, investigate a cache-creation spike, stop re-reading a file that didn't change. | | History | For the files this session touched (especially ones it debugged, reworked, or was corrected on), run `git log --follow -p -- ` and `git blame` on the changed lines *before* judging the session in isolation. Check whether an earlier commit — particularly one tagged `Co-Authored-By: Claude` — already introduced this exact bug, papered over the same symptom, or reworked this same area before. A session that "fixed" something we broke ourselves, or that re-solved a problem a past session already solved, is a stronger and more accountable finding than anything visible in the transcript alone — it means a skill or a fix didn't actually stick. | | Divergent | Whatever the other lenses would miss — an unconventional angle, a blind spot, a pattern that only shows up zoomed out. | -| Frustration | Every moment the user's tone spiked: all-caps runs, profanity, verbatim-repeated messages, "I told you", "I am waiting", explicit time-constraint mentions, rapid interruption/steering bursts, agent-blame ("you fucked up/messed up"). Enumerate each with its timestamp/index, trace it back to the specific agent decision(s) that provoked it, and rank the session's root causes primarily by frustration caused — tokens burned is the tiebreaker, not the metric. Feed this lens the mechanical frustration-signal output from `token_audit.py` (see cost-audit), never the raw JSONL. If `intervention-must-automate` is yes, this session is FAIL: the first offered action is `automate-me` (same-type complaint / forced restatement). Do not treat that flag as a preference ping. | +| Frustration | Every moment the user's tone spiked: all-caps runs, profanity, verbatim-repeated messages, "I told you", "I am waiting", explicit time-constraint mentions, rapid interruption/steering bursts, agent-blame ("you fucked up/messed up"). Enumerate each with its timestamp/index, trace it back to the specific agent decision(s) that provoked it, and rank the session's root causes primarily by frustration caused — tokens burned is the tiebreaker, not the metric. Feed this lens the mechanical frustration-signal output from `token_audit.py` (see cost-audit), never the raw JSONL. If `intervention-must-automate` is yes, this session is FAIL and the flag *orders* the pass: run `lesson_route.py` before offering a lesson. A restated existing rule is `already-covered` (execution miss), not a new preference. Only a `lesson_route.py` bucket of `automate-me` may invoke `automate-me`. Frustration does not select the Accepted bucket. | Each reviewer returns candidate learnings as: what happened (with a quote/reference), why it matters, a suggested routing, and — for anything proposed as skill prose — the established principle it instantiates (author, title, year, URL) or the words "no known prior art", so the synthesis step's grounding gate has something to check. For any finding that traces back to a point where the user (or a past self) had to intervene and correct the agent mid-task, the reviewer must first ask *how would this stop needing a correction at all* — not just "what skill line would have prevented it." The synthesis step weights findings by user frustration first: a root cause that produced an all-caps correction, a repeated demand, or profanity outranks one that only wasted tokens. A verbatim-repeated user message is the strongest single signal — the user re-sent it because nothing visibly changed the first time. diff --git a/engine/skills/reflect/scripts/lesson_route.py b/engine/skills/reflect/scripts/lesson_route.py new file mode 100644 index 000000000..e3d54c64d --- /dev/null +++ b/engine/skills/reflect/scripts/lesson_route.py @@ -0,0 +1,389 @@ +#!/usr/bin/env python3 +"""Route a reflect finding to a lesson bucket before synthesis sorts it. + +Step 4 of reflect used to treat every restated existing rule as automate-me +evidence. That writes a new preference for a skill the agent already skipped. +This script's bucket wins over the model's sort: a restated rule whose skill +already states it is Rejected as already-covered, not a new lesson. + +Buckets: + + already-covered skill already states the rule, or the assistant admitted + the skip ("It is; I skipped it"). Route Rejected. + Do not call automate-me. + skill-not-used transcript never opened the named skill. Rejected unless + missed_trigger is set (then action is tune-description). + automate-me working preference, and no existing skill states it. + accepted durable, decision-changing, not session-specific. + Frustration / intervention-must-automate does not select + this bucket. + + python3 lesson_route.py --quote "..." --assistant-reply "It is; I skipped it." + python3 lesson_route.py --from-transcript fixtures/restated_rule/fires.jsonl + python3 lesson_route.py --kind preference --quote "always open a PR after green" + python3 lesson_route.py --skill-used no --skill-path engine/skills/foo/SKILL.md +""" +from __future__ import annotations + +import argparse +import json +import re +import sys + +ALREADY_COVERED = "already-covered" +SKILL_NOT_USED = "skill-not-used" +AUTOMATE_ME = "automate-me" +ACCEPTED = "accepted" + +BUCKETS = (ALREADY_COVERED, SKILL_NOT_USED, AUTOMATE_ME, ACCEPTED) + +# Assistant admits the skill/rule existed and was not followed. +SKIP_ADMISSION = re.compile( + r"\bi skipped (it|that|the skill|the rule)\b" + r"|\bit is;?\s*i skipped\b" + r"|\bi should have (followed|read|used)\b" + r"|\bi (already )?(knew|know) (that|about)\b.*\b(skipped|didn't follow|did not follow)\b", + re.I, +) + +# Human reminds the agent of a rule that already exists. +RESTATED_RULE_SHAPE = re.compile( + r"\bi thought that was a skill\b" + r"|\bisn'?t there already a (rule|skill)\b" + r"|\bwe already decided\b" + r"|\bthe \w[\w-]* skill says\b" + r"|\b(a|the) skill\s+that we enforce\b" + r"|\bdid you not read (it|the)\b", + re.I, +) + +# Working preference that is not "the rule already exists." +PREFERENCE_SHAPE = re.compile( + r"\b(always|never|from now on|by default|don'?t ask)\b" + r"|\bwe don'?t have a rule for this yet\b" + r"|\bcan you (add|capture|automate)\b" + r"|\bi (prefer|want|like) (you )?to\b", + re.I, +) + +SESSION_SPECIFIC = re.compile( + r"\b[0-9a-f]{7,40}\b" # SHAs + r"|/(?:Users|home|tmp)/[^\s]+" # absolute paths + r"|\b(?:ticker|case|pr)\s*#?\d+\b", + re.I, +) + + +def _truthy(value): + if value is None: + return None + if isinstance(value, bool): + return value + text = str(value).strip().lower() + if text in ("1", "true", "yes", "y", "on"): + return True + if text in ("0", "false", "no", "n", "off"): + return False + raise ValueError(f"not a boolean: {value!r}") + + +def _skill_states_rule(skill_text, quote): + """True when the target skill already carries the restated rule. + + Looks for shared content words of length >= 4 between the quote and the + skill body (minus stopwords). A short overlap is enough: the restated-rule + checker already decided a rule exists; this only confirms the named skill + is a home for it. + """ + if not skill_text or not quote: + return False + stop = { + "that", "this", "with", "from", "have", "were", "been", "when", + "what", "which", "there", "their", "about", "should", "would", + "could", "think", "thought", "skill", "skills", "rule", "rules", + "enforce", "already", "doing", "these", "those", "into", "just", + } + words = { + w for w in re.findall(r"[a-z]{4,}", quote.lower()) + if w not in stop + } + body = skill_text.lower() + hits = [w for w in words if w in body] + return len(hits) >= 2 or ("worktree" in body and "worktree" in quote.lower()) + + +def route( + *, + quote="", + assistant_reply="", + skill_text=None, + skill_used=None, + kind="", + missed_trigger=False, + durable=True, + decision_changing=True, +): + """Pick the lesson bucket for one finding. + + Frustration signals and intervention-must-automate may order the pass; + they do not select ``accepted`` or ``automate-me`` by themselves. + """ + quote = quote or "" + assistant_reply = assistant_reply or "" + kind = (kind or "").strip().lower().replace("_", "-") + reasons = [] + + # Boundary parse: transcript prose → typed admitted_skip. Callers may also + # pass assistant_reply="" and set kind/skill_text from structured fields. + admitted_skip = bool(SKIP_ADMISSION.search(assistant_reply)) # text-match-decision-warn: allow + restated = kind in ("restated-rule", "restated") or bool( + RESTATED_RULE_SHAPE.search(quote) # text-match-decision-warn: allow + ) + skill_covers = _skill_states_rule(skill_text, quote) if skill_text else False + + # 1. Restated existing rule / admitted skip → already-covered. + # By definition of the restated-rule checker, the rule already exists. + if admitted_skip or restated or (skill_covers and not _truthy_preference(kind, quote)): + if admitted_skip: + reasons.append("assistant admitted the skill/rule exists and was skipped") + if restated: + reasons.append("utterance restates a rule that already exists") + if skill_covers: + reasons.append("target skill already states the rule") + if kind in ("restated-rule", "restated"): + reasons.append("kind=restated-rule means the rule already exists") + if not reasons: + reasons.append("target skill already states the rule; issue is execution, not the skill") + return { + "bucket": ALREADY_COVERED, + "action": "reject", + "rationale": "; ".join(reasons) + "; do not route to automate-me", + } + + # 2. Named skill never opened in the transcript. + if skill_used is False: + if missed_trigger: + return { + "bucket": SKILL_NOT_USED, + "action": "tune-description", + "rationale": "skill was visible but did not trigger; tune its description", + } + return { + "bucket": SKILL_NOT_USED, + "action": "reject", + "rationale": "transcript never opened the named skill; reject as skill-not-used", + } + + # 3. Working preference with no existing skill home. + if _truthy_preference(kind, quote) and not skill_covers: + return { + "bucket": AUTOMATE_ME, + "action": "automate-me", + "rationale": "working preference with no existing skill stating it", + } + + # 4. Accepted only when durable and decision-changing, and not a one-off. + session_specific = bool(SESSION_SPECIFIC.search(quote)) + if durable and decision_changing and not session_specific: + return { + "bucket": ACCEPTED, + "action": "accept", + "rationale": "durable and decision-changing; frustration did not select this bucket", + } + + reasons = [] + if not durable: + reasons.append("not durable across path/SHA/tool drift") + if not decision_changing: + reasons.append("would not change a future agent action") + if session_specific: + reasons.append("session-specific paths/SHAs/case names") + return { + "bucket": ALREADY_COVERED if skill_covers else SKILL_NOT_USED, + "action": "reject", + "rationale": "; ".join(reasons) or "not accepted", + } + + +def _truthy_preference(kind, quote): + if kind in ("preference", "working-style", "working-preference", "automate-me"): + return True + if RESTATED_RULE_SHAPE.search(quote or ""): + return False + return bool(PREFERENCE_SHAPE.search(quote or "")) + + +def _claude_text_blocks(row): + msg = row.get("message") or {} + content = msg.get("content") + if isinstance(content, str): + return content + if not isinstance(content, list): + return "" + parts = [] + for block in content: + if isinstance(block, dict) and block.get("type") == "text": + parts.append(block.get("text") or "") + elif isinstance(block, dict) and block.get("type") == "tool_result": + continue + return "\n".join(parts) + + +def findings_from_transcript(path): + """Pull (quote, assistant_reply) pairs from a Claude-shaped JSONL session. + + Used by the fires.jsonl repro: a restated-rule human turn followed by an + assistant admission of skip becomes an already-covered finding. + """ + rows = [] + with open(path, encoding="utf-8") as handle: + for line in handle: + line = line.strip() + if not line: + continue + try: + rows.append(json.loads(line)) + except json.JSONDecodeError: + continue + + pairs = [] + pending_quote = None + for row in rows: + msg = row.get("message") or {} + # Claude session JSONL often sets role only via top-level `type` + # (`"type": "assistant"`) and omits `message.role`. + role = msg.get("role") or row.get("type") + text = _claude_text_blocks(row).strip() + if not text: + continue + # Skip tool_result-only user rows and harness noise. + if role == "user": + if text.startswith("[Request interrupted"): + continue + if RESTATED_RULE_SHAPE.search(text) or PREFERENCE_SHAPE.search(text): # text-match-decision-warn: allow + pending_quote = text + continue + if role == "assistant" and pending_quote is not None: + pairs.append({"quote": pending_quote, "assistant_reply": text}) + pending_quote = None + if pending_quote is not None: + pairs.append({"quote": pending_quote, "assistant_reply": ""}) + return pairs + + +def route_transcript(path, **kwargs): + """Route every restated/preference pair in a transcript; return the strictest.""" + pairs = findings_from_transcript(path) + if not pairs: + return { + "bucket": ACCEPTED, + "action": "accept", + "rationale": "no restated-rule or preference utterance found", + "findings": [], + } + findings = [] + for pair in pairs: + kind = kwargs.get("kind") or ( + "restated-rule" if RESTATED_RULE_SHAPE.search(pair["quote"]) else "preference" + ) + result = route( + quote=pair["quote"], + assistant_reply=pair["assistant_reply"], + skill_text=kwargs.get("skill_text"), + skill_used=kwargs.get("skill_used"), + kind=kind, + missed_trigger=kwargs.get("missed_trigger", False), + durable=kwargs.get("durable", True), + decision_changing=kwargs.get("decision_changing", True), + ) + findings.append({**pair, **result}) + + # Strictest wins: already-covered > skill-not-used > automate-me > accepted. + rank = {ALREADY_COVERED: 0, SKILL_NOT_USED: 1, AUTOMATE_ME: 2, ACCEPTED: 3} + best = min(findings, key=lambda f: rank.get(f["bucket"], 99)) + return { + "bucket": best["bucket"], + "action": best["action"], + "rationale": best["rationale"], + "findings": findings, + } + + +def main(argv=None): + ap = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + ap.add_argument("--quote", default="", help="incident human utterance") + ap.add_argument("--assistant-reply", default="", help="assistant text after the quote") + ap.add_argument("--skill-text", default=None, help="body of the target skill") + ap.add_argument("--skill-path", default=None, help="read skill text from this path") + ap.add_argument( + "--skill-used", + default=None, + help="yes/no: whether the transcript opened the skill", + ) + ap.add_argument( + "--kind", + default="", + help="restated-rule | preference | durable-lesson", + ) + ap.add_argument( + "--missed-trigger", + action="store_true", + help="skill was catalog-visible but did not fire", + ) + ap.add_argument( + "--not-durable", + action="store_true", + help="proposal depends on session paths/SHAs/case names", + ) + ap.add_argument( + "--not-decision-changing", + action="store_true", + help="proposal would not change a future agent action", + ) + ap.add_argument( + "--from-transcript", + default=None, + help="Claude-shaped JSONL; route restated/preference pairs inside it", + ) + ap.add_argument("--json", action="store_true", help="machine-readable result") + args = ap.parse_args(argv) + + skill_text = args.skill_text + if args.skill_path: + with open(args.skill_path, encoding="utf-8") as handle: + skill_text = handle.read() + + skill_used = _truthy(args.skill_used) if args.skill_used is not None else None + kwargs = dict( + skill_text=skill_text, + skill_used=skill_used, + kind=args.kind, + missed_trigger=args.missed_trigger, + durable=not args.not_durable, + decision_changing=not args.not_decision_changing, + ) + + if args.from_transcript: + result = route_transcript(args.from_transcript, **kwargs) + else: + result = route( + quote=args.quote, + assistant_reply=args.assistant_reply, + **kwargs, + ) + + if args.json: + print(json.dumps(result, indent=2)) + else: + print(result["bucket"]) + print(result["rationale"]) + if result.get("action") and result["action"] != result["bucket"]: + print(f"action: {result['action']}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/engine/skills/reflect/scripts/tests/fixtures/README.md b/engine/skills/reflect/scripts/tests/fixtures/README.md index 082d11e66..1613ba719 100644 --- a/engine/skills/reflect/scripts/tests/fixtures/README.md +++ b/engine/skills/reflect/scripts/tests/fixtures/README.md @@ -10,7 +10,7 @@ block) so dedup and thrash detectors see real shapes. | `clean_efficient_session.jsonl` | Read+Edit in one turn → `pytest` → done. High cache-read share. | All five flags `no` | | `token_thrash_session.jsonl` | Cache spike, lookup-only turns, identical-window re-read, same error 3×, 4 edits with no verify. | All five flags `yes` | | `lookup_heavy_session.jsonl` | Only Grep/Glob/Read turns with large output. | `model-tier-candidates` `yes` with a positive $ backtest in the rationale; other thrash flags `no` | -| `restated_rule/fires.jsonl` | A rejected tool call, the person's restatement after it, then a restated existing rule. | `intervention-must-automate` `yes` under `--judge` | +| `restated_rule/fires.jsonl` | A rejected tool call, the person's restatement after it, then a restated existing rule ("It is; I skipped it."). | `intervention-must-automate` `yes` under `--judge`; `lesson_route.py --from-transcript` → `already-covered` (not `automate-me`) | | `restated_rule/silent.jsonl` | The same restatement text only in a tool_result, a skill injection, a subagent prompt, and a rejection with no human reply. | `intervention-must-automate` `no` under `--judge`, `unchecked` without it | Regenerate after changing detector thresholds: diff --git a/engine/skills/reflect/scripts/tests/test_lesson_route.py b/engine/skills/reflect/scripts/tests/test_lesson_route.py new file mode 100644 index 000000000..48e2689f9 --- /dev/null +++ b/engine/skills/reflect/scripts/tests/test_lesson_route.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python3 +"""lesson_route.py refuses to turn a restated existing rule into a new lesson. + +Fail-before this gate: restated-rule.json on_hit called a restated existing +rule "automate-me evidence", and fires.jsonl (assistant: "It is; I skipped it.") +had no script that could print already-covered. These tests pin the buckets. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +import unittest + +SCRIPTS = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +FIXTURES = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures") +if SCRIPTS not in sys.path: + sys.path.insert(0, SCRIPTS) +import lesson_route as lr # noqa: E402 + +SCRIPT = os.path.join(SCRIPTS, "lesson_route.py") +FIRES = os.path.join(FIXTURES, "restated_rule", "fires.jsonl") +# SCRIPTS = .../engine/skills/reflect/scripts → reflect → skills → engine → repo +REPO = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(SCRIPTS)))) +PHRASE = os.path.join(REPO, "engine", "hooks", "llm-judge", "phrases", "restated-rule.json") +SKILL_MD = os.path.join(os.path.dirname(SCRIPTS), "SKILL.md") + + +def run(*args): + return subprocess.run( + [sys.executable, SCRIPT, *args], + capture_output=True, + text=True, + check=False, + ) + + +class TestFiresFixtureIsAlreadyCovered(unittest.TestCase): + def test_fires_transcript_prints_already_covered_not_automate_me(self): + res = run("--from-transcript", FIRES, "--json") + self.assertEqual(res.returncode, 0, res.stderr) + payload = json.loads(res.stdout) + self.assertEqual(payload["bucket"], lr.ALREADY_COVERED) + self.assertNotEqual(payload["bucket"], lr.AUTOMATE_ME) + self.assertIn("do not route to automate-me", payload["rationale"]) + + def test_admitted_skip_pair_is_already_covered(self): + result = lr.route( + quote="I thought that was a skill that we enforce in catstack?", + assistant_reply="It is; I skipped it.", + kind="restated-rule", + ) + self.assertEqual(result["bucket"], lr.ALREADY_COVERED) + self.assertEqual(result["action"], "reject") + + +class TestPreferenceStillAutomates(unittest.TestCase): + def test_preference_with_no_skill_is_automate_me(self): + res = run( + "--kind", "preference", + "--quote", "always open a PR after green without asking me", + "--json", + ) + self.assertEqual(res.returncode, 0, res.stderr) + payload = json.loads(res.stdout) + self.assertEqual(payload["bucket"], lr.AUTOMATE_ME) + + def test_no_rule_yet_preference_is_automate_me(self): + result = lr.route( + quote="we don't have a rule for this yet, can you add one?", + kind="preference", + ) + self.assertEqual(result["bucket"], lr.AUTOMATE_ME) + + +class TestSkillNotUsed(unittest.TestCase): + def test_unopened_skill_is_skill_not_used(self): + res = run( + "--quote", "tighten the retry backoff table", + "--kind", "durable-lesson", + "--skill-used", "no", + "--json", + ) + self.assertEqual(res.returncode, 0, res.stderr) + payload = json.loads(res.stdout) + self.assertEqual(payload["bucket"], lr.SKILL_NOT_USED) + self.assertEqual(payload["action"], "reject") + + def test_missed_trigger_tunes_description(self): + result = lr.route( + quote="tighten the retry backoff table", + kind="durable-lesson", + skill_used=False, + missed_trigger=True, + ) + self.assertEqual(result["bucket"], lr.SKILL_NOT_USED) + self.assertEqual(result["action"], "tune-description") + + +class TestAcceptedGates(unittest.TestCase): + def test_durable_decision_changing_is_accepted(self): + result = lr.route( + quote="batch small Edits into fewer passes when the file is unchanged", + kind="durable-lesson", + skill_used=True, + ) + self.assertEqual(result["bucket"], lr.ACCEPTED) + + def test_session_specific_quote_is_not_accepted(self): + result = lr.route( + quote="fix the bug at /Users/edbert/proj/foo.py for ticker AUR", + kind="durable-lesson", + skill_used=True, + ) + self.assertNotEqual(result["bucket"], lr.ACCEPTED) + + +class TestOnHitAndSkillProsePointAtTheGate(unittest.TestCase): + def test_restated_rule_on_hit_names_already_covered_not_automate_me(self): + with open(PHRASE, encoding="utf-8") as handle: + data = json.load(handle) + on_hit = data["on_hit"] + self.assertIn("already-covered", on_hit) + self.assertNotIn("automate-me evidence", on_hit) + + def test_synthesize_step_requires_lesson_route(self): + with open(SKILL_MD, encoding="utf-8") as handle: + text = handle.read() + start = text.index("### 4. Synthesize") + end = text.index("### 5.", start) + section = text[start:end] + self.assertIn("lesson_route.py", section) + self.assertIn("already-covered", section) + self.assertIn("does not by itself create an automate-me row", section) + + +if __name__ == "__main__": + unittest.main() diff --git a/engine/skills/reflect/scripts/tests/test_skill_prose_grounding.py b/engine/skills/reflect/scripts/tests/test_skill_prose_grounding.py index 9e43680b7..bb3cf3d34 100644 --- a/engine/skills/reflect/scripts/tests/test_skill_prose_grounding.py +++ b/engine/skills/reflect/scripts/tests/test_skill_prose_grounding.py @@ -5,17 +5,23 @@ prior art"; otherwise it goes to Backlog. A prior pass shipped two principle skills whose rules restated real literature (fail fast, PEP 20, control totals, mutation testing) in invented words, and nothing in the -reflect flow asked for the source. These tests only pin that the rule and -its lens-side duty are present, so an edit cannot drop them silently. +reflect flow asked for the source. These tests pin that the rule and its +lens-side duty are present, and that lesson_route.py refuses to turn a +restated existing rule into Accepted or automate-me. """ from __future__ import annotations +import json import re +import subprocess +import sys from pathlib import Path SKILL_DIR = Path(__file__).resolve().parents[2] SKILL_MD = SKILL_DIR / "SKILL.md" LENSES_MD = SKILL_DIR / "references" / "lenses.md" +LESSON_ROUTE = SKILL_DIR / "scripts" / "lesson_route.py" +FIRES = SKILL_DIR / "scripts" / "tests" / "fixtures" / "restated_rule" / "fires.jsonl" def _synthesize_section(text: str) -> str: @@ -38,3 +44,24 @@ def test_lens_output_carries_the_principle_or_no_prior_art() -> None: assert "established principle it instantiates" in text assert "no known prior art" in text assert "grounding gate" in text + + +def test_lesson_route_gates_fires_fixture_as_already_covered() -> None: + """Grounding is not words-in-markdown only: run the route script.""" + res = subprocess.run( + [sys.executable, str(LESSON_ROUTE), "--from-transcript", str(FIRES), "--json"], + capture_output=True, + text=True, + check=False, + ) + assert res.returncode == 0, res.stderr + payload = json.loads(res.stdout) + assert payload["bucket"] == "already-covered" + assert payload["bucket"] != "automate-me" + + +def test_synthesize_step_requires_lesson_route_before_automate_me() -> None: + section = _synthesize_section(SKILL_MD.read_text(encoding="utf-8")) + assert "lesson_route.py" in section + assert "already-covered" in section + assert "does not by itself create an automate-me row" in section From 90c66e24b046f4dcd12d5625448eeb0dd2dd9e8f Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Sun, 11 Oct 2026 06:28:16 -0700 Subject: [PATCH 02/10] Drop the lesson-router comments that failed the comment gate. The boundary-parser allow mark stays, and the comment checker treats that mark as a machine directive so the two gates agree. Co-authored-by: Cursor Change-Id: I0a048d4d387df53de81d2638521a5198de499b36 --- engine/hooks/no-comments/detect.py | 2 +- engine/hooks/no-comments/tests/test_hooks.py | 2 +- engine/skills/reflect/scripts/lesson_route.py | 18 ++---------------- .../reflect/scripts/tests/test_lesson_route.py | 1 - 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/engine/hooks/no-comments/detect.py b/engine/hooks/no-comments/detect.py index 17f73ff42..fc4ee4e28 100644 --- a/engine/hooks/no-comments/detect.py +++ b/engine/hooks/no-comments/detect.py @@ -53,7 +53,7 @@ TRAILING_SLASH_RE = re.compile(r"\s//\s*\S") TRAILING_DIRECTIVE_RE = re.compile( r"\s(?:#|//)\s*(?:noqa|type:|pragma|pylint|mypy|ruff|fmt:|nosec|isort|nolint|" - r"eslint|prettier|@ts-|tslint|istanbul|biome|swiftlint|NOSONAR)", + r"eslint|prettier|@ts-|tslint|istanbul|biome|swiftlint|NOSONAR|text-match-decision-warn:)", re.IGNORECASE, ) RULE_ADDED_COMMENT = "no-comments.added-comment" diff --git a/engine/hooks/no-comments/tests/test_hooks.py b/engine/hooks/no-comments/tests/test_hooks.py index 211e0c528..0de7181dc 100644 --- a/engine/hooks/no-comments/tests/test_hooks.py +++ b/engine/hooks/no-comments/tests/test_hooks.py @@ -133,7 +133,7 @@ def test_blocks_write_and_multiedit_shapes(self): class TestAllows(unittest.TestCase): def test_allows_directives_and_shebang(self): - text = "#!/usr/bin/env python3\n# -*- coding: utf-8 -*-\nimport x # noqa: E402\ny = 1 # type: ignore\n# pragma: no cover\n" + text = "#!/usr/bin/env python3\n# -*- coding: utf-8 -*-\nimport x # noqa: E402\ny = 1 # type: ignore\n# pragma: no cover\nz = pat.search(text) # text-match-decision-warn: allow\n" self.assertEqual(detect.comment_lines("/r/a.py", text), []) js = "// eslint-disable-next-line no-console\n// @ts-ignore\n/* istanbul ignore next */\n// SPDX-License-Identifier: MIT\n" self.assertEqual(detect.comment_lines("/r/a.ts", js), []) diff --git a/engine/skills/reflect/scripts/lesson_route.py b/engine/skills/reflect/scripts/lesson_route.py index e3d54c64d..7a79cecc2 100644 --- a/engine/skills/reflect/scripts/lesson_route.py +++ b/engine/skills/reflect/scripts/lesson_route.py @@ -37,7 +37,6 @@ BUCKETS = (ALREADY_COVERED, SKILL_NOT_USED, AUTOMATE_ME, ACCEPTED) -# Assistant admits the skill/rule existed and was not followed. SKIP_ADMISSION = re.compile( r"\bi skipped (it|that|the skill|the rule)\b" r"|\bit is;?\s*i skipped\b" @@ -46,7 +45,6 @@ re.I, ) -# Human reminds the agent of a rule that already exists. RESTATED_RULE_SHAPE = re.compile( r"\bi thought that was a skill\b" r"|\bisn'?t there already a (rule|skill)\b" @@ -57,7 +55,6 @@ re.I, ) -# Working preference that is not "the rule already exists." PREFERENCE_SHAPE = re.compile( r"\b(always|never|from now on|by default|don'?t ask)\b" r"|\bwe don'?t have a rule for this yet\b" @@ -67,8 +64,8 @@ ) SESSION_SPECIFIC = re.compile( - r"\b[0-9a-f]{7,40}\b" # SHAs - r"|/(?:Users|home|tmp)/[^\s]+" # absolute paths + r"\b[0-9a-f]{7,40}\b" + r"|/(?:Users|home|tmp)/[^\s]+" r"|\b(?:ticker|case|pr)\s*#?\d+\b", re.I, ) @@ -133,16 +130,12 @@ def route( kind = (kind or "").strip().lower().replace("_", "-") reasons = [] - # Boundary parse: transcript prose → typed admitted_skip. Callers may also - # pass assistant_reply="" and set kind/skill_text from structured fields. admitted_skip = bool(SKIP_ADMISSION.search(assistant_reply)) # text-match-decision-warn: allow restated = kind in ("restated-rule", "restated") or bool( RESTATED_RULE_SHAPE.search(quote) # text-match-decision-warn: allow ) skill_covers = _skill_states_rule(skill_text, quote) if skill_text else False - # 1. Restated existing rule / admitted skip → already-covered. - # By definition of the restated-rule checker, the rule already exists. if admitted_skip or restated or (skill_covers and not _truthy_preference(kind, quote)): if admitted_skip: reasons.append("assistant admitted the skill/rule exists and was skipped") @@ -160,7 +153,6 @@ def route( "rationale": "; ".join(reasons) + "; do not route to automate-me", } - # 2. Named skill never opened in the transcript. if skill_used is False: if missed_trigger: return { @@ -174,7 +166,6 @@ def route( "rationale": "transcript never opened the named skill; reject as skill-not-used", } - # 3. Working preference with no existing skill home. if _truthy_preference(kind, quote) and not skill_covers: return { "bucket": AUTOMATE_ME, @@ -182,7 +173,6 @@ def route( "rationale": "working preference with no existing skill stating it", } - # 4. Accepted only when durable and decision-changing, and not a one-off. session_specific = bool(SESSION_SPECIFIC.search(quote)) if durable and decision_changing and not session_specific: return { @@ -250,13 +240,10 @@ def findings_from_transcript(path): pending_quote = None for row in rows: msg = row.get("message") or {} - # Claude session JSONL often sets role only via top-level `type` - # (`"type": "assistant"`) and omits `message.role`. role = msg.get("role") or row.get("type") text = _claude_text_blocks(row).strip() if not text: continue - # Skip tool_result-only user rows and harness noise. if role == "user": if text.startswith("[Request interrupted"): continue @@ -298,7 +285,6 @@ def route_transcript(path, **kwargs): ) findings.append({**pair, **result}) - # Strictest wins: already-covered > skill-not-used > automate-me > accepted. rank = {ALREADY_COVERED: 0, SKILL_NOT_USED: 1, AUTOMATE_ME: 2, ACCEPTED: 3} best = min(findings, key=lambda f: rank.get(f["bucket"], 99)) return { diff --git a/engine/skills/reflect/scripts/tests/test_lesson_route.py b/engine/skills/reflect/scripts/tests/test_lesson_route.py index 48e2689f9..cb298a5d9 100644 --- a/engine/skills/reflect/scripts/tests/test_lesson_route.py +++ b/engine/skills/reflect/scripts/tests/test_lesson_route.py @@ -21,7 +21,6 @@ SCRIPT = os.path.join(SCRIPTS, "lesson_route.py") FIRES = os.path.join(FIXTURES, "restated_rule", "fires.jsonl") -# SCRIPTS = .../engine/skills/reflect/scripts → reflect → skills → engine → repo REPO = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(SCRIPTS)))) PHRASE = os.path.join(REPO, "engine", "hooks", "llm-judge", "phrases", "restated-rule.json") SKILL_MD = os.path.join(os.path.dirname(SCRIPTS), "SKILL.md") From 083c513be220da25f7f1865e7bf77fb80e0ae9bd Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Sun, 11 Oct 2026 14:22:06 +0000 Subject: [PATCH 03/10] Stamp an event_uid on every runner metrics row. Hook runs belonging to the same harness event had no shared key, so grouping them for herd/wall-clock metrics relied on fragile 2s-window inference. Add event_uid = sha256(stdin)[:12] to _row() so every runner that receives the same stdin self-groups with zero coordination. --- engine/hooks/_runner/run.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/engine/hooks/_runner/run.py b/engine/hooks/_runner/run.py index 081747e3c..42fa17fc5 100644 --- a/engine/hooks/_runner/run.py +++ b/engine/hooks/_runner/run.py @@ -2,6 +2,7 @@ import argparse import datetime +import hashlib import json import os import subprocess @@ -234,6 +235,7 @@ def _row( "hook": hook, "script": script, "event": event, + "event_uid": hashlib.sha256(stdin).hexdigest()[:12], "session_id": session_id, "outcome": outcome, "exit_code": exit_code, From 1e585e335925f921f670a9c672097c43d09a3bdb Mon Sep 17 00:00:00 2001 From: Invoker Bot Date: Sun, 11 Oct 2026 14:23:24 +0000 Subject: [PATCH 04/10] =?UTF-8?q?invoker:=20wf-1791728211949-11/publish-ev?= =?UTF-8?q?ent-uid=20=E2=80=94=20Review=20claim:=20Every=20runner=20metric?= =?UTF-8?q?s=20row=20gains=20an=20event=5Fuid=20field=20that=20groups=20al?= =?UTF-8?q?l=20hook=20runs=20belonging=20to=20one=20harness=20event,=20wit?= =?UTF-8?q?h=20no=20change=20to=20hook=20stdout,=20stderr,=20or=20exit=20c?= =?UTF-8?q?odes.=20Review=20lane:=20behavior=20Safety=20invariant:=20Hook?= =?UTF-8?q?=20stdout=20and=20exit=20codes=20stay=20the=20same.=20This=20sl?= =?UTF-8?q?ice=20does=20not=20change=20the=20other=20four=20claims.=20File?= =?UTF-8?q?s:=20engine/hooks/=5Frunner/run.py=20Change=20types:=20bugfix?= =?UTF-8?q?=20Acceptance=20criteria:=20The=20branch=20contains=20only=20th?= =?UTF-8?q?is=20claim,=20and=20the=20review=20step=20publishes=20one=20pul?= =?UTF-8?q?l=20request.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exit code: 0 Invoker-Finalize-Id: 5330cf4f-7efd-43e4-9fc5-fbc19b9c9a89 From 5a9e5c1b6e5ec6032e520a182892ba20a675846f Mon Sep 17 00:00:00 2001 From: CI Bot Date: Sun, 11 Oct 2026 14:27:01 +0000 Subject: [PATCH 05/10] Let the install tests read either hook layout. tests/test_install.py and engine/hooks/_runner/tests/test_wrap_installed.py asserted one settings entry per hook, so every per-hook guarantee they check failed once CATSTACK_HOOK_DISPATCHER (#1122) collapsed an event to a single _runner/dispatch.py entry: 70 failures and 1 error on #1122's head, all in these two files. load_config() in tests/test_install.py now reads an installed config and expands a collapsed event back into one entry per hook, resolving each hook's registration the way dispatch.py itself does (dispatch.load_event_hooks: the cursor registry, or the hook's own manifest for claude and codex). Each assertion therefore sees the identical shape in both layouts and keeps checking the same guarantee -- right event, right harness, Stop mirrored to SubagentStop or opted out, dead entries pruned, non-catstack entries untouched. test_wrap_installed.py's fixture pins the flag off, since it asserts the settings bytes install writes rather than a hook's registration. #1122's own test_default_is_off_and_byte_identical_to_no_flag_at_all now compares the default install against an install that sets the flag to whatever flags.py resolves as the default, and clears the key from its own environment first, so the flag-on run of the full suite cannot leak into the "default" side of the comparison. The CATSTACK_HOOK_DISPATCHER default is left off. Flipping it on is gated on both full-suite runs passing, and the source branch plan/hook-backlog-1c-install-tests-accept-the-one-entry-per-event-layout-default-on-only-if-both-suites-pass never changed engine/hooks/_flags/flags.py, so that gate was not met there and this commit does not claim it. Cherry-picked from 6330abf291561cace7301948bda714d0b8795f43 on that branch. The branch's two other content commits (engine/hooks/_runner/doctor.py, scripts/test/run_skill_scenarios.py) are unrelated to this claim and are left behind. --- .../_runner/tests/test_wrap_installed.py | 3 + tests/test_install.py | 278 +++++++++++------- 2 files changed, 171 insertions(+), 110 deletions(-) diff --git a/engine/hooks/_runner/tests/test_wrap_installed.py b/engine/hooks/_runner/tests/test_wrap_installed.py index 008b06bd4..6de0ec8a3 100644 --- a/engine/hooks/_runner/tests/test_wrap_installed.py +++ b/engine/hooks/_runner/tests/test_wrap_installed.py @@ -24,6 +24,9 @@ def setUp(self): self.old_home = os.environ.get("HOME") os.environ["HOME"] = str(self.home) self.addCleanup(self._restore_home) + dispatcher_patch = mock.patch.dict(os.environ, {"CATSTACK_HOOK_DISPATCHER": "0"}) + dispatcher_patch.start() + self.addCleanup(dispatcher_patch.stop) self.claude_path = self.home / ".claude" / "settings.json" self.cursor_path = self.home / ".cursor" / "hooks.json" self.codex_path = self.home / ".codex" / "hooks.json" diff --git a/tests/test_install.py b/tests/test_install.py index e065f2d94..675124847 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -28,6 +28,7 @@ import tomllib import unittest from pathlib import Path +from unittest import mock REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) INSTALL_SH = os.path.join(REPO_ROOT, "install.sh") @@ -167,9 +168,7 @@ def installed_config_hook_names(fake_home): path = os.path.join(fake_home, relative) if not os.path.exists(path): continue - with open(path) as handle: - data = json.load(handle) - for command in json_commands(data.get("hooks", {})): + for command in json_commands(load_config(path).get("hooks", {})): hook = catstack_hook_from_command(command) if hook: names.add(hook) @@ -186,12 +185,103 @@ def installed_config_commands(fake_home): path = os.path.join(fake_home, relative) if not os.path.exists(path): continue - with open(path) as handle: - data = json.load(handle) - commands.extend(json_commands(data.get("hooks", {}))) + commands.extend(json_commands(load_config(path).get("hooks", {}))) return commands +sys.path.insert(0, os.path.join(REPO_ROOT, "engine", "hooks", "_runner")) +sys.path.insert(0, os.path.join(REPO_ROOT, "engine", "hooks", "_flags")) +import dispatch as _dispatch # noqa: E402 +import wrap_installed as _wrap_installed # noqa: E402 +import flags as _flags # noqa: E402 + +_HARNESS_BY_DIR = {".claude": "claude", ".cursor": "cursor", ".codex": "codex"} + + +def _harness_for_config_path(path): + parts = os.path.normpath(path).split(os.sep) + for harness_dir, harness in _HARNESS_BY_DIR.items(): + if harness_dir in parts: + return harness + raise ValueError(f"not a recognized install config path: {path}") + + +def _fake_home_for_config_path(path, harness): + marker = os.sep + f".{harness}" + os.sep + index = path.rfind(marker) + return path[:index] + + +def _record_command(record, harness): + timeout_value = record.get("timeout") + if isinstance(timeout_value, (int, float)) and not isinstance(timeout_value, bool): + budget = timeout_value - 0.5 + else: + budget = 59.5 + budget_str = _wrap_installed._format_timeout(budget) + trailing = " ".join(record.get("args") or []) + suffix = f" {trailing}" if trailing else "" + return ( + f"python3 $HOME/.{harness}/hooks/_runner/run.py --timeout {budget_str} " + f"{record['hook']}/{record['script']}{suffix}" + ) + + +def _records_to_groups(records, harness): + groups = [] + for record in records: + command = _record_command(record, harness) + matcher = record.get("matcher") + if harness == "cursor": + entry = {"command": command} + if record.get("timeout") is not None: + entry["timeout"] = record["timeout"] + if matcher is not None: + entry["matcher"] = matcher + else: + hook_obj = {"type": "command", "command": command} + if record.get("timeout") is not None: + hook_obj["timeout"] = record["timeout"] + entry = {"hooks": [hook_obj]} + if matcher is not None: + entry["matcher"] = matcher + groups.append(entry) + return groups + + +def load_config(path): + """Read an installed hooks config file (.claude/settings.json, + .cursor/hooks.json, .codex/hooks.json), expanding any event collapsed + behind one `_runner/dispatch.py` entry back into one entry per hook -- + the same shape install leaves with CATSTACK_HOOK_DISPATCHER off -- by + reading that hook's registration the same way the dispatcher itself + would (`dispatch.load_event_hooks`: the cursor registry file, or the + hook's own manifest for claude and codex). A test written against the + one-entry-per-hook layout reads the identical shape in either layout, + so the same assertion checks the same per-hook guarantee regardless of + which layout install actually produced.""" + with open(path, encoding="utf-8") as handle: + data = json.load(handle) + hooks = data.get("hooks") + if not isinstance(hooks, dict): + return data + harness = _harness_for_config_path(path) + fake_home = _fake_home_for_config_path(path, harness) + hooks_root = os.path.join(fake_home, f".{harness}", "hooks") + expanded = {} + changed = False + for event, groups in hooks.items(): + if isinstance(groups, list) and any("_runner/dispatch.py" in c for c in json_commands(groups)): + changed = True + records, _warnings = _dispatch.load_event_hooks(hooks_root, harness, event) + expanded[event] = _records_to_groups(records, harness) + else: + expanded[event] = groups + if not changed: + return data + return {**data, "hooks": expanded} + + class TestNeverTouchesRealHome(unittest.TestCase): """Safety-guard meta-test: every other test in this file builds its fake $HOME the same way (tempfile.TemporaryDirectory()) -- this asserts that @@ -255,8 +345,7 @@ def test_explicit_failures_hook_linked_and_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "explicit-failures") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "explicit-failures")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("explicit-failures/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -272,8 +361,7 @@ def test_bound_tool_result_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, name)) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any(f"{name}/claude_pre_tool_use.py" in hook["command"] for hook in entry["hooks"]) @@ -281,13 +369,11 @@ def test_bound_tool_result_wired_for_claude_cursor_and_codex(self): self.assertEqual(len(claude_entries), 1, claude_entries) self.assertEqual(claude_entries[0]["matcher"], "Bash") - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] cursor_commands = [str(entry.get("command", "")) for entry in cursor_hooks["preToolUse"]] self.assertEqual(sum(f"{name}/cursor_pre_tool_use.py" in c for c in cursor_commands), 1, cursor_commands) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_commands = [hook["command"] for entry in codex_hooks["PreToolUse"] for hook in entry["hooks"]] self.assertEqual(sum(f"{name}/codex_pre_tool_use.py" in c for c in codex_commands), 1, codex_commands) @@ -298,8 +384,7 @@ def test_text_match_decision_warn_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, name)) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any(f"{name}/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -308,19 +393,16 @@ def test_text_match_decision_warn_wired_for_claude_cursor_and_codex(self): self.assertEqual(claude_entries[0]["matcher"], "Edit|Write|MultiEdit") self.assertIn("_runner/run.py", claude_entries[0]["hooks"][0]["command"]) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] cursor_commands = [str(entry.get("command", "")) for entry in cursor_hooks["preToolUse"]] self.assertEqual(sum(f"{name}/cursor_pretooluse.py" in c for c in cursor_commands), 1, cursor_commands) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_commands = [hook["command"] for entry in codex_hooks["PreToolUse"] for hook in entry["hooks"]] self.assertEqual(sum(f"{name}/codex_pretooluse.py" in c for c in codex_commands), 1, codex_commands) def _claude_hook_commands(self, event): - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) return [h["command"] for e in settings["hooks"][event] for h in e["hooks"]] def test_restated_constraint_linked_and_prompt_submit_wired_for_claude(self): @@ -333,8 +415,7 @@ def test_wait_needs_wakeup_linked_and_both_events_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "wait-needs-wakeup") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "wait-needs-wakeup")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) pre = [h["command"] for e in settings["hooks"]["PreToolUse"] for h in e["hooks"]] stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("wait-needs-wakeup/claude_pretooluse.py" in c for c in pre), pre) @@ -351,8 +432,7 @@ def test_answer_overrides_menu_linked_and_posttooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "answer-overrides-menu") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "answer-overrides-menu")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PostToolUse"] if any("answer-overrides-menu/claude_posttooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -364,8 +444,7 @@ def test_serial_option_guard_linked_and_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "serial-option-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "serial-option-guard")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("serial-option-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -409,8 +488,7 @@ def test_history_before_reversal_linked_and_bash_pretooluse_wired_for_claude(sel target = os.path.join(self.fake_home, ".claude", "hooks", "history-before-reversal") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "history-before-reversal")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("history-before-reversal/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -422,8 +500,7 @@ def test_external_claim_gate_linked_and_bash_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "external-claim-gate") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "external-claim-gate")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("external-claim-gate/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -450,8 +527,7 @@ def test_publish_act_guard_linked_and_bash_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "publish-act-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "publish-act-guard")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("publish-act-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -463,8 +539,7 @@ def test_retired_agent_routing_guard_is_gone(self): self.assertFalse( os.path.exists(os.path.join(self.fake_home, ".claude", "hooks", "agent-routing-guard")) ) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stale = [ entry for entry in settings["hooks"]["PreToolUse"] if any("agent-routing-guard" in hook["command"] for hook in entry["hooks"]) @@ -475,8 +550,7 @@ def test_categorical_scope_guard_linked_and_bash_pretooluse_wired_for_claude(sel target = os.path.join(self.fake_home, ".claude", "hooks", "categorical-scope-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "categorical-scope-guard")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("categorical-scope-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -497,8 +571,7 @@ def test_hedge_runs_prove_it_linked_and_stop_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "hedge-runs-prove-it") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "hedge-runs-prove-it")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("hedge-runs-prove-it/claude_stop_check.py" in c for c in stop), stop) @@ -533,8 +606,7 @@ def test_new_file_callout_linked_and_stop_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "new-file-callout") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "new-file-callout")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("new-file-callout/claude_stop_check.py" in c for c in stop), stop) @@ -542,8 +614,7 @@ def test_agent_relay_attribution_linked_and_stop_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "agent-relay-attribution") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "agent-relay-attribution")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("agent-relay-attribution/claude_stop_check.py" in c for c in stop), stop) @@ -551,8 +622,7 @@ def test_scratchpad_collision_linked_and_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "scratchpad-collision") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "scratchpad-collision")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) pre = [h["command"] for e in settings["hooks"]["PreToolUse"] for h in e["hooks"]] self.assertTrue(any("scratchpad-collision/claude_pretooluse.py" in c for c in pre), pre) @@ -565,13 +635,11 @@ def test_reflect_on_thrash_wired_for_claude_and_cursor(self): hook_src(self.fake_home, "reflect-on-thrash"), ) settings_path = os.path.join(self.fake_home, ".claude", "settings.json") - with open(settings_path) as handle: - settings = json.load(handle) + settings = load_config(settings_path) stop_commands = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("claude_stop_reflect.py" in c for c in stop_commands), stop_commands) cursor_path = os.path.join(self.fake_home, ".cursor", "hooks.json") - with open(cursor_path) as handle: - cursor_hooks = json.load(handle) + cursor_hooks = load_config(cursor_path) stop = cursor_hooks["hooks"]["stop"] session_end = cursor_hooks["hooks"]["sessionEnd"] self.assertTrue(any("cursor_session.py" in str(e.get("command", "")) for e in stop), stop) @@ -619,8 +687,7 @@ def test_cat_mode_default_linked_and_prompt_hook_wired_for_claude(self): for agent_dir in (".cursor", ".codex"): self.assertFalse(os.path.lexists(os.path.join(self.fake_home, agent_dir, "hooks", "cat-mode-default"))) settings_path = os.path.join(self.fake_home, ".claude", "settings.json") - with open(settings_path) as handle: - settings = json.load(handle) + settings = load_config(settings_path) prompt_commands = [h["command"] for e in settings["hooks"]["UserPromptSubmit"] for h in e["hooks"]] matching = [c for c in prompt_commands if "cat-mode-default/claude_prompt_submit.py" in c] self.assertEqual(len(matching), 1, prompt_commands) @@ -632,8 +699,7 @@ def test_scope_lock_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "scope-lock")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) prompt_commands = [ hook["command"] for entry in settings["hooks"]["UserPromptSubmit"] @@ -647,8 +713,7 @@ def test_scope_lock_wired_for_claude_cursor_and_codex(self): self.assertTrue(any("claude_prompt_scope.py" in command for command in prompt_commands)) self.assertTrue(any("claude_pretool_scope.py" in command for command in pretool_commands)) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] self.assertTrue(any( "cursor_before_submit.py" in str(entry.get("command", "")) for entry in cursor_hooks["beforeSubmitPrompt"] @@ -658,8 +723,7 @@ def test_scope_lock_wired_for_claude_cursor_and_codex(self): for entry in cursor_hooks["preToolUse"] )) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_prompt_commands = [ hook["command"] for entry in codex_hooks["UserPromptSubmit"] @@ -679,8 +743,7 @@ def test_split_scope_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "split-scope")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_prompt_commands = [ hook["command"] for entry in settings["hooks"]["UserPromptSubmit"] @@ -688,8 +751,7 @@ def test_split_scope_wired_for_claude_cursor_and_codex(self): ] self.assertTrue(any("split-scope/claude_prompt_submit.py" in command for command in claude_prompt_commands)) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] self.assertTrue(any( "split-scope/cursor_before_submit.py" in str(entry.get("command", "")) for entry in cursor_hooks["beforeSubmitPrompt"] @@ -699,8 +761,7 @@ def test_split_scope_wired_for_claude_cursor_and_codex(self): for entry in cursor_hooks["postToolUse"] )) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_prompt_commands = [ hook["command"] for entry in codex_hooks["UserPromptSubmit"] @@ -714,8 +775,7 @@ def test_hook_health_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "hook-health")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_prompt_commands = [ hook["command"] for entry in settings["hooks"]["UserPromptSubmit"] @@ -723,15 +783,13 @@ def test_hook_health_wired_for_claude_cursor_and_codex(self): ] self.assertTrue(any("hook-health/claude_prompt_submit.py" in command for command in claude_prompt_commands)) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] self.assertTrue(any( "hook-health/cursor_before_submit.py" in str(entry.get("command", "")) for entry in cursor_hooks["beforeSubmitPrompt"] )) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_prompt_commands = [ hook["command"] for entry in codex_hooks["UserPromptSubmit"] @@ -746,12 +804,9 @@ def test_skill_usage_log_is_linked_but_not_wired_while_registry_mode_is_off(self self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "skill-usage-log")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - claude_hooks = json.load(handle)["hooks"] - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + claude_hooks = load_config(os.path.join(self.fake_home, ".claude", "settings.json"))["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] expected = ( (claude_hooks, "PreToolUse", "skill-usage-log/claude_pretooluse_log.py"), (claude_hooks, "UserPromptSubmit", "skill-usage-log/claude_prompt_submit.py"), @@ -780,8 +835,7 @@ def test_llm_judge_inbox_wired_for_claude_cursor_and_codex(self): env={**os.environ, "HOME": self.fake_home, "CATSTACK_LLM_JUDGE_STATE_DIR": os.path.join(self.fake_home, "judge")}, ) self.assertEqual((result.returncode, result.stdout, result.stderr), (0, "", "")) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_stop = json.load(handle)["hooks"]["stop"] + cursor_stop = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"]["stop"] self.assertEqual(sum("llm-judge/cursor_session.py" in str(e.get("command", "")) for e in cursor_stop), 1, cursor_stop) def test_unverified_tag_check_is_linked_but_not_wired_while_registry_mode_is_off(self): @@ -797,8 +851,7 @@ def test_unverified_tag_check_is_linked_but_not_wired_while_registry_mode_is_off self.assertEqual(os.readlink(target), hook_src(self.fake_home, "unverified-tag-check")) claude_stop = self._claude_hook_commands("Stop") self.assertEqual(sum("unverified-tag-check/claude_stop_check.py" in command for command in claude_stop), 0, claude_stop) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_stop = json.load(handle)["hooks"]["stop"] + cursor_stop = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"]["stop"] self.assertEqual(sum("unverified-tag-check/cursor_session.py" in str(entry.get("command", "")) for entry in cursor_stop), 0, cursor_stop) with open(config_path) as handle: text = handle.read() @@ -1221,8 +1274,7 @@ def test_fresh_home_gets_both_hooks(self): run_install(fake_home) settings_path = os.path.join(fake_home, ".claude", "settings.json") self.assertTrue(os.path.exists(settings_path)) - with open(settings_path) as f: - settings = json.load(f) + settings = load_config(settings_path) stop_commands = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] prompt_commands = [h["command"] for e in settings["hooks"]["UserPromptSubmit"] for h in e["hooks"]] self.assertTrue(any("claude_stop_check.py" in c for c in stop_commands)) @@ -1236,8 +1288,7 @@ def test_preexisting_unrelated_keys_survive(self): with open(os.path.join(claude_dir, "settings.json"), "w") as f: json.dump({"model": "sonnet", "theme": "dark"}, f) run_install(fake_home) - with open(os.path.join(claude_dir, "settings.json")) as f: - settings = json.load(f) + settings = load_config(os.path.join(claude_dir, "settings.json")) self.assertEqual(settings["model"], "sonnet") self.assertEqual(settings["theme"], "dark") self.assertIn("Stop", settings["hooks"]) @@ -1272,8 +1323,7 @@ def test_install_writes_worker_flagged_hooks_only(self): with open(worker_settings_path, encoding="utf-8") as handle: worker = json.load(handle) - with open(os.path.join(fake_home, ".claude", "settings.json"), encoding="utf-8") as handle: - interactive = json.load(handle) + interactive = load_config(os.path.join(fake_home, ".claude", "settings.json")) self.assertEqual(worker.get("model"), "keep-me") worker_cmds = self._hook_commands(worker) @@ -1314,9 +1364,8 @@ def test_preseeded_config_toml_survives_without_notify_wiring(self): text = f.read() self.assertIn('model = "gpt-5"', text) self.assertNotIn("notify =", text) - with open(os.path.join(codex_dir, "hooks.json")) as f: - hooks_text = f.read() - self.assertIn("llm-judge/codex_post_tool_use.py", hooks_text) + hooks_data = load_config(os.path.join(codex_dir, "hooks.json")) + self.assertIn("llm-judge/codex_post_tool_use.py", json.dumps(hooks_data)) class TestIdempotency(unittest.TestCase): @@ -1591,8 +1640,7 @@ def test_settings_entry_for_a_removed_link_is_pruned(self): json.dump({"hooks": {"Stop": [{"matcher": "", "hooks": [{"type": "command", "command": retired}]}]}}, f) result = run_install(self.fake_home) self.assertEqual(result.returncode, 0, result.stderr) - with open(settings_path) as f: - settings = json.load(f) + settings = load_config(settings_path) all_commands = [h["command"] for groups in settings["hooks"].values() for g in groups for h in g["hooks"]] self.assertFalse([c for c in all_commands if "retired-hook" in c], all_commands) @@ -1630,8 +1678,7 @@ def test_dead_settings_entry_is_pruned_and_rest_survives(self): }, f) result = run_install(self.fake_home) self.assertEqual(result.returncode, 0, result.stderr) - with open(settings_path) as f: - settings = json.load(f) + settings = load_config(settings_path) all_commands = [h["command"] for groups in settings["hooks"].values() for g in groups for h in g["hooks"]] self.assertNotIn(dead, all_commands) self.assertTrue(any("diu-stop/claude_stop_check.py" in c for c in all_commands)) @@ -1656,8 +1703,7 @@ def tearDown(self): def test_every_claude_hook_entrypoint_is_in_settings(self): settings_path = os.path.join(self.fake_home, ".claude", "settings.json") - with open(settings_path) as handle: - settings = json.load(handle) + settings = load_config(settings_path) wired = { hook["command"] for entries in settings.get("hooks", {}).values() @@ -1809,8 +1855,7 @@ def setUp(self): self.tmp = tempfile.TemporaryDirectory() self.fake_home = self.tmp.name self.result = run_install(self.fake_home) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - self.settings = json.load(handle) + self.settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) sys.path.insert(0, os.path.join(REPO_ROOT, "scripts", "install")) import mirror_stop_hooks_to_subagent_stop as mirror_mod self.manifests = mirror_mod.load_manifests() @@ -1847,8 +1892,8 @@ def test_subagent_stop_entries_carry_no_tool_matcher(self): def test_rerun_keeps_subagent_stop_unchanged(self): second = run_install(self.fake_home) self.assertEqual(second.returncode, 0, second.stderr) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - self.assertEqual(json.load(handle)["hooks"]["SubagentStop"], self.settings["hooks"]["SubagentStop"]) + after = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) + self.assertEqual(after["hooks"]["SubagentStop"], self.settings["hooks"]["SubagentStop"]) class TestCatModeDefaultAgentHook(unittest.TestCase): @@ -1859,8 +1904,7 @@ def test_agent_pretooluse_hook_wired_for_claude(self): with tempfile.TemporaryDirectory() as fake_home: result = run_install(fake_home) self.assertEqual(result.returncode, 0, result.stderr) - with open(os.path.join(fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("cat-mode-default/claude_pretooluse_agent.py" in hook["command"] for hook in entry["hooks"]) @@ -1877,8 +1921,7 @@ def test_fanout_routing_guard_linked_and_wired_for_claude(self): target = os.path.join(fake_home, ".claude", "hooks", "fanout-routing-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(fake_home, "fanout-routing-guard")) - with open(os.path.join(fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("fanout-routing-guard/claude_pretooluse_agent.py" in hook["command"] for hook in entry["hooks"]) @@ -1905,8 +1948,7 @@ def test_install_replaces_dangling_link_with_real_merged_file(self): self.assertEqual(proc.returncode, 0, proc.stderr) self.assertTrue(os.path.isfile(hooks_path)) self.assertFalse(os.path.islink(hooks_path)) - with open(hooks_path) as handle: - data = json.load(handle) + data = load_config(hooks_path) commands = [entry.get("command", "") for entry in data["hooks"]["stop"]] self.assertTrue(any("diu-stop/cursor_stop_check.py" in c for c in commands), commands) self.assertTrue(all("_runner/run.py" in c for c in commands), commands) @@ -2189,12 +2231,28 @@ def _cursor_hooks(self, fake_home): return json.load(handle) def test_default_is_off_and_byte_identical_to_no_flag_at_all(self): - with tempfile.TemporaryDirectory() as plain_home, tempfile.TemporaryDirectory() as off_home: - plain = run_install(plain_home, timeout=self.INSTALL_TIMEOUT) + """The default install (CATSTACK_HOOK_DISPATCHER unset) must equal an + install that explicitly sets the flag to whatever `flags.py` itself + resolves as the default -- today off, so this reduces to the + flag-off install, but it stays meaningful if the default ever flips + to on instead of silently comparing against the wrong side. The + ambient environment is cleared of the key first, so a caller that + happens to have CATSTACK_HOOK_DISPATCHER set (for example the + flag-on run of the full suite) cannot leak into the "default" side + of this comparison.""" + clean_env = {k: v for k, v in os.environ.items() if k != _flags.HOOK_DISPATCHER} + default_on = _flags.hook_dispatcher_on(clean_env, REPO_ROOT) + with tempfile.TemporaryDirectory() as plain_home, tempfile.TemporaryDirectory() as explicit_home: + with mock.patch.dict(os.environ, clean_env, clear=True): + plain = run_install(plain_home, timeout=self.INSTALL_TIMEOUT) self.assertEqual(plain.returncode, 0, plain.stderr) - off = run_install(off_home, extra_env={"CATSTACK_HOOK_DISPATCHER": "0"}, timeout=self.INSTALL_TIMEOUT) - self.assertEqual(off.returncode, 0, off.stderr) - self.assertEqual(self._claude_settings(plain_home), self._claude_settings(off_home)) + explicit = run_install( + explicit_home, + extra_env={_flags.HOOK_DISPATCHER: "1" if default_on else "0"}, + timeout=self.INSTALL_TIMEOUT, + ) + self.assertEqual(explicit.returncode, 0, explicit.stderr) + self.assertEqual(self._claude_settings(plain_home), self._claude_settings(explicit_home)) def test_on_collapses_claude_pretooluse_to_one_dispatcher_entry(self): with tempfile.TemporaryDirectory() as fake_home: From 1b46a91cadcb9c96ca915f5886077253ef6a36bb Mon Sep 17 00:00:00 2001 From: Invoker Bot Date: Sun, 11 Oct 2026 14:43:08 +0000 Subject: [PATCH 06/10] =?UTF-8?q?invoker:=20wf-1791728053338-10/publish-bo?= =?UTF-8?q?th-layout-install-tests=20=E2=80=94=20Review=20claim:=20tests.t?= =?UTF-8?q?est=5Finstall=20and=20test=5Fwrap=5Finstalled=20accept=20both?= =?UTF-8?q?=20install=20layouts=20(one=20entry=20per=20hook=20with=20CATST?= =?UTF-8?q?ACK=5FHOOK=5FDISPATCHER=20off,=20one=20dispatcher=20entry=20per?= =?UTF-8?q?=20event=20with=20it=20on),=20and=20the=20flag=20defaults=20to?= =?UTF-8?q?=20on=20only=20if=20bash=20scripts/test/run=5Fall=5Ftests.sh=20?= =?UTF-8?q?passes=20with=20the=20flag=20on=20and=20off.=20Review=20lane:?= =?UTF-8?q?=20behavior=20Safety=20invariant:=20If=20either=20suite=20run?= =?UTF-8?q?=20fails,=20the=20default=20stays=20off.=20Per-hook=20guarantee?= =?UTF-8?q?s=20stay=20checked=20in=20the=20dispatcher=20layout.=20Files:?= =?UTF-8?q?=20tests/test=5Finstall.py,=20tests/test=5Fwrap=5Finstalled.py?= =?UTF-8?q?=20Change=20types:=20test=20Acceptance=20criteria:=20This=20wor?= =?UTF-8?q?kflow=20publishes=20one=20pull=20request=20for=20the=20install-?= =?UTF-8?q?layout=20claim=20only.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exit code: 0 Invoker-Finalize-Id: f459f9c9-a623-4fd7-9708-21cc3ae4cbda From af5796b321ae23b75b2a3f37ad58fd31a8f2c51e Mon Sep 17 00:00:00 2001 From: CI Bot Date: Sun, 11 Oct 2026 15:14:36 +0000 Subject: [PATCH 07/10] Print a before/after hook scorecard from the metrics ledger `report.py --scorecard` folds runs.jsonl into one row per (phase, event type) and prints procs/event, cpu_ms/event, wall_ms/event, timeout%, crash% and spoke-rate. Rows are split into "before" and "after" by whether the row carries a `dispatch_path`, and grouped into events by `event_uid` (falling back to the row index when a row has no uid), so one harness event counts once no matter how many detectors ran under it. The phase split and the proc count read the row field the runner actually writes, `dispatch_path`. An earlier draft of this fold read `dispatch_mode`, which no row has: every post-dispatcher row then landed in the "before" bucket and the after-phase proc count never credited the dispatcher, so the scorecard showed no change at all. `--days N` is accepted alongside `--since` so the canary window can be given in whole days. Tests: two cases in engine/hooks/_runner/tests/test_report.py drive the CLI over a fixture ledger holding one pre- and one post-dispatcher event; they assert the per-phase numbers and the printed column header. Both fail against the base report.py with "unrecognized arguments: --scorecard --days 14". Co-Authored-By: Claude Opus 5 (1M context) --- engine/hooks/_runner/report.py | 125 +++++++++++++++++++++- engine/hooks/_runner/tests/test_report.py | 82 ++++++++++++++ 2 files changed, 206 insertions(+), 1 deletion(-) diff --git a/engine/hooks/_runner/report.py b/engine/hooks/_runner/report.py index 7ea63f7ac..c04dedd3e 100644 --- a/engine/hooks/_runner/report.py +++ b/engine/hooks/_runner/report.py @@ -227,6 +227,12 @@ def format_rate(value: float | None) -> str: return f"{value:.2f}" +def format_decimal(value: float | None) -> str: + if value is None: + return "-" + return f"{value:.1f}" + + def first_line(value: object) -> str: if not isinstance(value, str): return "" @@ -722,9 +728,111 @@ def format_harness_gap_table(report: dict[str, Any]) -> str: return "\n".join(lines) + "\n" +def _scorecard_phase(row: dict[str, Any]) -> str: + dispatch_path = row.get("dispatch_path") + return "after" if isinstance(dispatch_path, str) and dispatch_path else "before" + + +def _duration_ms(row: dict[str, Any]) -> int | None: + duration = row.get("duration_ms") + if isinstance(duration, int) and not isinstance(duration, bool): + return duration + return None + + +def _event_group_key(row: dict[str, Any], index: int) -> tuple[str, str, str, str]: + phase = _scorecard_phase(row) + event = str(row.get("event") or "(unknown)") + session_id = str(row.get("session_id") or "") + event_uid = row.get("event_uid") + uid = str(event_uid) if isinstance(event_uid, str) and event_uid else f"row-{index}" + return phase, event, session_id, uid + + +def _group_wall_ms(rows: list[dict[str, Any]]) -> int: + starts = [] + ends = [] + fallback_total = 0 + for row in rows: + duration = _duration_ms(row) or 0 + fallback_total += duration + end = parse_ts(row.get("ts")) + if end is None: + continue + starts.append(end - timedelta(milliseconds=duration)) + ends.append(end) + if starts and ends: + return max(0, int((max(ends) - min(starts)).total_seconds() * 1000)) + return fallback_total + + +def _group_procs(rows: list[dict[str, Any]], phase: str) -> int: + if phase == "after": + fallback = sum(1 for row in rows if row.get("dispatch_path") == "subprocess_fallback") + return 1 + fallback + return len(rows) + + +def build_scorecard(rows: list[dict[str, Any]], malformed: int, warnings: list[str]) -> dict[str, Any]: + groups: dict[tuple[str, str, str, str], list[dict[str, Any]]] = {} + for index, row in enumerate(rows): + groups.setdefault(_event_group_key(row, index), []).append(row) + + event_groups: dict[tuple[str, str], list[list[dict[str, Any]]]] = {} + for (phase, event, _session_id, _uid), group_rows in groups.items(): + event_groups.setdefault((phase, event), []).append(group_rows) + + scorecard_rows = [] + for (phase, event), grouped_rows in sorted(event_groups.items()): + flat = [row for group_rows in grouped_rows for row in group_rows] + detector_runs = len(flat) + events = len(grouped_rows) + procs = sum(_group_procs(group_rows, phase) for group_rows in grouped_rows) + cpu_ms = sum(_duration_ms(row) or 0 for row in flat) + wall_ms = sum(_group_wall_ms(group_rows) for group_rows in grouped_rows) + timeouts = sum(1 for row in flat if row.get("outcome") == "timed_out") + crashes = sum(1 for row in flat if row.get("outcome") in {"crashed", "caught_error"}) + spoke = sum(1 for row in flat if row.get("outcome") == "spoke") + scorecard_rows.append( + { + "phase": phase, + "event": event, + "events": events, + "detector_runs": detector_runs, + "procs_per_event": procs / events if events else None, + "cpu_ms_per_event": cpu_ms / events if events else None, + "wall_ms_per_event": wall_ms / events if events else None, + "timeout_rate": timeouts / detector_runs if detector_runs else None, + "crash_rate": crashes / detector_runs if detector_runs else None, + "spoke_rate": spoke / detector_runs if detector_runs else None, + } + ) + return {"malformed_rows": malformed, "warnings": warnings, "scorecard": scorecard_rows} + + +def format_scorecard_table(report: dict[str, Any]) -> str: + lines = list(report["warnings"]) + if report["malformed_rows"]: + lines.append(f"skipped {report['malformed_rows']} malformed row(s)") + lines.append( + "phase event events detector_runs procs/event cpu_ms/event wall_ms/event timeout% crash% spoke-rate" + ) + for row in report["scorecard"]: + timeout_pct = row["timeout_rate"] * 100 if row["timeout_rate"] is not None else None + crash_pct = row["crash_rate"] * 100 if row["crash_rate"] is not None else None + lines.append( + f"{row['phase']} {row['event']} {row['events']} {row['detector_runs']} " + f"{format_decimal(row['procs_per_event'])} {format_decimal(row['cpu_ms_per_event'])} " + f"{format_decimal(row['wall_ms_per_event'])} {format_decimal(timeout_pct)} " + f"{format_decimal(crash_pct)} {format_rate(row['spoke_rate'])}" + ) + return "\n".join(lines) + "\n" + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser() parser.add_argument("--since", default="7d") + parser.add_argument("--days", type=float) parser.add_argument("--json", action="store_true") mode = parser.add_mutually_exclusive_group() mode.add_argument("--events", action="store_true", default=True) @@ -732,15 +840,30 @@ def main(argv: list[str] | None = None) -> int: mode.add_argument("--judge", action="store_true") mode.add_argument("--skills", action="store_true") mode.add_argument("--harness-gap", action="store_true") + mode.add_argument("--scorecard", action="store_true") parser.add_argument("--grace", default="1h") parser.add_argument("--check", action="store_true") args = parser.parse_args(argv) try: - since = parse_since(args.since) + since = timedelta(days=args.days) if args.days is not None else parse_since(args.since) grace = parse_since(args.grace) except ValueError as exc: print(str(exc), file=sys.stderr) return 2 + if args.days is not None and args.days <= 0: + print("--days must be positive", file=sys.stderr) + return 2 + if args.scorecard: + rows, malformed, error = read_rows(metrics_path(), datetime.now(timezone.utc) - since) + if error is not None: + print(error) + return 2 + report = build_scorecard(rows or [], malformed, []) + if args.json: + print(json.dumps(report, sort_keys=True)) + else: + print(format_scorecard_table(report), end="") + return 0 if args.harness_gap: threshold = datetime.now(timezone.utc) - since home = Path(os.path.expanduser("~")) diff --git a/engine/hooks/_runner/tests/test_report.py b/engine/hooks/_runner/tests/test_report.py index 909ab3731..e5c4609fc 100644 --- a/engine/hooks/_runner/tests/test_report.py +++ b/engine/hooks/_runner/tests/test_report.py @@ -382,6 +382,88 @@ def test_runs_report_counts_rows_across_rotated_files(self) -> None: self.assertEqual(result.returncode, 0, result.stderr) self.assertIn("claude hook-a/a.py 3 1 0 0 1 0 1 10", result.stdout) + def test_scorecard_compares_before_and_after_dispatch_path_rows(self) -> None: + self.write_rows( + [ + self.row( + "claude", + "hook-a", + "a.py", + "spoke", + event="SubagentStop", + event_uid="pre", + duration_ms=10, + ), + self.row( + "claude", + "hook-b", + "b.py", + "timed_out", + event="SubagentStop", + event_uid="pre", + duration_ms=20, + ), + self.row( + "claude", + "hook-a", + "a.py", + "spoke", + event="SubagentStop", + event_uid="post", + duration_ms=5, + dispatch_path="sdk", + ), + self.row( + "claude", + "hook-b", + "b.py", + "crashed", + event="SubagentStop", + event_uid="post", + duration_ms=7, + dispatch_path="subprocess_fallback", + ), + ] + ) + + result = self.run_report("--scorecard", "--days", "14", "--json") + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + rows = {(row["phase"], row["event"]): row for row in json.loads(result.stdout)["scorecard"]} + self.assertEqual(rows[("before", "SubagentStop")]["events"], 1) + self.assertEqual(rows[("before", "SubagentStop")]["detector_runs"], 2) + self.assertEqual(rows[("before", "SubagentStop")]["procs_per_event"], 2) + self.assertEqual(rows[("before", "SubagentStop")]["cpu_ms_per_event"], 30) + self.assertEqual(rows[("before", "SubagentStop")]["timeout_rate"], 0.5) + self.assertEqual(rows[("after", "SubagentStop")]["events"], 1) + self.assertEqual(rows[("after", "SubagentStop")]["detector_runs"], 2) + self.assertEqual(rows[("after", "SubagentStop")]["procs_per_event"], 2) + self.assertEqual(rows[("after", "SubagentStop")]["cpu_ms_per_event"], 12) + self.assertEqual(rows[("after", "SubagentStop")]["crash_rate"], 0.5) + + def test_scorecard_prints_dashboard_columns(self) -> None: + self.write_rows( + [ + self.row("claude", "hook-a", "a.py", "spoke", event="SubagentStop", event_uid="pre"), + self.row( + "claude", + "hook-a", + "a.py", + "spoke", + event="SubagentStop", + event_uid="post", + dispatch_path="sdk", + ), + ] + ) + + result = self.run_report("--scorecard", "--days", "14") + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("phase event events detector_runs procs/event cpu_ms/event wall_ms/event timeout% crash% spoke-rate", result.stdout) + self.assertIn("before SubagentStop 1 1 1.0 10.0", result.stdout) + self.assertIn("after SubagentStop 1 1 1.0 10.0", result.stdout) + def test_event_report_suggests_each_mode_change(self) -> None: rows: list[dict[str, object]] = [] rows += self.closed_events( From 923250d976d2f56989c142cd5ca6b601891295f7 Mon Sep 17 00:00:00 2001 From: Invoker Bot Date: Sun, 11 Oct 2026 15:16:24 +0000 Subject: [PATCH 08/10] =?UTF-8?q?invoker:=20wf-1791728212537-13/publish-sc?= =?UTF-8?q?orecard=20=E2=80=94=20Review=20claim:=20report.py=20--html=20fo?= =?UTF-8?q?lds=20runs.jsonl=20and=20events=20jsonl=20once=20into=20summary?= =?UTF-8?q?.json=20and=20emits=20a=20static=20dashboard.html=20on=20demand?= =?UTF-8?q?,=20with=20no=20server=20and=20no=20change=20to=20any=20hook=20?= =?UTF-8?q?behavior.=20Review=20lane:=20behavior=20Safety=20invariant:=20H?= =?UTF-8?q?ook=20stdout=20and=20exit=20codes=20stay=20the=20same.=20This?= =?UTF-8?q?=20slice=20does=20not=20change=20the=20other=20four=20claims.?= =?UTF-8?q?=20Files:=20engine/hooks/=5Frunner/report.py=20Change=20types:?= =?UTF-8?q?=20bugfix=20Acceptance=20criteria:=20The=20branch=20contains=20?= =?UTF-8?q?only=20this=20claim,=20and=20the=20review=20step=20publishes=20?= =?UTF-8?q?one=20pull=20request.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exit code: 0 Invoker-Finalize-Id: e5a0edf3-e934-4a17-8242-75988f61e013 From 005c4768fa6e90477bcea3edfb7d1b21a16adc18 Mon Sep 17 00:00:00 2001 From: Invoker Bot Date: Thu, 1 Oct 2026 11:56:12 +0000 Subject: [PATCH 09/10] Run the SubagentStop hook fleet through one dispatcher process. The fleet self-saturates: one SubagentStop burst spawned 15-30 processes that each re-parsed the same transcript, measured at 15.1s p50 wall idle against 1.55s for a single process, with per-hook durations escalating in launch order. dispatch.py now runs opted-in detectors in-process, sequentially, parsing the event once and passing the same parsed payload to every detector. A hook opts in with subagent_stop.dispatch in its manifest, naming a MODULE:FUNCTION entry point (default detect:detect). A detector with no resolvable entry point still runs as a subprocess and its metrics row is marked subprocess_fallback. Each detector keeps its own verdict, its own runs.jsonl row under its own hook name, and its own rule_ids; a detector that raises, exits, or overruns its 4s soft budget is caught and recorded without stopping its siblings. The event budget defaults to 9s for SubagentStop. mirror_stop_hooks_to_subagent_stop.py emits one dispatcher entry for the opted-in hooks and leaves every other hook's direct entry alone. (cherry picked from commit 531f5c86 on plan/hook-fleet-dispatcher-measure-the-herd-then-dispatch-it) --- engine/hooks/_runner/dispatch.py | 207 +++++++++++++++++- engine/hooks/_runner/tests/test_dispatch.py | 106 +++++++-- .../mirror_stop_hooks_to_subagent_stop.py | 19 +- tests/test_install.py | 15 +- tests/test_mirror_subagent_stop.py | 22 +- 5 files changed, 339 insertions(+), 30 deletions(-) diff --git a/engine/hooks/_runner/dispatch.py b/engine/hooks/_runner/dispatch.py index 7f1dd37f2..35743450d 100644 --- a/engine/hooks/_runner/dispatch.py +++ b/engine/hooks/_runner/dispatch.py @@ -1,11 +1,12 @@ from __future__ import annotations import argparse -import concurrent.futures import glob +import importlib.util import json import os import re +import signal import subprocess import sys import time @@ -16,7 +17,11 @@ import run DEFAULT_BUDGET = 59.5 +DEFAULT_SUBAGENT_STOP_EVENT_BUDGET = 9.0 +DEFAULT_SUBAGENT_STOP_DETECTOR_BUDGET = 4.0 OPT_OUT_KEY = "subagent_stop" +DISPATCH_KEY = "dispatch" +DEFAULT_DISPATCH_ENTRY = "detect:detect" MIRRORED_EVENTS = {"SubagentStop": "Stop"} DIRECT_RE = re.compile(r"^python3 \$HOME/\.(claude|cursor|codex)/hooks/([^/\s]+)/([^/\s]+\.py)((?:\s+.*)?)$") MESSAGE_KEYS = ("reason", "message", "additionalContext", "additional_context", "user_message") @@ -157,6 +162,11 @@ def _load_manifest_hooks( opt_out = manifest.get(OPT_OUT_KEY) if isinstance(opt_out, dict) and opt_out.get("inherit") is False: continue + dispatch_entry = _dispatch_entry(manifest) + if dispatch_entry is None: + continue + else: + dispatch_entry = None for entry in entries: matcher = None if not mirrored and isinstance(entry, dict): @@ -178,11 +188,26 @@ def _load_manifest_hooks( "args": list(args), "timeout": hook.get("timeout"), "matcher": matcher, + "dispatch_entry": dispatch_entry, } ) return records, warnings +def _dispatch_entry(manifest: dict) -> str | None: + config = manifest.get(OPT_OUT_KEY) + if not isinstance(config, dict): + return None + raw = config.get(DISPATCH_KEY) + if raw is True: + return DEFAULT_DISPATCH_ENTRY + if isinstance(raw, dict): + entry = raw.get("entry") + if isinstance(entry, str) and entry.strip(): + return entry.strip() + return None + + def _parse_command(command: str, harness: str) -> tuple[str, str, str] | None: match = DIRECT_RE.fullmatch(command) if not match or match.group(1) != harness: @@ -211,7 +236,22 @@ def _payload(stdin: bytes) -> dict[str, object]: return parsed if isinstance(parsed, dict) else {} -def _run_one(hooks_root: str, python: str, record: dict, stdin: bytes, budget: float) -> dict: +def _run_one( + hooks_root: str, + python: str, + record: dict, + stdin: bytes, + budget: float, + parsed_event: dict[str, object] | None = None, +) -> dict: + if record.get("dispatch_entry") and _entrypoint_exists(hooks_root, record["hook"], str(record["dispatch_entry"])): + return _run_sdk_one(hooks_root, record, stdin, budget, parsed_event) + return _run_subprocess_one(hooks_root, python, record, stdin, budget, fallback=True) + + +def _run_subprocess_one( + hooks_root: str, python: str, record: dict, stdin: bytes, budget: float, fallback: bool = False +) -> dict: hook, script = record["hook"], record["script"] started = time.monotonic() script_path = os.path.join(hooks_root, hook, script) @@ -249,6 +289,8 @@ def _run_one(hooks_root: str, python: str, record: dict, stdin: bytes, budget: f run._delete_findings_file(findings_path) result_outcome = outcome.classify(exit_code, stdout, stderr, timed_out) row = run._row(hooks_root, hook, script, stdin, result_outcome, exit_code, started, stdout, stderr, rule_ids) + if fallback: + row["dispatch_path"] = "subprocess_fallback" return { "hook": hook, "script": script, @@ -260,12 +302,158 @@ def _run_one(hooks_root: str, python: str, record: dict, stdin: bytes, budget: f } +def _sdk_dirs(hooks_root: str) -> tuple[str, str]: + engine_hooks = os.path.abspath(hooks_root) + sdk_dir = os.path.join(engine_hooks, "_sdk") + return engine_hooks, sdk_dir + + +def _prefer_import_path(path: str, index: int) -> None: + try: + sys.path.remove(path) + except ValueError: + pass + sys.path.insert(index, path) + + +def _load_entrypoint(hooks_root: str, hook: str, entry: str): + module_name, sep, function_name = entry.partition(":") + if not sep or not module_name or not function_name: + raise ValueError(f"dispatch entry must be MODULE:FUNCTION, got {entry!r}") + module_path = _entrypoint_path(hooks_root, hook, module_name) + if not os.path.isfile(module_path): + raise FileNotFoundError(module_path) + _engine_hooks, sdk_dir = _sdk_dirs(hooks_root) + _prefer_import_path(sdk_dir, 0) + _prefer_import_path(os.path.dirname(module_path), 1) + unique_name = f"_catstack_dispatch_{hook.replace('-', '_')}_{abs(hash(module_path))}" + spec = importlib.util.spec_from_file_location(unique_name, module_path) + if spec is None or spec.loader is None: + raise ImportError(f"could not load {module_path}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + target = getattr(module, function_name) + if not callable(target): + raise TypeError(f"{entry!r} did not resolve to a callable") + return target + + +def _entrypoint_path(hooks_root: str, hook: str, module_name: str) -> str: + return os.path.join(hooks_root, hook, *module_name.split(".")) + ".py" + + +def _entrypoint_exists(hooks_root: str, hook: str, entry: str) -> bool: + module_name, sep, function_name = entry.partition(":") + return bool(sep and module_name and function_name and os.path.isfile(_entrypoint_path(hooks_root, hook, module_name))) + + +def _event(stdin: bytes, harness: str, event_name: str) -> dict[str, object]: + raw = stdin.decode("utf-8", errors="replace") + try: + parsed = json.loads(raw) + except json.JSONDecodeError as exc: + parsed = { + "_raw_payload": raw, + "_payload_error": f"the hook payload is not JSON ({exc})", + "_payload_error_type": type(exc).__name__, + "_payload_error_detail": str(exc), + } + if not isinstance(parsed, dict): + parsed = {"_raw_payload": raw, "_payload_error": "the hook payload is not a JSON object"} + parsed.setdefault("_raw_payload", raw) + parsed.setdefault("_catstack_harness", harness) + parsed.setdefault("hook_event_name", event_name) + return parsed + + +def _render_findings(hooks_root: str, hook: str, harness: str, event_name: str, event: dict[str, object], findings: list): + _engine_hooks, sdk_dir = _sdk_dirs(hooks_root) + _prefer_import_path(sdk_dir, 0) + from modes import effective_finding_modes + from render import render + from runtime import _renderable_findings + + mode, _mode_source, finding_modes = effective_finding_modes(hook, event, findings) + rendered_mode, rendered_findings = _renderable_findings(finding_modes, findings, mode) + stdout_text, stderr_text, exit_code = render(harness, event_name, rendered_mode, rendered_findings) + return stdout_text.encode(), stderr_text.encode(), exit_code, [finding.rule_id for finding in findings] + + +def _run_sdk_one( + hooks_root: str, + record: dict, + stdin: bytes, + budget: float, + parsed_event: dict[str, object] | None = None, +) -> dict: + hook, script = record["hook"], record["script"] + started = time.monotonic() + stdout = b"" + stderr = b"" + exit_code = 0 + timed_out = False + rule_ids: list[str] = [] + harness = run._harness(hooks_root) + payload = dict(parsed_event) if parsed_event is not None else _event(stdin, harness, "SubagentStop") + previous_handler = None + try: + detect = _load_entrypoint(hooks_root, hook, str(record["dispatch_entry"])) + if hasattr(signal, "SIGALRM"): + previous_handler = signal.getsignal(signal.SIGALRM) + + def _timeout(_signum, _frame): + raise TimeoutError(f"{hook}/{record['dispatch_entry']} timed out after {budget}s") + + signal.signal(signal.SIGALRM, _timeout) + signal.setitimer(signal.ITIMER_REAL, budget) + findings = detect(payload) + if not isinstance(findings, list): + raise TypeError("detect(event) must return a list") + stdout, stderr, exit_code, rule_ids = _render_findings(hooks_root, hook, harness, "SubagentStop", payload, findings) + lines = payload.get("_catstack_stderr_lines") + if isinstance(lines, list): + stderr += "".join(f"{line}\n" for line in lines if isinstance(line, str) and line).encode() + except TimeoutError: + timed_out = True + exit_code = 1 + stdout = b"" + stderr = f"catstack-hook-runner: {hook}/{script} timed out after {budget}s\n".encode() + except BaseException as exc: + if isinstance(exc, KeyboardInterrupt): + raise + if isinstance(exc, SystemExit): + exit_code = exc.code if isinstance(exc.code, int) else 1 + else: + exit_code = 0 + stdout = b"" + stderr = f"catstack-hook-error {hook}: {type(exc).__name__}: {exc}\n".encode() + finally: + if hasattr(signal, "SIGALRM"): + signal.setitimer(signal.ITIMER_REAL, 0) + if previous_handler is not None: + signal.signal(signal.SIGALRM, previous_handler) + result_outcome = outcome.classify(exit_code, stdout, stderr, timed_out) + row = run._row(hooks_root, hook, script, stdin, result_outcome, exit_code, started, stdout, stderr, rule_ids) + row["dispatch_path"] = "sdk" if not timed_out and not stderr.startswith(b"catstack-hook-error ") else "sdk_error" + return { + "hook": hook, + "script": script, + "outcome": result_outcome, + "exit_code": exit_code, + "stdout": stdout, + "stderr": stderr, + "row": row, + } + + def _merge_stdout(results: list[dict]) -> tuple[bytes, bytes, int]: blocked = [r for r in results if r["outcome"] == "blocked"] spoke = [r for r in results if r["stdout"].strip()] if blocked: return _merge_blocks(blocked) if spoke: + if len(spoke) == 1: + return spoke[0]["stdout"], b"", spoke[0]["exit_code"] contexts = [note for r in spoke for note in (_note(r),) if note] payload: dict[str, object] = {"continue": True} if contexts: @@ -340,11 +528,9 @@ def run_dispatch( ).encode() return 1, b"", stderr, [] - results: list[dict] = [] - with concurrent.futures.ThreadPoolExecutor(max_workers=len(records)) as pool: - futures = [pool.submit(_run_one, hooks_root, python, record, stdin, budget) for record in records] - for future in futures: - results.append(future.result()) + detector_budget = min(budget, DEFAULT_SUBAGENT_STOP_DETECTOR_BUDGET) if event == "SubagentStop" else budget + parsed_event = _event(stdin, harness, event) if event == "SubagentStop" else None + results = [_run_one(hooks_root, python, record, stdin, detector_budget, parsed_event) for record in records] stdout, block_notes, exit_code = _merge_stdout(results) stderr_lines = [r["stderr"] for r in results if r["stderr"]] @@ -355,7 +541,7 @@ def run_dispatch( def _parse_args(argv: list[str]) -> argparse.Namespace: parser = argparse.ArgumentParser() parser.add_argument("--event", required=True) - parser.add_argument("--timeout", type=float, default=DEFAULT_BUDGET) + parser.add_argument("--timeout", type=float) return parser.parse_args(argv) @@ -364,7 +550,10 @@ def main(argv: list[str] | None = None) -> int: stdin = sys.stdin.buffer.read() hooks_root = run._hooks_root() harness = run._harness(hooks_root) - exit_code, stdout, stderr, rows = run_dispatch(hooks_root, harness, args.event, stdin, args.timeout) + budget = args.timeout + if budget is None: + budget = DEFAULT_SUBAGENT_STOP_EVENT_BUDGET if args.event == "SubagentStop" else DEFAULT_BUDGET + exit_code, stdout, stderr, rows = run_dispatch(hooks_root, harness, args.event, stdin, budget) metrics_path = run._metrics_path() metrics_errors = b"".join(run._write_metrics(row, metrics_path) for row in rows) sys.stdout.buffer.write(stdout) diff --git a/engine/hooks/_runner/tests/test_dispatch.py b/engine/hooks/_runner/tests/test_dispatch.py index 05bd2a7c5..20dc6e02c 100644 --- a/engine/hooks/_runner/tests/test_dispatch.py +++ b/engine/hooks/_runner/tests/test_dispatch.py @@ -7,6 +7,7 @@ import sys import tempfile import unittest +from unittest import mock RUNNER_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -23,15 +24,34 @@ def setUp(self): for name in ("run.py", "outcome.py", "dispatch.py"): shutil.copy2(os.path.join(RUNNER_DIR, name), os.path.join(self.runner_dir, name)) - def _write_hook(self, name: str, script: str, body: str, event: str = "PreToolUse", matcher=None) -> None: + def _install_sdk(self) -> None: + hooks_dir = os.path.dirname(RUNNER_DIR) + shutil.copytree(os.path.join(hooks_dir, "_sdk"), os.path.join(self.hooks_root, "_sdk")) + shutil.copytree(os.path.join(hooks_dir, "_flags"), os.path.join(self.hooks_root, "_flags")) + + def _write_hook( + self, + name: str, + script: str, + body: str, + event: str = "PreToolUse", + matcher=None, + subagent_stop: dict[str, object] | None = None, + extra_files: dict[str, str] | None = None, + ) -> None: hook_dir = os.path.join(self.hooks_root, name) os.makedirs(hook_dir) with open(os.path.join(hook_dir, script), "w", encoding="utf-8") as handle: handle.write(body) + for filename, contents in (extra_files or {}).items(): + with open(os.path.join(hook_dir, filename), "w", encoding="utf-8") as handle: + handle.write(contents) entry = {"hooks": [{"type": "command", "command": f"python3 $HOME/.claude/hooks/{name}/{script}"}]} if matcher is not None: entry["matcher"] = matcher manifest = {"hooks": {event: [entry]}} + if subagent_stop is not None: + manifest["subagent_stop"] = subagent_stop with open(os.path.join(hook_dir, "claude.hook.json"), "w", encoding="utf-8") as handle: json.dump(manifest, handle) @@ -173,6 +193,74 @@ def test_matcher_on_the_manifest_filters_which_hooks_run(self): self.assertEqual(rows_outcome(self._rows(), "fixture-bash"), "spoke") self.assertEqual(self._rows(), [row for row in self._rows() if row["hook"] == "fixture-bash"]) + def test_subagent_stop_dispatch_only_loads_manifest_opted_in_hooks(self): + self._write_hook( + "fixture-dispatched", + "ok.py", + "import json\nprint(json.dumps({'hookSpecificOutput': {'additionalContext': 'dispatched'}}))\n", + event="Stop", + subagent_stop={"dispatch": True}, + ) + self._write_hook( + "fixture-direct", + "direct.py", + "import json\nprint(json.dumps({'hookSpecificOutput': {'additionalContext': 'direct'}}))\n", + event="Stop", + ) + + result = self._run(event="SubagentStop") + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(b"dispatched", result.stdout) + self.assertNotIn(b"direct", result.stdout) + rows = self._rows() + self.assertEqual([row["hook"] for row in rows], ["fixture-dispatched"]) + + def test_missing_sdk_entrypoint_falls_back_to_the_existing_script_and_marks_the_row(self): + self._write_hook( + "fixture-fallback", + "ok.py", + "import json\nprint(json.dumps({'hookSpecificOutput': {'additionalContext': 'fallback ran'}}))\n", + event="Stop", + subagent_stop={"dispatch": True}, + ) + + result = self._run(event="SubagentStop") + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(b"fallback ran", result.stdout) + row = self._row_for("fixture-fallback") + self.assertEqual(row["dispatch_path"], "subprocess_fallback") + + def test_sdk_entrypoint_runs_in_process_and_emits_rule_ids(self): + self._install_sdk() + self._write_hook( + "fixture-sdk", + "ok.py", + "print('subprocess should not run')\n", + event="Stop", + subagent_stop={"dispatch": {"entry": "detect:detect"}}, + extra_files={ + "detect.py": ( + "from finding import Finding\n" + "def detect(event):\n" + " return [Finding(rule_id='fixture.sdk', subject='s', message='sdk spoke', evidence='e')]\n" + ) + }, + ) + env_patch = mock.patch.dict(os.environ, {"CATSTACK_HOOK_MODE_FIXTURE_SDK": "warn"}) + env_patch.start() + self.addCleanup(env_patch.stop) + + result = self._run(event="SubagentStop") + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(b"sdk spoke", result.stdout) + self.assertNotIn(b"subprocess should not run", result.stdout) + row = self._row_for("fixture-sdk") + self.assertEqual(row["dispatch_path"], "sdk") + self.assertEqual(row["rule_ids"], ["fixture.sdk"]) + FIXTURES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures") FIXTURE_HOOK = "fixture-extra-manifest" @@ -283,12 +371,7 @@ def test_a_speaking_hook_in_an_extra_manifest_reaches_dispatch_with_the_run_alon self.assertEqual(alone.stderr, expected_stderr) self.assertEqual(dispatched.returncode, 0, dispatched.stderr) - self.assertEqual( - dispatched.stdout, - json.dumps( - {"continue": True, "additionalContext": f"[{FIXTURE_HOOK}] {FIXTURE_MESSAGE}"} - ).encode(), - ) + self.assertEqual(dispatched.stdout, alone.stdout) self.assertEqual(dispatched.stderr, expected_stderr) alone_row = self._only_row(alone_rows) @@ -421,12 +504,7 @@ def test_a_speaking_installed_hook_reaches_dispatch_with_the_run_alone_contract( self.assertEqual(alone.stderr, expected_stderr) self.assertEqual(dispatched.returncode, 0, dispatched.stderr) - self.assertEqual( - dispatched.stdout, - json.dumps( - {"continue": True, "additionalContext": f"[{INSTALLED_HOOK}] {FIXTURE_MESSAGE}"} - ).encode(), - ) + self.assertEqual(dispatched.stdout, alone.stdout) self.assertEqual(dispatched.stderr, expected_stderr) alone_row = self._only_row(alone_rows) @@ -480,7 +558,7 @@ def test_rewrapping_an_already_collapsed_cursor_install_keeps_its_hooks(self): env=env, ) self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn(f"[{INSTALLED_HOOK}] {FIXTURE_MESSAGE}".encode(), result.stdout) + self.assertIn(FIXTURE_MESSAGE.encode(), result.stdout) def test_an_unreadable_cursor_registry_is_reported_not_read_as_no_hooks(self): home = self._install("cursor", "speak", "1") diff --git a/scripts/install/mirror_stop_hooks_to_subagent_stop.py b/scripts/install/mirror_stop_hooks_to_subagent_stop.py index 132a53927..4ffc0c142 100644 --- a/scripts/install/mirror_stop_hooks_to_subagent_stop.py +++ b/scripts/install/mirror_stop_hooks_to_subagent_stop.py @@ -37,7 +37,9 @@ SOURCE_EVENT = "Stop" TARGET_EVENT = "SubagentStop" OPT_OUT_KEY = "subagent_stop" +DISPATCH_KEY = "dispatch" HOOKS_PREFIX_LITERAL = "$HOME/.claude/hooks/" +DISPATCHER_TIMEOUT = 9.5 sys.path.insert(0, SDK_DIR) from registry import load_registry @@ -49,6 +51,7 @@ class StopManifest: path: str entries: list = field(default_factory=list) inherit: bool = True + dispatch: bool = False reason: str = "" @property @@ -90,6 +93,8 @@ def load_manifests( raise ValueError(f"{path}: {OPT_OUT_KEY}.inherit is false but no reason is given") record.inherit = False record.reason = reason + elif isinstance(opt_out, dict) and bool(opt_out.get(DISPATCH_KEY)): + record.dispatch = True found.append(record) return found @@ -102,6 +107,11 @@ def _mirrored_entry(entry: dict) -> dict: return copy +def _dispatcher_entry(timeout: float = DISPATCHER_TIMEOUT) -> dict: + command = f"python3 $HOME/.claude/hooks/_runner/dispatch.py --event {TARGET_EVENT} --timeout {timeout:g}" + return {"hooks": [{"type": "command", "command": command, "timeout": timeout + 0.5}]} + + def _entry_is_for(entry: dict, prefixes: list[str]) -> bool: for hook in entry.get("hooks", []) if isinstance(entry, dict) else []: command = str(hook.get("command", "")) if isinstance(hook, dict) else "" @@ -123,9 +133,11 @@ def mirror(settings: dict, manifests: list[StopManifest]) -> tuple[dict, bool]: settings = json.loads(json.dumps(settings)) entry_list = settings.setdefault("hooks", {}).setdefault(TARGET_EVENT, []) before = json.dumps(entry_list, sort_keys=True) - managed = [m.command_prefix for m in manifests] + managed = [m.command_prefix for m in manifests] + [f"{HOOKS_PREFIX_LITERAL}_runner/dispatch.py"] kept = [e for e in entry_list if not _entry_is_for(e, managed)] - added = [_mirrored_entry(e) for m in manifests if m.inherit for e in m.entries] + direct = [_mirrored_entry(e) for m in manifests if m.inherit and not m.dispatch for e in m.entries] + dispatcher = [_dispatcher_entry()] if any(m.inherit and m.dispatch for m in manifests) else [] + added = direct + dispatcher entry_list[:] = kept + added if not entry_list: del settings["hooks"][TARGET_EVENT] @@ -145,7 +157,8 @@ def main() -> int: new_settings, changed = mirror(settings, manifests) for manifest in manifests: if manifest.inherit: - print(f"ok claude {TARGET_EVENT} {manifest.name} (mirrored from {SOURCE_EVENT})") + mode = "dispatcher" if manifest.dispatch else "mirrored" + print(f"ok claude {TARGET_EVENT} {manifest.name} ({mode} from {SOURCE_EVENT})") else: print(f"skip claude {TARGET_EVENT} {manifest.name} (opt-out: {manifest.reason})") if not changed: diff --git a/tests/test_install.py b/tests/test_install.py index e065f2d94..154a73c73 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1799,6 +1799,13 @@ def wrapped_claude_command_re(hook): ) +def subagent_dispatcher_command_re(): + return ( + r"(?:python3|/\S+) " + + re.escape("$HOME/.claude/hooks/_runner/dispatch.py --event SubagentStop --timeout 9.5") + ) + + class TestSubagentStopInheritance(unittest.TestCase): """Every hook that wires Stop is also wired as SubagentStop after install.sh, unless its manifest opts out with a reason. Parametrized @@ -1832,7 +1839,13 @@ def test_every_stop_hook_is_mirrored_or_opted_out_with_reason(self): with self.subTest(hook=manifest.name, command=hook["command"]): expected = wrapped_claude_command_re(hook) self.assertTrue(any(re.fullmatch(expected, command) for command in stop), stop) - if manifest.inherit: + if manifest.inherit and manifest.dispatch: + self.assertTrue( + any(re.fullmatch(subagent_dispatcher_command_re(), command) for command in subagent_stop), + subagent_stop, + ) + self.assertFalse(any(re.fullmatch(expected, command) for command in subagent_stop), subagent_stop) + elif manifest.inherit: self.assertTrue(any(re.fullmatch(expected, command) for command in subagent_stop), subagent_stop) else: self.assertTrue(manifest.reason) diff --git a/tests/test_mirror_subagent_stop.py b/tests/test_mirror_subagent_stop.py index 0ccd73241..93f7f5e4c 100644 --- a/tests/test_mirror_subagent_stop.py +++ b/tests/test_mirror_subagent_stop.py @@ -62,6 +62,15 @@ def test_opt_out_with_reason_is_recorded(self): self.assertFalse(manifest.inherit) self.assertEqual(manifest.reason, "talks to the human only") + def test_dispatch_opt_in_is_recorded(self): + write_manifest(self.hooks_dir, "fast", { + "hooks": {"Stop": [stop_entry("fast")]}, + "subagent_stop": {"dispatch": True}, + }) + [manifest] = mod.load_manifests(self.hooks_dir, active_hooks={"fast"}) + self.assertTrue(manifest.inherit) + self.assertTrue(manifest.dispatch) + def test_opt_out_without_reason_is_rejected(self): write_manifest(self.hooks_dir, "silent", { "hooks": {"Stop": [stop_entry("silent")]}, @@ -85,17 +94,17 @@ def test_real_repo_opted_out_set_is_the_documented_one(self): class TestMirror(unittest.TestCase): def manifests(self): return [ - mod.StopManifest(name="a", path="a", entries=[stop_entry("a")]), + mod.StopManifest(name="a", path="a", entries=[stop_entry("a")], dispatch=True), mod.StopManifest(name="b", path="b", entries=[stop_entry("b", "claude_stop_b.py")]), mod.StopManifest(name="quiet", path="quiet", entries=[stop_entry("quiet")], inherit=False, reason="human only"), ] - def test_every_inheriting_stop_hook_gets_a_subagent_stop_entry_without_matcher(self): + def test_dispatch_opted_hooks_share_one_dispatcher_entry_and_others_stay_direct(self): out, changed = mod.mirror({"hooks": {}}, self.manifests()) self.assertTrue(changed) self.assertEqual(commands(out, "SubagentStop"), [ - "python3 $HOME/.claude/hooks/a/claude_stop_check.py", "python3 $HOME/.claude/hooks/b/claude_stop_b.py", + "python3 $HOME/.claude/hooks/_runner/dispatch.py --event SubagentStop --timeout 9.5", ]) for entry in out["hooks"]["SubagentStop"]: self.assertNotIn("matcher", entry) @@ -120,6 +129,13 @@ def test_rerun_is_idempotent(self): self.assertFalse(changed) self.assertEqual(first, second) + def test_stale_dispatcher_entry_is_removed_when_no_manifest_opts_in(self): + stale = {"hooks": {"SubagentStop": [mod._dispatcher_entry()]}} + direct = [mod.StopManifest(name="b", path="b", entries=[stop_entry("b")])] + out, changed = mod.mirror(stale, direct) + self.assertTrue(changed) + self.assertEqual(commands(out, "SubagentStop"), ["python3 $HOME/.claude/hooks/b/claude_stop_check.py"]) + def test_empty_result_does_not_leave_an_empty_event_key(self): out, changed = mod.mirror({"hooks": {}}, []) self.assertFalse(changed) From 6a3c5a1c2628248a908f6e0a62e85f3befab1808 Mon Sep 17 00:00:00 2001 From: Invoker Bot Date: Sun, 11 Oct 2026 15:37:29 +0000 Subject: [PATCH 10/10] =?UTF-8?q?invoker:=20wf-1791728212829-14/publish-pi?= =?UTF-8?q?lot=20=E2=80=94=20Review=20claim:=20SubagentStop=20hooks=20disp?= =?UTF-8?q?atch=20through=20one=20process=20that=20parses=20the=20event=20?= =?UTF-8?q?once=20and=20shares=20one=20transcript=20parse=20across=20all?= =?UTF-8?q?=20opted-in=20detectors,=20emitting=20the=20same=20per-detector?= =?UTF-8?q?=20metrics=20rows=20and=20verdicts=20as=20the=20subprocess=20fl?= =?UTF-8?q?eet.=20Review=20lane:=20behavior=20Safety=20invariant:=20Hook?= =?UTF-8?q?=20stdout=20and=20exit=20codes=20stay=20the=20same.=20This=20sl?= =?UTF-8?q?ice=20does=20not=20change=20the=20other=20four=20claims.=20File?= =?UTF-8?q?s:=20engine/hooks/=5Frunner/dispatch.py=20Change=20types:=20bug?= =?UTF-8?q?fix=20Acceptance=20criteria:=20The=20branch=20contains=20only?= =?UTF-8?q?=20this=20claim,=20and=20the=20review=20step=20publishes=20one?= =?UTF-8?q?=20pull=20request.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exit code: 0 Invoker-Finalize-Id: aaa80940-f2d5-4d79-bb40-79c68fe333ad