feat(pm): refuse a merge group whose queued PR still carries needs:contract-review - #17484
Merged
os-litant merged 2 commits intoSep 10, 2026
Merged
Conversation
…ontract-review`
SKILL.md 〈入队与落地〉 states the clause-② enqueue gate as binding —
「双肢命中任一 ⇒ 无席内条款②复核 PASS 在案 ⛔ 禁止入队」 — and
`references/contract-review.md` 〈载体纪律〉 reads the label itself:
「开着的载体恒 = 真实待审」. Nothing mechanical read it at the queue: a grep over
`.github/workflows` and this guard answered with one comment line in `lint.yml`
about a label description's byte length. The gate was advisory in practice while
the protocol treated it as binding, and eleven enqueues in one day (2026-09-09,
both repos, event logs read per PR) is what that cost.
The queue guard gains a second, independent leg on `merge_group`: every pull
request the group is landing is enumerated per commit — the same decomposition
the governed leg uses, so a bare PR cannot carry a gated sibling through — and
its labels are read from the pull object. Refuse while the carrier is open
(exit 6); refuse when the label set cannot be read or the group names no pull
request (exit 7); pass otherwise. The `pull_request` leg is untouched and reads
no label, so its output stays byte-identical.
The label read uses `GET /pulls/{n}` rather than the issues-API labels route,
so `pull-requests: read` — already declared for the review read — is the whole
permission it needs and the workflow is unchanged.
Two things the header states out loud rather than leaving to be discovered. The
leg reads the LABEL, not the verdict: of the eleven measured enqueues it refuses
five and passes six, because those six stripped the carrier seconds before
enqueue with no PASS on record. And it has no cheap local pre-filter, so a
GitHub outage now refuses a merge group whose diff touches nothing governed; the
zero-cost-clear rendering is scoped on that leg rather than left making a promise
this change took away.
The carrier spelling is mirrored, not imported: `check-half-states.mjs` owns the
constant, but importing it here is a module-eval cycle — its H43 awaits
`import('./check-governed-queue-guard.mjs')` at module scope — which deadlocks
both modules (node exit 13). The self-test pins the mirror to that file's source
and pins the reason it is a mirror.
Claude-Session: https://claude.ai/code/session_01YKEjmbYNvYWJvWGSWx26zK
Co-authored-by: Claude <noreply@anthropic.com>
…eue-guard-contract-review-carrier
os-litant
added
the
skip-changeset
Claude
This was referenced Sep 10, 2026
os-litant
marked this pull request as ready for review
September 10, 2026 17:41
os-litant
enabled auto-merge
September 10, 2026 17:41
os-litant
deleted the
claude/issue-17040-queue-guard-contract-review-carrier
branch
September 10, 2026 18:05
This was referenced Sep 10, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17040
The rule, and the gap
.claude/skills/pm-dispatch/SKILL.md〈入队与落地〉 (:638) states the clause-② enqueue gate as binding, and the quote IS the operative criterion, so it is reproduced from that file's own bytes rather than paraphrased:with
references/contract-review.md〈载体纪律〉 (:19) supplying the reading of the label itself:The gap, re-measured on this branch's base:
One hit, and it is a comment about a label description's byte length. Nothing at the queue read the carrier. The gate was advisory in practice while the protocol treated it as binding.
What this adds
One new fail-closed leg in
scripts/pm/check-governed-queue-guard.mjs, in the same shape as the existingGOVERNED_APPROVERSrefusal. It is a second, independent predicate in the same check run, not a change to the governed one.merge_grouponly. Thepull_requestleg returnsnot-applicable, renders the empty string and reads no label, so that leg's output is byte-identical.GET /repos/{owner}/{repo}/pulls/{n}→labels[].name.needs:contract-review.The refusal quotes the rule verbatim and names who can act: park the PR back to draft (disarming auto-merge alone does not dequeue it), the dispatching seat completes the in-seat review and posts the verdict, and on PASS that same seat strips the carrier, cites the record and re-enqueues — 「PASS ⇒ 同席剥标并引记录、ready、auto-merge」. The refusal also says out loud that stripping the label to get past the check is the defect it was built from, not a way through it.
The fixture reading — eleven measured enqueues, and the honest boundary
The card carries nine numbered incident rows plus the two in its body. Carrier state at
added_to_merge_queuewas read from each PR's own event log (GET /repos/{owner}/{repo}/issues/{n}/events), not transcribed from the card's prose — three of the rows the card's tables leave silent turned out to have the carrier still on.added_to_merge_queueFive refused, six passed. ⭐ The six are the honest boundary and they are not a rounding error: this leg reads the LABEL, not the verdict. Those six stripped the carrier seconds-to-minutes before enqueue with no PASS on record for that head — 「被剥」 and 「从未挂过」 are the same bytes to a label reader. Row 9 (objectui#8164) is the one row of the eleven whose merged head carried a real published-face defect, and this leg would not have caught it. Whether a verdict EXISTS is
check-clause2-carriers.mjs's question; the CLEAR rendering says so out loud so nobody reads a green carrier leg as "the review happened", and all eleven rows are pinned as--self-testreplay fixtures with their predicted direction.Governed Surface Queue Guardis absent fromREQUIRED_CONTEXTS(scripts/check-required-contexts.mjs), by the script header's own decision. So a refusal is a red check, not a blocked queue, until the maintainer's #6865 two-step (a registry row plus the Settings → Rulesets entry, in one sitting) is done. The card's "It would have stopped both rows" is conditional on that step. ⛔ Not this PR's to take.Judgement calls, on the four axes
Every call below is analysed on 实际业务需求 / 项目长远合理性 / 防 AI 写错(尤其元数据) / 创业阶段不扩散需求.
J1 — enumerate per commit, not just the queue ref's PR (deviation from the suggested route).
The dispatch's Zone 3 route was "queue ref → PR number → labels". Taken literally that under-enumerates:
merge_group.head_refnames only the last PR in a multi-PR group, so PR A's open carrier would ride intomainbehind PR B — the exact trap this file's own header calls "the one direction a governed-surface reading must never be wrong in (#9902)". 业务需求: merge groups really do carry several PRs; the failure is silent and reads as compliance. 长远合理性: reusingenumerateRows' decomposition means the two legs can never disagree about which PRs are in the group — no second mechanism. 防写错: the tightening is structural, not a consumer-side accommodation. 不扩散: it is the same data already in hand; no new input, no new config. ⇒ Deviation taken, declared here.J2 — read labels from the pull object, not the issues labels route.
GET /repos/{o}/{r}/issues/{n}/labelsis an issues-API route needingissues: read, which this workflow does not grant. The pull object carries the samelabels[]underpull-requests: read, which the review read already needs — verified live on #17442 (["ci/cd","size/l","skip-changeset"]). 业务需求: same answer, measured. 长远合理性: a permission the workflow does not need is one nobody has to review. 防写错: the wrong route fails as a 403 on every queue build, which this leg then correctly turns into a refusal — loud, not silent. 不扩散: ⇒ Zone 2(b) resolves to no YAML change at all;governed-surface-guard.ymlis untouched.J3 — mirror the label constant instead of importing it.
check-half-states.mjsownsCONTRACT_REVIEW_LABEL(H31; H51/H53 pin that ownership) andcheck-clause2-carriers.mjsimports it. This file cannot: its H43 resolves the governed register at module scope withexport const GOVERNED_REGISTER = await loadGovernedRegister();, which awaitsimport('./check-governed-queue-guard.mjs'). Adding the reverse edge is a module-eval cycle that deadlocks both modules — measured: node exits 13 with "Detected unsettled top-level await", andcheck-half-states.mjsstops loading standalone too. A dynamic import inside the self-test deadlocks identically, because this file reaches its own self-test through a top-level await. 防写错: the mirror is pinned to H31's source text (the same "read it from disk, a constant asserting against itself proves nothing" idiom the workflow wiring pin already uses), and a second case pins the reason — when H31's module-scope await goes away, that case fails and says to replace the mirror with a real import. 不扩散: inverting the dependency would editcheck-half-states.mjs, outside the declared file surface, and would break H51/H53. ⇒ Mirror, pinned both ways.J4 —
merge_grouponly; nopull_requestearly warning.An early warning would be useful. 不扩散 decides it: the ruling this leg implements is about enqueue, and the
pull_requestleg carries a byte-identity constraint from 2026-08-27. Widening a governance gate past its own ruling is how gates acquire policy nobody agreed to. ⇒ One line for a maintainer to change later; not taken here.J5 — two exit codes (6 and 7), not one.
Mirrors the file's own 3-vs-4 split: "the carrier is open" and "we could not find out" are different facts and a queue log must separate them. 防写错: a merged code invites a later reader to treat an outage as a policy refusal, or worse, to relax one because the other is noisy.
J6 — scope the merge_group zero-cost clear rather than leave a false sentence.
The governed leg's CLEAR prints "so a GitHub API outage can never block a diff that touches nothing governed". This leg has no cheap local pre-filter — a carrier is remote state a seat hangs, not a property of the diff — so every merge group now pays one label read per queued PR, and an outage does refuse a clear diff. ⚖️ That is the accepted cost, stated in the header where it is paid. Rather than ship a knowingly false sentence in a governance guard, the merge_group clear gains a scoping note; the three pinned lines stay byte-identical, and the
pull_requestclear is untouched. 长远合理性: #15406's lesson applied rather than re-learned — a verdict may not deny its own evidence. Fail-open was the alternative and is ruled out by construction in this file.J7 — the governed exit code wins when both legs refuse.
Both legs are always evaluated and both blocks are always printed, so no reading is lost. The governed refusal takes the exit code because its remedy is the stricter of the two (the maintainer's own merge, Prime Directive #14) and it subsumes the carrier's "take it out of the queue".
Verification
Gate families derived from the FINAL diff with
node scripts/pm/dispatch-gates.mjs --commands(no hand-written path list; three-dot change set), then reconciled with--ran. Exit codes captured before any pipe.Self-test — the case count moves by exactly the added cases.
3644fadc)✓ check-governed-queue-guard self-test: 144 cases pass …✓ check-governed-queue-guard self-test: 183 cases pass …+39, all of them in the one new battery
⛔ #17040: the contract-review carrier is the enqueue gate, whose declared floor is set to that measured 39 (a floor, not a total — the roster's own size pin moves 18 → 19 in the same edit). No existing battery's floor was touched and none dropped a case.The 32 derived families — every one run, every exit code captured before any pipe.
Reconciliation —
node scripts/pm/dispatch-gates.mjs --ran RANLIST(RANLIST being the list above, as a file), exit 0:The family set was derived three times — at the branch point, after
git fetch origin main, and again after mergingorigin/main(0a578c60, no stale-tree warning). Identical 32 both in and out; no family added, none dropped. The union was run twice: once at7fb02346, then again in full at the final head0a578c60after the merge — all 32 exit 0 both times, and the reconciliation above is from the final-head record, where every family carries its own captured exit code rather than a bare "I ran it". Re-confirmed at0a578c60: the guard self-test (183 cases, exit 0), the path face (exit 0), and eslint (exit 0).eslint —
npx eslint --no-inline-config scripts/pm/check-governed-queue-guard.mjs, exit 0, no output.Ablation — the new leg is proven able to fail, not assumed to be.
state: names.includes(CONTRACT_REVIEW_LABEL) ? 'gated' : 'bare'was mutated tostate: false ? 'gated' : 'bare'(fail-open). Mutation proven on disk by grep count in both directions (original marker 1→0, mutant 0→1) before the run; restore isgit checkout HEAD -- scripts/pm/check-governed-queue-guard.mjsunder anEXIT INT TERMtrap, verified by an emptygit diff HEADand a blob hash identical to the HEAD blob4fd3ccf4de9cdebc2a6d901a18d671d52303be01.Predicted direction was turns red, and that is what was observed.
Path face —
node scripts/pm/check-governed-merges.mjs --test scripts/pm/check-governed-queue-guard.mjs, exit 0, on the final file list:Control characters —
grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over the changed file: no match (grep exit 1), on top ofpnpm check:nul-bytes.Changeset — none.
scripts/pm/**is on the not-published fast track: nothing in any package'sfiles[]moves.skip-changesetapplied.Acceptance notes
noted, not filed:rows 1, 2 and 3 of the card (objectstack#16783, objectstack#17036, objectui#8779) merged while still carryingneeds:contract-review— the label was never stripped at all, before or after. That is the "closed card / merged PR still carrying a gate label" shape the director seat named in 5597753733 as a wantedcheck-half-statesrow. It is a live half-state onmaintoday, not a hypothetical. Carrier:check-half-states.mjsis the file that owns carrier symmetry; ⛔ out of this PR's declared surface, and it is the accepting seat's to route.noted, not filed:the objectui port of this leg. Named in the grading (5621508721) as the accepting seat's follow-up card withBlocked-by:this one; ⛔ not folded here.noted, not filed:the card and dispatch quote SKILL.md :638 as 「无席内契约复审档 PASS 在案」 where the file reads 「无席内条款②复核 PASS 在案」. Documentation-side wording only; no mechanism depends on it. Carrier: whoever next edits that clause.noted, not filed:this check is not inREQUIRED_CONTEXTS, so today the refusal is a red check rather than a blocked queue. Flipping it is the maintainer's finding: lint.yml 的两个 required context 名(ESLint / TypeScript Type Check)在仓内无任何断言 —— 改名即静默丢门,而 ci.yml 的同类名字有 check:shard-attestation 守着 #6865 two-step and is deliberately not an agent's step — the script header says so itself.Clause-②: no— the diff touches no contract surface (nopackages/spec/src/**, no*.zod.ts, no error-code ledger) and no published package'sfiles[]; it is PM tooling underscripts/pm/**with no accept set or public surface moving.🤖 Generated with Claude Code
https://claude.ai/code/session_01YKEjmbYNvYWJvWGSWx26zK
Generated by Claude Code