fix(claude-review): keep the reviewer single-pass and stop losing inline findings silently

[topcoder1]

Manual merge (ci-workflows policy). This changes the tool surface of the review lane every fleet PR runs, and adds a step that can fail a REQUIRED check — read the "What changes for consumers" section before merging.

HUMAN_READABLE_SUMMARY: Since the 1.0.190 action bump the reviewer's first move is the CLI's bundled multi-agent /code-review skill, which runs as a background subagent that --max-turns never sees — reviews went from 1–8 min to 20–134 min fleet-wide, and findings sometimes never reached the PR while the check stayed green. This removes the fan-out tools from the reviewer (--disallowedTools Task,Agent,Skill,Workflow) so the lane is single-pass and bounded again, and adds a guard that compares the summary's "Flagged N issues inline" claim against the inline comments actually posted, prints every permission denial WITH its tool name, recovers lost findings from the transcript into a fallback comment, and fails the job only when nothing was recoverable.

Auto-merge rationale: manual — topcoder1/ci-workflows is always manual; this touches .github/workflows/** and a fleet-wide required check.

---

The incident (whois-api-llc/wxa-graph#403, run 31925279662)

review / Claude Review — required fleet-wide — ran 03:52→04:45Z (52 min), posted "Flagged 6 issues inline — most critical: …", created zero inline review comments, and concluded SUCCESS. The log held only:

"num_turns": 3, "duration_ms": 3081851, "total_cost_usd": 4.49, "permission_denials_count": 3
Post buffered inline comments: No buffered inline comments

Four of the six findings were lost; the babysitting session recovered the two the summary happened to spell out (its comment).

What the investigation found (answers to the three questions asked)

1. What was denied? Not recoverable for that run — the action hides the SDK stream (show_full_output: false), prints only a count, and keeps no transcript artifact. This PR closes that gap: the guard prints every permission_denials[] entry with tool_name + input from the execution_file the action leaves on disk. What the evidence does establish:

• Tool-name mismatch: refuted. In v1.0.190 the MCP server is github_inline_comment, the tool create_inline_comment → mcp__github_inline_comment__create_inline_comment, byte-identical to the allowlist (install-mcp-server.ts, github-inline-comment-server.ts).
• classify_inline_comments: true: not the cause. Buffering has shipped since v1.0.151. Without confirmed=true a call is buffered to /tmp/inline-comments-buffer.jsonl and posted by the action's own post-step; with it the comment posts live and the buffer copy is dropped. The 03:41Z run on the same PR (permission_denials_count: 13, also "No buffered inline comments") posted 7 inline comments that way — for a summary that said "Flagged 3". "No buffered inline comments" in the bad run therefore means the tool was never successfully invoked, not that a buffer was dropped.
• 51 min ≠ retries on denials. Denials are instant in -p mode. num_turns: 3 at $4.49 is only possible when the work happens inside tool calls: subagents.

The actual mechanism, from two independent sources:

• Fleet timing. Every consumer's PR Review went from 1–8 min to 20–134 min starting 2026-08-14 ~19:16Z — the 1.0.181→1.0.190 bump (#136) merged at 19:10:44Z. That bump moved Claude Code 2.1.218→2.1.227. Sample: wxa-graph 08-14 03:07Z–08:01Z 1/3/3/1 min; 08-15 onward 41/36/20/56/16/31/34/25 min. ci-workflows 08-14 ≤8 min; 08-14 23:54Z 134 min, 00:03Z 101 min. Every run since carries 3–17 permission denials.
• Local replay on the same CLI build (2.1.227), same prompt/allowlist/model/turn cap, gh writes shimmed, action's MCP server with a dummy token — full transcript captured. Result block: num_turns: 11, 27 min, total_cost_usd: 7.95, permission_denials: 7 — all seven Bash. First tool call: Skill({"skill":"code-review"}); 246 of 637 stream messages carry a parent_tool_use_id (the CLI's bundled review runs as a background subagent — Claude Code 2.1.218 changelog: "Changed /code-review to run as a background subagent") and it spawned three nested Agent finders ("Angle A & B: Line-by-line diff scan…", "Angle C & Reuse…", "regression test correctness…"). The denied calls, verbatim: a for d in …; do echo loop, cat <<'EOF' > /tmp/regex_check.go (a finder writing a Go program to test a regex), find /Users/… -name CLAUDE.md, git -C … diff, tail -c 5000 …, and — the instructive two — cd … && gh pr diff 403 … and gh pr view … | python3 -c …: allowed prefixes, denied because compound/piped, which is exactly how a gh pr comment post can also vanish. The finders' CONFIRMED/PLAUSIBLE verdict labels are what the 03:41Z run's inline comments carry. The skill never ran gh pr diff for its scope (it runs git diff main...HEAD; in a CI checkout there is no local main) and its own report drifted onto files outside the PR — the parent's final message: "The skill's subagents drifted into files not touched by this PR … I discarded those findings and reviewed the actual six-file diff directly." Findings return to the parent as text; whether they reach the PR depends on the parent re-posting them through the MCP tool — or the skill's --comment fallback, gh api …/pulls/{pr}/comments, which the allowlist denies.

The fix

1. --disallowedTools "Task,Agent,Skill,Workflow" in claude_args. Verified on 2.1.227 -p: the four tools vanish from the init tool list; the model reports it has no Skill/Agent/Task tool. Verified through the action's own parseSdkOptions on this exact claude_args string → disallowedTools: ["Task","Agent","Skill","Workflow"], allowlist intact. Both subagent spellings are named because the tool has carried each across CLI versions (2.1.227's init lists Task; its tool_use blocks say Agent). Effect: the reviewer is single-pass again — reads gh pr diff, posts through the inline-comment MCP tool, and the turn cap is real. This is the design the lane's own comment describes ("the turn cap bounds worst-case tokens regardless of future model or binary behavior changes") — a statement that has been false since 08-14.

2. Lost-findings guard (new step, after the action, !cancelled()):

step	source	result
transcript digest	steps.claude.outputs.execution_file	::warning:: per denial with tool name + input (300 chars); inline-comment call / error / subagent counts
FACT	gh api …/pulls/N/comments --paginate	inline review comments by claude[bot] created since a clock anchor step taken just before the action
CLAIM	gh api …/issues/N/comments --paginate	max N in the bot's summaries: Flagged N issue(s)… + the word "inline"
verdict	claim > posted	recover from transcript: bodies of denied inline-comment calls (permission_denials[].tool_input), errored ones (422 etc.), buffered-but-never-posted ones (successful tool_use whose body no posted comment carries), any subagent/skill report, and a final message that is clearly more than the summary echoed back → ONE fallback comment (marker <!-- claude-review:lost-findings -->, phrased flagged N issue(s) so bb-unaddressed-findings.sh admits it)
exit		fail when posted = 0 and nothing beyond the summary was recoverable, or when recovered findings could not be posted (a log is not the PR); recovered and posted → warning, green; partial (0 < posted < N) → warning, recovered content posted if any, never fails (the model's count is noisy both ways — the 03:41Z run said 3, posted 7)

Fail-safe on the reads that decide the verdict: API errors listing either comment set, or phrasing outside the mandated shape → warning + exit 0. The transcript is evidence for recovery, not for the verdict — claim > posted is established from GitHub alone, so a missing/malformed transcript makes a real loss unrecoverable (red), never spurious; malformed entries are skipped, never fatal. Overlapping runs on one PR cannot happen (every installed caller carries concurrency: claude-review-<pr>-<draft|live>, cancel-in-progress: true; a cancelled run's guard does not execute).

Dry-run of the shipped bash against the real PR #403 data (writes shimmed): window of the bad run → "summary claims 6 inline finding(s); 0 inline review comment(s) … nothing recoverable → exit 1"; window of the good run → "claims 6 … 8 posted → exit 0".

Selftest

selftest/test_claude_review_lost_findings_guard.sh (listed in test_workflow_guards.py, README entry): static pin of the tool surface + step wiring, then extracts the SHIPPED guard bash and runs it against a stubbed gh through 24 scenarios — the incident shape with denied inline bodies (recovered, exit 0), the incident as it actually looked (unrecoverable, exit 1, denials named), the 03:41Z undercount (silent), clean review, Skill-report recovery, errored-call recovery, buffered-then-dropped recovery with dedup against posted bodies, both API-failure fail-safes, the accepted phrasing miss (asserted), partial loss with/without recoverable content, no transcript, malformed transcript entries, empty-body calls, substantive vs bare final message, anchor/author filtering, --paginate multi-page flattening, POST failure after recovery (fails).

Verification

• uv run pytest -q selftest/ → 75 passed (includes the new test + completeness guard)
• bash selftest/test_claude_review_lost_findings_guard.sh → 24/24 PASSED, under /bin/bash 3.2 as well
• actionlint -shellcheck= clean; shellcheck clean on the selftest at all severities
• action parser probe + CLI -p probe as above
• Codex rounds: 6 — r1 four fixes (partial-loss early exit ignored subagent reports; POST failure after recovery exited 0; malformed transcript entries could abort jq mid-way; substantive final message never counted); r2 two declined with rationale in-file (post-buffered step is a regular composite step per the run log; transcript-missing is deliberately not fail-safe); r3 one fix (empty-body calls no longer count as recovery); r4 one fix (buffered-but-unposted calls recovered from tool_use.input) + one declined (partial mismatch stays a warning); r5 one declined (overlap prevented by the callers' concurrency group); r6 clean.

What changes for consumers

• Reviews go back to single-pass Sonnet 4.6 within max_turns — no CONFIRMED/PLAUSIBLE verify pass. That is the pre-08-14 baseline the fleet ran on; if a repo wants the deep review it should be an explicit opt-in, not a side effect of a dependency bump (not added here).
• Expect durations back to minutes and cost back to cents per review.
• A summary that overclaims (N > 0, nothing inline, nothing recoverable) now fails review / Claude Review — re-run the lane. Recovered findings show up as one github-actions[bot] comment the babysitter sweep counts as unaddressed.

Follow-ups (not in this PR)

• verifier-on-high-risk.yml and claude-adversarial-review.yml pin the same action version; check their durations since 08-14 and whether they want the same --disallowedTools.
• A run whose bot posts NO summary at all is warned, not failed (the summary is the "review ran" signal; failing on its absence is a new failure mode for a required check — decide separately).
• Consider uploading execution_file as a short-retention artifact so future incidents keep the whole transcript, not just the digest.
• Upstream: classify_inline_comments's buffer reply invites a second confirmed=true call per finding (the good run posted 2 duplicates); worth an issue on claude-code-action.

Refs: wxa-graph#403 comments 5305797473, 5305810583; run 31925279662.

🤖 Generated with Claude Code

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[github-actions]

Risk class: blocked — manual merge required.

This PR touches one of the blocked path categories from .github/risk-paths.yml (Dockerfiles, docker-compose, .github/workflows/**, **/.env*, **/secrets*, infra/, terraform/, k8s/, or the classifier config itself).

Auto-merge is refused by claude-author-automerge.yml. A maintainer should review the diff and click "Squash and merge" themselves.

(This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.")

[github-actions]

Coverage Floor — mode: enforce

metric	value
measured	100.0%
floor (current)	99.0%
target	100.0%
last bumped	2026-05-12

[topcoder1]

Live data point from this PR's own review / Claude Review run (31933028054), which ran the current @main reusable (i.e. without this fix):

duration_ms: 3834171 (64 min)   num_turns: 19   total_cost_usd: 5.44   permission_denials_count: 10
Post buffered inline comments: No buffered inline comments

Outcome on the PR: no summary comment, no inline comments, check SUCCESS. That is the "review said nothing" variant this PR's guard currently only warns on (no summary comment by claude[bot] since <anchor>), listed under follow-ups. With --disallowedTools in place the parent posts the summary itself in a single pass, so this shape should become rare — but if you'd rather the guard fail the job when the bot posts no summary at all, it is a two-line change to the step (treat empty SUMMARIES as ::error:: + exit 1). Left as-is here to keep the change to what was asked; say the word and I'll fold it in.

Sweep before merge: no review findings to address (nothing was posted). All checks green; manual click-merge as always for this repo.