Skip to content

A Claude review that posted nothing no longer finishes green: read-only tools end the denial churn, the job fails when no verdict was submitted, and the transcript survives the runner #3657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Sep 18, 2026
Merged

A Claude review that posted nothing no longer finishes green: read-only tools end the denial churn, the job fails when no verdict was submitted, and the transcript survives the runner #3657

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 151 additions & 3 deletions .github/workflows/claude-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ name: Claude Auto Review
# enforcement lives in claude-review-guard.yml, whose verdict arm fails while the newest verdict
# is changes-requested, so a substantive finding becomes a red check instead of a comment that
# auto-merge outruns (the #3470-#3473 train shipped six findings in one night that way; one was
# real). It no-ops cleanly until the CLAUDE_CODE_OAUTH_TOKEN repo secret is set, and on fork PRs
# (which do not receive secrets), so neither case shows a failed check.
# real). Since #3650 the job also fails ITSELF when the run submitted no verdict, with the cause in
# its log and the transcript attached -- a green here means a verdict exists, and the guard is the
# second line. It no-ops cleanly until the CLAUDE_CODE_OAUTH_TOKEN repo secret is set, and on fork
# PRs (which do not receive secrets), so neither case shows a failed check.
on:
pull_request:
types: [opened, synchronize, reopened]
Expand Down Expand Up @@ -37,7 +39,19 @@ jobs:
with:
fetch-depth: 1

# #3650: the verdict check at the bottom counts the bot's formal reviews submitted AFTER this
# instant, so a verdict left by an earlier run on the same PR cannot vouch for this one. Read
# once, here, before the action installs anything -- GitHub stamps submitted_at in the same
# ISO-8601 UTC shape (2026-09-18T22:00:00Z), so the comparison below is a plain string one.
- name: Open the verdict window
id: window
if: ${{ env.CLAUDE_CODE_OAUTH_TOKEN != '' }}
run: echo "start=$(date -u +%FT%TZ)" >> "$GITHUB_OUTPUT"

# The step NAME is read by claude-review-guard.yml (REVIEW_STEP) to tell a clean no-op from a
# run that said nothing; renaming it blinds the guard. The id is for the steps below.
- name: Claude review
id: review
if: ${{ env.CLAUDE_CODE_OAUTH_TOKEN != '' }}
uses: anthropics/claude-code-action@v1
with:
Expand Down Expand Up @@ -78,5 +92,139 @@ jobs:
does not enable, and no gate needs it. The guard's rule is newest-verdict-wins: on
a re-review after new commits, review the NEW diff and submit a fresh verdict, and
a clean fresh verdict clears an earlier changes-requested by itself.
# #3650: the prompt above says the branch is checked out and asks for a correctness,
# parity and security review -- an invitation to read code -- while the allowlist used to
# permit only the four gh verbs and the inline-comment tool. Every Read, Grep, Glob and
# git call the reviewer reached for was a permission denial, and the swallowed runs' own
# result blocks put the ratio on record: 50 turns / 18 denials, 39 / 21, 18 / 17, each
# ending subtype=success with NOTHING posted. After enough denials the session ends
# without ever reaching the verdict protocol, so a paid run leaves no trace (three PRs,
# about thirteen runs, one night). Read-only tools are enough: the reviewer needs to open
# the files the diff touches, find a symbol's other callers and read a parity twin, and
# nothing about reviewing needs a write. Nothing here can edit, commit, push or post
# outside the gh verbs already listed; the git verbs are the read-only three.
claude_args: |
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr review:*)"
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr review:*),Read,Grep,Glob,Bash(git diff:*),Bash(git log:*),Bash(git show:*)"

# #3650: a review that posted nothing used to finish GREEN. The prompt mandates one formal
# review per run -- changes-requested on a substantive finding, a "LGTM" comment review
# otherwise -- so on this repo a run that submitted no verdict has broken its contract every
# time; a clean review is never silent, and a zero here is never legitimate. This step turns
# that into the job's own colour, with the diagnosis in the log, so the guard (#2229/#3492,
# still the REQUIRED check, still enforcing newest-verdict-wins and the drift arm) becomes the
# second line rather than the first. Counted: reviews by the bot, submitted inside this run's
# window, with a NON-EMPTY body. The body test is load-bearing: the inline-comment tool files
# each comment inside a review of its own with an empty body (#3647 carried four bot reviews,
# two of them bodiless carriers), and both verdict shapes must carry one (gh and the API both
# refuse a bodiless comment/changes-requested review) -- so "posted inline notes, never a
# verdict" is the #3470 shape and fails here, as it should.
# Two runs end with zero verdicts and both are defects on this repo, told apart by whether
# Claude ran at all: the action exits SUCCESS in seconds without running when this file
# differs from the default branch's copy (cause 1 in the guard's header), and then it sets no
# execution_file and writes no transcript. Expected on a PR that edits this file; a repo-wide
# outage otherwise -- the guard grades which, because it can read the PR's file list from a
# workflow that is free to change. The step's own outcome is left to speak for itself when it
# is not success (a failed step already reds the job; a cancelled one is a superseded push).
# A gh lookup failure is NOT a verdict on the review (#2309): it warns and stands down rather
# than forcing a paid re-run of the whole job to clear a transient API error -- the guard's
# own tally, a separate and free-to-rerun workflow, stays the enforcement.
- name: Verify the review posted a verdict
if: ${{ always() && env.CLAUDE_CODE_OAUTH_TOKEN != '' }}
env:
GH_TOKEN: ${{ github.token }}
R: ${{ github.repository }}
PR: ${{ github.event.pull_request.number }}
SINCE: ${{ steps.window.outputs.start }}
REVIEW_OUTCOME: ${{ steps.review.outcome }}
# Set by the action only after Claude actually ran; empty when it refused at validation.
EXECUTION_FILE: ${{ steps.review.outputs.execution_file }}
# Author of every artifact the review leaves behind (the action's bot_name default).
REVIEW_BOT: claude[bot]
run: |
set -euo pipefail
summary() { echo "$*" >> "$GITHUB_STEP_SUMMARY"; }

if [ "$REVIEW_OUTCOME" != "success" ]; then
echo "::notice title=Verdict check skipped::The review step ended '$REVIEW_OUTCOME'; its own"\
"outcome carries the story, so this step has nothing to add."
exit 0
fi

transcript="${EXECUTION_FILE:-$RUNNER_TEMP/claude-execution-output.json}"

errfile=$(mktemp 2>/dev/null || echo /dev/null)
if ! matched=$(gh api --paginate "repos/$R/pulls/$PR/reviews?per_page=100" \
--jq ".[] | select(.user.login == env.REVIEW_BOT
and .submitted_at != null
and .submitted_at >= env.SINCE
and ((.body // \"\") | length) > 0)
| \"\\(.state)\\t\\(.submitted_at)\\t\\(.html_url)\"" 2>"$errfile"); then
err=$(cat "$errfile" 2>/dev/null || true)
[ "$errfile" != /dev/null ] && rm -f "$errfile" || true
echo "::warning title=Verdict check could not read the PR's reviews::gh api"\
"repos/$R/pulls/$PR/reviews failed: ${err:-no stderr}. A lookup failure is not a"\
"review verdict (#2309), so the review is UNCONFIRMED here; the guard's own tally"\
"decides, or read the PR by eye."
summary "- Verdict check: lookup failed; review UNCONFIRMED (#2309)."
exit 0
fi
[ "$errfile" != /dev/null ] && rm -f "$errfile" || true

count=$(printf '%s' "$matched" | grep -c . || true)
echo "verdict reviews by $REVIEW_BOT on PR #$PR since $SINCE: $count"
if [ -n "$matched" ]; then printf '%s\n' "$matched"; fi

if [ "$count" -gt 0 ]; then
summary "### Claude review posted $count verdict review(s) since $SINCE"
exit 0
fi

summary '### Claude review posted NO verdict'

if [ -z "$EXECUTION_FILE" ] && [ ! -s "$transcript" ]; then
echo "::error title=Claude never ran::claude-code-action exited success without running"\
"Claude -- no execution file, no transcript. That is what it does when this branch's"\
".github/workflows/claude-review.yml differs from the default branch's copy (#2229,"\
"cause 1). Expected on a PR that edits this file; on any other PR it means the file"\
"has drifted and EVERY PR in the repo is going unreviewed -- see the guard's verdict"\
"on this PR. Either way, do not read this PR as reviewed."
summary '- Claude never ran (workflow validation refused). Not reviewed.'
exit 1
fi

echo "::error title=Review ran and posted no verdict::The review ran and posted no"\
"verdict: $REVIEW_BOT submitted no formal review on PR #$PR since $SINCE, and the"\
"prompt promises one every run even when clean. Real money was spent and the output"\
"vanished (#3650; the #2229 failure shape). Do NOT read this PR as reviewed. The"\
"transcript is attached to this run as the claude-review-transcript artifact; its"\
"result block follows."
summary '- The review ran and submitted no formal review. Not reviewed (#3650).'
if [ -s "$transcript" ]; then
echo "--- result block of $transcript ---"
jq -c '.[-1] | {type, subtype, is_error, num_turns, permission_denials_count,
total_cost_usd, duration_ms,
denied: [.permission_denials[]? | .tool_name]}' "$transcript" \
2>/dev/null || true
echo "--- tail of $transcript ---"
tail -c 4000 "$transcript" || true
echo
else
echo "(no transcript at $transcript)"
fi
exit 1

# #3650: the per-turn transcript is what proves WHY a run said nothing, and it used to die
# with the runner -- the denial-ratio evidence above had to be inferred from result blocks in
# the step log. Seven days is long enough to diagnose the next swallowed run in one click and
# short enough that nothing accumulates. The reviewer's tools are read-only over a public
# tree and gh reads of public PR data, so the transcript holds nothing that is not already
# public; keep the allowlist that way, because this artifact is readable by anyone who can
# read the repo. Missing file (the action refused to run, or was skipped) is not an error.
- name: Retain the review transcript
if: ${{ always() && env.CLAUDE_CODE_OAUTH_TOKEN != '' }}
uses: actions/upload-artifact@v6
with:
name: claude-review-transcript
path: ${{ steps.review.outputs.execution_file || format('{0}/claude-execution-output.json', runner.temp) }}
if-no-files-found: ignore
retention-days: 7
Loading