From 0458c4f9214ca37f8f9b6ee51b189eca63cfd055 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Tue, 28 Apr 2026 11:03:25 -0700 Subject: [PATCH 1/3] feat(claude-blocking-review)!: drop --max-turns cap (v3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviews were getting force-killed mid-run on small/medium diffs because the auto-estimator's turn floor (15) was tighter than reality. The "Review did not complete (likely exceeded turn limit or timed out)" failure mode hit ralph-burndown PR #119 and #122 in the same week. Diagnosis: PR #122 (288 lines, 4 files) hit the failure at ~4 min elapsed against a 9-min wall-clock timeout — well before the timeout could have fired. Max turns was the actual constraint. The estimator's 1.44-compounded buffer was insufficient because the agent's natural rhythm doesn't fit a fixed turn budget; tightening or loosening the floor would just shift the same failure mode. The right shape is to remove the artificial cap entirely and let the agent be bounded by: - the prompt's own scope discipline (soft constraint) - the wall-clock timeout-minutes (hard safety net) - the OAuth subscription quota (hard ceiling on cost) Subscription billing means unused turns are free — only actual elapsed time matters. Generous wall-clock with no turn cap is the right posture. Changes: - Remove `--max-turns` from `claude_args`. Remove the `max_turns` input parameter cleanly (caller workflows passing it now fail validation — intentional; internal-only callers in smartwatermelon/nightowlstudio orgs migrate to v3 deliberately). - Re-base `timeout_minutes` estimation on diff size directly: `10 + lines/100` minutes, floor 10, cap 30. Floor raised from 4 to 10 since the wall-clock is now the only hard backstop. - Tighten the prompt's scope constraints with explicit self-bounding: 3-file ceiling per hypothesis, "tight PASS / thorough BLOCK is a successful review; meandering exploration is wasted." Without a turn cap the prompt does the load-bearing work of keeping reviews focused. - Update example caller snippet `@v1` → `@v3`. - Update README migration table and `with:` example. Caller migration: remove any `max_turns:` input from your caller workflow. v2 remains supported for callers that haven't migrated. BREAKING CHANGE: the `max_turns` input parameter is removed. Callers on @v3 must remove the input from their workflow or workflow validation will fail. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-blocking-review.yml | 145 ++++++++----------- README.md | 29 ++-- 2 files changed, 80 insertions(+), 94 deletions(-) diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index 562617b..2739380 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -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: # @@ -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: | @@ -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. @@ -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 @@ -100,7 +107,6 @@ 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 @@ -108,15 +114,6 @@ jobs: 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" @@ -230,7 +227,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 ' ') @@ -245,64 +241,35 @@ 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" @@ -310,7 +277,6 @@ jobs: 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 @@ -391,16 +357,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 @@ -486,7 +465,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() diff --git a/README.md b/README.md index f417897..829329c 100644 --- a/README.md +++ b/README.md @@ -43,17 +43,25 @@ 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 @@ -82,7 +90,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: | @@ -116,10 +124,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) | --- From b4b1e0913b354e100bcce6dae7a11cb26edaf497 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Tue, 28 Apr 2026 11:07:42 -0700 Subject: [PATCH 2/3] fix(claude-blocking-review): update stale diagnostic referencing removed --max-turns Codebase review on the v3 commit caught two follow-on misses where the "exceeded turn limit" wording survived in error messages and a comment even though v3 removes the only cause of that error path. Without --max-turns the step can no longer hit a turn cap; the only remaining no-verdict failure modes are wall-clock timeout or a Claude-side agent error. Updated the verdict-fallback messages and the workflow-self-modification skip comment to match. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-blocking-review.yml | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index 2739380..f4f640d 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -154,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 @@ -517,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 From 0d09ff29487d0781ad9728702f9f5709bc91ecaa Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Tue, 28 Apr 2026 11:10:39 -0700 Subject: [PATCH 3/3] docs: update README auto-sizing table for v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codebase review caught the ### Auto-sizing section still documenting the v2 behavior — "Max turns" row, old timeout formula `turns × 30s`, "turn budget is intentionally tight" framing. A v3 caller copying from that table would write `max_turns:` and fail validation. Drop the Max turns row, replace the Timeout row formula with the new `10 + lines/100` direct calc, update the lead paragraph to reflect that wall-clock is now the sole hard bound. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 829329c..b1cdf90 100644 --- a/README.md +++ b/README.md @@ -28,15 +28,16 @@ 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: