fix(ci): read the reviewed label live so a queued synchronize cannot leave a false green (BACKLOG #1423) #822
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| name: review gate | |
| # A pull request cannot merge until a reviewer has marked it read. | |
| # | |
| # THE GAP THIS CLOSES. GitHub cannot start a session on this machine, and the live session channel | |
| # reads one config directory with no glob, so it stops at an account boundary. Builders run as | |
| # background sessions bound to different accounts; they open pull requests and cannot be reached. | |
| # Autofix covers the CI half by waking a LIVE session on a failed check, but it is a per-session | |
| # checkbox in a desktop window, so a background session can never have it. Nothing at all reported | |
| # "this is green and nobody has looked at it". | |
| # | |
| # WHY A LABEL AND NOT AN APPROVAL. Every session on this machine pushes as ONE GitHub identity: `gh` | |
| # auth lives at AppData\Roaming\GitHub CLI\hosts.yml, machine-wide, not per config root, and reports | |
| # `wshallwshall` from every account root. CODEOWNERS assigns every path, including `*`, to that same | |
| # account, and it authored 28 of the last 30 merged pull requests. GitHub does not let an author | |
| # approve their own pull request, so `required_approving_review_count: 1` would wedge every pull | |
| # request permanently. REVIEWER.md records the same conclusion at its section 5. | |
| # | |
| # So this is a PROCESS gate, not an IDENTITY gate. It enforces that a step happened. It does not | |
| # establish that an independent party looked, and it must not be described as if it does. The | |
| # identity version needs a second GitHub account whose token the reviewer session carries in | |
| # GH_TOKEN; this gate is compatible with that and does not have to be redone when it arrives. | |
| # | |
| # FAIL-CLOSED, WHICH IS WHY THE LABEL IS POSITIVE. An earlier draft used a `review-requested` label | |
| # written automatically and removed by a human, with the gate failing while it was present. That | |
| # fails OPEN: if the marking workflow never ran, the label is absent and the gate passes unreviewed | |
| # work. A gate whose safe state depends on another workflow having succeeded is not a gate. This one | |
| # requires the label to be PRESENT, so the default state of a brand-new pull request is blocked. | |
| # | |
| # HOW A REVIEWER SIGNALS. `gh pr edit <N> --add-label reviewed`. That is the whole protocol. Nothing | |
| # automated ever adds it. | |
| # | |
| # RE-REVIEW IS AUTOMATIC. On `synchronize` this workflow REMOVES the label: commits nobody has read | |
| # 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, NEVER FROM THE EVENT PAYLOAD (BACKLOG #1423, generalising #1417). A webhook | |
| # payload is a SNAPSHOT taken when the event fired, and a run can sit queued for minutes before it | |
| # executes. Measured on pull request 765, 2026-09-03, from the runs and timeline APIs: a reviewer | |
| # labelled at 20:43:19Z; run 33803911587 was created by that `labeled` event at 20:43:24Z; the earlier | |
| # `synchronize` run 33803677823, created 20:40:58Z and queued four minutes, finally executed and | |
| # removed the label at 20:45:14Z; then the `labeled` run's job started at 20:49:34Z and reported | |
| # SUCCESS at 20:49:37Z out of its own 20:43:24 payload -- six minutes stale, and four minutes after | |
| # the label it was reading had been deleted. The pull request carried this context GREEN with zero | |
| # labels until a human re-labelled at 20:52:55Z. So the step below asks the API what the labels ARE. | |
| # | |
| # AND NOTHING CORRECTS THAT GREEN, WHICH IS WHY READING LIVE IS THE FIX RATHER THAN AN IMPROVEMENT. | |
| # GitHub does not dispatch a workflow run from an event raised by the repository's own GITHUB_TOKEN -- | |
| # `workflow_dispatch` and `repository_dispatch` are the documented exceptions, and neither is a label | |
| # event. The removal above runs `gh` under `github.token`, so its `unlabeled` event emits no run. | |
| # Verified on the same head: the workflow-runs API reports exactly TWO review-gate runs on | |
| # ee2e7ec2423a73fd385d2afa27733ca050058cba, the two named above, and none for the 20:45:14Z removal. | |
| # | |
| # THE LIVE READ NARROWS THE WINDOW, IT DOES NOT CLOSE IT TO ZERO, and saying otherwise would be the | |
| # compensating control resting on a false premise. A `labeled` run can still read a label that a | |
| # `synchronize` run removes moments later, and its green then stands on a state that has just changed. | |
| # What the payload read cost was MINUTES of queue delay; what remains is the gap between this step's | |
| # API call and the job finishing, which is seconds. So the operational rule survives the fix rather | |
| # than expiring with it: for a BEHIND pull request, update-branch, WAIT for the resulting | |
| # `synchronize` review-gate run to COMPLETE, then label, then merge. Labelling while that run is still | |
| # queued is what produced the false green above, and it is still the shape to avoid. | |
| # | |
| # 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 | |
| # each other: measured 49 of 151 runs cancelled, 32.5 percent, zero failures, evicting them from the | |
| # queue. The job below runs in seconds, so there is nothing to supersede and no reason to risk it. | |
| # | |
| # ARMING IT IS A SEPARATE, OWNER-ONLY STEP. Merging this workflow changes nothing on its own. It | |
| # blocks a merge only once "a reviewer has read this" is added to the required status checks in | |
| # branch protection, and `.github/required-contexts.txt` is updated to match. | |
| on: | |
| pull_request: | |
| # THE LIST IS LOAD-BEARING AND FAILS SILENTLY. `labeled` is what lets a reviewer clear the check: | |
| # without it the label emits no run, the red check-run stands, and the only trigger left that | |
| # re-runs this job is `synchronize` -- whose first step REMOVES the label. The pull request then | |
| # cannot be merged by any action at all, and `strict = true` means it cannot even be brought up to | |
| # date past the block. `unlabeled` is the opposite half FOR A HUMAN WITHDRAWAL: without it, a | |
| # reviewer taking the label back would leave a GREEN context behind. It does NOT cover the removal | |
| # this workflow performs on `synchronize` -- that one is raised by GITHUB_TOKEN and emits no run at | |
| # all, measured above -- so `unlabeled` is not what protects against a stale green; the live read | |
| # in the last step is. 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. | |
| types: [opened, reopened, ready_for_review, synchronize, labeled, unlabeled] | |
| merge_group: | |
| permissions: | |
| pull-requests: write | |
| jobs: | |
| reviewed: | |
| # The name IS the required-context string. Keep it stable; changing it silently detaches the gate. | |
| name: a reviewer has read this | |
| runs-on: ubuntu-latest | |
| steps: | |
| # PASSES UNCONDITIONALLY IN THE QUEUE, and that is correct rather than lax. A merge_group event | |
| # carries no pull request, so there is no label to read. By the time an entry is in the queue | |
| # the gate has already been satisfied on the pull request itself; failing here would mean | |
| # nothing could ever merge. | |
| - name: Queue entries carry no pull request | |
| if: github.event_name == 'merge_group' | |
| # QUOTED. An unquoted YAML scalar cannot contain ": " -- it reads as a mapping. The sibling | |
| # lesson is in backlog-hygiene.yml, where an unquoted job name was silently truncated at " #". | |
| run: 'echo "merge_group carries no pull request. The gate was satisfied on the PR itself."' | |
| - name: A new commit makes it unread again | |
| if: github.event_name == 'pull_request' && github.event.action == 'synchronize' | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| GH_REPO: ${{ github.repository }} | |
| NUMBER: ${{ github.event.pull_request.number }} | |
| # The only write this workflow performs, and it only ever moves toward blocked. | |
| # | |
| # ITS EXIT CODE IS NOT DISCARDED, and that became load-bearing on 2026-08-31 when this job's | |
| # context entered branch protection. The line read `... 2>/dev/null || true` until then: a | |
| # REQUIRED job swallowing a failure, which is the shape tests/test_security_posture.py refuses | |
| # and caught here on the first run after the context was recorded. | |
| # | |
| # THERE WAS NOTHING TO SWALLOW. This step runs on `synchronize` only, and `synchronize` means | |
| # commits nobody has read, so the run ends RED regardless -- the next step returns that verdict | |
| # from `$ACTION` without consulting the API at all. Letting the removal's own failure through | |
| # changes no outcome; it only stops hiding the reason. | |
| # | |
| # A SILENTLY FAILED REMOVAL IS ITS OWN TRAP, which is why hiding it cost something. If the | |
| # label survives, re-adding it emits no `labeled` event, so nothing re-runs this job and the | |
| # pull request sits red with nothing a reviewer can do. Printed, it names itself. | |
| run: gh pr edit "$NUMBER" --remove-label reviewed | |
| - name: Require the reviewed label | |
| if: github.event_name == 'pull_request' | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| GH_REPO: ${{ github.repository }} | |
| NUMBER: ${{ github.event.pull_request.number }} | |
| ACTION: ${{ github.event.action }} | |
| # DIAGNOSTIC ONLY, AND NEVER THE VERDICT. This is the snapshot that produced the measured | |
| # false green, kept so the log can SAY it went stale instead of leaving a reader to infer it | |
| # from three API endpoints and two clocks. Nothing below branches on it. | |
| PAYLOAD_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} | |
| run: | | |
| # `synchronize` IS UNREAD BY DEFINITION, and it is answered BEFORE any read. The step above | |
| # has just removed the label, so a live read would usually agree -- but "usually" is what the | |
| # payload race already cost. Commits nobody has read are unread whatever any label says, so | |
| # this arm is decided by the action and consults nothing it could race against. | |
| 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 | |
| # READ LIVE, AND FAIL CLOSED ON A FAILED READ. A gate whose safe state depends on a network | |
| # call succeeding is not a gate, so the read is tested rather than trusted: `gh` absent, | |
| # unauthenticated, rate-limited, or answering with an error all land here and BLOCK. The | |
| # sibling shape to avoid is a checker that ignores a child's return code and reports clean. | |
| if ! LABELS=$(gh pr view "$NUMBER" --json labels --jq '[.labels[].name] | join(",")'); then | |
| echo "::error::Could not read the labels on pull request #$NUMBER, so this gate cannot say whether anybody has read it. Blocking rather than guessing -- re-run this job." | |
| exit 1 | |
| fi | |
| # EXACT-ELEMENT MATCH against the comma-joined list, not a substring: `reviewed-by-bot` and | |
| # `not-reviewed` are different labels, and one of them reads as the opposite. | |
| case ",$PAYLOAD_LABELS," in *,reviewed,*) WAS=yes ;; *) WAS=no ;; esac | |
| case ",$LABELS," in *,reviewed,*) NOW=yes ;; *) NOW=no ;; esac | |
| # Printed only when the two DISAGREE ON THIS LABEL, so ordering differences between the two | |
| # sources never make noise. A diagnostic that fires on every run is one nobody reads. | |
| if [ "$WAS" != "$NOW" ]; then | |
| echo "STALE PAYLOAD: the event payload recorded reviewed=$WAS, the live label set says reviewed=$NOW. The verdict below uses the LIVE set. Payload [$PAYLOAD_LABELS], live [$LABELS]." | |
| fi | |
| if [ "$NOW" = yes ]; then | |
| echo "reviewed label present in the live label set [$LABELS]. Gate satisfied." | |
| exit 0 | |
| fi | |
| echo "::error::Not yet read by a reviewer. When you have read it: gh pr edit <N> --add-label reviewed" | |
| exit 1 |