-
Notifications
You must be signed in to change notification settings - Fork 4
[DO NOT MERGE] Test harness for the AI review skip gate #98
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.
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
Changes from 5 commits
798cfb2
c94bc3b
096f415
f80a5ac
d30e911
1945767
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| #!/bin/bash | ||
|
|
||
| # Record that an AI review completed for a specific commit by setting a commit status. | ||
| # A later "ready for review" event reads this status and skips the duplicate review. | ||
| # Usage: recordReviewComplete.sh <HEAD_SHA> <CONTEXT> [DESCRIPTION] | ||
| # Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_SERVER_URL, GITHUB_RUN_ID | ||
| # | ||
| # CONTEXT names the reviewer, one per reviewer per repo (e.g. "ai-review-completed/claude", | ||
| # "ai-review-completed/codex"). GitHub allows one status per context per commit, so a second | ||
| # status with the same context replaces the first rather than stacking up. It is also | ||
| # the key shouldSkipReview.sh looks for, so both scripts must be passed the same value | ||
| # or the review will never be recognised as already done. It shows up as the status's | ||
| # label in the PR's checks list. | ||
| set -eu | ||
|
|
||
| if [[ $# -lt 2 ]]; then | ||
| echo "Usage: $0 <HEAD_SHA> <CONTEXT> [DESCRIPTION]" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! [[ "$1" =~ ^[0-9a-f]{40}$ ]]; then | ||
| echo "Error: HEAD_SHA must be a full 40-character commit SHA" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! [[ "$2" =~ ^[a-z0-9]([a-z0-9/_-]*[a-z0-9])?$ ]]; then | ||
| echo "Error: CONTEXT must be lowercase alphanumeric with '/', '_' or '-' separators" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| readonly HEAD_SHA="$1" | ||
| readonly CONTEXT="$2" | ||
| # GitHub rejects status descriptions longer than 140 characters. | ||
| readonly DESCRIPTION="${3:-Reviewed at this commit}" | ||
| readonly TRUNCATED_DESCRIPTION="${DESCRIPTION:0:140}" | ||
| readonly REPO="${GITHUB_REPOSITORY}" | ||
| readonly RUN_URL="${GITHUB_SERVER_URL}/${REPO}/actions/runs/${GITHUB_RUN_ID}" | ||
|
|
||
| gh api -X POST "/repos/$REPO/statuses/$HEAD_SHA" \ | ||
| -f state=success \ | ||
| -f context="$CONTEXT" \ | ||
| -f description="$TRUNCATED_DESCRIPTION" \ | ||
| -f target_url="$RUN_URL" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| #!/bin/bash | ||
|
|
||
| # Decide whether an AI review already completed for the PR's current head commit. Useful when a PR author manually requests a review while the PR is still a draft, | ||
| # and then marks it ready for review once the AI review passes. In that case, this skips running the AI review again, since it already completed successfully for the same commit. | ||
| # Writes "head_sha=<sha>" and "skip=true|false" to $GITHUB_OUTPUT. | ||
| # Usage: shouldSkipReview.sh <PR_NUMBER> <CONTEXT> | ||
| # Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT | ||
| # | ||
| # CONTEXT names the reviewer, one per reviewer per repo (e.g. "ai-review-completed/claude", | ||
| # "ai-review-completed/codex"). It must match the value recordReviewComplete.sh was given, since | ||
| # that is the commit status this looks for. | ||
| set -eu | ||
|
|
||
| if [[ $# -lt 2 ]]; then | ||
| echo "Usage: $0 <PR_NUMBER> <CONTEXT>" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! [[ "$1" =~ ^[0-9]+$ ]]; then | ||
| echo "Error: PR_NUMBER must be a positive integer" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! [[ "$2" =~ ^[a-z0-9]([a-z0-9/_-]*[a-z0-9])?$ ]]; then | ||
| echo "Error: CONTEXT must be lowercase alphanumeric with '/', '_' or '-' separators" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| readonly PR_NUMBER="$1" | ||
| readonly CONTEXT="$2" | ||
| readonly REPO="${GITHUB_REPOSITORY}" | ||
|
|
||
| HEAD_SHA=$(gh api "/repos/$REPO/pulls/$PR_NUMBER" --jq '.head.sha') | ||
| readonly HEAD_SHA | ||
|
|
||
| if [[ -z "$HEAD_SHA" ]]; then | ||
| echo "::error::Could not resolve head SHA for PR #$PR_NUMBER" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # The combined status endpoint returns only the most recent status per context. | ||
| STATE=$(gh api "/repos/$REPO/commits/$HEAD_SHA/status" --jq ".statuses[] | select(.context == \"$CONTEXT\") | .state") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In repositories where more than one page of commit-status contexts exists and this review context is not on the first page, the filter returns an empty Useful? React with 👍 / 👎. |
||
| readonly STATE | ||
|
|
||
| echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" | ||
|
|
||
| if [[ "$STATE" == "success" ]]; then | ||
| echo "$CONTEXT already completed for $HEAD_SHA, skipping review" >&2 | ||
| echo "skip=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "skip=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Throwaway workflow used to exercise shouldSkipReview.sh and recordReviewComplete.sh on a real PR. | ||
| # It stands in for an AI reviewer without calling one. Do not merge this file. | ||
| name: Test AI review skip gate | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, ready_for_review] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| statuses: write | ||
|
|
||
| jobs: | ||
| testSkipGate: | ||
| runs-on: blacksmith-2vcpu-ubuntu-2404 | ||
| steps: | ||
| - name: Checkout | ||
| # 4.2.2 | ||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 | ||
|
|
||
| - name: Add toolkit scripts to PATH | ||
| run: echo "$GITHUB_WORKSPACE/.github/actions/claude-review-toolkit/scripts" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Check for an existing review of this commit | ||
| id: skip | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| run: shouldSkipReview.sh "$PR_NUMBER" "ai-review-completed/test" | ||
|
|
||
| - name: Report the gate decision | ||
| env: | ||
| ACTION: ${{ github.event.action }} | ||
| SKIP: ${{ steps.skip.outputs.skip }} | ||
| HEAD_SHA: ${{ steps.skip.outputs.head_sha }} | ||
| run: echo "::notice::action=$ACTION skip=$SKIP head_sha=$HEAD_SHA" | ||
|
|
||
| - name: Stand in for the AI review | ||
| id: code-review | ||
| if: steps.skip.outputs.skip != 'true' | ||
| run: echo "The real reviewer would run here" | ||
|
|
||
| - name: Record review completion | ||
| if: steps.code-review.outcome == 'success' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| HEAD_SHA: ${{ steps.skip.outputs.head_sha }} | ||
| run: recordReviewComplete.sh "$HEAD_SHA" "ai-review-completed/test" "Reviewed at this commit - comment @claude review to re-run" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a private caller repository, copying this workflow-level permission block sets every unspecified permission to
none, soshouldSkipReview.shcannot perform itsGET /repos/{owner}/{repo}/pulls/{pull_number}request and the review job fails before reaching the gate. GitHub documents that this endpoint requires the fine-grained Pull requests: read permission, while unauthenticated access is limited to public resources; addpull-requests: readhere and to the test workflow. (workflow permission semantics, pull-request endpoint permissions)Useful? React with 👍 / 👎.