Skip to content

feat(claude-blocking-review)!: drop --max-turns cap (v3) #62

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 3 commits into from
Apr 28, 2026
Merged

feat(claude-blocking-review)!: drop --max-turns cap (v3) #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0458c4f
feat(claude-blocking-review)!: drop --max-turns cap (v3)
Apr 28, 2026
b4b1e09
fix(claude-blocking-review): update stale diagnostic referencing remo…
Apr 28, 2026
0d09ff2
docs: update README auto-sizing table for v3
Apr 28, 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
165 changes: 73 additions & 92 deletions .github/workflows/claude-blocking-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@ name: Claude Blocking Review
# Reusable workflow: blocks PR merges when Claude finds bugs, reliability
# regressions, security issues, or data-loss risks.
#
# By default, review parameters are auto-estimated from the PR diff size.
# The prompt is BLOCK-only (bug / reliability / security / async-error /
# data-loss), not a full code review — local reviewers cover style, coverage,
# and docs — so the estimation base is intentionally tight:
# - Model: sonnet (callers can override)
# - max_turns: 8 + lines/200, +20% buffer (min 15, max 40)
# - timeout: scaled from max_turns (min 4m, max 30m)
# Callers can override any parameter explicitly. Override range for
# max_turns is 1-50; timeout_minutes is 1-30.
# v3 (2026-04-28): drops the `--max-turns` cap on the Claude agent — it
# was force-killing reviews mid-run on small/medium diffs because the
# estimator's floor (15 turns) was tighter than reality. Reviews are now
# bounded only by the wall-clock `timeout-minutes` (the hard safety net),
# the prompt's own scope discipline, and the OAuth subscription quota.
# This trades a predictable artificial cap for a more permissive natural
# bound; the previous "Review did not complete (likely exceeded turn
# limit)" failure mode is gone. Subscription billing means unused turns
# are free — generous wall-clock is the right posture.
#
# By default, review parameters are auto-estimated from the PR diff size:
# - Model: claude-sonnet-4-6 (callers can override)
# - timeout: 10 + lines/100 minutes (min 10, max 30)
# Callers can override `model` and `timeout_minutes` explicitly. Override
# range for timeout_minutes is 1-30. Removed in v3: the `max_turns` input.
#
# Usage in a caller workflow:
#
Expand All @@ -23,7 +29,7 @@ name: Claude Blocking Review
#
# jobs:
# claude-review:
# uses: YOUR_ORG/github-workflows/.github/workflows/claude-blocking-review.yml@v1
# uses: smartwatermelon/github-workflows/.github/workflows/claude-blocking-review.yml@v3
# with:
# pr_number: ${{ github.event.pull_request.number }}
# extra_instructions: |
Expand All @@ -34,6 +40,9 @@ name: Claude Blocking Review
# The calling job name ("claude-review" above) becomes the required status
# check name: "Claude Blocking Review / claude-review / run-review" in branch protection.
#
# Migration from v2: remove any `max_turns:` input from caller workflows.
# Workflow validation will fail until the input is removed.
#
# Escape hatch: add [skip-claude-review: reason] to the PR body to bypass
# enforcement with an audit trail.

Expand All @@ -49,13 +58,11 @@ on:
type: string
required: false
default: ''
max_turns:
description: 'Maximum Claude API turns. 0 = auto-estimate from diff size. Callers can override.'
type: number
required: false
default: 0
timeout_minutes:
description: 'Hard timeout for the Claude review step in minutes. 0 = auto-estimate from diff size.'
description: |
Hard timeout for the Claude review step in minutes (max 30).
0 = auto-estimate from diff size. Since v3 dropped the
--max-turns cap, this is the only hard safety net.
type: number
required: false
default: 0
Expand Down Expand Up @@ -100,23 +107,13 @@ jobs:
- name: Validate inputs
env:
MODEL: ${{ inputs.model }}
MAX_TURNS: ${{ inputs.max_turns }}
TIMEOUT: ${{ inputs.timeout_minutes }}
run: |
# Validate model: "auto" or alphanumeric/dots/hyphens
if [ "$MODEL" != "auto" ] && ! echo "$MODEL" | grep -qE '^[a-zA-Z0-9._-]+$'; then
echo "::error::Invalid model name. Must be 'auto' or match [a-zA-Z0-9._-]+"
exit 1
fi
# Validate max_turns: 0 (auto) or 1-50
if ! echo "$MAX_TURNS" | grep -qE '^[0-9]+$'; then
echo "::error::max_turns must be a non-negative integer"
exit 1
fi
if [ "$MAX_TURNS" -gt 50 ]; then
echo "::error::max_turns must be between 0 and 50 (0 = auto)"
exit 1
fi
# Validate timeout_minutes: 0 (auto) or 1-30
if ! echo "$TIMEOUT" | grep -qE '^[0-9]+$'; then
echo "::error::timeout_minutes must be a non-negative integer"
Expand Down Expand Up @@ -157,12 +154,13 @@ jobs:
# PRIORITY SKIP — workflow-self-modification.
# When a PR modifies ANY .github/workflows/*.yml file, the
# anthropics/claude-code-action refuses to run by design (its
# security feature: a PR that modifies the reviewer cannot have the
# reviewer run against itself). Without a skip here, the step fails
# with a misleading "exceeded turn limit" error after 30s. This
# case hits EVERY Dependabot-generated PR that bumps the pin, plus
# any manual caller-workflow edit. Short-circuit cleanly; a real
# review runs on the next non-workflow PR after merge.
# security feature: a PR that modifies the reviewer cannot have
# the reviewer run against itself). Without a skip here, the step
# fails fast with a misleading "review did not complete" error.
# This case hits EVERY Dependabot-generated PR that bumps the
# pin, plus any manual caller-workflow edit. Short-circuit
# cleanly; a real review runs on the next non-workflow PR after
# merge.
while IFS= read -r f; do
[ -z "$f" ] && continue
case "$f" in
Expand Down Expand Up @@ -230,7 +228,6 @@ jobs:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ inputs.pr_number }}
MODEL_INPUT: ${{ inputs.model }}
MAX_TURNS_INPUT: ${{ inputs.max_turns }}
TIMEOUT_INPUT: ${{ inputs.timeout_minutes }}
run: |
DIFF_LINES=$(gh pr diff "$PR_NUMBER" --repo "${GITHUB_REPOSITORY}" | wc -l | tr -d ' ')
Expand All @@ -245,72 +242,42 @@ jobs:
echo "Model: claude-sonnet-4-6"
fi

# --- Turn estimation ---
# The prompt is now BLOCK-only (bug / reliability / security /
# data-loss / async-error criteria) — no style, no coverage, no
# CLAUDE.md read, no per-file reads for organizational context.
# Fixed overhead is therefore lower than the wide-prompt era:
# - Read diff (1-2 turns)
# - Scan for hazards, reason about findings (2-4 turns)
# - Write review, append verdict (2 turns)
# - Write verdict file, post comment (4 turns — 3-command post)
# ≈8-12 turns for a typical small diff. Floor of 15 keeps a
# comfortable margin against the "exceeded turn limit" failure
# mode that drove the earlier 25 floor (see AAR 2026-04-17).
# Unused turns are free; only turns actually spent cost tokens,
# so the floor is a ceiling-above-overhead, not a target.
if [ "$MAX_TURNS_INPUT" -gt 0 ]; then
MAX_TURNS="$MAX_TURNS_INPUT"
echo "max_turns override: $MAX_TURNS"
else
ESTIMATED=$((8 + DIFF_LINES / 200))
MAX_TURNS=$(( (ESTIMATED * 120 + 99) / 100 ))
if [ "$MAX_TURNS" -lt 15 ]; then MAX_TURNS=15; fi
if [ "$MAX_TURNS" -gt 40 ]; then MAX_TURNS=40; fi
echo "Estimated turns: $ESTIMATED → allocated: $MAX_TURNS"
fi

# --- Timeout estimation ---
# ~30s per turn as baseline, with a second 20% buffer on top of the
# max_turns value (which already carries its own 20% buffer from the
# turn estimation above). Compounded factor = 1.20 × 1.20 = 1.44 —
# so the timeout is ~44% over the raw `turns × 30s` baseline.
# This is INTENTIONAL: a max_turns exhaustion is what Claude hits
# when its budget runs out mid-run (produces "exceeded turn limit");
# a wall-clock timeout is the outer safety net for infrastructure
# flakes (network stalls, backend pressure) that don't consume turns
# but still eat wall-clock. Keeping slack on the wall-clock side
# means flakes that would otherwise kill the step instead give
# Claude room to recover and finish. The AAR 2026-04-17 documented
# three consecutive infra-flake timeouts in one session on
# kebab-tax#1162 — extra slack is the right posture.
# At current bounds (MAX_TURNS ∈ [15, 40]) the computed timeout is
# always 9–24 min, so the 4 min floor and 30 min ceiling are
# DEFENSIVE — they only activate if a future change lowers the
# MAX_TURNS floor below 8 or raises it above 50. The caller-side
# timeout_minutes validation in Validate inputs enforces a hard
# upper bound of 30 regardless of what the estimator computes.
# --- Timeout estimation (v3) ---
# The wall-clock `timeout-minutes` is the only hard safety net
# since v3 dropped the --max-turns cap. The previous formula
# derived timeout from max_turns × 30s × 1.44 buffer; with no
# turn cap, we compute timeout directly from diff size:
#
# TIMEOUT = max(10, 10 + lines/100) capped at 30
#
# Floor raised from 4 → 10 min because the wall-clock is now
# the SOLE mechanism that bounds a runaway / confused agent.
# Subscription billing means an oversized timeout is free if
# the agent finishes early; only actual elapsed time matters.
# Generous is the right posture.
#
# Sample sizing:
# 100-line diff → 11 min
# 1,000-line diff → 20 min
# 2,000-line diff → 30 min (cap)
if [ "$TIMEOUT_INPUT" -gt 0 ]; then
TIMEOUT="$TIMEOUT_INPUT"
echo "timeout_minutes override: $TIMEOUT"
else
TIMEOUT_SECS=$(( MAX_TURNS * 30 * 120 / 100 ))
TIMEOUT=$(( (TIMEOUT_SECS + 59) / 60 ))
if [ "$TIMEOUT" -lt 4 ]; then TIMEOUT=4; fi
TIMEOUT=$(( 10 + DIFF_LINES / 100 ))
if [ "$TIMEOUT" -lt 10 ]; then TIMEOUT=10; fi
if [ "$TIMEOUT" -gt 30 ]; then TIMEOUT=30; fi
echo "Estimated timeout: ${TIMEOUT}m"
echo "Estimated timeout: ${TIMEOUT}m (from ${DIFF_LINES} diff lines)"
fi

echo "model=$MODEL" >> "$GITHUB_OUTPUT"
echo "max_turns=$MAX_TURNS" >> "$GITHUB_OUTPUT"
echo "timeout_minutes=$TIMEOUT" >> "$GITHUB_OUTPUT"

echo "### Review Parameters" >> "$GITHUB_STEP_SUMMARY"
echo "| Parameter | Value |" >> "$GITHUB_STEP_SUMMARY"
echo "|-----------|-------|" >> "$GITHUB_STEP_SUMMARY"
echo "| Diff lines | $DIFF_LINES |" >> "$GITHUB_STEP_SUMMARY"
echo "| Model | $MODEL |" >> "$GITHUB_STEP_SUMMARY"
echo "| Max turns | $MAX_TURNS |" >> "$GITHUB_STEP_SUMMARY"
echo "| Timeout | ${TIMEOUT}m |" >> "$GITHUB_STEP_SUMMARY"

- name: Minimize prior review comments
Expand Down Expand Up @@ -391,16 +358,29 @@ jobs:
test coverage, and documentation. Do NOT duplicate that work —
it costs money and dilutes the signal of a real BLOCK.

SCOPE CONSTRAINTS — follow these strictly:
- Read the PR diff with `gh pr diff`
SCOPE CONSTRAINTS — follow these strictly. v3 dropped the
artificial --max-turns cap, so YOU are the soft constraint
keeping the review efficient. The wall-clock timeout is the
only hard backstop, and burning it on exploration costs
real subscription quota for no signal.

- Read the PR diff with `gh pr diff` — that's your primary input
- Read a changed file ONLY when you need execution-path context
to confirm a specific BLOCK hypothesis — never for "general
context" or style check
context", style check, or to prove a hunch you can already
answer from the diff alone
- Hard ceiling: if confirming a single hypothesis would require
reading more than 3 files, stop and either downgrade to PASS
with a named observation or BLOCK based on the evidence you
already have. Do not chase a fourth file.
- Do NOT read CLAUDE.md, README, or any doc — they describe
style and conventions, which are not in scope
- Do NOT explore the broader codebase, run tests, or investigate
unrelated files
- Complete your review in as few turns as possible
- Complete your review in as few turns as possible. A tight
PASS is a successful review; a thorough BLOCK is a successful
review; a meandering exploration that runs out of wall-clock
is a wasted review.

Check the diff for these BLOCK criteria ONLY:
- A clear bug that causes incorrect behavior in production
Expand Down Expand Up @@ -486,7 +466,7 @@ jobs:
VERDICT line inside the posted comment (Step 2 / Step 4) is the
fallback. Both must match.

claude_args: '--max-turns ${{ steps.estimate.outputs.max_turns }} --model ${{ steps.estimate.outputs.model }} --allowed-tools "Read,Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(echo *),Bash(cat *),Bash(tee *),Bash(printf *)"'
claude_args: '--model ${{ steps.estimate.outputs.model }} --allowed-tools "Read,Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(echo *),Bash(cat *),Bash(tee *),Bash(printf *)"'

- name: Check review verdict
if: always()
Expand Down Expand Up @@ -538,12 +518,13 @@ jobs:
VERDICT="VERDICT: PASS"
echo "Verdict source: PR comment (fallback)"
else
# No verdict found — distinguish exhaustion from infrastructure failure
# No verdict found — distinguish a real Claude-side failure
# (timeout, agent error) from an infrastructure flake.
if [ "$CLAUDE_OUTCOME" = "failure" ]; then
echo "::error::Review did not complete (likely exceeded turn limit or timed out). No verdict rendered."
echo "::error::Review did not complete (likely timed out or failed to render verdict). No verdict rendered."
echo "::error::Re-push to retry, or add [skip-claude-review: reason] to the PR body to bypass."
echo "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** INCOMPLETE — review exceeded turn limit or timed out." >> "$GITHUB_STEP_SUMMARY"
echo "**Verdict:** INCOMPLETE — review timed out or failed to render verdict." >> "$GITHUB_STEP_SUMMARY"
echo "Re-push to retry or add \`[skip-claude-review: reason]\` to bypass." >> "$GITHUB_STEP_SUMMARY"
exit 1
else
Expand Down
40 changes: 24 additions & 16 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,41 @@ test failures without traceable evidence default to PASS.

Review parameters are estimated automatically from the PR diff size. The
reviewer prompt is BLOCK-only (bug / reliability regression / security /
async-error / data-loss), not a full code review, so the turn budget is
intentionally tight — local reviewers are expected to cover style, test
coverage, and documentation concerns:
async-error / data-loss), not a full code review — local reviewers are
expected to cover style, test coverage, and documentation concerns. v3
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` |
| **Max turns** | `8 + lines/200`, +20% buffer | 15–40 |
| **Timeout** | `turns × 30s × 1.2` | 4–30 minutes |
| **Timeout** | `10 + lines/100` minutes | 10–30 minutes |

Callers can override any parameter:

```yaml
with:
pr_number: ${{ github.event.pull_request.number }}
model: claude-sonnet-4-6 # force sonnet for all diffs
max_turns: 30 # override turn estimate
model: claude-sonnet-4-6 # force sonnet for all diffs
timeout_minutes: 15 # override timeout estimate
```

Pass `0` for `max_turns` or `timeout_minutes` to use auto-estimation (the default).
Pass `0` for `timeout_minutes` to use auto-estimation (the default).
Pass `auto` for `model` to use auto-selection (the default).

If the review runs out of turns or times out before rendering a verdict, the
check **fails** (INCOMPLETE) instead of silently passing. Use the escape hatch
below to bypass if needed.
**v3 (2026-04-28):** the `max_turns` input was removed and the
`--max-turns` flag is no longer passed to the Claude agent. Reviews are
now bounded only by the wall-clock `timeout_minutes` (the hard safety
net), the prompt's own scope discipline, and the OAuth subscription
quota. The previous "Review did not complete (likely exceeded turn
limit)" failure mode is gone. **Caller migration:** remove `max_turns:`
from your caller workflow when bumping to `@v3`; it will fail
workflow validation if left in place.

If the review times out before rendering a verdict, the check **fails**
(INCOMPLETE) instead of silently passing. Use the escape hatch below to
bypass if needed.

### Escape hatch

Expand Down Expand Up @@ -82,7 +91,7 @@ on:

jobs:
claude-review:
uses: YOUR_ORG/github-workflows/.github/workflows/claude-blocking-review.yml@v1
uses: YOUR_ORG/github-workflows/.github/workflows/claude-blocking-review.yml@v3
with:
pr_number: ${{ github.event.pull_request.number }}
# extra_instructions: |
Expand Down Expand Up @@ -116,10 +125,9 @@ The BLOCK criteria are in the workflow prompt. To adjust:

| Tag | Meaning |
|-----|---------|
| `@v1` | Current stable major version (floating — gets minor updates) |
| `@v1.2.0` | Latest pinned release |
| `@v1.1.0` | Previous pinned release |
| `@v1.0.0` | Initial release |
| `@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 |
| `@main` | Latest (may include breaking changes) |

---
Expand Down
Loading