[CXH-2780] Let CI submit an approval, not only a block (verdict_mode)

.github/actions/pr-review/action.yml

@@ -22,6 +22,20 @@ inputs:
     description: "Optional override for the review summary heading: a single-line Markdown heading of the form '### ...:'. Empty uses the review_prompt profile's heading."
     required: false
     default: ""
+  verdict_mode:
+    description: >
+      How CI turns the review summary into a formal PR review. "baseline"
+      (default) reads the blocking count: N > 0 submits REQUEST_CHANGES,
+      N == 0 submits COMMENT, and it never approves — so its REQUEST_CHANGES
+      can only be cleared by a human dismissal. "judge" additionally asks the
+      reviewer for an explicit APPROVE / REQUEST CHANGES / DEFER verdict row
+      and submits APPROVE / REQUEST_CHANGES / COMMENT from it, which lets a
+      clean re-run clear its own earlier block — granted only when the base
+      branch's rulesets confirm 0 required approving reviews, and downgraded
+      to "baseline" otherwise, including when that cannot be read. "none"
+      publishes the report and submits no formal review at all.
+    required: false
+    default: baseline
 
 runs:
   using: composite
@@ -30,8 +44,11 @@ runs:
     id: review-config
     shell: bash
     env:
+      GH_TOKEN: ${{ inputs.github_token }}
       REVIEW_PROMPT: ${{ inputs.review_prompt }}
       SUMMARY_MARKER: ${{ inputs.summary_marker }}
+      VERDICT_MODE: ${{ inputs.verdict_mode }}
+      BASE_REF: ${{ github.event.pull_request.base.ref }}
     run: |
       # Captured before any review work: the publication gates require the
       # working summary comment to have been created/updated at or after this
@@ -76,6 +93,93 @@ runs:
         summary_heading="${SUMMARY_MARKER}"
       fi
       echo "summary_heading=${summary_heading}" >> "${GITHUB_OUTPUT}"
+      # Validated here as well as in publish-review-report.py: a typo should
+      # fail before the review burns a model run, and the publish step must
+      # not depend on this step having screened its input.
+      #
+      # Empty normalizes to baseline rather than failing, and must: a
+      # composite action's `default:` does NOT apply to an input that is
+      # explicitly passed as an empty string, so a caller forwarding its own
+      # optional input (`verdict_mode: ${{ inputs.verdict_mode }}`) sends "".
+      # Failing there would break the compatibility path verdict_mode() in
+      # publish-review-report.py documents and the review_prompt case above
+      # already honours.
+      case "${VERDICT_MODE}" in
+        ""|"baseline")
+          VERDICT_MODE="baseline"
+          ;;
+        "judge"|"none") ;;
+        *)
+          echo "::error::verdict_mode must be 'judge', 'baseline', or 'none'"
+          exit 1
+          ;;
+      esac
+      # Judge mode is only safe where a bot APPROVE cannot satisfy a human
+      # review requirement, so that precondition is CHECKED rather than
+      # documented: "connector repos require 0 approving reviews" is a fact
+      # about today, and a repo that quietly starts requiring one would hand
+      # the bot a vote nobody decided to give it.
+      #
+      # Anything other than a confirmed zero downgrades to baseline — an
+      # unreadable answer included. The classic
+      # /branches/{branch}/protection endpoint is NOT usable here: it answers
+      # 403 "Resource not accessible by integration" for GITHUB_TOKEN. The
+      # rulesets endpoint below does answer it.
+      #
+      # Every assignment below is guarded (`if`, or `|| true`): this step runs
+      # under `bash -eo pipefail`, where a bare `var=$(failing-cmd)` aborts the
+      # step instead of falling through to the unknown case.
+      if [ "${VERDICT_MODE}" = "judge" ]; then
+        approvals=""
+        if [ -n "${BASE_REF}" ] \
+          && rules=$(gh api "repos/${GITHUB_REPOSITORY}/rules/branches/${BASE_REF}" 2>/dev/null); then
+          # The response carries one entry per applicable ruleset, org- and
+          # repo-level mixed, so several pull_request rules can apply at once
+          # and the first one is not the governing one. Take the strictest.
+          approvals=$(printf '%s' "${rules}" | jq -r '
+            [.[] | select(.type == "pull_request") | .parameters.required_approving_review_count // 0] as $counts
+            | if ($counts | length) == 0 then "none" else ($counts | max) end
+          ' 2>/dev/null || true)
+          if [ "${approvals}" = "none" ]; then
+            # NO ruleset governs pull requests on this branch, yet something
+            # may still be protecting it, and a classic branch protection can
+            # require approvals where this token cannot look. Protection with
+            # no ruleset rule to explain it is unexplained, so it is unknown
+            # and loses judge; an unprotected branch requires nothing and is a
+            # real zero.
+            #
+            # This deliberately does NOT extend to a ruleset rule that says 0.
+            # Classic protection could in principle sit alongside one and ask
+            # for more, and that case is genuinely not covered (see "The
+            # limit of the check" in docs/pr-review.md — the check reads
+            # rulesets, and cannot see classic protection at all). But a
+            # governing pull_request rule IS an explanation for
+            # the branch being protected, where the "none" case has none. And
+            # the alternative is not a stricter check, it is no feature:
+            # every connector repo reads exactly 0-from-a-ruleset plus
+            # protected=true, so treating that as unknown would downgrade all
+            # of them and judge would never run anywhere.
+            if [ "$(gh api "repos/${GITHUB_REPOSITORY}/branches/${BASE_REF}" --jq '.protected' 2>/dev/null || true)" = "false" ]; then
+              approvals="0"
+            else
+              approvals=""
+            fi
+          fi
+        fi
+        case "${approvals}" in
+          "0")
+            ;;
+          ""|*[!0-9]*)
+            echo "::warning::Could not read the approving-review requirement for '${BASE_REF}'; running verdict_mode=baseline instead of judge, so this review cannot submit an approval."
+            VERDICT_MODE="baseline"
+            ;;
+          *)
+            echo "::notice::'${BASE_REF}' requires ${approvals} approving review(s); running verdict_mode=baseline instead of judge, so a bot approval cannot satisfy that requirement."
+            VERDICT_MODE="baseline"
+            ;;
+        esac
+      fi
+      echo "verdict_mode=${VERDICT_MODE}" >> "${GITHUB_OUTPUT}"
   - name: Fetch PR context
     shell: bash
     env:
@@ -101,12 +205,20 @@ runs:
     shell: bash
     env:
       BUILT_IN_MIXINS: ${{ steps.review-config.outputs.built_in_mixins }}
+      VERDICT_MODE: ${{ steps.review-config.outputs.verdict_mode }}
     run: |
       DELIM="PROMPT_EOF_$(openssl rand -hex 8)"
       PROMPT_DIR="${GITHUB_ACTION_PATH}/prompts"
       {
         echo "REVIEW_PROMPT<<${DELIM}"
         cat "${PROMPT_DIR}/base-pr-review.md"
+        # Judge mode needs the verdict row the base template does not ask for.
+        # It goes in before the domain mixins and repo-local criteria so those
+        # keep the last word on review content, as they do today.
+        if [ "${VERDICT_MODE}" = "judge" ]; then
+          echo
+          cat "${PROMPT_DIR}/verdict-judge.md"
+        fi
         if [ -n "${BUILT_IN_MIXINS}" ]; then
           IFS=',' read -ra MIXINS <<< "${BUILT_IN_MIXINS}"
           for mixin in "${MIXINS[@]}"; do
@@ -196,17 +308,25 @@ runs:
     #    and never output already consumed by a published report), refusing
     #    as obsolete when a later attempt already completed (actual attempt
     #    start times, never run-ID order),
-    # 2. validates the baseline count row and that the live PR head still
-    #    equals the reviewed checkout,
+    # 2. validates the canonical rows the run's verdict_mode requires — the
+    #    count row in every mode (including "none", where it is the only
+    #    proof the agent finished rather than being cut off), plus the
+    #    verdict row under "judge" — and that the live PR head still equals
+    #    the reviewed checkout,
     # 3. POSTs a NEW report comment (publication: pending) carrying a visible
     #    reviewed-commit link and CI-owned review-state metadata identifying
     #    the publication (workflow + marker + mode + run + attempt + head),
     # 4. submits the formal PR review with an explicit commit_id, linking
-    #    directly to the report — baseline mode only: request changes on
-    #    blocking findings, neutral comment otherwise, never approves,
+    #    directly to the report, with the event taken from verdict_mode:
+    #    "baseline" from the count (request changes on blocking findings,
+    #    neutral comment otherwise, never approves), "judge" from the
+    #    verdict row (APPROVE / REQUEST CHANGES / DEFER), "none" submits no
+    #    review at all, skipping this step and step 5 (but not step 6),
     # 5. transitions the report to publication: completed — a completed
     #    report is the report PLUS its formal result, so a report whose
-    #    review never landed stays pending and never becomes review state,
+    #    review never landed stays pending and never becomes review state
+    #    ("none" has no formal result to wait for, so its report is born
+    #    completed),
     # 6. only then collapses the superseded previous report(s) and the
     #    consumed working comment (bodies retained, linked to the new
     #    report).
@@ -221,6 +341,7 @@ runs:
       PR_NUMBER: ${{ inputs.pr_number }}
       SUMMARY_MARKER: ${{ steps.review-config.outputs.summary_heading }}
       REVIEW_RUN_STARTED_AT: ${{ steps.review-config.outputs.review_run_started_at }}
+      VERDICT_MODE: ${{ steps.review-config.outputs.verdict_mode }}
     run: python3 ${{ github.action_path }}/scripts/publish-review-report.py
   - name: Upload review context artifacts
     if: always()

.github/actions/pr-review/prompts/verdict-judge.md

@@ -0,0 +1,70 @@
+## Verdict Row (judge mode)
+
+This run is configured with `verdict_mode: judge`. CI submits the formal PR
+review from an explicit verdict you write, not from the blocking count alone.
+Everything the base prompt says still applies — including that you must NOT run
+`gh pr review` yourself — with one addition and one replacement.
+
+**The addition.** Your summary carries one more row, the canonical verdict row.
+Add it to the metadata rows directly under the heading, immediately after the
+`**Blocking Issues: ...**` row and before the `**Criteria:**` row. Everything
+else about the template is unchanged:
+
+```
+**Blocking Issues: N** | **Suggestions: M** | **Threads Resolved: R**
+**Verdict: APPROVE**
+**Criteria:** <as the base template describes>
+```
+
+Write exactly one of the three values below, spelled exactly as shown — bold,
+uppercase, nothing else on the line, and only once in the whole summary. CI
+rejects anything else and the run turns red, losing the entire review, so this
+row is as load-bearing as the count.
+
+| Verdict | Write it when | CI submits |
+| --- | --- | --- |
+| `**Verdict: APPROVE**` | You reviewed the entire diff and found nothing blocking. Requires `N == 0`. | `APPROVE` |
+| `**Verdict: REQUEST CHANGES**` | At least one blocking finding, or a non-defect reason the author must act on before merge. | `REQUEST_CHANGES` |
+| `**Verdict: DEFER**` | You cannot stand behind either of the above for the whole PR. | `COMMENT` at `N == 0`, `REQUEST_CHANGES` above |
+
+**The replacement.** The base prompt says CI derives the review from the count
+(`--request-changes` when N > 0, `--comment` when N == 0). In judge mode it
+derives it from the verdict row instead. The count still has to be accurate —
+it is what readers and the next run's audit work from — but it no longer selects
+the review event on its own.
+
+### Choosing between them
+
+`APPROVE` is a statement about the **entire pull request**, because that is what
+GitHub records: an approval satisfies the PR's review requirement no matter how
+much of the diff you actually read. Only approve when you are willing to make
+that claim.
+
+`DEFER` exists for the gap that creates: you found nothing blocking but cannot
+vouch for the whole change. Use it when part of the diff was outside what you
+could meaningfully review — generated or vendored files, a binary, a domain the
+criteria do not cover — or when you declared a coverage gap in the review
+summary. Do NOT defer merely because no repo-local criteria file exists: a
+`Criteria status:` line reporting that none was found is the normal state for
+most repos and says nothing about your coverage of the diff. Defer on a
+criteria line that reports the file was unavailable or invalid, which means
+rules you should have applied are missing.
+
+Be deliberate about reaching for it. At `N == 0`, `DEFER` submits a neutral
+comment, and a comment never clears an earlier `CHANGES_REQUESTED` — so
+deferring by habit re-creates exactly the stuck state judge mode exists to fix.
+
+`DEFER` is a statement about your coverage, not about severity, so it cannot
+be used to unblock: if your count row is above zero, CI submits
+`REQUEST_CHANGES` regardless. You do not get to list confirmed blockers and
+then decline to block.
+
+`REQUEST CHANGES` does not require `N > 0`. A PR can need author action for a
+reason that is not a defect in the diff — a missing migration note, an
+unanswered prior thread. Say so in the summary when you use it that way.
+
+An `APPROVE` over a positive blocking count is a contradiction, not a verdict:
+your own summary calls something blocking while the verdict clears it. CI
+refuses to pick a side and fails the run. If a prior finding is genuinely no
+longer blocking, resolve it in the audit and lower the count; if it still
+blocks, do not approve.