Skip to content

Commit df54d0e

Browse files
kyle-sextonclaude
andauthored
fix(claude-review): surface real review failures instead of a silent green (#117)
## Summary Every `claude-review` run across all consumer repos has been silently reporting green for the last 27+ hours while the underlying Claude Agent SDK call fails with `is_error: true` (0 cost, 1 turn, <1s). No red check, no warning, just a permanently stuck "I'll analyze this and get back to you" placeholder comment on every PR. **Root cause of the invisibility:** our pinned `anthropics/claude-code-action` version (v1.0.165→166) predates upstream fix [#1496](anthropics/claude-code-action#1496) for [#1495](anthropics/claude-code-action#1495) ("Action reports success when result is is_error:true"), which first shipped in v1.0.172. Before that fix, `steps.claude-review.outcome` was always `success` regardless of the SDK result, so our existing "Report review outcome" step's failure branch was dead code. **This PR does not fix why the SDK call itself is failing** — that's bisected to an Anthropic-account-side condition on the `CLAUDE_CODE_OAUTH_TOKEN` credential (two independent tokens broke simultaneously with no local change; 27+ hours of continuous failure rules out a simple 5-hour rate-limit reset). That requires checking the Anthropic Console, which isn't something this PR can address. This PR makes that failure *visible* instead of silently green, which is the actual gap that let it run unnoticed for over a day. ## Changes - Bump `anthropics/claude-code-action` pin from v1.0.166 → **v1.0.174** (past the #1495 fix; also picks up an unrelated `ghu_` token-redaction hardening from the same release range — reviewed every intermediate commit v1.0.172→v1.0.175 via `gh api compare`, nothing else in range). - Rewrite "Report review outcome": on a real failure, read the action's own `execution_file` output (no `show_full_output` needed) for the last SDK result, emit a real `::error::` annotation (was `::warning::`, and never fired), and expose `review_failed`/`review_detail` step outputs. - New "Comment on genuine review failure" step: posts (sticky, via `--edit-last --create-if-none`) an explicit PR comment labeling any lingering placeholder as an incomplete infra failure — not "no findings" — with the run URL and last SDK result. - `continue-on-error: true` stays on the review step for now — this is a visibility fix, not a new merge gate. Left as a deliberate follow-up decision whether a persistent failure should eventually block. ## Verification - `actionlint` — clean. - `zizmor` — one pre-existing `artipacked` medium finding, confirmed unchanged from `main` (unrelated to this change, already accepted by design per the checkout step's own comment). - Dry-ran the new shell logic locally against three cases (clean review / `is_error` with execution file / failure with missing execution file) — all three branch and format correctly. - Not yet verified against a live GitHub Actions run — the account-side failure currently affecting every repo means the very next PR push anywhere downstream will exercise the failure path for real once this rolls out. ## Scope notes - Did not bundle the still-unconsumed `runner` self-hosted-routing input (from `4dbb0df`) — that's a separate, deliberately staged multi-repo rollout per its own commit message. - Follow-up once merged: bump the pinned SHA in each of the 6 caller repos (`dotfiles`, `standards`, `claude-code-plugins`, `provisioning`, `github-iac`, `medley`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QSCqyt1o7XnDXCgyp8ejKT --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 0a53868 commit df54d0e

1 file changed

Lines changed: 110 additions & 4 deletions

File tree

.github/workflows/claude-review.yml

Lines changed: 110 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ jobs:
265265
# findings go to PR comments, not the exit code. continue-on-error keeps
266266
# an OIDC/usage-limit/SDK blip from showing as a red check.
267267
continue-on-error: true
268-
uses: anthropics/claude-code-action@f87768c6d25f92ae6efa7175e223ef77d4cbf97f # v1.0.166
268+
uses: anthropics/claude-code-action@12531344451323133b0493233c759991ac61da12 # v1.0.174
269269
with:
270270
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
271271
track_progress: ${{ inputs.track-progress }}
@@ -288,16 +288,122 @@ jobs:
288288
https://github.com/${{ github.repository }}/blob/${{ github.event.pull_request.head.sha }}/<path>#L<line>
289289
290290
- name: Report review outcome
291+
id: review-outcome
291292
if: always()
292293
env:
293294
REVIEW_OUTCOME: ${{ steps.claude-review.outcome }}
295+
EXECUTION_FILE: ${{ steps.claude-review.outputs.execution_file }}
294296
run: |
295297
if [ "$REVIEW_OUTCOME" = "success" ]; then
296298
echo "Claude review completed. Findings (if any) are in the PR comments."
299+
echo "review_failed=false" >> "$GITHUB_OUTPUT"
300+
exit 0
301+
fi
302+
303+
# Prior to claude-code-action v1.0.172, the action reported `success`
304+
# even when the underlying SDK result had is_error:true (upstream
305+
# #1495, fixed in #1496) — this branch was effectively dead. Bumping
306+
# past the fix makes REVIEW_OUTCOME finally reflect real failures, so
307+
# this now needs to actually surface them instead of just logging.
308+
#
309+
# This is a public repo, so the raw SDKResultMessage is NOT safe to
310+
# publish as-is: its `result` field is model-authored free text (per
311+
# the Claude Agent SDK message schema) and CLAUDE.md keeps
312+
# display_report/show_full_output off for exactly this leak risk.
313+
# Project only the safe, structured metadata fields — never the last
314+
# message wholesale.
315+
detail="(no execution file was produced)"
316+
if [ -n "${EXECUTION_FILE:-}" ] && [ -f "$EXECUTION_FILE" ]; then
317+
detail=$(jq -c '
318+
(.[-1] // empty) as $last
319+
| if $last == null then empty
320+
else $last | {subtype, is_error, num_turns, duration_ms, total_cost_usd}
321+
end
322+
' "$EXECUTION_FILE" 2>/dev/null || true)
323+
[ -n "$detail" ] || detail="(execution file present but unparsable)"
324+
fi
325+
326+
echo "::error::Claude review exited with: $REVIEW_OUTCOME" \
327+
"(infrastructure error, not a code-quality signal — e.g. usage limit," \
328+
"OIDC failure, SDK crash, is_error result, or max-turns exhaustion)." \
329+
"Last SDK result: $detail"
330+
331+
echo "review_failed=true" >> "$GITHUB_OUTPUT"
332+
echo "review_detail=$detail" >> "$GITHUB_OUTPUT"
333+
334+
# Runs only on a genuine infra failure (never on a clean review) and only
335+
# when the token can write: fork-triggered pull_request runs get a
336+
# read-only GITHUB_TOKEN and no secrets by design (CLAUDE.md), so
337+
# gh pr comment would just fail there — skip rather than error.
338+
# Targets the marker comment by ID (never --edit-last, which edits the
339+
# PR's last comment from ANY author) so a stuck placeholder comment from
340+
# track_progress is never mistaken for "no findings" and an unrelated
341+
# bot comment is never clobbered — this is the failure mode this whole
342+
# change exists to fix.
343+
- name: Comment on genuine review failure
344+
if: >-
345+
always() && steps.review-outcome.outputs.review_failed == 'true' &&
346+
github.event.pull_request.number != '' &&
347+
github.event.pull_request.head.repo.full_name == github.repository
348+
env:
349+
GH_TOKEN: ${{ github.token }}
350+
REPO: ${{ github.repository }}
351+
PR_NUMBER: ${{ github.event.pull_request.number }}
352+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
353+
REVIEW_DETAIL: ${{ steps.review-outcome.outputs.review_detail }}
354+
run: |
355+
marker="<!-- claude-review-infra-status -->"
356+
body=$(printf '%s\n' \
357+
"$marker" \
358+
"> [!WARNING]" \
359+
"> **Automated review did not complete** — this is an infrastructure failure, not a review verdict." \
360+
">" \
361+
"> Treat any Claude comment on this PR (including a placeholder like \"I'll analyze this and get back to you\") as incomplete, not \"no findings.\"" \
362+
">" \
363+
"> - Run: $RUN_URL" \
364+
"> - Last SDK result: \`$REVIEW_DETAIL\`" \
365+
">" \
366+
"> Re-running the job, or pushing a new commit, will retry the review.")
367+
368+
comment_id=$(gh api "repos/$REPO/issues/$PR_NUMBER/comments" --paginate |
369+
jq -r --arg marker "$marker" '
370+
[.[] | select(.user.login == "github-actions[bot]" and (.body | startswith($marker)))]
371+
| last | .id // empty
372+
')
373+
374+
if [ -n "$comment_id" ]; then
375+
gh api "repos/$REPO/issues/comments/$comment_id" -X PATCH -f "body=$body" --silent
297376
else
298-
echo "::warning::Claude review exited with: $REVIEW_OUTCOME" \
299-
"(infrastructure error, not a code-quality signal — e.g. usage limit," \
300-
"OIDC failure, SDK crash, or max-turns exhaustion)."
377+
gh pr comment "$PR_NUMBER" --body "$body"
378+
fi
379+
380+
# Symmetric counterpart to the failure step above: a transient infra
381+
# failure (e.g. an OIDC blip) can leave the marker comment behind on the
382+
# PR, and the failure-only condition previously meant no later code path
383+
# ever touched it again. Once a retry (rerun or new push) succeeds, clear
384+
# that stale warning so it is not mistaken for a still-open problem.
385+
# Same fork guard and marker lookup as the failure step — never
386+
# --edit-last, for the same clobbering risk.
387+
- name: Clear stale failure comment after successful review
388+
if: >-
389+
always() && steps.review-outcome.outputs.review_failed == 'false' &&
390+
github.event.pull_request.number != '' &&
391+
github.event.pull_request.head.repo.full_name == github.repository
392+
env:
393+
GH_TOKEN: ${{ github.token }}
394+
REPO: ${{ github.repository }}
395+
PR_NUMBER: ${{ github.event.pull_request.number }}
396+
run: |
397+
marker="<!-- claude-review-infra-status -->"
398+
399+
comment_id=$(gh api "repos/$REPO/issues/$PR_NUMBER/comments" --paginate |
400+
jq -r --arg marker "$marker" '
401+
[.[] | select(.user.login == "github-actions[bot]" and (.body | startswith($marker)))]
402+
| last | .id // empty
403+
')
404+
405+
if [ -n "$comment_id" ]; then
406+
gh api "repos/$REPO/issues/comments/$comment_id" -X DELETE --silent
301407
fi
302408
303409
# zizmor `artipacked` mitigation — scrub the GITHUB_TOKEN that checkout

0 commit comments

Comments
 (0)