Skip to content

fix(claude-blocking-review): distinct skip reasons + SHA-scoped skip marker (v3.1.0) #93

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

Merged
twistedmelonman merged 2 commits into from
Aug 7, 2026
Merged

fix(claude-blocking-review): distinct skip reasons + SHA-scoped skip marker (v3.1.0) #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e535652
fix(claude-blocking-review): distinct skip reasons + SHA-scoped skip …
Aug 7, 2026
a13e6fa
fix(claude-blocking-review): align minimize-step escape-hatch check w…
Aug 7, 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
124 changes: 106 additions & 18 deletions .github/workflows/claude-blocking-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,37 @@ name: Claude Blocking Review
# Migration from v2: remove any `max_turns:` input from caller workflows.
# Workflow validation will fail until the input is removed.
#
# v3.1.0 (2026-08-07): #86 — each skip path (workflow-self-modification,
# doc-only diff, Dependabot-authored PR) now writes its own skip/skip_reason
# output pair, and the "Check review verdict" step logs the actual reason
# that fired instead of a hardcoded "Doc-only skip" message regardless of
# which path triggered. #88 — the [skip-claude-review] escape hatch can now
# be scoped to a specific commit; see below. Both changes are additive:
# existing unscoped markers and log consumers are unaffected.
#
# Escape hatch: add [skip-claude-review: reason] to the PR body to bypass
# enforcement with an audit trail.
# enforcement with an audit trail. This unscoped form is honored
# unconditionally on every subsequent run for the life of the PR (deliberate
# grandfathered behavior — it's a visible, chosen opt-out, not a bug).
#
# To scope a skip to a single commit instead, use:
# [skip-claude-review sha=<short-sha>: reason]
# where <short-sha> is a >=7-character prefix of the commit SHA you want to
# bypass review for (github.event.pull_request.head.sha — the actual PR
# head commit, not github.sha, which is a synthetic merge-commit SHA on
# pull_request events). The marker only applies while it matches the
# CURRENT head SHA; if the PR is later pushed to, the head SHA changes and
# the marker stops applying (visible as an ::notice:: in the step summary),
# which is the intended anti-staleness behavior.
#
# IMPORTANT: editing the PR body does not retrigger this workflow (no
# `edited` in the trigger list), so to make a newly-added sha=-scoped
# marker actually take effect on an existing failed/blocked run, use:
# gh run rerun <run-id>
# on that run — do NOT push a new commit to "retry." A new commit changes
# the head SHA and would immediately invalidate a marker scoped to the old
# one; `gh run rerun` re-runs the job against the SAME SHA the marker names,
# which is the only way a sha=-scoped marker can take effect.

on:
workflow_call:
Expand Down Expand Up @@ -167,6 +196,7 @@ jobs:
.github/workflows/*.yml|.github/workflows/*.yaml)
echo "::notice::PR modifies .github/workflows/ ($f) — claude-code-action refuses to run by design. Skipping; real review will run on the next non-workflow PR after merge."
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "skip_reason=workflow-self-modification" >> "$GITHUB_OUTPUT"
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** SKIPPED (workflow-self-modification)" >> "$GITHUB_STEP_SUMMARY"
exit 0
Expand Down Expand Up @@ -214,6 +244,7 @@ jobs:
FILE_COUNT=$(printf '%s\n' "$FILES" | grep -c .)
echo "::notice::Doc-only diff (${FILE_COUNT} file(s)) — skipping Claude review."
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "skip_reason=doc-only diff (${FILE_COUNT} file(s))" >> "$GITHUB_OUTPUT"
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** SKIPPED (doc-only diff, ${FILE_COUNT} file(s))" >> "$GITHUB_STEP_SUMMARY"
else
Expand All @@ -228,6 +259,7 @@ jobs:
if [ "${{ github.actor }}" = "dependabot[bot]" ]; then
echo "::notice::Dependabot PR — skipping AI review (version bumps are low-risk)."
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "skip_reason=Dependabot version bump" >> "$GITHUB_OUTPUT"
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** SKIPPED (Dependabot version bump)" >> "$GITHUB_STEP_SUMMARY"
else
Expand Down Expand Up @@ -488,30 +520,73 @@ jobs:
PR_NUMBER: ${{ inputs.pr_number }}
CLAUDE_OUTCOME: ${{ steps.claude-review.outcome }}
DOC_SKIP: ${{ steps.doc-check.outputs.skip }}
DOC_SKIP_REASON: ${{ steps.doc-check.outputs.skip_reason }}
DEPENDABOT_SKIP: ${{ steps.dependabot-check.outputs.skip }}
DEPENDABOT_SKIP_REASON: ${{ steps.dependabot-check.outputs.skip_reason }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
run: |
# Short-circuit: doc-only diff already wrote its summary in the
# Check for doc-only diff step. Nothing to verify, nothing to block.
# Short-circuit: an earlier step already wrote its own summary and
# decided to skip. Log the ACTUAL path that fired (from its own
# skip_reason output) rather than a hardcoded message — the
# doc-check step handles two distinct skip paths (workflow-self-
# modification and doc-only diff) that must not be conflated.
if [ "$DOC_SKIP" = "true" ]; then
echo "Doc-only skip — no verdict required."
echo "Review skipped (${DOC_SKIP_REASON}) — no verdict required."
exit 0
fi

if [ "$DEPENDABOT_SKIP" = "true" ]; then
echo "Dependabot skip — no verdict required."
echo "Review skipped (${DEPENDABOT_SKIP_REASON}) — no verdict required."
exit 0
fi

# Escape hatch: [skip-claude-review] or [skip-claude-review: reason] in PR body
# bypasses enforcement. The extended regex matches both the bare token and the
# documented `: reason` form advertised by our error messages. See issue #38
# for the prior -F fixed-string pattern that only matched the bare form.
# Escape hatch: [skip-claude-review: reason] or the SHA-scoped form
# [skip-claude-review sha=<short-sha>: reason] in the PR body bypasses
# enforcement.
#
# SHA scoping (see #88): a bare/unscoped marker (no `sha=`) is
# grandfathered and always honored, unconditionally, exactly as
# before — this is a deliberate, visible opt-out an operator chose,
# not the bug. A `sha=`-scoped marker is only honored when its SHA
# is a >=7-character prefix of the CURRENT head SHA
# (github.event.pull_request.head.sha — NOT github.sha, which is a
# synthetic merge-commit SHA on pull_request events that the
# operator never sees or types). Shorter prefixes are treated as
# invalid/unmatched to prevent a trivial bypass like `sha=a`.
#
# To make a sha=-scoped marker take effect after adding it to the
# PR body: use `gh run rerun <run-id>` on the existing failed/
# blocked run. A new commit/push changes the head SHA and correctly
# invalidates a SHA-scoped marker (intentional anti-staleness
# behavior) — `gh run rerun` re-checks the SAME commit, which is
# the only way a sha=-scoped marker can actually apply.
PR_BODY=$(gh pr view "$PR_NUMBER" --json body -q .body 2>/dev/null || echo "")
if echo "$PR_BODY" | grep -qE '\[skip-claude-review(\]|:)'; then
echo "::notice::Claude Code Review enforcement skipped via [skip-claude-review] marker in PR body."
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** SKIPPED (override in PR body)" >> "$GITHUB_STEP_SUMMARY"
exit 0
MARKER=$(echo "$PR_BODY" | grep -oE '\[skip-claude-review[^]]*\]' | head -1 || echo "")

if [ -n "$MARKER" ]; then
MARKER_SHA=$(echo "$MARKER" | grep -oE 'sha=[0-9a-fA-F]+' | head -1 | cut -d= -f2 || echo "")

if [ -z "$MARKER_SHA" ]; then
# Unscoped marker — grandfathered, unconditional skip.
echo "::notice::Claude Code Review enforcement skipped via unscoped [skip-claude-review] marker in PR body."
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** SKIPPED (override in PR body)" >> "$GITHUB_STEP_SUMMARY"
exit 0
elif [ "${#MARKER_SHA}" -lt 7 ]; then
# Too short to trust as a real SHA prefix — reject as invalid,
# fall through to a real review.
echo "::warning::[skip-claude-review sha=${MARKER_SHA}] marker rejected — sha= value must be at least 7 characters. Proceeding with review."
elif [ "${HEAD_SHA#"$MARKER_SHA"}" != "$HEAD_SHA" ]; then
# HEAD_SHA starts with MARKER_SHA — scoped marker matches current head.
echo "::notice::Claude Code Review enforcement skipped via [skip-claude-review sha=${MARKER_SHA}] marker matching current head sha=${HEAD_SHA}."
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** SKIPPED (override in PR body, sha=${MARKER_SHA})" >> "$GITHUB_STEP_SUMMARY"
exit 0
else
# Scoped marker present but doesn't match current head — make
# the non-application visible instead of silently proceeding.
echo "::notice::Skip marker present but scoped to sha=${MARKER_SHA}, current head is sha=${HEAD_SHA} — marker not honored. Use 'gh run rerun' on a run against sha=${MARKER_SHA} to apply it, or add a new marker scoped to sha=${HEAD_SHA}."
fi
fi

VERDICT_FILE="/tmp/review-verdict.txt"
Expand Down Expand Up @@ -586,11 +661,24 @@ jobs:
# comment body is preserved in the PR thread under a collapsed
# "Resolved" disclosure, so the audit trail is intact.

# Skip if the [skip-claude-review] escape hatch was used.
# Skip if the [skip-claude-review] escape hatch is ACTIVE for this
# commit (see #88) — i.e. an unscoped marker (always active) or a
# sha=-scoped marker whose SHA matches the current head. A
# sha=-scoped marker that does NOT match the current head means
# the review actually ran and a real PASS comment may exist to
# minimize, so that case must fall through rather than bail here
# (matching the same precedence used in "Check review verdict").
PR_BODY=$(gh pr view "$PR_NUMBER" --json body -q .body 2>/dev/null || echo "")
if echo "$PR_BODY" | grep -qE '\[skip-claude-review(\]|:)'; then
echo "[skip-claude-review] override active — nothing to minimize."
exit 0
MARKER=$(echo "$PR_BODY" | grep -oE '\[skip-claude-review[^]]*\]' | head -1 || echo "")
if [ -n "$MARKER" ]; then
MARKER_SHA=$(echo "$MARKER" | grep -oE 'sha=[0-9a-fA-F]+' | head -1 | cut -d= -f2 || echo "")
if [ -z "$MARKER_SHA" ]; then
echo "[skip-claude-review] unscoped override active — nothing to minimize."
exit 0
elif [ "${#MARKER_SHA}" -ge 7 ] && [ "${SHA#"$MARKER_SHA"}" != "$SHA" ]; then
echo "[skip-claude-review sha=${MARKER_SHA}] override active for current head — nothing to minimize."
exit 0
fi
fi

VERDICT_FILE="/tmp/review-verdict.txt"
Expand Down
40 changes: 33 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ bugs, reliability regressions, security vulnerabilities, or data-loss risks.
### What triggers a BLOCK

| Category | Examples |
|----------|---------|
| ---------- | --------- |
| Clear bug | Wrong calculation, inverted condition, off-by-one affecting real data |
| Reliability regression | Previously working path may now fail due to this PR |
| Security | Hardcoded credentials, auth bypass, unvalidated input to privileged op |
Expand All @@ -34,10 +34,10 @@ removed the `--max-turns` cap; reviews are now bounded only by the
wall-clock timeout, the prompt's scope discipline, and the OAuth
subscription quota.

| Parameter | Logic | Range |
|-----------|-------|-------|
| **Model** | Sonnet (callers can override) | `claude-sonnet-4-6` |
| **Timeout** | `10 + lines/100` minutes | 10–30 minutes |
| Parameter | Logic | Range |
| ----------- | ----------------------------- | ------------------- |
| **Model** | Sonnet (callers can override) | `claude-sonnet-4-6` |
| **Timeout** | `10 + lines/100` minutes | 10–30 minutes |

Callers can override any parameter:

Expand Down Expand Up @@ -69,6 +69,32 @@ bypass if needed.
Add `[skip-claude-review: reason]` to the PR body to bypass enforcement.
The override is logged in the step summary for audit.

This unscoped form is honored unconditionally on **every** subsequent run
for the life of the PR — it's a deliberate, visible opt-out, not scoped to
any particular commit. That's intentional grandfathered behavior for
markers already in use; see below for a way to bound the bypass to a
single commit.

**Scoping a skip to one commit (v3.1.0+):** add
`[skip-claude-review sha=<short-sha>: reason]` instead, where `<short-sha>`
is a prefix (7+ characters) of the commit SHA you want to bypass review
for — use the PR's head commit SHA, e.g. from `gh pr view <PR> --json
headRefOid -q .headRefOid`. The marker is only honored while it matches
the **current** head SHA of the PR. If the PR is pushed to again, the head
SHA changes and the marker stops applying automatically — a visible
`::notice::` in the step summary calls this out so it isn't only
discoverable by diffing raw run logs. Markers with a `sha=` value shorter
than 7 characters are rejected as invalid (prevents a trivial bypass like
`sha=a` matching anything) and the review proceeds normally.

**Important: to make a `sha=`-scoped marker take effect, use `gh run rerun
<run-id>` on the existing failed/blocked run — do not push a new commit.**
Editing the PR body does not retrigger this workflow (there's no `edited`
event in the trigger list), so the only way to get a fresh check run
against the same commit is `gh run rerun`. A new commit/push changes the
head SHA and immediately invalidates a marker scoped to the old SHA — that
is the intended anti-staleness behavior, not a bug to work around.

### Setup

#### 1. Add the secret
Expand Down Expand Up @@ -124,7 +150,7 @@ The BLOCK criteria are in the workflow prompt. To adjust:
### Versioning

| Tag | Meaning |
|-----|---------|
| ----- | --------- |
| `@v3` | Current stable major version (floating — gets minor updates). v3 dropped the `max_turns` input; remove it from caller workflows when bumping. |
| `@v2` | Previous stable major (still supported for callers that haven't migrated; passes `--max-turns` to the agent and accepts `max_turns:` input) |
| `@v1` | Initial release line |
Expand Down Expand Up @@ -222,7 +248,7 @@ in the "New workflow" picker for `smartwatermelon/*` repos.
The script classifies each repo:

| Class | Action |
|-------|--------|
| ------- | -------- |
| `CURRENT` | Already on the target version. No-op. |
| `STALE` | Different pin or floating tag. Opens a PR bumping the pin. |
| `MISSING` | No caller workflow at all. Opens a PR adding the canonical stub. |
Expand Down
Loading