diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index d635abd..a9eb4ad 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -123,8 +123,57 @@ 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) ;; + # 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 + 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 +240,63 @@ 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#*/}" + + # `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(last: 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. + + The verdict file (Step 3) is the primary signal read by CI. The + 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 *)"' @@ -299,7 +418,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 @@ -321,12 +448,17 @@ jobs: else # Fallback: Claude sometimes includes the verdict in the comment body # instead of writing a separate file. Parse the most recent PR comment. + # Anchor the grep to line start/end — the prompt instructs Claude to + # emit the VERDICT line as the terminal line of the comment. An + # unanchored match will hit any "VERDICT: BLOCK" or "VERDICT: PASS" + # string in the review prose (e.g., when the review discusses the + # verdict contract itself, as PR #47's self-review demonstrated). echo "::warning::Verdict file not found — falling back to PR comment parsing." COMMENT=$(gh pr view "$PR_NUMBER" --json comments -q '.comments[-1].body' 2>/dev/null || echo "") - if echo "$COMMENT" | grep -q "VERDICT: BLOCK"; then + if echo "$COMMENT" | grep -qE '^VERDICT: BLOCK$'; then VERDICT="VERDICT: BLOCK" echo "Verdict source: PR comment (fallback)" - elif echo "$COMMENT" | grep -q "VERDICT: PASS"; then + elif echo "$COMMENT" | grep -qE '^VERDICT: PASS$'; then VERDICT="VERDICT: PASS" echo "Verdict source: PR comment (fallback)" else