Skip to content

feat(claude-review): set a pending status while the review runs #114

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

Open
crowecawcaw wants to merge 2 commits into
base: mainline
Choose a base branch
from
Open

feat(claude-review): set a pending status while the review runs #114

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2aba70c
feat(claude-review): set a pending status while the review runs
crowecawcaw Oct 8, 2026
429bb5d
fix(claude-review): don't leave the status pending when post fails
crowecawcaw Oct 8, 2026
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
Binary file added .github/scripts/__pycache__/claude_pr_review.cpython-314.pyc
View file
Open in desktop
Binary file not shown.
122 changes: 86 additions & 36 deletions .github/workflows/reusable_claude_pr_review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@
# only on the full review.
# - A sticky summary comment and a "Claude review" commit status report
# what is still open, so authors know when the review has settled. The
# status is set when a review finishes, so a head commit with no status
# has not been reviewed yet. It needs `statuses: write` from the caller
# (see below); without it the summary comment alone carries the signal.
# status is "pending" while a review runs and is set to the result when
# it finishes, so a head commit with no status has not been picked up for
# review. It needs `statuses: write` from the caller (see below); without
# it the summary comment alone carries the signal.
name: Claude PR Review

on:
Expand Down Expand Up @@ -84,8 +85,11 @@ on:
description: "ARN of the OIDC role that can mint a Bedrock bearer token. Kept in a secret so the account id is configured in one place."
required: true

# Permissions are split across two jobs so the job that runs the model never
# Permissions are split across jobs so the job that runs the model never
# holds a token that can write anything:
# resolve finds the PR for the head SHA. It declares a read-only token.
# start sets the pending commit status. Like post, it declares no
# permissions and runs no model.
# review runs the agent on the untrusted PR. It declares a read-only token
# (plus id-token: write for Bedrock) and hands its results to the
# post job as an artifact.
Expand All @@ -101,29 +105,28 @@ on:
# issues: write post/update the summary comment
# id-token: write mint the OIDC token for Bedrock
# statuses: write optional: set the "Claude review" commit status
# (start and post jobs)
# contents: write optional, instead of contents: read: resolve review
# threads the review judges addressed (GitHub requires
# it for resolveReviewThread). Only the post job, which
# runs no model, receives it.
# No admin scopes.

jobs:
review:
# Resolve the PR, then set the pending status before the review starts.

@github-actions github-actions Bot Oct 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix: The PR commits compiled bytecode: .github/scripts/__pycache__/claude_pr_review.cpython-314.pyc and test/__pycache__/test_claude_pr_review.cpython-314-pytest-9.0.2.pyc. These are local build artifacts. They go stale as soon as the source changes, and they bloat history. The repo has no .gitignore, so the next local test run will add them again.

Fix: remove both files from the PR and add a .gitignore containing __pycache__/ and *.pyc.

@crowecawcaw crowecawcaw Oct 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 429bb5d: removed both files and added a .gitignore for __pycache__/ and *.pyc.

@github-actions github-actions Bot Oct 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: Both .pyc files are gone from the PR head, and the new .gitignore excludes pycache/ and *.pyc.

# These are separate jobs so the status is written by a job that runs no
# model; the review job's token stays read-only.
resolve:
runs-on: ubuntu-latest
timeout-minutes: ${{ inputs.timeout_minutes }}
# Read-only: everything this job writes goes through the post job.
timeout-minutes: 10
permissions:
contents: read
pull-requests: read
issues: read
id-token: write
outputs:
prepared: ${{ steps.prepare.outcome == 'success' }}
resolved: ${{ steps.meta.outputs.resolved }}
pr_number: ${{ steps.meta.outputs.pr_number }}
head_sha: ${{ steps.meta.outputs.head_sha }}
base_sha: ${{ steps.meta.outputs.base_sha }}
mode: ${{ steps.prepare.outputs.mode }}
prior_sha: ${{ steps.prepare.outputs.prior_sha }}
steps:
# Resolve the PR from the trusted head SHA in the workflow_run payload --
# never from anything Stage 1 produced. Stage 1 runs a fork's copy of the
Expand Down Expand Up @@ -223,21 +226,56 @@ jobs:
echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT"
echo "base_sha=$BASE_SHA" >> "$GITHUB_OUTPUT"

# All steps below run only when a unique open PR was resolved above. If
# the step skipped (no unique open PR, bot actor, superseded head),
# `resolved` is false and every remaining step (including the credential
# and agent steps) is skipped, so the job finishes green without touching
# AWS or posting anything.
# Mark the head commit "pending" so the PR shows a review is running. Without
# it the PR shows only the collect check, which finishes in seconds, and
# nothing more until the review posts. The post job overwrites this status.
# A run cancelled by a newer push leaves it pending on a commit that is no
# longer the head, which the PR does not show. No permissions block, like
# post: needs `statuses: write` from the caller, and is best effort without it.
start:
needs: resolve
if: needs.resolve.outputs.resolved == 'true'
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Set pending review status
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
HEAD_SHA: ${{ needs.resolve.outputs.head_sha }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
# The context must match STATUS_CONTEXT in claude_pr_review.py so post
# replaces this status rather than adding a second one.
run: |
gh api "repos/$REPO/statuses/$HEAD_SHA" --silent \
-f state=pending -f "context=Claude review" \
-f "description=Review in progress" -f "target_url=$RUN_URL" \
|| echo "::warning::Could not set the pending status; does the caller grant statuses: write?"

review:
needs: resolve
if: needs.resolve.outputs.resolved == 'true'
runs-on: ubuntu-latest
timeout-minutes: ${{ inputs.timeout_minutes }}
# Read-only: everything this job writes goes through the post job.
permissions:
contents: read
pull-requests: read
issues: read
id-token: write
outputs:
prepared: ${{ steps.prepare.outcome == 'success' }}
mode: ${{ steps.prepare.outputs.mode }}
prior_sha: ${{ steps.prepare.outputs.prior_sha }}
steps:
# Check out the PR head read-only into a subdirectory. persist-credentials
# is false so no token is left on disk, submodules are not fetched, and the
# repo root is left untouched. The agent reads pr-head/ but never runs it.
- name: Checkout PR head (read-only)
if: steps.meta.outputs.resolved == 'true'
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.0
with:
repository: ${{ inputs.head_repository }}
ref: ${{ steps.meta.outputs.head_sha }}
ref: ${{ needs.resolve.outputs.head_sha }}
path: pr-head
persist-credentials: false
submodules: false
Expand All @@ -246,7 +284,6 @@ jobs:
# Remove any symlinks from the checkout so the agent's read tools can only
# see files inside pr-head/.
- name: Strip symlinks from PR head
if: steps.meta.outputs.resolved == 'true'
run: find pr-head -type l -delete

# A reusable workflow does not check out its own repository -- the only
Expand All @@ -256,7 +293,6 @@ jobs:
# see tooling_repository / tooling_ref inputs) into workflow-tooling/.
# persist-credentials is false so no token is left on disk.
- name: Check out workflow tooling (requirements, review script)
if: steps.meta.outputs.resolved == 'true'
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.0
with:
repository: ${{ inputs.tooling_repository }}
Expand All @@ -278,13 +314,12 @@ jobs:
# rather than diffing from the wrong commit.
- name: Prepare review context
id: prepare
if: steps.meta.outputs.resolved == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ steps.meta.outputs.pr_number }}
HEAD_SHA: ${{ steps.meta.outputs.head_sha }}
BASE_SHA: ${{ steps.meta.outputs.base_sha }}
PR_NUMBER: ${{ needs.resolve.outputs.pr_number }}
HEAD_SHA: ${{ needs.resolve.outputs.head_sha }}
BASE_SHA: ${{ needs.resolve.outputs.base_sha }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
run: python3 workflow-tooling/.github/scripts/claude_pr_review.py prepare

Expand All @@ -294,7 +329,6 @@ jobs:
# present means a compromised release has no role credentials to exfiltrate
# at install time. --only-binary blocks arbitrary setup.py execution.
- name: Install Bedrock token generator (no AWS creds in env)
if: steps.meta.outputs.resolved == 'true'
run: |
set -euo pipefail
python3 -m pip install --quiet --only-binary :all: \
Expand All @@ -304,15 +338,13 @@ jobs:
# the IAM credentials, then unset those credentials before the agent runs.
# role-duration-seconds bounds the token's lifetime to about an hour.
- name: Configure AWS credentials (OIDC, mint scope only)
if: steps.meta.outputs.resolved == 'true'
uses: aws-actions/configure-aws-credentials@e1253824e5c10ff9df46874f81ed3ec929e19cfd # v6.0.0
with:
role-to-assume: ${{ secrets.bedrock_role_arn }}
aws-region: ${{ inputs.aws_region }}
role-duration-seconds: 3600

- name: Mint Bedrock bearer token and drop IAM credentials
if: steps.meta.outputs.resolved == 'true'
env:
AWS_REGION: ${{ inputs.aws_region }}
run: |
Expand Down Expand Up @@ -341,7 +373,6 @@ jobs:
# re-reviews the same changes.
- name: Claude PR review
id: agent
if: steps.meta.outputs.resolved == 'true'
timeout-minutes: ${{ inputs.review_timeout_minutes }}
uses: anthropics/claude-code-action@58985842b834ed26087302ba27d07bc24ca8697a # v1.0.244
env:
Expand All @@ -368,8 +399,8 @@ jobs:
instruct maintainers.

Repository: ${{ github.repository }}
PR number: ${{ steps.meta.outputs.pr_number }}
Head SHA: ${{ steps.meta.outputs.head_sha }}
PR number: ${{ needs.resolve.outputs.pr_number }}
Head SHA: ${{ needs.resolve.outputs.head_sha }}
Review mode: ${{ steps.prepare.outputs.mode }}

The PR head working tree is checked out (read-only) under `pr-head/`.
Expand Down Expand Up @@ -506,18 +537,36 @@ jobs:
# superseded this run. No permissions block: inherits the caller's grants
# (see the top of this file). Runs no model and no PR code.
post:
needs: review
if: ${{ !cancelled() && needs.review.outputs.prepared == 'true' }}
needs: [resolve, start, review]
if: ${{ !cancelled() && needs.resolve.outputs.resolved == 'true' }}
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
# The review job failed before it produced anything to post (a checkout
# or context step failed). Replace the pending status so it does not
# stay pending on the PR head.
- name: Report unfinished review
if: needs.review.outputs.prepared != 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
HEAD_SHA: ${{ needs.resolve.outputs.head_sha }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
run: |
gh api "repos/$REPO/statuses/$HEAD_SHA" --silent \
-f state=error -f "context=Claude review" \
-f "description=Review did not finish; push again or re-run to retry" -f "target_url=$RUN_URL" \
|| echo "::warning::Could not set the review status; does the caller grant statuses: write?"

- name: Download review results
if: needs.review.outputs.prepared == 'true'
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
with:
name: claude-review-${{ github.run_id }}
path: review-context

- name: Check out workflow tooling (review script)
if: needs.review.outputs.prepared == 'true'
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.0
with:
repository: ${{ inputs.tooling_repository }}
Expand All @@ -527,12 +576,13 @@ jobs:
submodules: false

- name: Post review

@github-actions github-actions Bot Oct 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix: If the Post review step fails, the head commit stays at the pending status that start set, and it stays there indefinitely. post() calls gh with check=True in several places (fetch_threads, fetch_summary, the summary PATCH/POST), so a transient GitHub API error or a failed artifact download ends the job before set_status runs. The PR then shows "Review in progress" with nothing running. Before this PR the status was simply absent in that case, which the docs define as "not picked up".

Fix: add a final step that runs on failure() and sets state=error for context=Claude review, the same way Report unfinished review does.

@crowecawcaw crowecawcaw Oct 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 429bb5d: Report unfinished review now runs last, on !cancelled() && (prepared != 'true' || failure()), so a failed download, checkout, or Post review step replaces pending with error.

@github-actions github-actions Bot Oct 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: 'Report unfinished review' now runs as the last step on !cancelled() && (prepared != 'true' || failure()), so if the download, checkout, or Post review step fails, the pending status becomes error.

if: needs.review.outputs.prepared == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ needs.review.outputs.pr_number }}
HEAD_SHA: ${{ needs.review.outputs.head_sha }}
BASE_SHA: ${{ needs.review.outputs.base_sha }}
PR_NUMBER: ${{ needs.resolve.outputs.pr_number }}
HEAD_SHA: ${{ needs.resolve.outputs.head_sha }}
BASE_SHA: ${{ needs.resolve.outputs.base_sha }}
MODE: ${{ needs.review.outputs.mode }}
PRIOR_SHA: ${{ needs.review.outputs.prior_sha }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
Expand Down
Binary file added test/__pycache__/test_claude_pr_review.cpython-314-pytest-9.0.2.pyc
View file
Open in desktop
Binary file not shown.
Loading