Skip to content

perf(testing): run the task-end test judge with asyncRewake - #6662

Merged
kyle-sexton merged 3 commits into
mainfrom
perf/testing-judge-async-rewake
Oct 9, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
perf/testing-judge-async-rewake

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

No related issue: perf-audit item B4 (non-blocking test judge), from the user-approved change set.

Summary

The task-end test judge's Stop hook held every turn end for up to 240 s. It now runs with asyncRewake, so the turn ends at once. Only an attended FLAG (or an UNKNOWN that started as a FLAG) wakes Claude; pass and count summaries reach Claude on its next turn.

Fix

  • hooks.json / gen-hook-filters.sh: the Stop row carries "asyncRewake": true and keeps its 240 s timeout, which Claude Code still enforces on an asyncRewake hook. The SubagentStop row is removed.
  • test-judge.sh: a relay that needs a decision exits 2 with the fixed template (then any count lines) on stderr; every other path exits 0 with a systemMessage-only JSON. The SubagentStop branch is removed: a subagent's writes are judged at the parent's Stop, and a still-running subagent's writes wait for a later Stop.
  • judge-lib.sh: drops the JUDGE_AGENT_ONLY filter that only SubagentStop used.
  • README.md: describes the background Stop hook and cites the hooks reference with an as-of date and recheck trigger.
  • Changelog fragment, bump: patch.

Docs basis: hooks reference, run hooks in the background (as of 2026-10-09): asyncRewake wakes Claude on exit 2 and shows it stderr; timeout is still enforced; an async hook's systemMessage is delivered to Claude on the next turn.

Verification

  • scripts/affected-tests.sh --run --jobs 4: 5 suites selected (test-judge, test-judge-bg, test-judge-start, gen-hook-filters, judge-calibration metrics), all passed. test-judge.test.sh: 228 passed, 0 failed, including the killed-subagent relay at the parent Stop (once that subagent has finished, its unrelayed FLAG is relayed at the parent, now asserting exit 2 and the stderr reason).
  • gen-hook-filters.sh --check: in sync.
  • ShellCheck on the five changed .sh files: clean. shfmt drift unchanged from main.
  • check-hooks-description.sh, check-hook-wiring-liveness.sh, check-hook-slow-shapes.sh, check-purged-em-dashes.sh, check-changelog-fragments.sh --check and --check-required origin/main, validate-plugins.sh: pass.
  • markdownlint-cli2, typos, editorconfig-checker on the changed files: clean.

Related

  • Perf-audit work order T1 (B4).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VxGrZW26Qs1qeo3RFgmbvp

The Stop hook now runs in the background, so a turn no longer waits up
to 240 s for the judge. An attended FLAG (or an UNKNOWN that started as
a FLAG) exits 2 with the relay on stderr, which wakes Claude; every
other path exits 0 with a systemMessage that reaches Claude on its next
turn. The SubagentStop hook and judge-lib's JUDGE_AGENT_ONLY filter are
removed: a subagent's tests are judged at the parent's Stop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VxGrZW26Qs1qeo3RFgmbvp
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 9, 2026 12:54
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T13:00:31.141032Z 5f60178 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 2s —— View job


Security review (/review:security-review)

  • Gather context (PR diff, body, changed files)
  • Invoke /review:security-review per repo instructions
  • Review diff against criteria (injection, auth/access, trust boundaries, secret handling, instruction-surface deletions)
  • Post findings

Scope reviewed: plugins/testing/hooks/hooks.json, judge-lib.sh, test-judge.sh, test-judge.test.sh, plugins/testing/scripts/gen-hook-filters.sh/.test.sh, plugins/testing/README.md, and the changelog fragment — i.e., the full diff (no GitHub Actions workflow files are touched, so zizmor's lane is out of scope here).

No security issues found.

Notes from the review, for context (none rise to a finding):

  • The Stop→async relay refactor (decision: block JSON → exit 2 + raw stderr via fd 4, test-judge.sh#L63-L70) changes delivery mechanism but not what's relayed or to whom; content size/escaping concerns don't apply since it's no longer JSON-encoded, and the reason text is still built from internally-derived strings (counts, a .work/reviews/... path), not raw external input.
  • Removing JUDGE_AGENT_ONLY / the SubagentStop hook (hooks.json#L2133-L2143, judge-lib.sh#L103-L117) was checked against the instruction-exception-register lens (deletion of a standing guardrail): this isn't a deletion of coverage, just a relocation of when a subagent's flagged test is surfaced — a finished subagent's unrelayed FLAG is still picked up and shown to the user at the parent's next Stop (confirmed by the retained/adapted test "once that subagent has finished, its unrelayed FLAG is relayed at the parent" in test-judge.test.sh). JUDGE_AGENT_SKIP still correctly excludes writes from subagents still running, so nothing is judged prematurely either.
  • TEST_JUDGE_TIMEOUT/TEST_JUDGE_DEBOUNCE arithmetic-injection hardening (the sec7 test) is untouched by this diff's logic changes, only its stderr-capture plumbing changed in the test harness.
  • The non-blocking (asyncRewake) turn-end is a deliberate, documented perf tradeoff (perf-audit item B4) with a bounded 240s window before a FLAG wakes Claude — an acknowledged latency, not an exploitable gap introduced by this change.

@claude

claude Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 5m 10s —— View job


Code review complete

  • Gather context (PR diff, changed files, CLAUDE.md)
  • Review hooks.json / gen-hook-filters.sh changes
  • Review test-judge.sh relay/exit-code logic
  • Review judge-lib.sh filter removal
  • Review test file changes for coverage
  • Review README/changelog fragment
  • Adversarial validation of candidate findings
  • Post findings and summary

No blocking or high-confidence findings.

What I checked, statically (diff-only, scoped to the 8 changed files):

  • test-judge.sh's new emit() (plugins/testing/hooks/test-judge.sh:71-77): reason → stderr (fd 4) + exit 2 via the EXIT trap's exit "$RC" (line 68); message-only → stdout (fd 3) as systemMessage. The fd setup at line 65 (exec 3>&1 4>&2 >/dev/null 2>>"$JUDGE_LOG") correctly captures original stdout/stderr into 3/4 before stdout/stderr are redirected to the judge log, so the relay and the log never collide.
  • Traced the exit-2 path through exec-bash.mjs (plugins/testing/hooks/exec-bash.mjs:299-318): since the Stop row has no --skip-unless-stdin-contains flag, stdio is "inherit" for stdin/stdout/stderr, and child.on("exit") forwards the raw exit code via process.exit(code ?? 1). So test-judge.sh's exit 2 and its stderr reach Claude Code's hook layer unmodified — the new asyncRewake/exit-2 relay mechanism is plumbed correctly end to end.
  • Confirmed no stale references to the removed SubagentStop hook or the removed JUDGE_AGENT_ONLY jq filter remain anywhere in plugins/testing/ outside of comments/tests that were intentionally updated.
  • hooks.json's Stop row (plugins/testing/hooks/hooks.json:2122-2142) parses as valid JSON with asyncRewake: true alongside the existing timeout: 240, matching gen-hook-filters.sh's generator output 1:1.
  • judge-lib.sh's judge::load (plugins/testing/hooks/judge-lib.sh:181-208): removing the $only/JUDGE_AGENT_ONLY branch from the group jq filter is consistent with SubagentStop going away — JUDGE_AGENT_SKIP (dropping a still-running subagent's in-flight writes) is the only filter left, which is still exercised by test-judge.sh.
  • The emit/reason/message split matches the updated assertions in test-judge.test.sh (e.g. the relay() helper reading rc/stderr instead of .decision/.reason), including the "reason carries the counts, count lines follow it on stderr" case (test-judge.test.sh:400-416).

Not independently verified: I could not execute the test suite, ShellCheck, or any of the other check scripts myself — this sandbox denies all non-trivial Bash execution (only inspection commands like grep/find/wc are permitted). The PR body's test/lint results are the author's own claims; I did not re-run them.

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through 9c60f12; a later push is reviewed from there.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f60178588

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/testing/scripts/gen-hook-filters.sh
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

claude-review has reviewed this pull request through 9c60f12; a later push is reviewed from there.

claude -p and Agent SDK sessions end before the async Stop judge finishes, so
they get no task-end verdict; CI is their gate.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VxGrZW26Qs1qeo3RFgmbvp
@claude

claude Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 10s —— View job


Security review (/review:security-review) — incremental pass

Scope: per the lane's incremental diff (.claude-lane/incremental.diff), only the two files changed since the last-reviewed commit 5f60178:

  • .changes/testing/perf-testing-judge-async-rewake-da85f122.md

  • plugins/testing/README.md

  • Gather context (incremental diff, PR body, prior review comments)

  • Invoke /review:security-review procedure for this scope

  • Review diff against criteria (injection, auth/access, trust boundaries, secret handling, instruction-surface deletions)

  • Post findings

No security issues found.

Both files' changes are prose-only: they add one sentence (changelog) and one paragraph (README) disclosing that a claude -p or Agent SDK session ends before the async Stop judge finishes and gets no task-end verdict, naming CI as the gate for that case. This is the documentation half of the response to the Codex P1 finding on this PR (kyle-sexton's comment on that finding deferred the CLAUDE_CODE_ENTRYPOINT-based synchronous-fallback design to a follow-up and chose to document the gap here instead).

  • No code, hook wiring, or filter logic changed in this increment — hooks.json, test-judge.sh, judge-lib.sh, and gen-hook-filters.sh are unchanged since 5f60178 and were already covered by this lane's prior pass on that commit.
  • Instruction-surface lens: this diff only adds disclosure about an existing, already-reviewed behavior change (the asyncRewake Stop hook and SubagentStop removal landed in 5f60178); it does not delete, narrow, or soften any standing instruction, hook allow/deny list, or guardrail. Nothing here reduces an existing check.
  • No injection surface, secret handling, or trust-boundary change: both edits are static Markdown text with no templating or interpolation of external/untrusted input.

@claude

claude Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 8s —— View job


Code review complete

  • Gather context (PR diff, incremental diff, custom instructions)
  • Read .claude-lane/incremental.diff
  • Invoke /review:code-review and follow its instructions
  • Review the two changed files (changelog fragment, README.md)
  • Post findings

Scope reviewed (incremental): only what changed since 5f60178 — .changes/testing/perf-testing-judge-async-rewake-da85f122.md and plugins/testing/README.md (the doc follow-up to the claude -p/Agent SDK no-verdict gap raised in Codex's earlier review). Every other file was reviewed at 5f60178 and is unchanged.

Finding posted inline: plugins/testing/README.md:102-104 — the new paragraph about claude -p/Agent SDK sessions getting no task-end verdict was spliced into the middle of an existing sentence, leaving a dangling Each and a garbled paragraph (...changes). Each A \claude -p` or Agent SDK session ends...gate. Each UNKNOWN records...`). Suggested fix included inline.

The changelog fragment (.changes/testing/perf-testing-judge-async-rewake-da85f122.md) reads cleanly and accurately mirrors the README's new claim; no issues there.

Comment thread plugins/testing/README.md Outdated
@claude

claude Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 36s —— View job


Code review complete

  • Gather context (incremental diff, PR body, prior review comments)
  • Read .claude-lane/incremental.diff
  • Invoke /review:code-review and follow its instructions
  • Review the README.md change since d23b667
  • Post findings

Scope reviewed (incremental): only what changed since d23b667 — plugins/testing/README.md (commit 9c60f12). Every other file was reviewed at d23b667 and is unchanged.

No issues found. This commit is exactly the one-line fix for the dangling-Each finding raised in the prior incremental review pass (d23b667): plugins/testing/README.md#L99-L105.

  • The stray trailing Each after ...output delivery changes). is removed; the sentence now ends cleanly at the period, and Each UNKNOWN records its reason kind... (line 105) remains its own, now un-garbled, sentence.
  • Diffed the before/after text directly (d23b667 vs 9c60f12): no other wording changed, no line-wrap or whitespace drift introduced elsewhere in the paragraph.
  • The asyncRewake citation still carries its as-of date and recheck trigger per skill-bodies-state-current-rules, unaffected by this edit.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit b229484 Oct 9, 2026
23 checks passed
@kyle-sexton
kyle-sexton deleted the perf/testing-judge-async-rewake branch October 9, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant