ci: both AI review lanes reported success having reviewed nothing, and the #3147 evidence guard passes a lane that posts nothing at all

[kyle-sexton]

This was generated by AI during work.

What happened

On PR #4059 (head c94be900), both AI review lanes concluded success and produced no review and no finding. The check rows went green, the required ci-status check went green, and the PR was mergeable on that evidence. The only substantive review on that PR came from Codex, which found three real bugs.

The initial hypothesis was quota exhaustion on the account behind the lanes. That is not what happened. Both sessions ran, were billed, and exited cleanly well inside every budget.

Evidence

From the two sessions' own result records in the job logs:

	claude-review	claude-security-review
Run	34569637210	34569636940
subtype / is_error	success / false	success / false
Turns used	20 of 75	34 of 75
Wall time	3m17s of a 15m cap	8m03s
permission_denials_count	23	8
total_cost_usd	1.70	2.11
Review posted	none	none
Findings produced	none	none

No quota, rate-limit, 429, overload, or credit signal appears anywhere in either log. Neither ran out of turns or time. Roughly $3.81 was spent to produce nothing, and both lanes reported green.

Each session's tracking comment records its own incomplete state. claude-review logged:

Invoke /review:code-review (skill errored in this environment; proceeding with an equivalent manual review: correctness + maintainability, security out of scope)

and then stopped with all seven of its review items unchecked. claude-security-review stopped with all four of its passes unchecked.

Root cause, and what is inferred rather than proven

Proven: the string Skill appears zero times in the claude-review job log, including in the resolved ALLOWED_TOOLS, which is:

Glob, Grep, LS, Read, mcp__github_comment__update_claude_comment,
Bash(git add:*), Bash(git commit:*), Bash(<action>/scripts/git-push.sh:*), Bash(git rm:*)

plus Bash(gh pr diff:*) and mcp__github_inline_comment__create_inline_comment from claude_args. The prompt handed to that session is Invoke /review:code-review now and follow its instructions exactly for this pull request. The tool required to satisfy that instruction is not in the grant.

Inferred: that the missing Skill grant accounts for the 23 denials. The per-turn stream is not in the job log (only the init and result records are), so the denied tool names cannot be read back. The count, the absent grant, and the session's own admission that the skill errored all point the same way, but the attribution is not directly observable from the log.

This is the same failure named in #3147 ("claude-review lane cannot invoke /review:code-review: Skill tool errors in the workflow environment, lane degrades to manual review"), which is closed with all three of its acceptance criteria unchecked. Criteria 2 and 3 are both still failing:

• "A claude-review run on a PR invokes /review:code-review successfully, or the lane fails loudly when it cannot"
• "The silent-degrade path (manual review with a green check row) no longer masks a broken skill chain"

The guard built for this does not catch it

review-skill-evidence exists in .github/workflows/claude-review.yml precisely to redden this class, and it passed. Reproduced locally against the real PR at the real head:

$ GITHUB_EVENT_NAME=pull_request GITHUB_ACTOR=kyle-sexton PR_NUMBER=4059 \
  EVENT_HEAD_SHA=c94be900... LANE_RESULT=success REVIEWER_LOGINS='claude[bot]' \
  bash scripts/verify-claude-review-skill.sh
claude-review skill evidence OK (no silent Skill-tool fallback in posted review bodies)
$ echo $?
0

The guard fetches review bodies posted by claude[bot] and classifies each one for a Skill-failure admission. On this PR claude[bot] posted 0 reviews and 3 issue comments. With no review body to classify, the loop body never executes and the script falls through to its success line.

Two gaps compound:

1. Absence of a bad body is read as absence of a problem. The guard can only find a degrade inside a posted review. A lane that posts nothing has nothing to classify and passes. "No review was posted" and "a clean review was posted" are the same verdict.
2. The admission was in a surface the guard does not read. The skill errored in this environment sentence was in the tracking issue comment, not in a review body. The guard reads review bodies by design (deliberately not the job log, per security-evidence guard fails every successful in-scope PR by matching the lane's echoed script source #2517), so the one piece of direct evidence sat where it could not be seen.

The sibling verify-security-review-evidence.sh is structurally sounder: it branches on LANE_REVIEW_RAN, a positive declaration that a review ran, and errors when that is false. It passed here because the upstream reusable declared review_ran=true, which evidently means "the session ran", not "a review was produced".

Proposed work

• Decide whether to reopen ci: claude-review lane cannot invoke /review:code-review — Skill tool errors in the workflow environment, lane degrades to manual review #3147 or track the remainder here. Its criteria 2 and 3 are unmet.
• Grant the Skill tool to the review lane's session, or confirm with the reusable workflow's owner how a skill is meant to be invoked there, and make the lane fail loudly when the invocation is refused.
• Give verify-claude-review-skill.sh a zero-review branch: an in-scope lane that concluded success and posted no review body at all is a failure, not a pass. Distinguish "reviewed and found nothing" from "never reviewed".
• Have the guard read the tracking issue comment as well as review bodies, since that is where a degrading session actually confesses.
• Reconcile what review_ran=true means upstream. If it reports session completion rather than review production, the security guard's central signal does not mean what its name says.
• Consider surfacing permission_denials_count from the result record as a lane signal. A double-digit denial count is a strong indicator that a session could not do the job it was given.

Acceptance criteria

• A claude-review run that cannot invoke /review:code-review fails its check row rather than reporting success.
• verify-claude-review-skill.sh fails for an in-scope lane that concluded success and posted no review body, with a regression case covering the zero-review input.
• verify-security-review-evidence.sh's LANE_REVIEW_RAN signal is confirmed to mean a review was produced, or the guard stops relying on it as though it does.
• A lane that produced no review and no finding is visible as such on the PR, without reading the job log.

Workaround now in place

plugins/source-control 0.55.75 adds step B2 to the PR monitor's per-iteration checklist: a review lane that posted no review body and no finding is classified ABSENT rather than PASS, and a local review (/review:fanout, or /review:code-review for the correctness lane alone) is run over the same diff and named in the report. The readiness gate in step E does not clear until every review lane is productive or substituted. That is an agent-side compensation for a CI-side defect and does not close this issue.

Related

• ci: claude-review lane cannot invoke /review:code-review — Skill tool errors in the workflow environment, lane degrades to manual review #3147, closed with unchecked criteria; same root cause, and the guard it produced is what this issue reports a gap in.
• security-evidence guard fails every successful in-scope PR by matching the lane's echoed script source #2517, why the guard reads posted bodies rather than the job log.
• PR refactor: twenty-one module deepenings across the hook library, guardrails and the check scripts #4059, where this was observed. Codex reviewed the same head and found three real defects, all confirmed and fixed, which is the measure of what the two green lanes missed.

[kyle-sexton]

The agent-side workaround named in the issue body is committed and pushed, but not yet proposed as a pull request:

• Branch: claude/architecture-improvements-hqfc0i
• Commit: 6a8a1db9 — feat(source-control): judge a review lane by what it produced, not by its check row
• Files: plugins/source-control/skills/pull-request/SKILL.md (step B2, step E gate, one gotcha), CHANGELOG.md, manifest bump to 0.55.75

It is branched fresh from main at b5350ca2. Gates run clean against that base: em-dash gate, all four changelog-parity modes, markdownlint, manifest JSON. affected-tests.sh selects no suites, every changed file being a recorded no-suite class.

This compensates on the agent side and changes nothing about the CI defect the issue reports. The acceptance criteria above are all still open.

---

Generated by Claude Code

[kyle-sexton]

Third reproduction, on the PR that compensates for it

#4106 hit the same failure while it was open. On head 9ac42aaa:

• claude-review (run 34625139xx) concluded success in 3m09s.
• claude-security-review concluded success in 38s.
• claude[bot] posted 0 reviews. Both check rows green.

Every finding on that PR came from Codex, which raised three (one P1, two P2). All three were verified against the files and all three were real, including a genuine false-pass hole in the very step meant to catch this class. Two consecutive PRs now where the Claude lanes were green and contributed nothing while another reviewer found real defects.

The 38-second security-review run is a new data point worth keeping: that is far too short for the four passes its own tracking comment enumerates, which suggests the lane can also exit almost immediately rather than only stalling partway. Whatever the guard eventually checks should catch a suspiciously short run as well as an empty one.

This does not change the acceptance criteria above; it raises the confidence that the failure is systematic rather than incidental to one PR.

---

Generated by Claude Code

[kyle-sexton]

Correction: my "third reproduction" comment above is wrong

Retracting the previous comment's claim that #4106 reproduced this. It did not. Both lanes ran correctly there, and I measured the wrong surface.

I checked GET /pulls/4106/reviews filtered to claude[bot], got 0, and reported "posted zero reviews / produced nothing". These lanes do not post to the reviews surface. They post to issue comments, and on #4106 both completed with every checklist item ticked:

• claude-review (3m09s): ran the skill and produced a substantive cross-file finding, that the new B2 step was not wired into the reference docs that operationalize "run the readiness gates". That finding was correct against the head it reviewed, and the fix landed before merge.
• claude-security-review (38s): invoked the skill and hit its documented skip gate ("no security-relevant surface"), naming all three changed files and why none carries security surface. That is a reasoned no-findings verdict, not a failure.

The speculation in my previous comment that a 38-second run is "far too short for the four passes its own tracking comment enumerates" was also wrong. That comment's four passes were from the #4059 run; on #4106 the lane enumerated three items, ticked all three, and exited early through a skip gate that exists for exactly this case. Please disregard that paragraph, including the suggestion that a guard should flag short runs.

What still stands

The issue body is unaffected. It rests on #4059, where I read the issue-comment surface and both lanes had left checklists with items unchecked and no findings, claude-review having logged that the skill errored. That reproduction is solid, as are the log-derived numbers (23 and 8 permission denials, 20 and 34 turns of 75, Skill absent from ALLOWED_TOOLS).

So the failure is intermittent, not systematic. One confirmed occurrence, not three. That lowers the priority I implied and changes the shape of the fix: the question is why the skill loaded on #4106 and not on #4059, not why it never loads.

One thing this sharpens rather than weakens

The guard gap is now better characterized, and worse than the body states. verify-claude-review-skill.sh reads review bodies. These lanes write to issue comments. So the guard is not merely blind to a lane that posted nothing, it reads a surface the lane never writes to at all, in the success case as much as the failure case. On #4106 the lane produced a genuine finding and the guard still had nothing to classify.

That makes the second acceptance criterion above more pointed: a zero-review branch is not enough, the guard has to read the surface the lane actually uses.

My apologies for the noise on an issue someone else may act on.

---

Generated by Claude Code