From 7e5d8f380730319c6ab2edb15a9553c0480f502f Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Sat, 18 Apr 2026 13:04:41 -0700 Subject: [PATCH 1/4] feat: add doc-only skip, SHA marker, and prior-comment collapse to reviewer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three CI-cost reductions aimed at the failure modes documented in the 2026-04-17 AAR. All three land together because they share the same `Check for doc-only diff` step output and the same marker contract. 1. Doc-only fast-skip (new Check for doc-only diff step, lines 126-170) Before spinning up the Claude action, check whether every changed file matches a doc/meta pattern (*.md, LICENSE, CHANGELOG, .gitignore, .github/ISSUE_TEMPLATE/**, docs/, etc.). If yes, skip the paid review and post a SKIPPED verdict to the step summary. The BLOCK criteria (bug, reliability regression, security, data-loss) don't apply to prose changes, so the review was adding cost and flake surface area for zero signal. kebab-tax#1162 was a doc-only PR that failed three times in a row on infrastructure flakes this week — this skip would have saved the full cost of all three runs. 2. SHA marker in every posted review (prompt Step 3 change) Each review comment now starts with: Downstream consumers (status scrapers, the new minimize step below, future tooling) can filter to the review for the current HEAD and ignore phantom findings from prior runs. kebab-tax#1165's post-push loop saw FINDINGS=1→2→3→4 across four iterations precisely because top-level issue comments carry no original_commit_id; the marker closes that gap without changing the comment's rendered body. 3. Collapse prior review comments (new Minimize prior step, lines 240-284) Before the Claude action runs, query the PR for comments whose body begins with the claude-blocking-review marker and minimize them via the minimizeComment GraphQL mutation (classifier: OUTDATED). Keeps the PR conversation readable across iterations — GitHub collapses minimized comments under a "Show resolved" affordance and `gh pr view --comments` excludes them by default. Non-fatal: any failure here logs a warning but does not block the review. Implementation notes: - Estimate, Minimize, and Run Claude steps are all guarded with `if: steps.doc-check.outputs.skip != 'true'`. The final Check review verdict step keeps `if: always()` and short-circuits at the top when DOC_SKIP=true so doc-only PRs exit 0 cleanly. - The marker emission is three sequential commands (printf > file, cat >> file, gh pr comment --body-file file) rather than a pipeline so every command stays within the existing allowed-tools prefix allowlist at line 294 without broadening it. - The minimize query uses --jq to filter for `.isMinimized == false` AND body starts-with the marker, so repeated runs are idempotent (already-minimized comments are skipped). - OUTDATED classifier confirmed valid via GraphQL introspection; requires pull-requests: write (already declared). Expected cost impact: - Doc-only PRs: eliminated (was the full ~5-10 min Claude review cost). - PRs with multiple push iterations: each run still costs one review, but prior reviews collapse instead of accumulating noise. - Diff-unrelated: no change. No change to the BLOCK/PASS contract, no change to the required status check name, no change to the reusable workflow's inputs or secrets. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-blocking-review.yml | 117 ++++++++++++++++++- 1 file changed, 115 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index d635abd..bdee721 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -123,8 +123,54 @@ jobs: exit 1 fi + - name: Check for doc-only diff + id: doc-check + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ inputs.pr_number }} + run: | + # Short-circuit: diffs that only touch docs/meta files don't need a + # paid Claude review — the BLOCK criteria (bug, reliability regression, + # security, data-loss) don't apply to prose changes. Skips saves real + # money per run and eliminates the "3 flakes in a row on a doc-only + # PR" pattern observed in the 2026-04-17 AAR (kebab-tax#1162). + FILES=$(gh pr diff "$PR_NUMBER" --repo "${GITHUB_REPOSITORY}" --name-only 2>/dev/null || echo "") + if [ -z "$FILES" ]; then + echo "::warning::Could not determine changed files — proceeding with review." + echo "skip=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + ALL_DOCS=true + NON_DOC="" + while IFS= read -r f; do + [ -z "$f" ] && continue + case "$f" in + *.md|*.markdown|*.rst|*.txt) ;; + LICENSE|LICENSE.*|COPYING|COPYING.*) ;; + CHANGELOG|CHANGELOG.*|HISTORY|HISTORY.*|AUTHORS|NOTICE) ;; + .gitignore|.gitattributes|.editorconfig|.mailmap) ;; + .github/ISSUE_TEMPLATE/*|.github/PULL_REQUEST_TEMPLATE.md) ;; + .github/CODEOWNERS|.github/FUNDING.yml|.github/dependabot.yml) ;; + docs/*|doc/*) ;; + *) ALL_DOCS=false; NON_DOC="$f"; break ;; + esac + done <<< "$FILES" + + if [ "$ALL_DOCS" = "true" ]; then + 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 "## Claude Code Review" >> "$GITHUB_STEP_SUMMARY" + echo "**Verdict:** SKIPPED (doc-only diff, ${FILE_COUNT} file(s))" >> "$GITHUB_STEP_SUMMARY" + else + echo "Non-doc file present ('$NON_DOC') — proceeding with review." + echo "skip=false" >> "$GITHUB_OUTPUT" + fi + - name: Estimate review parameters id: estimate + if: steps.doc-check.outputs.skip != 'true' env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ inputs.pr_number }} @@ -191,8 +237,56 @@ jobs: echo "| Max turns | $MAX_TURNS |" >> "$GITHUB_STEP_SUMMARY" echo "| Timeout | ${TIMEOUT}m |" >> "$GITHUB_STEP_SUMMARY" + - name: Minimize prior review comments + if: steps.doc-check.outputs.skip != 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ inputs.pr_number }} + run: | + # Collapse prior review comments tagged with the claude-blocking-review + # marker. Keeps the PR conversation readable across multiple review + # iterations (the AAR observed 4 stale reviews on kebab-tax#1165 + # polluting both the human-facing timeline and the post-push status + # scraper). Non-fatal: any failure here must not block the review. + OWNER="${REPO%/*}" + NAME="${REPO#*/}" + + PRIOR_IDS=$(gh api graphql \ + -f query='query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + comments(first: 100) { + nodes { id body isMinimized } + } + } + } + }' \ + -F owner="$OWNER" -F name="$NAME" -F number="$PR_NUMBER" \ + --jq '.data.repository.pullRequest.comments.nodes[] | select(.isMinimized == false) | select(.body | startswith("\n\n' '${{ github.event.pull_request.head.sha }}' '${{ github.run_id }}' > /tmp/review-final.md + cat /tmp/review.md >> /tmp/review-final.md + gh pr comment ${{ inputs.pr_number }} --body-file /tmp/review-final.md + + The marker lets downstream tooling (status scrapers, the + "Minimize prior review comments" step on the next run) identify + reviews produced by this workflow and associate each one with + its reviewed commit. Omitting it breaks those consumers. Step 4 — Write the verdict to the verdict file: echo "VERDICT: PASS" > /tmp/review-verdict.txt @@ -299,7 +404,15 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ inputs.pr_number }} CLAUDE_OUTCOME: ${{ steps.claude-review.outcome }} + DOC_SKIP: ${{ steps.doc-check.outputs.skip }} run: | + # Short-circuit: doc-only diff already wrote its summary in the + # Check for doc-only diff step. Nothing to verify, nothing to block. + if [ "$DOC_SKIP" = "true" ]; then + echo "Doc-only skip — 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 From 0b45b33f50eba659e61ca7760c53019f2bdc744c Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Sat, 18 Apr 2026 13:07:45 -0700 Subject: [PATCH 2/4] fix: exclude CODEOWNERS and dependabot.yml from doc-only allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten the doc-only fast-skip classification per local code-reviewer feedback. Both files live under .github/ alongside true display-only config (FUNDING.yml, issue templates), but they have real security impact: - .github/CODEOWNERS controls which reviewers are required to approve changes. A malicious PR that weakens CODEOWNERS (e.g., removes an approval requirement on a sensitive path, or adds the attacker as an owner of security-critical code) should not skip review. - .github/dependabot.yml controls which ecosystems, registries, and directories get dependency updates, and what the update cadence looks like. Changes here can silently weaken supply-chain posture (e.g., pointing to an attacker-controlled registry, or disabling security updates). FUNDING.yml stays in the allowlist — it's purely display metadata for the "Sponsor this project" button and has no authorization or dependency semantics. Issue templates and .gitignore-class meta files also stay; those are build-time noise, not runtime behavior. Follow-up to the preceding commit in this branch. No other changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-blocking-review.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index bdee721..1d2e912 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -151,7 +151,10 @@ jobs: CHANGELOG|CHANGELOG.*|HISTORY|HISTORY.*|AUTHORS|NOTICE) ;; .gitignore|.gitattributes|.editorconfig|.mailmap) ;; .github/ISSUE_TEMPLATE/*|.github/PULL_REQUEST_TEMPLATE.md) ;; - .github/CODEOWNERS|.github/FUNDING.yml|.github/dependabot.yml) ;; + # FUNDING.yml is display-only; CODEOWNERS (approval authority) + # and dependabot.yml (dependency sourcing) are intentionally + # EXCLUDED — they look meta but have real security impact. + .github/FUNDING.yml) ;; docs/*|doc/*) ;; *) ALL_DOCS=false; NON_DOC="$f"; break ;; esac From 8584ec1c08cd5f6544f05a256531bb365280380e Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Sat, 18 Apr 2026 13:11:35 -0700 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20minimize=20prior=20reviews=20?= =?UTF-8?q?=E2=80=94=20use=20last:100=20and=20explicit-string=20vars?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two non-blocking observations from the pre-push codebase reviewer (issues #42 and #43). #42: GraphQL `comments(first: 100)` returns the OLDEST 100 comments first. On a PR that has collected >100 comments before any claude-blocking-review ran, or on a very active PR that accumulates non-bot comments faster than the minimize step can keep up, recent markers would fall outside the window and never be minimized. `last: 100` gets the newest 100 instead, which is the natural fit for "find my recent review markers to collapse" and aligns the window with how reviews accumulate chronologically. #43: `gh api -F` auto-detects argument types — repo names that happen to parse as integers (e.g. a name like "123456") would be coerced to int and fail the `String!` schema check. Switch owner/name to `-f` (explicit string). Keep `-F number=...` where int coercion is desired for the Int! PR number variable. No behavior change on the current PRs in flight. Follow-up to the two preceding commits in this branch. Closes #42 and #43 on merge. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-blocking-review.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index 1d2e912..8124acf 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -255,17 +255,24 @@ jobs: OWNER="${REPO%/*}" NAME="${REPO#*/}" + # `last: 100` (newest comments first) rather than `first:` (oldest) + # so markers from recent review iterations are always inside the + # window even on noisy PRs. Filter to our marker AND un-minimized + # to keep this idempotent across repeated runs. + # Use -f (explicit string) for owner/name — -F auto-detects types, + # and a repo name that happens to parse as an integer would fail + # the String! schema check. PRIOR_IDS=$(gh api graphql \ -f query='query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $number) { - comments(first: 100) { + comments(last: 100) { nodes { id body isMinimized } } } } }' \ - -F owner="$OWNER" -F name="$NAME" -F number="$PR_NUMBER" \ + -f owner="$OWNER" -f name="$NAME" -F number="$PR_NUMBER" \ --jq '.data.repository.pullRequest.comments.nodes[] | select(.isMinimized == false) | select(.body | startswith("