ci(pm): refuse governed surfaces at the merge queue — the prevention half - #12102
Merged
Conversation
The prevention half of the governed-surface regime. check:pm-governed-merges is the detection half -- report-only, post-merge, by design -- and three incidents (#9550, #10580, the #9319 landing) each ended with a governed diff enqueued or merged with zero reviews, each caught by accident. New check 'Governed Surface Queue Guard': * merge_group -> REFUSES a governed diff carrying no APPROVED review * pull_request -> the same finding as a green EARLY WARNING The event split is what keeps this from rebuilding the gate the 2026-08-18 ruling retired for being red on every governed PR by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ
…dled rejection Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ
…rt the collector Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ
hotlong
marked this pull request as ready for review
August 25, 2026 09:30
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
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.
Closes #11704
The prevention half of the governed-surface regime.
check:pm-governed-mergesis the detection half — report-only, post-merge, by design, and lint.yml's own comment calls it exactly that. This adds the refusal.The card's first measurement was pre-answered in its grading comment (the queue refuses nothing today), so this starts past it.
The check
Governed Surface Queue Guard.github/workflows/governed-surface-guard.yml/governed-surface-guardscripts/pm/check-governed-queue-guard.mjsnode scripts/pm/check-governed-queue-guard.mjs --self-test— 80 cases⭐ The one decision worth reviewing: the event split
The 2026-08-18 ruling retired
ADR maintainer approvalbecause it was red on every governed PR by design — 红灯常态化本身有毒. Rebuilding that is the obvious way to get this wrong, so the split is deliberate:pull_requestleg is green, and says out loud that it is deliberately green.main. That is the prevention.merge_groupevent. Under this regime that is not a hole; the human merge is the review record. What it closes is the seat path: flip ready → enqueue → the queue is the entire review.What satisfies it — and one deliberate divergence from the dispatch
The dispatch said "no APPROVED review from the CODEOWNER/maintainer". I implemented "an APPROVED review exists", with no person named, because that is a standing ruling about this exact predicate (#8161, verbatim): 「门禁改成只要求「APPROVED review 存在」」/「不要指定具体的人」. The identity proxy was tried and became unsatisfiable — human and agent accounts are not stably partitioned, cloud sessions author under the maintainer's own account, and GitHub forbids self-approval, so an identity-keyed gate went permanently red exactly when the human was driving. Flagging the divergence rather than quietly picking one.
#11704's own retraction re-derives this from the other side: it built a three-instance case out of the GitHub actor field and had to withdraw a third of it. Its closing lesson — "attribution from the GitHub actor field is not a reading" — is why this guard keys on the diff's paths and needs no attribution at all.
The accepted cost is unchanged from #8161 and is printed on every clear verdict: no identity signal proves a review is human, so a seat can satisfy this by approving. What it can no longer do is the thing all three incidents actually were — a silent flip-and-enqueue with nothing recorded anywhere. An approval is a separate, deliberate, timestamped action. That is the whole delta, and it is real.
Restraint worth naming: an outstanding
CHANGES_REQUESTEDis reported loudly but does not flip the verdict, because the ruled predicate is "an APPROVED review exists" and widening a governance gate past its own ruling is how gates acquire policy nobody agreed to. One-line change if you want it.Design choices, argued
A dedicated workflow, not a job in
lint.yml. Three reasons: it needspull-requests: read, which the lint job neither has nor should grow; a red here means "a governed surface is being merged unreviewed", which must not be mixed into a gate family whose red reads as "fix your lint"; and a standalone job yields one clean context name for branch protection. The retired gate was its own workflow for the same reasons.No
paths:filter, on either leg. A skipped job counts as SUCCESS in branch protection, so a filter would hand the queue a greenGoverned Surface Queue Guardfor a PR it mis-scoped — the #4928 shape, on the one check whose entire job is to refuse. The path test lives inside the script.Ordering, so fail-closed and fail-open never collide. Fail-open is wrong here specifically — this guard exists because everything else failed open — so an unreadable review list is a refusal with its own exit code. But a diff touching nothing governed must never be blocked by an API hiccup either. Reconciled by order, not tolerance:
runGuarddecomposes the diff and returns before constructing a single request. Pinned with afetchReviewsspy that throws if called at all — a mock returning[]would have passed against a broken version.Multi-PR merge groups are decomposed per commit.
merge_group.head_refnames only the last PR in a batch, so keying the group's diff to it would check the wrong PR's reviews — and in the direction that reads as compliance (PR B approved, PR A's governed diff rides in behind it). Each first-parent commit is attributed via the register's ownpullNumberFromSubject; a governed commit naming no PR isUNATTRIBUTEDwith its own exit code, not dropped.GOVERNED_SURFACESis read at runtime, never restated (#9840) — including the #9866/#10277 generated-artifact exception, applied through the register's ownapplyGeneratedExceptionswith provenance recomputed byte-exact against this build's base sha. Without it every page-adding docs PR would be refused (measured 5-for-5 on #9866) — routine traffic reddening the guard is the same poison one level down.Evidence
Self-test —
node scripts/pm/check-governed-queue-guard.mjs --self-test:Live end-to-end, real git + real GitHub API, replaying the merge of PR #11387 (
docs/adr/0120-…md) as a merge group:exit
3. The0 reviewsreading matches the card's ownget_reviews → [].pull_requestleg exits 0 with the early warning, as designed.Ablations — three mutations, each proven on disk by marker count and each restored (
trap … EXIT INT TERM); no build/distis involved, the script runs from source:.some(state === 'APPROVED')for the approval reductionname:A first attempt at the second ablation injected
q{APPROVED}and died onSyntaxError— red for the wrong reason. That reading was void and is recorded here rather than silently re-run; the table is the corrected run.Gates — derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(no hand-written path list), run at final commit1080f6c24, all 26 green. The nine that first answered "a fresh worktree has nonode_modules… nothing was measured" were re-run afterpnpm installand are counted as measured, not as passes:check:agent-test-spelling·check:cross-package-test-inputs·check:entry-guard·check:node-version·check:parse-guard·check:pnpm-acquisition·check:pnpm-filter-targets·check:required-contexts·check:shard-attestation·check:workflow-status-functions·check-aggregator-roster·check-ci-filter-parity·check-cross-package-test-inputs·check-required-contexts·check-self-test-wired·check-self-test-workflow-commands·check-shard-attestation·check-step-collectors·check-whole-set-label-write·check-drift-comment·check-governed-queue-guard --self-test·ci-failure --self-test·check:nul-bytes·check:pm-governed-merges·check:pm-governed-prose·check:pm-dispatch-gatescheck-required-contextsprints its own verdict, which is the one that matters for a PR adding a workflow:Lint, narrowed and declared. Repo-wide
pnpm lintis CI's run. Locally:eslint --no-inline-config --format jsonover the diff. Population read from eslint's own config, not guessed — the.ymlreports "File ignored because no matching configuration was supplied", so the real population for this diff is 1 file,scripts/pm/check-governed-queue-guard.mjs, with 0 errors, 0 warnings.--print-configshowslanguageOptions.parserOptions.project: null, i.e. type-aware linting is off, so this diff cannot move the verdict on any untouched file. The.ymlis covered instead by the four workflow-scanning gates above, which parse it and pass.⛔ Not done here — your step
Making this a required context is yours. Branch protection is the maintainer's, and #6865's two-step is a
REQUIRED_CONTEXTSrow inscripts/check-required-contexts.mjsplus the Settings → Rulesets entry, in one sitting. Adding the row alone would put it indirection A — registered here, NOT in the live required set, which that gate reports as a problem — so no row is added in this PR. The literal to pin, verbatim:The script's
--self-testreads the workflow file and fails if that literal ever drifts, so the pin cannot later be taken against a name nothing publishes.No token permission was widened.
contents: read+pull-requests: read, both default read scopes; no write scope anywhere.No changeset (
skip-changeset): a CI workflow and a PM script publish nothing.Until the context is required, this check is advisory — it will report on queue builds and go red, but nothing enforces it. That is #5617's own defect, and it is the reason the flip above is a real step rather than a formality.
Generated by Claude Code