Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
38 changes: 38 additions & 0 deletions .github/actions/claude-review-toolkit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,44 @@ Caller repos must ship a `.claude/skills/coding-standards/rules/` directory with
| `createInlineComment.sh` | `<PR_NUMBER> <path> <body> <line>` | 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` | `<PR_NUMBER>` | 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` | `<rules-dir> <output-file>` | Walks `<rules-dir>` for `.md` rule files and writes their `ruleId:` tags to `<output-file>`. Invoked automatically by the action; rarely called directly. |
| `shouldSkipReview.sh` | `<PR_NUMBER> <CONTEXT>` | 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 `<CONTEXT>/pr-<PR_NUMBER>`, 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` | `<HEAD_SHA> <CONTEXT> [DESCRIPTION]` | Sets a `success` commit status with `<CONTEXT>` on `<HEAD_SHA>`, 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`:

```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'
Comment thread
neil-marcellini marked this conversation as resolved.
Outdated
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.

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

Expand Down
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 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
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"
57 changes: 57 additions & 0 deletions .github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/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>", "context=<status context>" and "skip=true|false" to $GITHUB_OUTPUT.
# Usage: shouldSkipReview.sh <PR_NUMBER> <CONTEXT>
# Env: GH_TOKEN, GITHUB_REPOSITORY, GITHUB_OUTPUT
#
# 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
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 STATUS_CONTEXT="$CONTEXT/pr-$PR_NUMBER"
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, 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 "$STATUS_CONTEXT already completed for $HEAD_SHA, skipping review" >&2
echo "skip=true" >> "$GITHUB_OUTPUT"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
Loading