ci(claude-review): an inline review after the head's summary fails the check, and the test drives that step

.github/scripts/claude-review-test.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
-# claude-review.yml's scope and diff steps against fixtures, and the JSON its
-# claude_args carries; run from the repo root.
+# claude-review.yml's scope, diff and verdict steps against fixtures, and the
+# JSON its claude_args carries; run from the repo root.
 set -euo pipefail
 
 fail() {
@@ -19,8 +19,8 @@ work="$(mktemp -d)"
 trap 'rm -rf "$work"' EXIT
 
 # Serves $PAGES/<kind>.<n>.json in page order, each through --jq as gh does,
-# and only page 1 without --paginate; anything else is a call the step must
-# not make.
+# and only page 1 without --paginate; anything else is a call no step may
+# make.
 gh() {
     local paginate=false path="" expr="" kind
     [[ "${1:-}" == api ]] || { echo "gh stub: unexpected: $*" >&2; return 1; }
@@ -336,3 +336,62 @@ for agent in ("correctness-reviewer", "design-reviewer", "verifier"):
 # shellcheck disable=SC2016 # the backticks are the prompt's own, not expansions
 [[ "$prompt" == *'an incomplete one ends with a `Read:` line, then the `- <id> <path>:<range>` line of each scope unit'* ]] ||
     fail "prompt: step 3 no longer asks for the \`Read:\` format VERDICTS parses"
+[[ "$(yq -r "$action | .with.classify_inline_comments" "$WORKFLOW_FILE")" == false ]] ||
+    fail "classify_inline_comments: buffering would post inline reviews after the summary VERDICT needs as the newest"
+
+# The step that sets the required check, run over a case's reviews at head 9.
+# A crash fails the check too, so a fail counts only with the step's ::error::.
+check_step='.jobs.claude-review.steps[] | select(.env.VERDICT != null)'
+check_script="$(yq -e -r "$check_step | .run" "$WORKFLOW_FILE")"
+check_jq="$(yq -e -r "$check_step | .env.VERDICT" "$WORKFLOW_FILE")"
+assert_check() {
+    local name="$1" want="$2" got=pass
+    PAGES="$PAGES" GITHUB_REPOSITORY=org/repo PR=7 HEAD_SHA="$(sha 9)" VERDICT="$check_jq" \
+        bash --noprofile --norc -eo pipefail -c "$check_script" >"$work/$name.out" 2>"$work/$name.err" || got=fail
+    [[ "$got" == pass ]] || grep -q '^::error::' "$work/$name.out" || fail "check $name: the step crashed: $(cat "$work/$name.err")"
+    [[ "$got" == "$want" ]] || fail "check $name: the check would $got, want $want"
+}
+clean=$'Blocking: 0\n\n**Verdict:** no findings'
+
+new_case check-clean
+review "$bot" 9 "$clean" | page reviews 1
+assert_check check-clean pass
+
+new_case check-none
+assert_check check-none fail
+
+new_case check-blocking
+review "$bot" 9 'Blocking: 1' | page reviews 1
+assert_check check-blocking fail
+
+new_case check-incomplete
+review "$bot" 9 'Blocking: incomplete' | page reviews 1
+assert_check check-incomplete fail
+
+# An inline comment is a review with an empty body, so alone it is no verdict,
+# and after a summary it is a finding of a run that stopped before its own.
+new_case check-inline-only
+review "$bot" 9 '' | page reviews 1
+assert_check check-inline-only fail
+new_case check-inline-after
+{ review "$bot" 9 "$clean"; review "$bot" 9 '' 10; } | page reviews 1
+assert_check check-inline-after fail
+
+new_case check-other-user
+review someone 9 "$clean" | page reviews 1
+assert_check check-other-user fail
+
+# A head whose own review ended without its summary must not pass on the
+# previous head's.
+new_case check-older-head
+review "$bot" 8 "$clean" | page reviews 1
+assert_check check-older-head fail
+
+# A re-review at the head decides, its page past the first included.
+new_case check-rereview-clean
+review "$bot" 9 'Blocking: 1' | page reviews 1
+review "$bot" 9 "$clean" 10 | page reviews 2
+assert_check check-rereview-clean pass
+new_case check-rereview-blocks
+{ review "$bot" 9 "$clean"; review "$bot" 9 'Blocking: 1' 10; } | page reviews 1
+assert_check check-rereview-blocks fail

.github/workflows/claude-review.yml

@@ -194,6 +194,9 @@ jobs:
           # The job's token, so neither the Claude GitHub App nor id-token.
           github_token: ${{ github.token }}
           # Buffering screens out probe comments, and only this prompt posts.
+          # It also holds each inline comment not sent with confirmed=true until
+          # the session ends (the action's action.yml), past the summary the
+          # verdict step needs as the newest review.
           classify_inline_comments: false
           prompt: |
             Review pull request #${{ github.event.pull_request.number }} of ${{ github.repository }}: base ${{ github.event.pull_request.base.sha }}, head ${{ github.event.pull_request.head.sha }}, checked out here.
@@ -250,11 +253,11 @@ jobs:
           HEAD_SHA: ${{ github.event.pull_request.head.sha }}
           PR: ${{ github.event.pull_request.number }}
           VERDICT: >-
-            .[] | select(.user.login == "github-actions[bot]" and .commit_id == env.HEAD_SHA and .body != "")
+            .[] | select(.user.login == "github-actions[bot]" and .commit_id == env.HEAD_SHA)
             | .body | split("\n")[0]
         run: |
           set -euo pipefail
-          verdict="$(gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR/reviews" --jq "$VERDICT" | tail -n 1)"
+          verdict="$(gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR/reviews?per_page=100" --jq "$VERDICT" | tail -n 1)"
           if [[ "$verdict" != "Blocking: 0" ]]; then
             echo "::error::the review at ${HEAD_SHA::7} reports a blocking finding or no verdict: fix it, or disposition and resolve its thread, then re-run"
             exit 1