Repository navigation
feat(testing): route test-judge verdicts to the writing subagent and decide judge placement #6060
Description
Problem
The task-end test judge (testing 0.21.3 on main) attributes every test write to the session id, so a test a subagent writes is judged and relayed at the parent session's Stop, not to the subagent that wrote it and can still fix it. It also starts a judge run after each write (after a 20 s debounce), so a block edited several times in one task gets a judge run for each version. Each run but the last is wasted.
What this issue does NOT ask, because #6037 already covers it: skipping temp and scratchpad copies, judging files outside a git repository, and not blocking Stop when a run has no FLAG. Once #6037 lands, all-PASS runs stop blocking. That alone would have removed every block in the incident below. What remains here is narrower:
- Attribution. A subagent's FLAG still lands in the parent session. The author never sees it, and the parent's user is asked about code they did not touch.
- Cost. There is a judge run per edit, not per finished change.
- Placement. The decision is whether the model judge belongs per edit, or once at a boundary (subagent end, commit, PR prep, CI review lane).
Code on main (c3376b14d):
- Recorders store
agent_id:plugins/testing/hooks/test-scan.sh:54reads.agent_id, and:95writes it to the record.plugins/testing/hooks/test-scan-bash.sh:75-81does the same. - Records are filed under
sessions/<pkey>/<session_id>/(judge-lib.sh:8).judge::loadgroups them per session and file (judge-lib.sh:107-129), and it keepswritersas{sid, agent}pairs (:115). The agent id is used only to pick a judge model class that differs from the writer's (judge::pick,:390-405, subagent transcript at:397). test-judge.sh(Stop) loads every record of the session set (:84-95) and never filters by agent. It blocks once with the relay template (:252-258).hooks.jsonregisters PreToolUse, PostToolUse, Stop and SessionStart only. It has noSubagentStopentry.test-judge-bg.sh:33readssession_id,tool_use_id,transcript_path,cwdandfile_path, but notagent_id. Its no-record fallback hardcodeswriters: [{sid: $s, agent: ""}](:69-71), so attribution is lost on that path even today.plugins/testing/hooks/hook-utils.sh:261-266already notes that subagents share the session id, and keys a different marker on session plus agent for that reason.- The per-edit cost comes from
test-judge-bg.sh:5-9and:51-53: waitTEST_JUDGE_DEBOUNCE(default 20 s,judge-lib.sh:32), then judge every in-doubt block of the file that has no verdict. A later edit to the same block gives it a new key (below) and a new run.
Evidence
Verified this pass:
- All code locations above, read with
git show origin/main:<path>atc3376b14d. - Claude Code hooks reference (
https://code.claude.com/docs/en/hooks.md, fetched 2026-10-03):- "Hooks from settings files, managed policy settings, and plugins also run inside subagents. When a subagent calls a tool, tool events such as PreToolUse and PostToolUse fire the same configured hooks as in the main conversation, and the input carries the agent_id and agent_type common input fields".
- SubagentStop input carries
agent_id,agent_type,agent_transcript_pathandlast_assistant_message. "The transcript_path is the main session's transcript, while agent_transcript_path is the subagent's own transcript stored in a nested subagents/ folder". Its example showssession_id: "abc123"withagent_transcript_pathunderabc123/subagents/, so a subagent's hook input carries the parent'ssession_id. The plugin's own path$tdir/$sid/subagents/agent-$agent.jsonl(judge-lib.sh:397) relies on the same layout. - "Returning decision: "block" with a reason keeps the subagent running and delivers reason to the subagent as its next instruction." Also: "To inject context into the parent session after a subagent returns, use a PostToolUse hook on the Agent tool instead."
- SubagentStop also accepts
hookSpecificOutput.additionalContextfor non-error feedback.
- Incident counts, from the review-findings files one background subagent's worktree held (
.work/reviews/agent-a245b911f52d408b9/): 7 findings files and 29 verdicts, with 0 FLAG, 28 PASS and 1 UNKNOWN. The parent session wrote no test files, but each of those runs was relayed at the parent's Stop. Per file:test_registry_manager.py6 verdicts,test_save_point.py3,clean-batch.test.sh3, two files 2 each, and 13 files 1 each.
What the item claimed that did NOT reproduce as stated:
- "Re-judges unchanged tests" is not true for files the scanner splits into blocks. A key is
file::name::ordinal::shaof the block's current text (judge-lib.sh:17-19, built at:207-217), so an unchanged block keeps its verdict.judge::relayedskips keys already shown (test-judge.sh:92). The 6test_registry_manager.pyverdicts cover 4 distinct tests. The 2 repeats had changed text (lines 110-116 became 110-117, and 118-122 became 119-125). The 3test_save_point.pyverdicts are 3 different tests. The item's proposed "cache by block content hash" is therefore already on main. fix(testing): reject comment-only judge repairs and judge identical tests consistently #6044 asks for something different: reusing a verdict across blocks with identical normalized bodies. - Whole-file judging of shell tests is documented design, not a defect.
plugins/testing/README.md:125-126says "A bash test script with no test blocks is judged as one whole file, with the changed lines as a hint". A whole-file key is formed atjudge-lib.sh:182-186. So any edit anywhere inclean-batch.test.sh(verdicts on lines 1-733, 1-734, 1-734) re-judges the whole file, and the judge is told the changed lines. One verdict shows the hint in use: "The changed tests (lines 799-811)". - The item's totals were not re-counted. It reports 43 blocks over 10 Stops, across two subagent worktrees. Only the a245 worktree's findings were read here (29 verdicts in 7 files). The other worktree (
...-agent-a4b2f59daea66d5a7) was not read.
Inferred, not verified:
- Whether SubagentStop fires for a background (
run_in_background) subagent, and when. The docs do not say. - That the findings files landed in the subagents' worktrees because the findings directory follows the test file's repository (
README.md:87,.work/reviews/<branch>/).
Proposed approach
Two parts. Part A is a scoped fix with a clear design. Part B needs a decision from the owner before anyone builds it.
Part A: route by agent (recommended, independent of B).
- Add a
SubagentStopentry inhooks.jsonthat runs the Stop relay scoped to the finishingagent_id: load only records whoseagent_idmatches, judge or collect their verdicts, and write the findings file.- On a FLAG (or, after fix(testing): skip temp-copy tests and stop blocking Stop on judge runs with nothing to decide #6037, a non-environmental UNKNOWN), return
decision: "block"with the relay reason. Per the docs, this is delivered to the subagent as its next instruction, so the author sees the verdict and proposed diff and can fix the test or report it in its hand-back. - On 0 FLAG, do not block. Use
systemMessageoradditionalContextwith the counts.
- On a FLAG (or, after fix(testing): skip temp-copy tests and stop blocking Stop on judge runs with nothing to decide #6037, a non-environmental UNKNOWN), return
- In
test-judge.sh(parent Stop), skip records that carry anagent_idand whose agent's SubagentStop already ran (mark them relayed at SubagentStop). As a fallback, the parent still relays agent records that no SubagentStop handled, such as a subagent that was killed, so nothing is lost silently. - In
test-judge-bg.sh, read.agent_idand carry it into the fallback record (:69-71) and the pending marker. - Optional: a PostToolUse hook on the
Agenttool adds one line to the parent ("subagent X: N tests judged, F FLAG, findings at ...") only when F > 0.
Files: plugins/testing/hooks/hooks.json, test-judge.sh (or a new test-judge-subagent.sh sharing judge-lib.sh), judge-lib.sh (judge::load agent filter), test-judge-bg.sh, test-judge.test.sh, test-judge-bg.test.sh, README.md (Task-end test judge section), plugin version and CHANGELOG.
Alternative considered: filter agent records out of the parent Stop entirely. This is rejected because a subagent's FLAG would then be shown nowhere.
Part B: judge placement (needs-decision). These options come from the producing session and are judgment, not verified:
- B1. Keep per-edit background judging (status quo). Verdicts are mostly ready at Stop. The cost is one run per edited version of a block.
- B2. Drop the PostToolUse background job and judge once at the boundary: the parent's Stop and subagent's SubagentStop for interactive work. Add a criterion in the CI review lane for PRs. The Stop wait is then bounded by
TEST_JUDGE_TIMEOUT(180 s) more often, because no verdicts are pre-computed. - B3. Keep per-edit judging but raise the debounce, or judge only when the file has been quiet across a turn boundary.
- B4. Shell test files: key per changed hunk (or per
assert_*group) instead of one whole-file key, so an edit far from earlier changes is judged only on its hunk. This changes the documented design atREADME.md:125. - Run the deterministic
/testing:auditdetectors first, everywhere, with no model call.test-scan.shalready does this at write time.
The item also proposes earning the keep with data: record FLAGs acted on versus dismissed. If per-edit judging cannot show a real hit rate, retire it in favor of a boundary judge. The one sample here (29 verdicts, 0 FLAG) cannot settle that. /overengineering:justify is the repo's tool for that call.
Acceptance criteria
Part A:
- A test file written by a subagent (record carries
agent_id) with a stubbed FLAG verdict: that subagent's SubagentStop returnsdecision: "block"with the relay reason, and the parent session's next Stop does not relay that key. - The same subagent case with only PASS verdicts: SubagentStop does not block, and the parent Stop does not block for it.
- An agent record whose SubagentStop never ran is still relayed at the parent's Stop.
- A write the main thread made (no
agent_id) is relayed at the parent Stop exactly as today. - The
test-judge-bg.shfallback record (no test-scan record) keeps the payload'sagent_id. - Each case above has a test in
test-judge.test.sh/test-judge-bg.test.shthat fails before the change.
Part B, once decided (fill in for the chosen option):
- Editing a block N times within one task produces at most one judge run for its final text (if B2 or B3 is chosen).
- A one-line change to a shell test file sends the judge only the changed hunk's key (if B4 is chosen).
Constraints and gotchas
- A judge at SubagentStop makes the subagent wait up to
TEST_JUDGE_TIMEOUT(180 s, below the hooks.json Stop timeout of 240 s) before it can hand back. Give the SubagentStop entry its own timeout, and reuse the existing handoff to a detachedtest-judge-bg.shfor overflow. - SubagentStop input carries
stop_hook_active. Mirrortest-judge.sh:68-78: do not judge or block again on the forced turn. - Do not block on a 0-FLAG run at SubagentStop. Keep the same rule fix(testing): skip temp-copy tests and stop blocking Stop on judge runs with nothing to decide #6037 sets for Stop, so the two hooks behave alike.
- Whether SubagentStop fires for background subagents is unverified. The parent-Stop fallback in Part A item 2 is what keeps verdicts from being lost if it does not.
- Session-set adoption for
/clearand fork successors (judge::session_set,judge-lib.sh:86-97) must keep working with the agent filter. - The judge's model class must still differ from the subagent's model (
judge::pick). The SubagentStop payload givesagent_transcript_pathdirectly. - Hook changes need a stay-quiet test that fails before the fix (
docs/conventions/hook-precision/README.md). The fixtures live under temp. If fix(testing): skip temp-copy tests and stop blocking Stop on judge runs with nothing to decide #6037's temp skip lands first, keep its skip root injectable. - Version bump and CHANGELOG entry for testing.
- Coordinate with fix(testing): skip temp-copy tests and stop blocking Stop on judge runs with nothing to decide #6037 (same files:
test-judge.sh,test-scan*.sh) and fix(testing): reject comment-only judge repairs and judge identical tests consistently #6044 (judge-lib.shverdict reuse). Land fix(testing): skip temp-copy tests and stop blocking Stop on judge runs with nothing to decide #6037 first, because it changes the block rule this issue mirrors.
Context
Source: local handoff item 20261003-063320-testing-test-judge-trigger-scope-and-placement.md (retired into this issue). The owner asked: "Why is test judge firing on things we haven't modified... this seems inefficient", and then, setting current state aside, what the best setup for the hook, plugin and skill is given the goal: tests whose expected values restate the implementation do not land.
Related: #6037 (temp-copy skip, non-repo, no block without FLAG), #6016 (checkout-brought test files recorded as authored), #6044 (comment-only repairs, identical-body verdict reuse), #5725 (judge calibration).
Activity
kyle-sexton commentedon Oct 4, 2026
Part A shipped in #6305 (testing 0.22.18): a subagent's judged tests are relayed to that subagent at SubagentStop, and the parent Stop skips tests whose subagent is still running.
Open follow-up for part B: it is not verified that the SubagentStop systemMessage reaches the user. A subagent can act on FLAG verdicts before the user sees them. Check delivery on the current Claude Code release, and if the user does not see it, gate the subagent relay on the attended flag the same way the main Stop is gated.
kyle-sexton commentedon Oct 13, 2026
Decision recorded. The owner accepted the adjudicated recommendation on 2026-10-12.
Option (a): accept that #6662 superseded Part A; drop the attended-gating follow-up; narrow this issue to Part B as B1 (keep per-edit judging, add a FLAG outcome counter before any placement change).
Why: On origin/main hooks.json:2122-2137 has one asyncRewake Stop row and no SubagentStop row, and #6662 (kyle-sexton, merged 2026-10-09) names that removal, so the owner's 2026-10-04 follow-up question has no subject; by precedence rule 2 the newer merged decision wins, but it does not cite #6060, so the attribution loss is flagged, not treated as closed. #6662 removed B2's stated cost (the turn-end wait), but hooks.md:4026 says Claude Code kills any async hook still running at -p teardown and README.md:22-23 already records that a -p or SDK session gets no task-end verdict, so B2 would drop judging in those lanes while B1's per-edit verdicts persist in the ledger and test-judge-start.sh names them at the next session start. No FLAG acted-versus-dismissed data exists anywhere on main, so the issue's own 'earn the keep with data' step is the only way to settle B1 against B3/B4 later.
Precedence applied: Rule 2 (owner's newest explicit decision): #6662 names the SubagentStop removal and beats the 2026-10-04 comment, flagged because it does not cite #6060. Rule 4 (current upstream docs) settles B1 over B2: hooks.md:4026 (-p teardown kills async hooks) and :4033 (async systemMessage unseen by the user), corroborated by README.md:22-23 on main.
Sources:
Next: Read-only this pass. On (a): comment on #6060 recording Part A as superseded by #6662 and the attended-gate follow-up as dropped, retitle it to the Part B scope, and open one draft PR in plugins/testing that (1) adds a FLAG outcome counter in the judge ledger (relayed, acted on, dismissed) with stay-quiet tests that fail before the change, (2) adds to the test-judge.sh:28 pointer comment that CLAUDE_CODE_SESSION_ATTENDED is set by Claude Code 2.1.296 but absent from env-vars.md and the changelog, with a recheck trigger (the env-vars page or changelog naming it), and (3) leaves hooks.json placement as is. Separately probe on 2.1.296 that the attended exit-2 reason leads Claude to show the user the FLAG and diff before acting (hooks.md:459 says only that stderr is shown to Claude), and whether a claude --bg session kills async hooks at teardown (hooks.md:4026 names only -p). Revisit B2/B3/B4 only when the counter shows a FLAG rate.
Researched by two independent agents (local evidence and upstream docs) and adjudicated by a third.
kyle-sexton commentedon Oct 13, 2026
Owner batch-ratified this admission in an attended session on 2026-10-12: work-class: scoped. Rule: class C1-C3, no trust surface, the independent check confirmed the recorded decision, and no owner step remains.
kyle-sexton commentedon Oct 13, 2026
Correction: this issue already carried work-class: structural, which batch ratification never admits. The admission above is reverted; it stays escalated for an attended decision.