diff --git a/engine/hooks/gate-blame-needs-evidence/detect.py b/engine/hooks/gate-blame-needs-evidence/detect.py index f4dea0a5..2c24ed9b 100644 --- a/engine/hooks/gate-blame-needs-evidence/detect.py +++ b/engine/hooks/gate-blame-needs-evidence/detect.py @@ -337,8 +337,10 @@ def blamed_gates(message: str, lines: list[dict], hooks_dir: str = HOOKS_DIR) -> out: dict[str, dict] = {} for blame in blames: gates = gates_in(blame["window"], known) - if not gates and (blame["specific"] or GATE_NOUN_RE.search(blame["window"])): + if not gates and blame["specific"]: gates = latest_refusal_gate(lines) + in_reply + elif not gates and GATE_NOUN_RE.search(blame["window"]): + gates = latest_refusal_gate(lines) for gate in gates: out.setdefault(gate["name"], {"gate": gate, "phrase": blame["phrase"]}) for delete in deletes: diff --git a/engine/hooks/gate-blame-needs-evidence/tests/fixtures/real_session.json b/engine/hooks/gate-blame-needs-evidence/tests/fixtures/real_session.json index 676c3102..97830d69 100644 --- a/engine/hooks/gate-blame-needs-evidence/tests/fixtures/real_session.json +++ b/engine/hooks/gate-blame-needs-evidence/tests/fixtures/real_session.json @@ -161,6 +161,57 @@ "label": "1527", "reply": "Found it. `scripts/review-unit-rules.mjs:78-85` — the `activation-surface` unit matches the bare words **`cli`** and **`command`**. `:96-100` — `proof` matches **`test`**.\n\nSo a plan that adds a CLI command can't describe itself without tripping `activation-surface`. Every rewrite I tried was me avoiding the word \"command\" in a plan about adding a command. That makes the plan worse, not more reviewable.\n\nThe design is unchanged and still right: **`invoker-cli run-chain` compiled into the binary**, skill documents the command and carries no script, watchers call the binary rather than a repo path.\n\nTwo ways forward, and I'd take the first:\n\n1. **Implement `run-chain` directly** in `packages/cli/src/index.ts` with its test, as one reviewed PR. One package, one review slice — the routing rule says that stays local, no Invoker plan needed.\n2. **Separately fix the detector**, since matching bare `cli`/`command` leaves it unable to gate any CLI work.\n\nNothing is lost meanwhile: all 11 workflows are live, and `submit-workflow-chain.sh` is untouched.\n\nGo implement `run-chain` now?" }, + "silent_after_agent_routing_guard_read": { + "label": "agent-routing-real-generic-blame", + "reply": "Its source at engine/hooks/agent-routing-guard/detect.py:66 counts any bare merge word as a publication verb, and my read-only prompt said PRs 288, 289 (merged). I'll note the guard's false positive on a read-only prompt as a backlog item.", + "transcript": [ + { + "type": "user", + "message": { + "role": "user", + "content": "PreToolUse:Bash hook error: [python3 $HOME/.claude/hooks/agent-routing-guard/claude_pretool_agent_routing.py]: agent-routing-guard refused a read-only prompt that mentioned merged PRs." + } + }, + { + "type": "assistant", + "message": { + "role": "assistant", + "content": [ + { + "type": "tool_use", + "id": "t-grep-agent-routing", + "name": "Bash", + "input": { + "command": "grep -n \"bare merge\" engine/hooks/agent-routing-guard/detect.py", + "description": "Read the agent-routing-guard detector" + } + } + ] + } + }, + { + "type": "user", + "message": { + "role": "user", + "content": [ + { + "type": "tool_result", + "tool_use_id": "t-grep-agent-routing", + "is_error": false, + "content": "66: PUBLICATION_RE = re.compile(r\"\\\\b(?:merge|merged|ship|land)\\\\b\", re.IGNORECASE)" + } + ] + } + }, + { + "type": "user", + "message": { + "role": "user", + "content": "Stop hook feedback:\n[python3 $HOME/.claude/hooks/wrong-check-reflect/claude_stop_check.py]: wrong-check-reflect would require reflection if the assistant admitted a previous check was wrong." + } + } + ] + }, "stop_feedback_after_refusal": [ { "type": "user", diff --git a/engine/hooks/gate-blame-needs-evidence/tests/test_hooks.py b/engine/hooks/gate-blame-needs-evidence/tests/test_hooks.py index 52e633cc..d7b23640 100644 --- a/engine/hooks/gate-blame-needs-evidence/tests/test_hooks.py +++ b/engine/hooks/gate-blame-needs-evidence/tests/test_hooks.py @@ -176,6 +176,23 @@ def test_silent_on_real_review_unit_reply_after_its_source_was_read(self): reply = REAL["silent_after_review_unit_read"]["reply"] self.assertIsNone(detect.decide_stop_from_lines(reply, lines_of(REAL["review_unit_read"]), HOOKS_DIR)) + def test_silent_on_generic_blame_after_latest_refusing_gate_was_read(self): + case = REAL["silent_after_agent_routing_guard_read"] + lines = lines_of(case["transcript"]) + reply = ( + "The wrong-check-reflect hook was named only in Stop feedback. " + "The read-only prompt evidence is below. " + "The guard's false positive belongs on the backlog." + ) + blames = detect.blamed_gates(reply, lines, HOOKS_DIR) + self.assertEqual([b["gate"]["name"] for b in blames], ["agent-routing-guard"]) + self.assertIsNone(detect.decide_stop_from_lines(reply, lines, HOOKS_DIR)) + self.assertEqual( + [b["gate"]["name"] for b in detect.blamed_gates(case["reply"], lines, HOOKS_DIR)], + ["agent-routing-guard"], + ) + self.assertIsNone(detect.decide_stop_from_lines(case["reply"], lines, HOOKS_DIR)) + def test_silent_when_user_ran_the_read_themselves(self): lines = [{"type": "user", "message": {"role": "user", "content": "cat ~/.claude/hooks/scope-lock/detect.py"}}] diff --git a/engine/hooks/llm-judge/phrases/wrong-check-reflect.json b/engine/hooks/llm-judge/phrases/wrong-check-reflect.json index a9932a15..0aaf4200 100644 --- a/engine/hooks/llm-judge/phrases/wrong-check-reflect.json +++ b/engine/hooks/llm-judge/phrases/wrong-check-reflect.json @@ -16,7 +16,10 @@ "I double-checked my earlier count and it holds; nothing in it was wrong.", "If my earlier check was wrong, say so.", "The test was wrong, not the code.", - "He said I was wrong about the timeout." + "He said I was wrong about the timeout.", + "I'll note the guard's false positive on a read-only prompt as a backlog item.", + "That refusal was a false alarm from the spawn guard, not my error.", + "The scan reported a false negative on the second fixture." ], "on_hit": "Wrong-check admission on this transcript. This is a FAILURE, not a preference ping: a claim went out before a real check. Finish the live correction first. Then read the reflect skill and spawn a subagent for steps 1-4 on this exact transcript. Present Accepted / Backlog / Route-to-automate-me / Rejected. Do not skip because the task also finished." } diff --git a/engine/skills/reflect/SKILL.md b/engine/skills/reflect/SKILL.md index 68d4f3b6..258a52fe 100644 --- a/engine/skills/reflect/SKILL.md +++ b/engine/skills/reflect/SKILL.md @@ -41,6 +41,10 @@ Not every finding belongs in a skill edit here. A finding about the *user's work Every invocation of this skill — single-transcript or multi-conversation mode — runs inside a subagent, no exceptions. The parent launches it with the harness subagent tool (`subagent_type: general-purpose` / `generalPurpose`: the process reads transcripts fresh from disk and doesn't need the parent's own conversation context) with the user's original reflect arguments/scope, then waits for it to report back. The subagent runs steps 1-4 (locate transcript(s), cost audit, lens fan-out, synthesis) — that's the large part. The parent runs steps 5 and 6 itself, in the main thread, never delegated for the investigation: presenting the Accepted / Backlog / Route-to-automate-me / Rejected list, then **immediately** firing a catstack worktree (+ background agent) to apply Accepted items (PR is the landing gate — do not wait for a second “apply those” turn). This keeps the bulk of the investigation out of the parent's context window — the parent only needs the final synthesized findings list. +The subagent does not end its turn while a task it launched is still running; +it waits for that task and returns once, because a resumed subagent replays its +whole context. + ## When to invoke - The `reflect-on-thrash` Stop/sessionEnd hook fired. Treat the named transcript as the scope; still present the list, then auto-worktree-apply Accepted (step 5) — never merge without a PR. @@ -92,7 +96,9 @@ One more `Agent` call, given all reviewers' output, merges overlapping findings, - **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. - **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. +- **Rejected** — one-offs, already covered, 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. **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. diff --git a/engine/skills/reflect/scripts/self_retraction_scan.py b/engine/skills/reflect/scripts/self_retraction_scan.py index d0d533da..e70d17a3 100644 --- a/engine/skills/reflect/scripts/self_retraction_scan.py +++ b/engine/skills/reflect/scripts/self_retraction_scan.py @@ -21,7 +21,7 @@ r"read|grep|assumption|number|count)\b" ) WRONGNESS_RE = re.compile( - r"(?i)\b(?:wrong|incorrect|inaccurate|false|untrue|not\s+true|mistaken|" + r"(?i)\b(?:wrong|incorrect|inaccurate|false(?![- ](?:positives?|negatives?|alarms?))|untrue|not\s+true|mistaken|" r"misread|mis-read|misstated|overstated|vacuous|premature|bogus|" r"retract(?:ing|ed)?|take\s+(?:that|it)\s+back|" r"does(?:n'?t|\s+not)\s+hold|did(?:n'?t|\s+not)\s+hold)\b" diff --git a/engine/skills/reflect/scripts/tests/test_help_flags.py b/engine/skills/reflect/scripts/tests/test_help_flags.py new file mode 100644 index 00000000..be7aa011 --- /dev/null +++ b/engine/skills/reflect/scripts/tests/test_help_flags.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +import os +import subprocess +import sys +import unittest + + +SCRIPTS_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +class TestHelpFlags(unittest.TestCase): + def test_token_audit_help(self): + proc = subprocess.run( + [sys.executable, os.path.join(SCRIPTS_DIR, "token_audit.py"), "--help"], + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn("Usage", proc.stdout) + + def test_top_sessions_help(self): + proc = subprocess.run( + [sys.executable, os.path.join(SCRIPTS_DIR, "top_sessions.py"), "--help"], + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn("Usage", proc.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py b/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py index ca451368..fb68c79d 100644 --- a/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py +++ b/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py @@ -37,6 +37,14 @@ def test_quoted_admission_does_not_fire(self): text = 'The rule says "my earlier check was wrong" is an admission.' self.assertIsNone(self_retraction_scan.find_admission(text)) + def test_false_positive_about_another_checker_stays_clean(self): + text = "I'll note the guard's false positive on a read-only prompt as a backlog item." + self.assertIsNone(self_retraction_scan.find_admission(text)) + + def test_false_claim_still_matches(self): + text = "My earlier claim was false; the real count is 12." + self.assertIsNotNone(self_retraction_scan.find_admission(text)) + class TestScanAssistantTexts(unittest.TestCase): def test_collects_one_hit_per_admission(self): diff --git a/engine/skills/reflect/scripts/token_audit.py b/engine/skills/reflect/scripts/token_audit.py index 0004d8f0..056bbd5a 100644 --- a/engine/skills/reflect/scripts/token_audit.py +++ b/engine/skills/reflect/scripts/token_audit.py @@ -1415,6 +1415,9 @@ def _parse_argv(argv): non-zero. Returns (mode, path, out_path, include_subagents).""" if len(argv) < 2: return None, None, None, True + if argv[1] in ("--help", "-h"): + print(__doc__) + sys.exit(0) mode = argv[1] out_path = None path = None diff --git a/engine/skills/reflect/scripts/top_sessions.py b/engine/skills/reflect/scripts/top_sessions.py index c4f877ac..6c74173f 100644 --- a/engine/skills/reflect/scripts/top_sessions.py +++ b/engine/skills/reflect/scripts/top_sessions.py @@ -224,6 +224,9 @@ class of shell-portability issue. def main(): + if len(sys.argv) > 1 and sys.argv[1] in ("--help", "-h"): + print("Usage: top_sessions.py [N]\n\nRank local sessions by total token usage.") + sys.exit(0) top_n = int(sys.argv[1]) if len(sys.argv) > 1 else 5 t0 = time.time() claude_files = glob.glob(os.path.expanduser("~/.claude/projects/*/*.jsonl"))