CI fails when the external reviewer did not actually review

.github/workflows/ci.yml

@@ -585,8 +585,67 @@ jobs:
   # on an empty list. Both of those are the same point, which is the point of the
   # whole workflow: a check nobody ran reads exactly like a check that passed,
   # and a gate that covers nothing is the purest form of that.
+  # The external reviewer is a check like any other, and it failed the way this
+  # gate's own comment warns about: silently. Sourcery answers every pull request
+  # with a *review*, and a review is what a reader looking for one finds -- so
+  #
+  #     Sorry @blooop, you have reached your weekly rate limit of 500000 diff
+  #     characters.
+  #
+  # arrives in the same shape as a review that found nothing, and reads as one to
+  # everything downstream. Twenty-six consecutive pull requests merged behind that
+  # sentence, among them the largest changes in the repo, and nothing said so.
+  #
+  # The classification is in `scripts/external_review_verdict.sh` rather than in
+  # this block, for the reason the public-api script is: a `case` statement that
+  # exists only inside a `run:` can be tested only by copying it, and the copy is
+  # what goes stale. What is here is the fetch, the polling and the label.
+  external-review:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: read
+    steps:
+      - uses: actions/checkout@v4
+      - name: An external reviewer that refused is not an external reviewer
+        env:
+          GH_TOKEN: ${{ github.token }}
+          EVENT: ${{ github.event_name }}
+          PR: ${{ github.event.pull_request.number }}
+          REPO: ${{ github.repository }}
+          OVERRIDE_LABEL: no-external-review
+        run: |
+          set -euo pipefail
+
+          if [ "$EVENT" != pull_request ]; then
+            echo "not a pull request, so there is nothing to have been reviewed"
+            exit 0
+          fi
+
+          if gh pr view "$PR" --repo "$REPO" --json labels --jq '.labels[].name' \
+              | grep -qxF "$OVERRIDE_LABEL"; then
+            echo "::notice::'$OVERRIDE_LABEL' is set, so this pull request merges"\
+                 "without an external review. That is the label's whole purpose;"\
+                 "it is recorded here so the choice is visible afterwards."
+            exit 0
+          fi
+
+          # Sourcery posts within a minute of the pull request opening, well
+          # inside the shortest job in this workflow -- but "well inside" is not
+          # "before", and a race lost here fails a pull request for the
+          # reviewer's latency rather than for anything about the change. So
+          # poll, and let the remaining ~2 minutes of CI absorb the wait.
+          reviews=
+          for _ in $(seq 1 12); do
+            reviews=$(gh api "repos/$REPO/pulls/$PR/reviews" \
+              --jq '.[] | select(.user.login == "sourcery-ai[bot]") | .body' || true)
+            [ -n "$reviews" ] && break
+            sleep 10
+          done
+
+          printf '%s' "$reviews" | ./scripts/external_review_verdict.sh
+
   gate:
-    needs: [ci, e2e, rust, rust-coverage, public-api, packaging]
+    needs: [ci, e2e, rust, rust-coverage, public-api, packaging, external-review]
     if: always()
     runs-on: ubuntu-latest
     steps:

CHANGELOG.md

@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- **CI fails when the external reviewer did not actually review.** Sourcery
+  answers a quota refusal *as a review*, so `Sorry @blooop, you have reached your
+  weekly rate limit…` arrives in the same shape as a review that found nothing.
+  Twenty-six consecutive pull requests merged behind that sentence — the largest
+  changes in this repo among them — and nothing anywhere said so. The new
+  `external-review` job, inside `gate`'s `needs`, refuses a quota refusal, a
+  refusal on diff size (which fires on exactly the changes least safe to merge
+  unread), and no review at all. The way past it is the `no-external-review`
+  label, so skipping external review is a decision on a named pull request rather
+  than a default that quietly stopped applying.
+
 ## [0.13.0] - 2026-08-24
 
 ### Changed

README.md

@@ -2362,6 +2362,34 @@ means adding it to `gate`'s `needs`, in the same pull request, where it can be
 seen. It reaches only as far as its own workflow file, so the `prek` lint job is
 not behind it and has to be required alongside it.
 
+One of `gate`'s jobs is not a test. `external-review` reads the pull request's
+reviews and fails unless the external reviewer actually reviewed the code. It is
+there because that reviewer failed silently once and nobody noticed for a day and
+a half: Sourcery answers a quota refusal *as a review*, so
+
+```
+Sorry @blooop, you have reached your weekly rate limit of 500000 diff characters.
+```
+
+arrives in the same shape as a review that found nothing and reads as one to
+everything downstream. Twenty-six consecutive pull requests merged behind that
+sentence, among them the largest changes in the repo. The other refusal is worse
+targeted still — a per-pull-request diff cap, which fires on exactly the changes
+least safe to merge unread.
+
+So the job refuses three things: a refusal on quota, a refusal on size, and no
+review at all after two minutes of polling. It is a failure rather than a warning
+because a warning is precisely what already did not work, and the way past it is
+the `no-external-review` label — a decision recorded on a named pull request,
+rather than a default that quietly stops applying. Labelling a change is fine;
+not knowing the reviewer stopped reading is not.
+
+The classification itself is `scripts/external_review_verdict.sh`, which the
+workflow calls and `test/test_external_review_guard.py` executes, for the reason
+the public-API script is a script: a `case` statement inside a `run:` block can
+be tested only by copying it, and the copy is the half that goes stale. Its tests
+run against the refusal bodies those twenty-six pull requests actually received.
+
 Running it yourself is a different proposition. This repo's devcontainer carries
 a Docker daemon of its own, through the `docker-in-docker` feature, and pins the
 same devpod a host installs, so `pixi run test-e2e` from inside it builds its

scripts/external_review_verdict.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# The verdict on whether an external reviewer actually reviewed a pull request.
+#
+# Separate from the workflow that calls it so that the classification is
+# executable outside CI, which is what `test/test_external_review_guard.py`
+# executes. A `case` statement living only inside a `run:` block is testable
+# only by copying it, and a copied guard is a guard that drifts.
+#
+# Reads the concatenated review bodies on stdin. Exit 0 means an external
+# reviewer answered about the code; exit 1 means it did not, and the reason is
+# on stdout as a GitHub workflow-command error so the annotation appears on the
+# pull request rather than only in a log nobody opens.
+set -euo pipefail
+
+OVERRIDE=${OVERRIDE_LABEL:-no-external-review}
+reviews=$(cat)
+
+if [ -z "${reviews//[[:space:]]/}" ]; then
+  echo "::error::no external review. Either the reviewer is not installed on"\
+       "this repository any more, or it is not reaching this pull request."\
+       "Label '$OVERRIDE' to merge anyway."
+  exit 1
+fi
+
+# Both refusals are quota refusals, and neither says anything about the code.
+# They are matched on the stable half of each sentence: the numbers move when
+# the plan does, and `@blooop` is not the only account that can open a pull
+# request here.
+case $reviews in
+  *"you have reached your weekly rate limit"*)
+    echo "::error::the external reviewer is out of weekly quota and reviewed"\
+         "nothing. It posts that refusal *as a review*, which is why this is"\
+         "not otherwise visible. Wait for the quota, or label '$OVERRIDE' to"\
+         "merge unreviewed."
+    exit 1
+    ;;
+  *"larger than the review limit"*)
+    echo "::error::this pull request is over the external reviewer's per-diff"\
+         "cap, so it went unreviewed -- on the size of change that least"\
+         "tolerates that. Split it, or label '$OVERRIDE' to merge unreviewed."
+    exit 1
+    ;;
+esac
+
+echo "the external reviewer ran and answered about the code"

test/test_external_review_guard.py

@@ -0,0 +1,119 @@
+"""The guard that notices when the external reviewer stopped reviewing.
+
+Between 2026-08-22 and 2026-08-24, twenty-six consecutive pull requests merged
+with no external review. Sourcery had run out of weekly quota and was answering
+
+    Sorry @blooop, you have reached your weekly rate limit of 500000 diff
+    characters.
+
+-- posted *as a review*. So `gh pr view --json reviews` showed a review present,
+the merge path was satisfied, and the outage was invisible for a day and a half.
+The largest changes in the repo went through in that window, including a
+breaking CLI grammar change and a 1,862-line feature that was withdrawn again
+twenty-two hours later.
+
+The strings below are the real refusals, copied from those pull requests, and
+the accepted body is the real "reviewed your changes and they look great"
+Sourcery posts when it found nothing -- which is the case the guard must NOT
+fail, and the case a naive "did the review say anything useful" check gets
+wrong.
+
+`gate`'s `needs` is asserted here for the same reason `test_bench_workflow.py`
+asserts the bench job is *absent* from it: the wiring is one edit away from
+being lost, and a guard nothing requires is not a guard.
+"""
+
+import subprocess
+from pathlib import Path
+
+ROOT = Path(__file__).parent.parent
+VERDICT = ROOT / "scripts" / "external_review_verdict.sh"
+CI = ROOT / ".github" / "workflows" / "ci.yml"
+JOB = "external-review"
+
+# Verbatim from PRs #343, #344, #345, #347, #348, #350-#368 and #370-#376.
+WEEKLY_QUOTA = (
+    "Sorry @blooop, you have reached your weekly rate limit of 500000 diff "
+    "characters.\n\nPlease try again later or [upgrade](https://app.sourcery.ai/"
+    "login?connection=github)"
+)
+# Verbatim from PR #369, the 1,631-line `--rm` grammar change.
+OVER_DIFF_CAP = (
+    "Sorry @blooop, your pull request is larger than the review limit of 150000 diff characters"
+)
+# Verbatim from PRs #330, #331, #333, #334, #337 and #342.
+REVIEWED_AND_CLEAN = (
+    "Hey - I've reviewed your changes and they look great!\n\n***\n\n"
+    "<details>\n<summary>Sourcery is free for open source</summary>"
+)
+REVIEWED_WITH_FINDINGS = "Hey - I've found 2 issues\n\n## Individual Comments"
+
+
+def verdict(bodies: str) -> subprocess.CompletedProcess:
+    return subprocess.run(
+        ["bash", str(VERDICT)],
+        input=bodies,
+        capture_output=True,
+        text=True,
+        check=False,
+    )
+
+
+def test_the_weekly_quota_refusal_is_not_a_review():
+    result = verdict(WEEKLY_QUOTA)
+    assert result.returncode == 1, "the outage that started this ran for 26 PRs"
+    assert "out of weekly quota" in result.stdout
+
+
+def test_a_pull_request_over_the_diff_cap_is_not_reviewed():
+    # The worse of the two: it fires on size, so it exempts exactly the changes
+    # that least tolerate going unread. #369 was 1,631 lines of CLI grammar.
+    result = verdict(OVER_DIFF_CAP)
+    assert result.returncode == 1
+    assert "per-diff cap" in result.stdout
+
+
+def test_no_review_at_all_is_not_a_review():
+    for empty in ["", "\n", "   \n  "]:
+        result = verdict(empty)
+        assert result.returncode == 1, f"{empty!r} left the reviewer unaccounted for"
+        assert "no external review" in result.stdout
+
+
+def test_a_review_that_found_nothing_passes():
+    # The case that makes this guard usable rather than merely loud: most
+    # reviews find nothing, and failing those would get the job deleted.
+    result = verdict(REVIEWED_AND_CLEAN)
+    assert result.returncode == 0, result.stdout
+
+
+def test_a_review_that_found_something_passes():
+    result = verdict(REVIEWED_WITH_FINDINGS)
+    assert result.returncode == 0, result.stdout
+
+
+def test_a_refusal_among_several_reviews_still_fails():
+    # Bodies arrive concatenated. A re-review after a push can leave a real
+    # review and a later refusal side by side, and the refusal is the newer
+    # fact: the current head is what went unread.
+    result = verdict(REVIEWED_AND_CLEAN + "\n" + WEEKLY_QUOTA)
+    assert result.returncode == 1
+
+
+def test_the_gate_requires_the_job():
+    ci = CI.read_text(encoding="utf-8")
+    assert f"\n  {JOB}:\n" in ci, f"{JOB} is the job this file is about"
+    needs = next(line for line in ci.splitlines() if line.strip().startswith("needs: [ci,"))
+    assert JOB in needs, (
+        "a job outside `gate`'s needs is not required by the branch ruleset, "
+        "which is the same shape of nothing-gating-anything this guard exists "
+        "to catch"
+    )
+
+
+def test_the_workflow_runs_this_script_rather_than_its_own_copy():
+    ci = CI.read_text(encoding="utf-8")
+    assert "./scripts/external_review_verdict.sh" in ci, (
+        "the classification must be the one under test; a copy inside a `run:` "
+        "block is what goes stale"
+    )