Repository navigation
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
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Sep 17, 2026
…:contract-review (objectstack-ai#17831) The half-state patrol could see a contract-review gate on an OPEN carrier (H31 compares the card's and the PR's, H51 reads one outliving a verdict on the current head, H53 reads one with no increment behind it) and it could see a gate REMOVAL (H35). It could see nothing at the other end: a pull request that MERGED, or a card that CLOSED, with `needs:contract-review` still on it. 载体纪律's invariant is 「开着的载体恒 = 真实待审」, so every one of those labels still tells a later reader — a seat, a post-merge tier audit, any candidate query that forgets to scope `state=open` — that a real contract review is owed on work that is already in `main`. This adds H61, one report-only row per landed carrier, printing the merge or closure stamp and the PR's head sha, so the post-merge tier audit has a list instead of a memory. It costs no request. Both populations were already in hand by the time the row runs: merged PRs from H8's 8-day window (`listRecentlyMergedPullRequests`) and closed cards from H22's 3-day window (`listRecentlyClosedIssues`), whose list rows already carry `labels`, `merged_at` / `closed_at` and `head.sha`. Exit codes, bands and every other row are untouched — a completed sweep still exits 0 whatever it finds, and the script still never writes a label: clearing a carrier is 清标即落地, a seat's audited write with its provenance comment, and a sweeper striking a review gate would be issuing the verdict. Fixes objectstack-ai#17510 ## Acceptance notes ### The director seat's ask, verbatim From comment 5597753733 on objectstack-ai#17040 (os-bill, 2026-09-09T07:11Z, "What the mechanisable fix should now also cover"), quoted exactly as written: > A `check-half-states` row for a **closed card still carrying `pm:*` or a carrier** (row 1 left objectstack-ai#16231 closed with `pm:queue` + `needs:contract-review`; row 2 left the carrier on a merged PR). The `pm:*` half of that sentence is H22's and stays there. The CARRIER half is H61. The card paraphrases it as 「merged PR / closed card still carrying a gate label」; the sentence above is the actual wording. ### Premises, re-checked on `origin/main` at `f830fa21` 1. **Holds.** No predicate named a merged PR or a closed card still carrying the gate. `h51SpeaksAbout` excludes a merged or closed PR by name; H31's population test excludes a merged delivering PR and a closed card by name; H35 reads removal events; H8 reads `pm:dispatched`; H22 reads `pm:*` on closed cards and files no row at all since objectstack-ai#14072. `CONTRACT_REVIEW_LABEL` is reused — no second spelling, pinned the way H51 and H53 pin it. 2. **Half false, and corrected in place.** H60 is indeed the highest number in use and H99 is the self-test sentinel, so H61 is the next free one. But the parenthetical is wrong: the top docblock's "## The invariants" index stops at **H28**. H29 through H60 have no entry there at all. H59 and H60 were added as a banner comment block above their predicate plus a `HALF_STATE_FAMILY_BAND` entry, and `familyRegistryCoverage` is the mechanical index that holds the band registry equal to the codes the sweep actually pushes. H61 follows that idiom; adding a lone H61 line to an index that stops thirty-three rows earlier would have been worse than leaving it out. 3. **Holds.** `33e07f7c` (PR objectstack-ai#17484) is an ancestor of `origin/main`. The front end is closed, so this row is residue reporting, not a bypass, and the row says so in its own text. 4. **Updated, larger, and measured.** See below. ### The measured population, 2026-09-12 `GET /repos/{owner}/{repo}/issues?state=closed&labels=needs:contract-review`, one page per repo, both complete (39 and 29 rows, under the 100-row page size): | repo | total | merged PRs | closed-UNMERGED PRs | closed cards | |:--|--:|--:|--:|--:| | objectstack | 39 | 23 | 2 | 14 | | objectui | 29 | 17 | 0 | 12 | | **both** | **68** | **40** | **2** | **26** | Oldest closure still wearing the label: 2026-08-20T23:58Z (objectstack), 2026-08-30T17:32Z (objectui). The card named three specimens; the class is an order of magnitude larger. **What the row reaches, and what it does not.** H61 reads the two windows the sweep already holds, so a merged carrier is visible for 8 days after it lands and a closed-card carrier for 3. At the patrol's 6-hour cadence that is roughly 32 and 12 consecutive sweeps: every NEW carrier is seen, repeatedly. What ages out is the standing backlog above. That boundary is deliberate — the alternative is a `state=closed&labels=…` listing, which is both a new fetch class and a second closed-card reader beside `pmLabelListingPath`, whose `state=open` scoping is the entire mechanism by which maintainer ruling 批 objectstack-ai#13 holds. The header records the number so a reader can tell a short reach from a clean board. ### Work item B — closed-UNMERGED PRs are EXCLUDED Stated, not folded. Two of the 68 (objectstack objectstack-ai#16733 and objectstack-ai#14923; objectui 0 — 2.9%) are pull requests closed without merging, carrier still on. They are out of this row for three reasons: 1. **The harm model does not reach them.** This row exists because a gate on a LANDED increment says a review is owed on work already in `main`; the post-merge tier audit is the consumer. A PR closed unmerged delivered nothing and owes no post-merge verdict — the shape H51's own header already calls moot, 「载体不迁移」. 2. **The data path does not hold them.** `listRecentlyMergedPullRequests` SELECTS on `merged_at`, and its header says why in as many words: an unmerged closed PR is an abandoned attempt, not a delivery. Widening that selector would change what H8 and H49 are handed — two other rows' inputs, and out of scope here. 3. **Size.** 2 of 68, both on one install, neither inside the window this row reads. A separate count rendered on every run for a class that contributes nothing to it is a number a reader learns to ignore. Both legs refuse them, and both refusals are pinned. Writing that pin is what found a real hole in my own first draft: a closed-unmerged PR arriving from the PULLS listing carries no `pull_request` field at all, while `state` is `closed` and `closed_at` is readable — so the card leg accepted it. `h61SpeaksAboutClosedCard` now refuses a row carrying `head` as well. Live, the two collections never cross; a population test that is only correct because of its caller is one refactor from being wrong. ### The sha is the HEAD, not the merge commit The row prints `head.sha`. 载体纪律 says a clearing write cites 「所判 head」 and H51 resolves a verdict against `pr.head.sha`, so the head is the only sha a contract-review record can be matched on. Measured on the three specimens that filed this card: objectstack-ai#16783 head `47eea7a8` / merge commit `854639b3`; objectstack-ai#17036 head `530469ff` / merge commit `e4fd55d9`; objectui#8779 head `a04441f7` / merge commit `6cc48c4e`. The three shas quoted on the card and in the dispatch are the MERGE COMMITS — a reader searching a thread for one of them finds nothing. Noted, not filed: a card-text inaccuracy, not a defect in any code. ### Band, and the repo column `inventory`. Refused in turn: not `gate` (that band is for a row that can tell a STRIPPED gate from an ungated one — an absence reading as a green light; here the carrier is present), not `stall` (nothing is stopped — the increment merged), not `state` (whose definition says a LIVE card, while every subject here is merged or closed, ARCHIVE in ruling 批 objectstack-ai#13's own word for this population). Being first in line for the body trim is correct for it, and the ledger callout names the family and the omitted count rather than dropping rows silently. The row text is deliberately short for this file for the same reason: it fires per carrier, and 20 of the 23 objectstack merged carriers sit inside the 8-day window today. No repo column: each install sweeps its own repository through `resolveSweepRepo`, one token and one board, so this row can never see more than one and the column would be a constant. The two-install figures above are a hand measurement taken for the header, not something a run reproduces. ### Verification - `node scripts/pm/check-half-states.mjs --self-test` :: exit 0 — 3748 cases pass (3707 before this change, 41 new). Both directions per the dispatch: a merged PR carrier fires, a closed card carrier fires, an OPEN PR carrier does not (and H51 still speaks about exactly that PR, so nothing fell between the two rows), a merged PR without the label does not, a closed-unmerged PR does not through either leg, unreadable `labels` is excluded rather than read as unlabelled, an unreadable stamp yields no row. The row text is pinned on the invariant, the remedy (清标即落地 / 同笔剥双载体 / the provenance comment / who acts), the "never a label written from this script" clause, the neighbouring rows it defers to, and the fact that it prints `CONTRACT_REVIEW_LABEL` rather than a re-typed literal. - Exit codes and bands pinned unchanged: `EXIT_PREREQUISITE_NOT_MET` is still 3, `HALF_STATE_FAMILY_BANDS` still spells `gate,unregistered,stall,state,inventory`, `isLoudFinding` is false on the row, and `familyRegistryCoverage().missing.length` is 0. - All 41 gate families derived by `node scripts/pm/dispatch-gates.mjs --commands` in this worktree were run; each exit code is in the report on objectstack-ai#17510. - No changeset: `scripts/pm/**` publishes nothing from any released package, which lint.yml calls the textbook `skip-changeset` case in as many words. ### Out of scope, noted not filed - The card's and the dispatch's three shas are merge-commit shas presented as head shas (measured above). Successor: this PR body and the row's own header carry the correction; no code is wrong. - objectstack-ai#16231 no longer carries `pm:queue` — the director's row 1 recorded it on 2026-09-09 and it has since been cleared. The carrier is still on, which is why it serves as this row's closed-card specimen. --- _Generated by [Claude Code](https://claude.ai/code/session_01MCLBsUgfykL74aU716rzVK)_ Co-authored-by: Claude <noreply@anthropic.com>
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