From 86435c2ed346beb2b5c416f74373cee9c7b77fe2 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:12:18 -0700 Subject: [PATCH] feat: surface Claude PR review progress as a check run The review stage runs on workflow_run, and workflow_run runs never attach to a pull request -- so the PR only ever shows Stage 1's collect check going green in seconds, while the review itself runs invisibly and its comments trickle in with no signal for whether it is still working or already dead. Stage 1 cannot wait for the review: its completion is what fires the workflow_run event, so blocking on it would deadlock. A check run created against the PR head SHA does attach to the PR, so the review stage now opens one as in_progress once it resolves the PR and closes it in an always() step. Both calls are best-effort. The check run needs checks: write, which only a caller can grant, so a caller that bumps this workflow without adding the scope would otherwise 403 here and get no review at all -- a progress indicator must never be the reason a review does not run. A failed create warns and leaves the step output empty, which gates the closing step off. Cancellation reports neutral, not failure: a superseded push and the job timeout are indistinguishable from the step outcome, and neither says anything about the change -- findings posted before the cut still stand. Not safe to mark required in branch protection: the check is only created when a unique open PR resolves, so the existing clean-skip path would leave it pending forever. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../workflows/reusable_claude_pr_review.yml | 110 +++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/.github/workflows/reusable_claude_pr_review.yml b/.github/workflows/reusable_claude_pr_review.yml index 7b9b557..602c85c 100644 --- a/.github/workflows/reusable_claude_pr_review.yml +++ b/.github/workflows/reusable_claude_pr_review.yml @@ -60,13 +60,14 @@ on: required: true # Least privilege: read the repo, post PR/issue comments (pull-requests: write -# also covers the commits/{sha}/pulls lookup used to resolve the PR), and mint -# the OIDC token (id-token: write). No write access to contents and no admin -# scopes. +# also covers the commits/{sha}/pulls lookup used to resolve the PR), publish +# this stage's own check run (checks: write), and mint the OIDC token +# (id-token: write). No write access to contents and no admin scopes. permissions: contents: read pull-requests: write issues: write + checks: write id-token: write jobs: @@ -148,6 +149,65 @@ jobs: echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" echo "base_sha=$BASE_SHA" >> "$GITHUB_OUTPUT" + # Publish this stage's progress as a check run on the PR head. + # + # Stage 2 runs on workflow_run, and workflow_run runs are never associated + # with a pull request -- so none of the work below shows up in the PR's + # Checks list. All a maintainer sees is Stage 1's "collect" check going + # green in seconds, then review comments trickling in with no indication + # that a review is still running (or has died). Stage 1 cannot wait for + # this stage either: its completion is what fires the workflow_run event, + # so blocking on the review would deadlock. + # + # A check run created against the head SHA does attach to the PR, so this + # is the one honest progress signal available: it opens as in_progress + # here and is closed out by the final step below, whatever happens in + # between. + # + # Deliberately NOT a required check in branch protection: it is only + # created when a unique open PR resolves, so the clean-skip path above + # would leave a required check permanently pending and block the merge. + # + # Best-effort by design. The check run needs `checks: write`, which a + # caller has to grant (a caller's permissions block is the ceiling for a + # reusable workflow), so a caller that bumps this workflow without adding + # the scope gets a 403 here -- and this is a progress indicator, not the + # review. Failing the job over it would mean no review at all. So a + # failure warns and leaves `id` empty, which gates the closing step off, + # and the review proceeds exactly as it did before this step existed. + # + # Note the create call requires a GitHub App token: the Checks API rejects + # user tokens outright ("You must authenticate via a GitHub App"). The + # Actions GITHUB_TOKEN is an installation token, so it qualifies -- but + # this step cannot be reproduced by hand with a PAT. + - name: Open review check run on the PR head + id: check + if: steps.meta.outputs.resolved == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ steps.meta.outputs.head_sha }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + err="$(mktemp)" + # Body built with jq rather than -f field=value: the payload nests + # (output.title / output.summary) and jq keeps the quoting honest. + if ! id=$(jq -n --arg sha "$HEAD_SHA" --arg url "$RUN_URL" '{ + name: "Claude PR Review", + head_sha: $sha, + status: "in_progress", + details_url: $url, + output: { + title: "Review in progress", + summary: "Claude is reviewing this revision. Findings are posted as inline comments as they are found, so comments appearing one at a time is expected -- this check completes when the review is done." + } + }' | gh api "repos/$REPO/check-runs" --input - --jq '.id' 2>"$err"); then + echo "::warning::Could not open the review check run; continuing without the PR-side progress signal. Does this caller grant checks: write? $(tr -d '\n' < "$err")" + exit 0 + fi + echo "id=$id" >> "$GITHUB_OUTPUT" + # Collect the state of this bot's prior review comments so the agent can # avoid re-posting findings it already raised on earlier revisions. This is # done here, deterministically and cheaply, rather than spending agent turns @@ -325,6 +385,7 @@ jobs: # MCP tools -- and the run is bounded by the job-level timeout-minutes. The # agent step's env carries only the Bedrock bearer token and GITHUB_TOKEN. - name: Claude PR review + id: review if: steps.meta.outputs.resolved == 'true' uses: anthropics/claude-code-action@0b1b62002952733671bde978d429b50b51c51c85 # v1.0.136 env: @@ -424,3 +485,46 @@ jobs: claude_args: | --model ${{ inputs.model_id }} --allowedTools "Read,Glob,Grep,Bash(cat:*),Bash(head:*),Bash(wc:*),Bash(jq:*),Bash(git -C pr-head diff:*),Bash(git -C pr-head log:*),Bash(git -C pr-head show:*),Bash(git -C pr-head blame:*),Bash(gh api:*)" + + # Close out the check run opened above. always() so the check never sticks + # at in_progress: it also runs when the job is cancelled, which is the + # common case here (a newer push cancels this run via the caller's + # concurrency group, and the job-level timeout-minutes cancels a stuck + # run). + # + # Those two cancellation causes are indistinguishable from the step + # outcome, and neither deserves a red check: findings already posted stand + # on their own, so both report `neutral` with wording that covers both. + # Only a genuine agent-step failure reports `failure`. + - name: Close review check run + if: always() && steps.check.outputs.id != '' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + CHECK_ID: ${{ steps.check.outputs.id }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + AGENT_OUTCOME: ${{ steps.review.outcome }} + run: | + set -euo pipefail + case "$AGENT_OUTCOME" in + success) + conclusion=success + title="Review complete" + detail="Claude finished reviewing this revision. Any findings are posted as inline comments." ;; + cancelled) + conclusion=neutral + title="Review stopped early (superseded or timed out)" + detail="The review was cut short -- either a newer push superseded it, or it hit the job time limit. Findings posted before that point still stand; push again to re-review." ;; + *) + conclusion=failure + title="Review did not complete" + detail="The review step failed. This says nothing about the change itself -- see the job log." ;; + esac + # Also best-effort: a failed PATCH would otherwise redden a job whose + # review actually succeeded. The check run then ages out on its own. + jq -n --arg c "$conclusion" --arg t "$title" --arg d "$detail" --arg url "$RUN_URL" '{ + status: "completed", + conclusion: $c, + output: { title: $t, summary: ($d + "\n\nJob log: " + $url) } + }' | gh api -X PATCH "repos/$REPO/check-runs/$CHECK_ID" --input - --silent \ + || echo "::warning::Could not close the review check run $CHECK_ID; it may sit at in_progress until it ages out."