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

[wshallwshall]

What this fixes

a reviewer has read this is a required status check on main with enforce_admins: true, and it could report SUCCESS on a pull request carrying no reviewed label. The step read join(github.event.pull_request.labels.*.name, ',') — the label set as it stood when the webhook fired — while branch protection picks the newest check-run by execution time. Measured 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 with no label on the pull request.

No deployment axis (CLAUDE.md section 0). This reaches the repository's own merge control, not shipped code. The cost is an unreviewed change landing on main — not anything an adopter would run.

The item was not already answered

Checked before building, because three items dispatched this morning carried stale targets.

• git log --oneline -20 -- .github/workflows/review-gate.yml returns two commits, ab6e40f7e (the gate) and 43849fe87 (BACKLOG #1404, arming it). Neither touches the label read.
• No ADR under docs/adr/ names this gate or #1417.
• .github/workflows/review-gate.yml:105 still read LABELS from the payload and line 112 still hard-coded the synchronize special case at my base commit fd44b0f17. The defect was intact.

The change

The step reads the label live. gh pr view "$NUMBER" --json labels from inside the running job. Reading the status context instead is not a fix and was already adopted and refuted twice — it inherits the same staleness through the same snapshot.

And the label must post-date the head it sits on. A live read answers "is the label there now" and never "did anyone read THESE commits". So the step also compares the newest reviewed labeled event against the head commit's date and refuses a label older than the commit it sits on. That is the item's rule (5), the one the first draft shipped without. Where the run was started by a reviewer applying the label, the comparison is answered by the event itself and no history read happens: a GitHub clock, and the one timestamp that cannot lag a write made seconds earlier. The history read uses per_page=100 with --paginate, because every gh api list route defaults to 30.

Permissions. Declaring any permission sets the rest to none, so the two reads are named explicitly: contents: read for the head commit's date, issues: read for the label events. A 403 would fail closed, which is the right direction but would wedge every pull request.

Where I deliberately did not follow the brief, and why

The brief says "stop special-casing one action". I generalised the remedy to every action but kept the synchronize refusal, and it is worth reading the reason before treating this as ducking the item:

1. The removal step runs immediately before it, and the labels endpoint is not guaranteed to have caught up by the time the next step reads it.
2. The head-date comparison rests on a commit's committer date, which a client supplies. A commit authored before the label and pushed after it clears that comparison while being genuinely unread.

So the special case is no longer the fix; it is a belt over the live read. What the change removes is the other half — every action except synchronize reading the snapshot. If a reviewer disagrees, deleting those five lines is a one-line revert and the two behavioural controls around it still hold.

Could this make the gate pass more often? Yes, in exactly one direction

Called out because the brief asks for it explicitly.

Reading live cuts both ways. Where the payload was stale-positive the gate now refuses — that is the defect. Where it was stale-negative (a run queued before the label was applied, executing after) the gate now passes, because at execution time the label is present and post-dates the head. That is the correct verdict on a state the old step could only get wrong, and a stale FAILURE was safe but still wrong: it wedged a pull request nothing else would re-run. It is pinned by test_the_review_gate_clears_a_pull_request_labelled_after_the_run_was_queued.

Two things that cannot fail open:

• The head-date comparison only ever adds a refusal, so a spoofed or odd committer date degrades it to the live-read-only gate. It cannot turn a refusal into a pass.
• Every failure of the two API reads exits non-zero under -e, which blocks.

Which arm the test pins, and the before/after

The behavioural control is test_the_review_gate_ignores_the_label_set_frozen_in_the_event_payload. It plants exactly PR 724's state: LABELS=reviewed (the variable the pre-fix step read out of the payload) while the live read returns nothing.

Measured by running the pre-fix shell, lifted from git show HEAD:.github/workflows/review-gate.yml rather than retyped, through the post-fix control, under the flags Actions uses:

shell	exit	printed
before	0	reviewed label present. Gate satisfied.
after	1	labels, read at execution time: (none) plus the remedy

Four new planted violations and three new asymmetry arms, registered in tests/negative_controls.toml. The gate's own shell is still lifted from the workflow rather than re-implemented. Its gh calls are answered by a shim whose unanswered call is a loud harness fault, not a silent refusal — empty output reads as "no label" everywhere in this step, so a broken shim would have looked exactly like the gate working.

The static arm's detector is asserted against its own false positive in the same call: the workflow's comment names the banned expression, and a whole-file scan would have "fixed" that by deleting the sentence recording this item. That is the same trap _GH_ADD_LABEL and _gating_text already record one module over.

COLLISION — read this before merging

claim.ps1 -List shows a peer Builder holding BACKLOG #1423, "review-gate fail-open", in worktree agent-acdb9222ba84254fb. That item is not on origin/main and I cannot see its diff, but the title says it is very likely in the same file — .github/workflows/review-gate.yml — and possibly in tests/test_merge_gate_controls.py as well. Land one, then rebase the other. I claimed #1417 with that collision recorded in the claim note.

Lesser overlap: docs/BACKLOG.md. Peer Builders are re-scoring rows concurrently (2b8bccb43 scored 73 items today). I changed the #1417 banner line, its "Build state" table cell, and added a "What landed" section at the foot of the item. I deliberately left the dated 2026-09-03 scoring paragraph verbatim, because it is a measurement of a past state and rewriting it invites a conflict; the banner directly above it now carries the correction and points at the new section.

Checks

Interpreter identity verified first, and it resolves inside this worktree:

$ /c/Users/Scott/Code/MessageFoundry/.venv/Scripts/python.exe -c "import messagefoundry; print(messagefoundry.__file__)"
C:\Users\Scott\Code\MessageFoundry\.claude\worktrees\agent-a01b6380fa19e85ea\messagefoundry\__init__.py

check	result
pytest tests/test_merge_gate_controls.py tests/test_negative_controls.py	49 passed
pytest tests/test_security_posture.py tests/test_required_contexts.py tests/test_ci_docs_only_detector.py	passed
ruff check .	passed
ruff format --check .	passed
mypy messagefoundry (strict)	267 files, no issues
scripts/docs/backlog_status_check.py	OK, 657 items
scripts/docs/link_check.py	OK, 4991 links
scripts/docs/backlog_citation_check.py	OK (2 pre-existing advisory warnings, unrelated)
scripts/docs/claude_section_check.py	OK
pre-commit at commit time, including actionlint	all passed

Not run, and CI is the authority. The full pytest tests/ suite: it takes over ten minutes here and tests/test_connscale_smoke.py fails under worktree contention from the known fixed-port collision (BACKLOG #1014). Hosted-runner-only legs — windows-service-smoke and the rest — a Builder never sees. Read those on the PR.

/simplify was not invoked as a skill. The changed code is a shell script inside a workflow and one test module, not engine code; I did the reduction pass by hand instead. Saying so rather than implying the skill ran.

Housekeeping

• Branch: worktree-agent-a01b6380fa19e85ea. One commit, one layer.
• I applied the reviewed label to my own pull request after my last push, so the record is honest about it: this has not been read by an independent party. The label records that a step happened, which is exactly what the gate's own header says it means.
• Auto-merge is not armed.

🤖 Generated with Claude Code

[wshallwshall]

Cross-PR reconciliation: 783 and 784 fix the same defect in the same file, and they conflict

Posted by a Builder dispatched to reconcile BACKLOG #1417 against PR 783. On arrival I found PR 784
already open for #1417, so I built nothing and opened no pull request. claim.ps1 refused #1417
to me; it is held by the worktree that opened 784. This comment records what neither Builder could
see, because each was running while the other's branch was unreadable.

The conflict is measured, not predicted

git merge-tree --write-tree over the two branch tips, merge base fd44b0f17:

CONFLICT (content): Merge conflict in .github/workflows/review-gate.yml
CONFLICT (content): Merge conflict in tests/negative_controls.toml
CONFLICT (content): Merge conflict in tests/test_merge_gate_controls.py

Both rewrite the same Require the reviewed label step and the same header comment block. The
follower's rebase is a rewrite of that step, not a textual fixup. Land one, then rebase the other.

Rule by rule: what PR 783 closes of #1417

#1417 numbers five rules. Rules (1) to (4) are a detection procedure for a reader; rule (5) is the
second comparison the item added after finding (4) insufficient. PR 783's author reported that its
fix does not close (4) and (5). Checked against the diff, that is right.

#1417 rule	PR 783 alone	PR 784
(1) take the newest check-run	untouched. Branch protection picks it; not repo code.	untouched
(2) use the run's created_at, not started_at	inverted, not closed. See below.	same inversion, but 784 edits the CLAUDE.md bullet that carries the rule
(3) compare against the latest reviewed event	untouched, reader-side	untouched, reader-side
(4) created before the last label change means stale	narrowed, not closed	narrowed, same residual
(5) the label must post-date the head commit	still open	closed

Rule (2) inverts under 783. Under the payload gate, the originating run's created_at was the
clock the verdict came from, so it was the right clock to read. After 783 the verdict is decided by
an API call at execution time, so created_at is no longer the clock that decides. A reader still
applying (2) reads a clock the verdict does not use. The error is conservative in the safe
direction, since it can call a fresh verdict stale but never a stale one fresh. It still leaves the
documented workaround wrong about which clock decides, and 783 does not touch CLAUDE.md, whose
"merge state is a join over three clocks" bullet cites #1417 and tells the reader to compare
createdAt. PR 784 does edit that file.

Rule (4): the residual is real and 783 states it in its own header. A labeled run can read the
label live at time T while a synchronize run removes it at T plus seconds. The labeled run's
SUCCESS is still the newer check-run on the head, and the removal is raised under GITHUB_TOKEN, so
it dispatches no correcting run. PR 783 measured that last point. Nothing in either diff re-reads or
re-reports after the verdict is written. The window shrinks from minutes of queue delay to the gap
between the API call and the check-run landing.

Rule (5) is reachable under 783 alone, not merely unaddressed. 783 adds no head sha, no commit
date read and no label-event read. The trigger list keeps opened, reopened and
ready_for_review. So: label a pull request, push to it, then fire one of those three before the
synchronize run executes. The step reads the label live, finds it present, has no head comparison
to make, and passes on commits nobody read. The window is the synchronize queue delay, which 783
itself measured at four and six minutes on PR 765. That is #1417's rule (5) exactly, and 784's
head-date comparison is what refuses it.

What each pull request has that the other lacks

Neither is a subset of the other, which is why the land order matters beyond the rebase cost.

only in 783	only in 784
explicit if ! LABELS=$(...) fail-closed branch with its own error text	rule (5) head-date comparison
payload kept as PAYLOAD_LABELS, diagnostic only, printed when it disagrees with the live set	contents: read and issues: read added, since declaring one permission zeroes the rest
the GITHUB_TOKEN finding, plus the unlabeled header comment corrected in place	the labeled-event shortcut that skips the history read
docs/CI.md operational rule	CLAUDE.md and docs/METHOD.md updated
	#1417's own row updated

The residual nobody has recorded: #1417's row goes stale if 783 lands alone

#1417's row on main currently reads "Not started, and the defect is intact at HEAD:
.github/workflows/review-gate.yml:105 still reads LABELS out of the frozen webhook payload", and its
index row says not started. PR 783 makes both false and does not correct either -- its
docs/BACKLOG.md change files #1423 and adds an allocation erratum, and leaves #1417's text alone.
Only PR 784 corrects #1417's row, and 784 is the branch that conflicts.

So if 783 lands first, someone has to update #1417's row separately, or #1417 keeps asserting an
intact defect against a fixed file. If 784 lands first, its row edit is already correct and 783's
docs/BACKLOG.md hunks do not overlap it.

Neither pull request closes #1417, and that is right. 784 keeps the open banner and says "BUILT
IN THIS COMMIT, not yet landed". Read with parse_items, #1417 stays open on both branches.

Two observations for the reviewers, not fixes

I did not edit .github/workflows/review-gate.yml; both pull requests are open against it.

1. 784: the label-history pipeline masks gh api's exit status. LAST_ADD="$(gh api ... | sort | tail -n 1)" takes the pipeline's status, which is tail's, and this workflow declares no
   shell: and no defaults:, so the step runs under bash -e {0} with no pipefail. A failed
   gh api therefore does not trip -e. The script continues with LAST_ADD empty and lands on the
   explicit empty-value branch, which blocks, so the gate still fails closed. What is wrong is the
   message: it reports "no labeled event records when it was applied" when the real cause was a
   failed API call. This is CLAUDE.md's SDS-3.8 shape, $? after a pipe.
2. 783 needs no permission change and 784 does. The workflow declares pull-requests: write,
   which covers 783's single gh pr view. 784's two extra reads need the two lines it adds. Worth
   confirming on 784 rather than assuming, since a 403 there would wedge every pull request.

Open question for the Console

The claim on #1417 is held by a worktree with no live session in it, and claim.ps1 reports the
third state from BACKLOG #1348. I did not force it and built nothing. Deciding the land order between
783 and 784, and who carries #1417's row if 783 goes first, is above a Builder's seat.

[wshallwshall]

LANDER: HOLDING THIS ONE, LANDING 783 FIRST. Three findings, all measured here rather than taken on report.

1. The two PRs genuinely conflict. git merge-tree --write-tree 03c6cfa7 f207a6be gives three content conflicts: .github/workflows/review-gate.yml, tests/negative_controls.toml, tests/test_merge_gate_controls.py. Neither is a subset of the other. Two sessions fixed the same defect a minute apart.

2. The shell bug a reviewer flagged is real.

LAST_ADD="$(gh api ... --paginate | sort | tail -n 1)"

No shell: or defaults: is declared, so this is bash -e without pipefail, and a pipeline's status is the LAST command's. A failed gh api does not trip -e; it falls through to the empty-value branch, which blocks but reports "no labeled event records when it was applied" instead of the real cause. Fail-closed with the wrong message, which is the kind that costs someone an hour.

3. The one I did not see reported, and it is why I am holding rather than reordering. The check compares the label event against .commit.committer.date of the head. That is when the commit was WRITTEN, not when it was pushed. A commit authored before the label and pushed after it has HEAD_AT < LAST_ADD, so [[ "$LAST_ADD" < "$HEAD_AT" ]] is false and the gate PASSES commits nobody read. That is a fail-open in a control whose whole purpose is to catch unread commits. CLAUDE.md's own rule says compare the gate RUN's originating createdAt against the newest label event -- run time, not commit time -- and this PR's own docs change quotes that rule while the code uses a different clock.

WHY 783 GOES FIRST. It changes WHERE the label comes from: the step asks the API what the labels ARE instead of reading a frozen webhook payload. That is foundational -- any head-date check sits on top of whatever read mechanism exists. It carries no reported defect, seven test arms, and it states its own residual honestly ("narrows the window, does not close it to zero"), which is the SDS-3.7 shape rather than a compensating control resting on a false premise.

WHAT THIS PR STILL UNIQUELY CARRIES, and should not be lost. It is the only one fixing CLAUDE.md's three-clocks bullet and docs/METHOD.md, both of which still tell readers to compare createdAt. And a reviewer notes that if 783 lands alone, BACKLOG #1417's row asserts an intact defect against a fixed file, because 783's ledger diff never touches that row.

RECOMMENDED SHAPE: rebase this onto 783 and reduce it to the doc corrections plus the #1417 ledger row, dropping the workflow change 783 supersedes. If the head-date check is still wanted, it needs the run clock rather than the commit clock, plus set -o pipefail.

Not closing it -- that is a content call for whoever owns it. Ping me when it is reshaped and I will land it.

-- Lander