From 798cfb2277f46bff2bc86dd52c97f790fd25193a Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Thu, 6 Aug 2026 19:15:02 -0700 Subject: [PATCH 1/7] Add shared scripts to skip AI reviews already completed for a commit --- .../actions/claude-review-toolkit/README.md | 33 +++++++++++++ .../scripts/recordReviewComplete.sh | 36 ++++++++++++++ .../scripts/shouldSkipReview.sh | 47 +++++++++++++++++++ 3 files changed, 116 insertions(+) create mode 100755 .github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh create mode 100755 .github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh diff --git a/.github/actions/claude-review-toolkit/README.md b/.github/actions/claude-review-toolkit/README.md index 0f68cad..05a925a 100644 --- a/.github/actions/claude-review-toolkit/README.md +++ b/.github/actions/claude-review-toolkit/README.md @@ -46,6 +46,39 @@ Caller repos must ship a `.claude/skills/coding-standards/rules/` directory with | `createInlineComment.sh` | ` ` | Posts an inline review comment. Requires `GITHUB_REPOSITORY`, `GH_TOKEN`, and `ALLOWED_RULES_FILE` in env. The body must reference a rule tag matching `[A-Z]+(-[A-Z]+)*-[0-9]+` (e.g. `PERF-1`) that is present in the allowlist; otherwise the comment is rejected. | | `postCodeReviewResults.sh` | `` | Posts the result of a Claude code review. With no violations, adds a `+1` reaction to the PR; with violations, posts one inline comment per violation. Reads the JSON output from env `STRUCTURED_OUTPUT`. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `ALLOWED_RULES_FILE`, and `STRUCTURED_OUTPUT` in env. Individual comment failures are swallowed so one rejected comment does not kill the loop. | | `extractAllowedRules.sh` | ` ` | Walks `` for `.md` rule files and writes their `ruleId:` tags to ``. Invoked automatically by the action; rarely called directly. | +| `shouldSkipReview.sh` | ` ` | Resolves the PR's head SHA and writes `head_sha` plus `skip=true\|false` to `$GITHUB_OUTPUT`. `skip` is `true` when a `success` commit status with `` already exists on that SHA. Requires `GH_TOKEN` and `GITHUB_REPOSITORY`. | +| `recordReviewComplete.sh` | ` [DESCRIPTION]` | Sets a `success` commit status with `` on ``, linking back to the workflow run. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `statuses: write`. | + +## Skipping duplicate reviews + +A PR marked ready for review after being reviewed as a draft would otherwise be reviewed twice on the same commit. To avoid that, gate the review on `shouldSkipReview.sh` and record completion with `recordReviewComplete.sh`: + +```yaml +permissions: + statuses: write + +steps: + - name: Check for an existing review of this commit + id: skip + env: + GH_TOKEN: ${{ github.token }} + run: shouldSkipReview.sh "$PR_NUMBER" "ai-review/claude" + + - name: Run Claude Code + if: steps.skip.outputs.skip != 'true' + # ... + + - name: Record review completion + if: steps.skip.outputs.skip != 'true' && steps.code-review.outcome == 'success' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ steps.skip.outputs.head_sha }} + run: recordReviewComplete.sh "$HEAD_SHA" "ai-review/claude" "Reviewed at this commit" +``` + +Record against `steps.skip.outputs.head_sha` — the SHA captured before the review started — rather than re-resolving it at the end. If the author pushed while the review was running, the status lands on the commit that was actually reviewed and the next event correctly triggers a fresh review. + +The status is recorded whether or not the review found anything, so re-running on an unchanged commit never reposts the same findings. A comment trigger (`@claude review`, `/codex-review`) bypasses the gate and is the way to force a re-review. ## Schema extension diff --git a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh new file mode 100755 index 0000000..bd7ceed --- /dev/null +++ b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh @@ -0,0 +1,36 @@ +#!/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 [DESCRIPTION] +# Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_SERVER_URL, GITHUB_RUN_ID +set -eu + +if [[ $# -lt 2 ]]; then + echo "Usage: $0 [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" diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh new file mode 100755 index 0000000..2f9ff0f --- /dev/null +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -0,0 +1,47 @@ +#!/bin/bash + +# Decide whether an AI review already completed for the PR's current head commit. +# Writes "head_sha=" and "skip=true|false" to $GITHUB_OUTPUT. +# Usage: shouldSkipReview.sh +# Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT +set -eu + +if [[ $# -lt 2 ]]; then + echo "Usage: $0 " >&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") +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 From c94bc3b1e68a5b85094efe476c964d4435cb270b Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Thu, 6 Aug 2026 19:40:05 -0700 Subject: [PATCH 2/7] Document what the review context argument is for --- .../claude-review-toolkit/scripts/recordReviewComplete.sh | 7 +++++++ .../claude-review-toolkit/scripts/shouldSkipReview.sh | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh index bd7ceed..abc6bf5 100755 --- a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh +++ b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh @@ -4,6 +4,13 @@ # A later "ready for review" event reads this status and skips the duplicate review. # Usage: recordReviewComplete.sh [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/claude", +# "ai-review/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 diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh index 2f9ff0f..e7194d7 100755 --- a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -4,6 +4,10 @@ # Writes "head_sha=" and "skip=true|false" to $GITHUB_OUTPUT. # Usage: shouldSkipReview.sh # Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT +# +# CONTEXT names the reviewer, one per reviewer per repo (e.g. "ai-review/claude", +# "ai-review/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 From 096f415ac7bb77cae9189d1061350563def00a99 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Thu, 6 Aug 2026 19:42:57 -0700 Subject: [PATCH 3/7] Name the marker status ai-review-completed so it is not read as a review result --- .github/actions/claude-review-toolkit/README.md | 6 ++++-- .../claude-review-toolkit/scripts/recordReviewComplete.sh | 4 ++-- .../claude-review-toolkit/scripts/shouldSkipReview.sh | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/actions/claude-review-toolkit/README.md b/.github/actions/claude-review-toolkit/README.md index 05a925a..ef3ee7a 100644 --- a/.github/actions/claude-review-toolkit/README.md +++ b/.github/actions/claude-review-toolkit/README.md @@ -62,7 +62,7 @@ steps: id: skip env: GH_TOKEN: ${{ github.token }} - run: shouldSkipReview.sh "$PR_NUMBER" "ai-review/claude" + run: shouldSkipReview.sh "$PR_NUMBER" "ai-review-completed/claude" - name: Run Claude Code if: steps.skip.outputs.skip != 'true' @@ -73,13 +73,15 @@ steps: env: GH_TOKEN: ${{ github.token }} HEAD_SHA: ${{ steps.skip.outputs.head_sha }} - run: recordReviewComplete.sh "$HEAD_SHA" "ai-review/claude" "Reviewed at this commit" + run: recordReviewComplete.sh "$HEAD_SHA" "ai-review-completed/claude" "Reviewed at this commit" ``` Record against `steps.skip.outputs.head_sha` — the SHA captured before the review started — rather than re-resolving it at the end. If the author pushed while the review was running, the status lands on the commit that was actually reviewed and the next event correctly triggers a fresh review. The status is recorded whether or not the review found anything, so re-running on an unchanged commit never reposts the same findings. A comment trigger (`@claude review`, `/codex-review`) bypasses the gate and is the way to force a re-review. +Name the context so it does not read as a second review result. The review job itself already appears in the PR's checks list, and a neighbouring green row called `ai-review/claude` looks like a duplicate verdict — `ai-review-completed/claude` reads as the record of a past run, which is what it is. + ## Schema extension Repos that need extra fields on top of the canonical schema should `jq`-merge them in a follow-up step before feeding `claude_args`. Read the canonical schema from `schema_path` (a file) rather than piping `schema_json` through `echo`, so the shell never sees the schema's `"` characters: diff --git a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh index abc6bf5..657c062 100755 --- a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh +++ b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh @@ -5,8 +5,8 @@ # Usage: recordReviewComplete.sh [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/claude", -# "ai-review/codex"). GitHub allows one status per context per commit, so a second +# 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 diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh index e7194d7..7cdc9ef 100755 --- a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -5,8 +5,8 @@ # Usage: shouldSkipReview.sh # Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT # -# CONTEXT names the reviewer, one per reviewer per repo (e.g. "ai-review/claude", -# "ai-review/codex"). It must match the value recordReviewComplete.sh was given, since +# 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 From f80a5ac9480edb3633861efcb6e5d70464f5a362 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Thu, 6 Aug 2026 20:12:06 -0700 Subject: [PATCH 4/7] Explain use of shouldSkipReview script --- .../actions/claude-review-toolkit/scripts/shouldSkipReview.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh index 7cdc9ef..f706c77 100755 --- a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -1,6 +1,7 @@ #!/bin/bash -# Decide whether an AI review already completed for the PR's current head commit. +# 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=" and "skip=true|false" to $GITHUB_OUTPUT. # Usage: shouldSkipReview.sh # Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT From 1a4fa52bc5a99da25ef70afa6859f2aea3e9d0e9 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Fri, 7 Aug 2026 17:39:28 -0700 Subject: [PATCH 5/7] Scope the review marker to the PR and paginate the status lookup Commit statuses hang off a repository commit rather than a PR, so two PRs sharing a head SHA could read each other's completion marker and skip a review of a different diff. shouldSkipReview.sh now appends the PR number to the context it was given and publishes the result as a "context" output for recordReviewComplete.sh to record against. The combined status endpoint pages at 30 statuses, so add --paginate to keep the marker visible on commits carrying many other statuses. --- .../actions/claude-review-toolkit/README.md | 9 ++++++--- .../scripts/recordReviewComplete.sh | 12 ++++++------ .../scripts/shouldSkipReview.sh | 19 ++++++++++++------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.github/actions/claude-review-toolkit/README.md b/.github/actions/claude-review-toolkit/README.md index ef3ee7a..7c57438 100644 --- a/.github/actions/claude-review-toolkit/README.md +++ b/.github/actions/claude-review-toolkit/README.md @@ -46,8 +46,8 @@ Caller repos must ship a `.claude/skills/coding-standards/rules/` directory with | `createInlineComment.sh` | ` ` | Posts an inline review comment. Requires `GITHUB_REPOSITORY`, `GH_TOKEN`, and `ALLOWED_RULES_FILE` in env. The body must reference a rule tag matching `[A-Z]+(-[A-Z]+)*-[0-9]+` (e.g. `PERF-1`) that is present in the allowlist; otherwise the comment is rejected. | | `postCodeReviewResults.sh` | `` | Posts the result of a Claude code review. With no violations, adds a `+1` reaction to the PR; with violations, posts one inline comment per violation. Reads the JSON output from env `STRUCTURED_OUTPUT`. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `ALLOWED_RULES_FILE`, and `STRUCTURED_OUTPUT` in env. Individual comment failures are swallowed so one rejected comment does not kill the loop. | | `extractAllowedRules.sh` | ` ` | Walks `` for `.md` rule files and writes their `ruleId:` tags to ``. Invoked automatically by the action; rarely called directly. | -| `shouldSkipReview.sh` | ` ` | Resolves the PR's head SHA and writes `head_sha` plus `skip=true\|false` to `$GITHUB_OUTPUT`. `skip` is `true` when a `success` commit status with `` already exists on that SHA. Requires `GH_TOKEN` and `GITHUB_REPOSITORY`. | -| `recordReviewComplete.sh` | ` [DESCRIPTION]` | Sets a `success` commit status with `` on ``, linking back to the workflow run. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `statuses: write`. | +| `shouldSkipReview.sh` | ` ` | Resolves the PR's head SHA and writes `head_sha`, `context` and `skip=true\|false` to `$GITHUB_OUTPUT`. The status context it looks for is `/pr-`, and `skip` is `true` when a `success` commit status with that context already exists on the head SHA. Requires `GH_TOKEN` and `GITHUB_REPOSITORY`. | +| `recordReviewComplete.sh` | ` [DESCRIPTION]` | Sets a `success` commit status with `` on ``, linking back to the workflow run. Pass `shouldSkipReview.sh`'s `context` output so the two agree. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `statuses: write`. | ## Skipping duplicate reviews @@ -73,11 +73,14 @@ steps: env: GH_TOKEN: ${{ github.token }} HEAD_SHA: ${{ steps.skip.outputs.head_sha }} - run: recordReviewComplete.sh "$HEAD_SHA" "ai-review-completed/claude" "Reviewed at this commit" + CONTEXT: ${{ steps.skip.outputs.context }} + run: recordReviewComplete.sh "$HEAD_SHA" "$CONTEXT" "Reviewed at this commit" ``` Record against `steps.skip.outputs.head_sha` — the SHA captured before the review started — rather than re-resolving it at the end. If the author pushed while the review was running, the status lands on the commit that was actually reviewed and the next event correctly triggers a fresh review. +Record against `steps.skip.outputs.context` for the same reason. Commit statuses hang off a repository commit, not a PR, so two PRs sharing a head SHA — the same branch opened against two different base branches — would otherwise read each other's marker and skip a review of a different diff. `shouldSkipReview.sh` appends the PR number to the context it was given and publishes the result, so the gate and the record stay in step without the workflow rebuilding the string. + The status is recorded whether or not the review found anything, so re-running on an unchanged commit never reposts the same findings. A comment trigger (`@claude review`, `/codex-review`) bypasses the gate and is the way to force a re-review. Name the context so it does not read as a second review result. The review job itself already appears in the PR's checks list, and a neighbouring green row called `ai-review/claude` looks like a duplicate verdict — `ai-review-completed/claude` reads as the record of a past run, which is what it is. diff --git a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh index 657c062..a5fdc0e 100755 --- a/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh +++ b/.github/actions/claude-review-toolkit/scripts/recordReviewComplete.sh @@ -5,12 +5,12 @@ # Usage: recordReviewComplete.sh [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. +# CONTEXT is the key shouldSkipReview.sh looks for, so pass that step's "context" output rather +# than rebuilding the string here - it names the reviewer and the PR (e.g. +# "ai-review-completed/claude/pr-97"), and a mismatch means the review is never recognised as +# already done. GitHub allows one status per context per commit, so a second status with the same +# context replaces the first rather than stacking up. It shows up as the status's label in the +# PR's checks list. set -eu if [[ $# -lt 2 ]]; then diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh index f706c77..7c61f74 100755 --- a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -2,13 +2,15 @@ # 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=" and "skip=true|false" to $GITHUB_OUTPUT. +# Writes "head_sha=", "context=" and "skip=true|false" to $GITHUB_OUTPUT. # Usage: shouldSkipReview.sh # 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. +# CONTEXT names the reviewer (e.g. "ai-review-completed/claude", "ai-review-completed/codex"). +# Commit statuses belong to a repository commit rather than to a PR, and two PRs can share a head +# SHA - the same branch opened against two different base branches, for example - so the PR number +# is appended to form the status context this looks for. That full context is written to +# $GITHUB_OUTPUT as "context" and must be the value recordReviewComplete.sh is given. set -eu if [[ $# -lt 2 ]]; then @@ -28,6 +30,7 @@ fi readonly PR_NUMBER="$1" readonly CONTEXT="$2" +readonly STATUS_CONTEXT="$CONTEXT/pr-$PR_NUMBER" readonly REPO="${GITHUB_REPOSITORY}" HEAD_SHA=$(gh api "/repos/$REPO/pulls/$PR_NUMBER" --jq '.head.sha') @@ -38,14 +41,16 @@ if [[ -z "$HEAD_SHA" ]]; then 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") +# The combined status endpoint returns only the most recent status per context, but it pages at 30 +# statuses, so --paginate is needed to see a marker on a commit that carries many other statuses. +STATE=$(gh api --paginate "/repos/$REPO/commits/$HEAD_SHA/status" --jq ".statuses[] | select(.context == \"$STATUS_CONTEXT\") | .state") readonly STATE echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" +echo "context=$STATUS_CONTEXT" >> "$GITHUB_OUTPUT" if [[ "$STATE" == "success" ]]; then - echo "$CONTEXT already completed for $HEAD_SHA, skipping review" >&2 + echo "$STATUS_CONTEXT already completed for $HEAD_SHA, skipping review" >&2 echo "skip=true" >> "$GITHUB_OUTPUT" else echo "skip=false" >> "$GITHUB_OUTPUT" From dde402fdd8f73dbb0511d40505f3481a8b4077bf Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Fri, 7 Aug 2026 17:45:47 -0700 Subject: [PATCH 6/7] Never skip a review requested by comment An issue_comment run is someone asking for a review by hand, so it is a deliberate request to review a commit the gate has most likely already marked complete - the one case where repeating the review is the point. The README claimed this already worked; now it does. Also drop the workflow walkthrough from the README in favour of a high-level description, and document the token scopes the gate needs. --- .../actions/claude-review-toolkit/README.md | 34 +++---------------- .../scripts/shouldSkipReview.sh | 11 ++++-- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/.github/actions/claude-review-toolkit/README.md b/.github/actions/claude-review-toolkit/README.md index 7c57438..11783b3 100644 --- a/.github/actions/claude-review-toolkit/README.md +++ b/.github/actions/claude-review-toolkit/README.md @@ -46,42 +46,16 @@ Caller repos must ship a `.claude/skills/coding-standards/rules/` directory with | `createInlineComment.sh` | ` ` | Posts an inline review comment. Requires `GITHUB_REPOSITORY`, `GH_TOKEN`, and `ALLOWED_RULES_FILE` in env. The body must reference a rule tag matching `[A-Z]+(-[A-Z]+)*-[0-9]+` (e.g. `PERF-1`) that is present in the allowlist; otherwise the comment is rejected. | | `postCodeReviewResults.sh` | `` | Posts the result of a Claude code review. With no violations, adds a `+1` reaction to the PR; with violations, posts one inline comment per violation. Reads the JSON output from env `STRUCTURED_OUTPUT`. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `ALLOWED_RULES_FILE`, and `STRUCTURED_OUTPUT` in env. Individual comment failures are swallowed so one rejected comment does not kill the loop. | | `extractAllowedRules.sh` | ` ` | Walks `` for `.md` rule files and writes their `ruleId:` tags to ``. Invoked automatically by the action; rarely called directly. | -| `shouldSkipReview.sh` | ` ` | Resolves the PR's head SHA and writes `head_sha`, `context` and `skip=true\|false` to `$GITHUB_OUTPUT`. The status context it looks for is `/pr-`, and `skip` is `true` when a `success` commit status with that context already exists on the head SHA. Requires `GH_TOKEN` and `GITHUB_REPOSITORY`. | +| `shouldSkipReview.sh` | ` ` | Resolves the PR's head SHA and writes `head_sha`, `context` and `skip=true\|false` to `$GITHUB_OUTPUT`. The status context it looks for is `/pr-`, and `skip` is `true` when a `success` commit status with that context already exists on the head SHA. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `GITHUB_EVENT_NAME`, `pull-requests: read`, `statuses: read`. | | `recordReviewComplete.sh` | ` [DESCRIPTION]` | Sets a `success` commit status with `` on ``, linking back to the workflow run. Pass `shouldSkipReview.sh`'s `context` output so the two agree. Requires `GH_TOKEN`, `GITHUB_REPOSITORY`, `statuses: write`. | ## Skipping duplicate reviews -A PR marked ready for review after being reviewed as a draft would otherwise be reviewed twice on the same commit. To avoid that, gate the review on `shouldSkipReview.sh` and record completion with `recordReviewComplete.sh`: +A PR marked ready for review after being reviewed as a draft would otherwise be reviewed twice on the same commit. `shouldSkipReview.sh` gates the review and `recordReviewComplete.sh` records that it happened, using a commit status as the marker. -```yaml -permissions: - statuses: write - -steps: - - name: Check for an existing review of this commit - id: skip - env: - GH_TOKEN: ${{ github.token }} - run: shouldSkipReview.sh "$PR_NUMBER" "ai-review-completed/claude" - - - name: Run Claude Code - if: steps.skip.outputs.skip != 'true' - # ... - - - name: Record review completion - if: steps.skip.outputs.skip != 'true' && steps.code-review.outcome == 'success' - env: - GH_TOKEN: ${{ github.token }} - HEAD_SHA: ${{ steps.skip.outputs.head_sha }} - CONTEXT: ${{ steps.skip.outputs.context }} - run: recordReviewComplete.sh "$HEAD_SHA" "$CONTEXT" "Reviewed at this commit" -``` - -Record against `steps.skip.outputs.head_sha` — the SHA captured before the review started — rather than re-resolving it at the end. If the author pushed while the review was running, the status lands on the commit that was actually reviewed and the next event correctly triggers a fresh review. - -Record against `steps.skip.outputs.context` for the same reason. Commit statuses hang off a repository commit, not a PR, so two PRs sharing a head SHA — the same branch opened against two different base branches — would otherwise read each other's marker and skip a review of a different diff. `shouldSkipReview.sh` appends the PR number to the context it was given and publishes the result, so the gate and the record stay in step without the workflow rebuilding the string. +Run the gate first and skip the reviewer when its `skip` output is `true`. Once the review succeeds, record completion against the gate's `head_sha` and `context` outputs rather than re-deriving either at the end — `head_sha` is the SHA captured before the review started, so a push mid-review leaves the marker on the commit that was actually reviewed and the next event triggers a fresh one. Record the status whether or not the review found anything, so re-running on an unchanged commit never reposts the same findings. -The status is recorded whether or not the review found anything, so re-running on an unchanged commit never reposts the same findings. A comment trigger (`@claude review`, `/codex-review`) bypasses the gate and is the way to force a re-review. +An `issue_comment` run never skips, so a comment trigger (`@claude review`, `/codex-review`) is the way to force a re-review of a commit already marked complete. Name the context so it does not read as a second review result. The review job itself already appears in the PR's checks list, and a neighbouring green row called `ai-review/claude` looks like a duplicate verdict — `ai-review-completed/claude` reads as the record of a past run, which is what it is. diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh index 7c61f74..6362505 100755 --- a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -4,7 +4,11 @@ # 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=", "context=" and "skip=true|false" to $GITHUB_OUTPUT. # Usage: shouldSkipReview.sh -# Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT +# Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT, GITHUB_EVENT_NAME +# +# An issue_comment run never skips. That event is how someone asks for a review by hand +# ("@claude review", "/codex-review"), so it is a deliberate request to review a commit that has +# most likely already been reviewed - the one case where repeating the review is the point. # # CONTEXT names the reviewer (e.g. "ai-review-completed/claude", "ai-review-completed/codex"). # Commit statuses belong to a repository commit rather than to a PR, and two PRs can share a head @@ -49,7 +53,10 @@ readonly STATE echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" echo "context=$STATUS_CONTEXT" >> "$GITHUB_OUTPUT" -if [[ "$STATE" == "success" ]]; then +if [[ "${GITHUB_EVENT_NAME:-}" == "issue_comment" ]]; then + echo "Review requested by comment, running it even if $STATUS_CONTEXT already completed for $HEAD_SHA" >&2 + echo "skip=false" >> "$GITHUB_OUTPUT" +elif [[ "$STATE" == "success" ]]; then echo "$STATUS_CONTEXT already completed for $HEAD_SHA, skipping review" >&2 echo "skip=true" >> "$GITHUB_OUTPUT" else From 4f48808cfd9f483d07866a3ad4af1f259d4fcdde Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Wed, 19 Aug 2026 14:24:17 -0700 Subject: [PATCH 7/7] fix codex manual review example --- .../actions/claude-review-toolkit/scripts/shouldSkipReview.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh index 6362505..7a9be59 100755 --- a/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh +++ b/.github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh @@ -7,7 +7,7 @@ # Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT, GITHUB_EVENT_NAME # # An issue_comment run never skips. That event is how someone asks for a review by hand -# ("@claude review", "/codex-review"), so it is a deliberate request to review a commit that has +# ("@claude review", "@codex review"), so it is a deliberate request to review a commit that has # most likely already been reviewed - the one case where repeating the review is the point. # # CONTEXT names the reviewer (e.g. "ai-review-completed/claude", "ai-review-completed/codex").