Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
35 changes: 35 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,41 @@ 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` plus `skip=true\|false` to `$GITHUB_OUTPUT`. `skip` is `true` when a `success` commit status with `<CONTEXT>` already exists on that 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. 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 }}
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

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 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"
52 changes: 52 additions & 0 deletions .github/actions/claude-review-toolkit/scripts/shouldSkipReview.sh
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")
Comment thread
neil-marcellini marked this conversation as resolved.
Outdated
Comment thread
neil-marcellini marked this conversation as resolved.
Outdated
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
Loading