fix(ci): read the reviewed label live inside the job, not from the frozen event payload (BACKLOG #1417)

.github/workflows/review-gate.yml

@@ -34,6 +34,25 @@ name: review gate
 # are unread again, even if the pull request was marked an hour ago. That is the one thing the
 # workflow writes, and it only ever writes in the blocking direction.
 #
+# THE LABEL IS READ LIVE, FROM INSIDE THE JOB (BACKLOG #1417). This step used to read
+# `join(github.event.pull_request.labels.*.name, ',')` -- the label set as it stood when the WEBHOOK
+# FIRED. A queued run therefore reported the truth of its own CREATION time, while branch protection
+# picks the newest check-run by EXECUTION time. Two clocks, and when they disagree the older one is
+# what gets reported. Measured end to end on PR 724 on 2026-09-01: a run created 13:24 executed at
+# 13:43:46, twenty-odd seconds after the label was removed, and reported SUCCESS from its 13:24
+# payload; that success stood for ten minutes on a pull request carrying no `reviewed` label. Nothing
+# but `strict = true` -- an unrelated control -- kept it from merging.
+#
+# READING THE STATUS CONTEXT INSTEAD IS NOT A FIX, and was adopted by two sessions before being
+# refuted: the context inherits the same staleness through the same snapshot. A lower-level proxy of
+# a snapshotted value is still the snapshot. The only live reading is an API call made while the job
+# is running, which is what the label step now does.
+#
+# AND THE LABEL HAS TO POST-DATE THE HEAD IT SITS ON. A live read answers "is the label there now";
+# it does not answer "did it ever cover THESE commits". Label a pull request, push to it, and read
+# the state before the `synchronize` strip lands, and the label is live-present on a head nobody has
+# read. So the step makes a second comparison, and refuses a label older than the head.
+#
 # NO CONCURRENCY BLOCK, DELIBERATELY. This job's name is intended to become a required status
 # context, and a cancelled required check can never go green. backlog-hygiene.yml carried
 # `cancel-in-progress` on a key that collapsed to one group on `merge_group`, and entries cancelled
@@ -54,10 +73,32 @@ on:
     # GREEN context behind. Both are pinned by
     # tests/test_merge_gate_controls.py::test_the_review_gate_reruns_when_a_reviewer_adds_the_label,
     # because dropping `labeled` reddened nothing before that test existed.
+    #
+    # `unlabeled` COVERS LESS THAN IT LOOKS LIKE IT COVERS, and this comment used to imply otherwise.
+    # It catches a HUMAN withdrawing the label. It does NOT catch the strip step below removing it:
+    # GitHub does not dispatch a run from an event raised by the repository's own `GITHUB_TOKEN`, and
+    # that step runs `gh` under `github.token`. Measured on PR 765: exactly TWO review-gate runs on
+    # that head, and none for the bot's 20:45:14Z removal. Nothing is wedged by that, because the
+    # `synchronize` that caused the strip dispatches its own run and that run ends red -- but the
+    # coverage comes from `synchronize`, not from `unlabeled`, and a reader who believed otherwise
+    # would think deleting the `synchronize` arm was safe.
     types: [opened, reopened, ready_for_review, synchronize, labeled, unlabeled]
   merge_group:
 
 permissions:
+  # `pull-requests: write` is the label REMOVAL on `synchronize`, and the only write here.
+  #
+  # THE TWO READS ARE WHAT THE LIVE CHECK COSTS. Declaring any permission sets every undeclared one
+  # to `none`, so the label step's API calls would 403 without them: `contents: read` for the head
+  # commit's date, `issues: read` for the pull request's label events -- label history lives on the
+  # issue side of a pull request, not the pulls side. A 403 would fail the gate CLOSED, which is the
+  # right direction but would wedge every pull request, so both are named rather than assumed.
+  #
+  # THAT "FAILS CLOSED" HOLDS BECAUSE THE STEP DECLARES `shell: bash`, and it did not before. Under
+  # the default shell one of the two reads swallowed its own 403. The mechanism is recorded once, at
+  # that declaration, rather than restated here.
+  contents: read
+  issues: read
   pull-requests: write
 
 jobs:
@@ -102,21 +143,110 @@ jobs:
       - name: Require the reviewed label
         if: github.event_name == 'pull_request'
         env:
-          LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }}
+          GH_TOKEN: ${{ github.token }}
+          GH_REPO: ${{ github.repository }}
+          NUMBER: ${{ github.event.pull_request.number }}
+          # THESE THREE ARE FACTS ABOUT THE EVENT, NOT STATE READ THROUGH IT, and the distinction is
+          # the whole of #1417. `ACTION` and `LABEL_NAME` describe what happened; an event cannot go
+          # stale about itself. `HEAD_SHA` is the commit this job's check-run ATTACHES to, so it is
+          # the subject of the verdict rather than a mutable property of it. Everything the verdict
+          # DEPENDS on -- whether the label is on the pull request, and when it was applied -- is
+          # fetched below, at execution time, and never taken from the payload.
           ACTION: ${{ github.event.action }}
+          LABEL_NAME: ${{ github.event.label.name }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        # `shell: bash` IS LOAD-BEARING HERE, NOT TIDINESS. A `run:` block with no `shell:` key runs
+        # under `bash -e {0}`; naming bash gets `bash --noprofile --norc -e -o pipefail {0}`. The
+        # option that goes missing is `pipefail`, and one of this step's two API reads is a PIPELINE
+        # (`gh api ... | sort | tail -n 1`). A pipeline's status is its LAST command's, so `tail`
+        # exits 0 over a `gh` that 403'd, `-e` never sees the failure, and the step carries on with an
+        # empty result. Measured 2026-09-03 under both flag sets: without `pipefail` the script ran
+        # past a failing history read; with it the script ends at the assignment.
+        #
+        # It also settles the bashism. The comparison at the bottom is `[[ ]]`, which the `sh -e {0}`
+        # fallback -- what a runner without bash gets -- would reject outright.
+        #
+        # THE TESTS IN tests/test_merge_gate_controls.py RUN THIS STEP UNDER `-e -o pipefail`, and
+        # before this line that was a friendlier shell than CI used, so they were measuring a step
+        # Actions never executed. The declaration is what makes those runs faithful.
+        shell: bash
         run: |
-          # On `synchronize` the label was just removed above, so the event payload is stale by one
-          # step. Treat that action as unreviewed by definition rather than reading a value that is
-          # already wrong -- reading the payload here would pass a pull request that was just
-          # invalidated, which is the exact failure this step exists to prevent.
+          # A NEW COMMIT IS UNREAD BY DEFINITION, and this arm is kept as a belt over the live read
+          # rather than as the fix for staleness -- it used to be the fix, applied to the one action
+          # anyone had an instance of. Two reasons it stays. The step above has just removed the
+          # label, and the labels endpoint is not guaranteed to have caught up by the time this step
+          # reads it. And the head-date comparison at the bottom rests on a commit's COMMITTER DATE,
+          # which is client-supplied: a commit authored before the label and pushed after it would
+          # clear that comparison while being genuinely unread.
           if [ "$ACTION" = "synchronize" ]; then
             echo "::error::New commits have not been read. Re-review, then: gh pr edit <N> --add-label reviewed"
             exit 1
           fi
+
+          # LIVE. Not `github.event.pull_request.labels`, which is the snapshot #1417 records.
+          LABELS="$(gh pr view "$NUMBER" --json labels --jq '[.labels[].name] | join(",")')"
+          echo "labels, read at execution time: ${LABELS:-(none)}"
+
+          # EXACT ELEMENT, not substring: `reviewed-by-bot` and `not-reviewed` are not a review.
           case ",$LABELS," in
-            *,reviewed,*)
-              echo "reviewed label present. Gate satisfied." ;;
+            *,reviewed,*) ;;
             *)
               echo "::error::Not yet read by a reviewer. When you have read it: gh pr edit <N> --add-label reviewed"
               exit 1 ;;
           esac
+
+          # THE SECOND COMPARISON: the label must post-date the head it sits on.
+          #
+          # THE CHEAP ANSWER FIRST, and it is also the better-founded one. If a reviewer applied
+          # `reviewed` in the very event that started this run, the label was applied while this head
+          # already existed -- that is a GitHub clock, needs no API call, and cannot lag behind a
+          # write made moments earlier.
+          #
+          # SAY WHAT THIS ARM IS: a PASS that returns before either API read. So the head-date
+          # machinery does NOT "only ever add a refusal", which is how the first draft of this change
+          # described it. The shortcut is sound -- a `labeled` event necessarily post-dates the head
+          # named in its own payload -- and it stays. But it is a new way to exit 0, and a control
+          # described as refusal-only while it carries a pass path is a compensating control resting
+          # on a false premise.
+          if [ "$ACTION" = "labeled" ] && [ "$LABEL_NAME" = "reviewed" ]; then
+            echo "the reviewed label was applied by the event that started this run. Gate satisfied."
+            exit 0
+          fi
+
+          # Otherwise ask when it was last applied. `per_page=100` AND `--paginate`: every `gh api`
+          # list route defaults to 30, and a label history read 30 at a time answers a question about
+          # a population it cannot see. `sort | tail -n 1` rather than trusting the page order.
+          LAST_ADD="$(gh api "repos/$GH_REPO/issues/$NUMBER/events?per_page=100" --paginate \
+            --jq '.[] | select(.event == "labeled" and .label.name == "reviewed") | .created_at' \
+            | sort | tail -n 1)"
+          HEAD_AT="$(gh api "repos/$GH_REPO/commits/$HEAD_SHA" --jq '.commit.committer.date')"
+          echo "reviewed last applied ${LAST_ADD:-(no recorded event)}; head $HEAD_SHA dated $HEAD_AT"
+
+          if [ -z "$LAST_ADD" ]; then
+            echo "::error::The reviewed label is on this pull request but no labeled event records when it was applied, so it cannot be shown to cover this head. Re-apply it: gh pr edit <N> --remove-label reviewed && gh pr edit <N> --add-label reviewed"
+            exit 1
+          fi
+
+          # BOTH TIMESTAMPS ARE GUARDED, and the first draft guarded only the one above. An empty
+          # `$HEAD_AT` FAILS OPEN: every string sorts at or after the empty string, so
+          # `[[ "$LAST_ADD" < "" ]]` is FALSE and the comparison below falls straight through to
+          # `Gate satisfied` on a head this step could not date at all.
+          #
+          # AND IT IS REACHABLE, which is the part that reads as impossible until you check what `gh`
+          # actually prints. `gh api --jq` on a path that does not resolve prints an EMPTY LINE and
+          # exits 0 -- not `null`, and not non-zero -- so neither `-e` nor `pipefail` sees anything
+          # wrong. Measured 2026-09-03 against this repository: one byte of output, a newline, exit 0.
+          # A `null` would have sorted AFTER any `2026-...` timestamp and refused, so the safe-looking
+          # failure mode is the one that never happens.
+          if [ -z "$HEAD_AT" ]; then
+            echo "::error::The head commit $HEAD_SHA could not be dated, so the reviewed label cannot be shown to cover it. Re-run this check; if it persists, re-apply the label: gh pr edit <N> --remove-label reviewed && gh pr edit <N> --add-label reviewed"
+            exit 1
+          fi
+
+          # Both timestamps are fixed-width ISO-8601 in UTC, so a string compare is a time compare.
+          if [[ "$LAST_ADD" < "$HEAD_AT" ]]; then
+            echo "::error::The reviewed label was applied at $LAST_ADD, before this head was dated $HEAD_AT, so it never covered these commits. Re-review, then: gh pr edit <N> --remove-label reviewed && gh pr edit <N> --add-label reviewed"
+            exit 1
+          fi
+
+          echo "reviewed label present at execution time and applied after this head. Gate satisfied."

CLAUDE.md

@@ -411,7 +411,10 @@ went red, and no workflow reads that label back. So the Console finds both by as
   queued synchronize run has not stripped the label yet, so the label can be present and invalid at
   the same time. When no run is newer than the label event at all, the state is unknown: the Console
   keeps polling, and nobody inherits the last verdict. A Builder never evaluates this, because its
-  process exits before any run reports. Filed as BACKLOG #1417, open in PR 731 and not yet on main.
+  process exits before any run reports. BACKLOG #1417 was the gate's own half of the same staleness
+  and it is fixed: the gate reads the label live inside the job, so a SUCCESS is no longer a
+  snapshot. The join above still binds, because a run that has not reported yet is still not a
+  verdict.
 - Never write the required-context count into a document. `.github/required-contexts.txt` is a
   checked-in claim that can lag the server, so read branch protection for the live set. When the set
   moves, move that file and the pinned count in `tests/test_required_contexts.py` in the same PR, or