Skip to content

fix(governed-guard): an authorised approval record satisfies the queue leg on any commit - #7606

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-7601-queue-guard-unpinned-approval
Sep 4, 2026
Merged

fix(governed-guard): an authorised approval record satisfies the queue leg on any commit#7606
os-steve merged 1 commit into
mainfrom
claude/issue-7601-queue-guard-unpinned-approval

Conversation

@os-steve

@os-steve os-steve commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7601

The merge_group leg counted an authorised APPROVED review only when its commit_id equalled the pull request's current head sha, so any push after an approval — a merge of main, a patch round, a rebase — turned the approval stale and reopened the refusal. Maintainer ruling in the live PM chat, 2026-09-04, quoted verbatim and untranslated:

你的门禁有问题,只需要有人工批准记录就行,不需要卡最新的提交。

A human approval record suffices, and it is never pinned to the latest commit.

It was ruled while objectui PR #7473 was being approved over and over by an authorised approver as its head moved. That PR remains open; nothing here touches it.

The predicate, before and after

before after
name pinnedApprovalVerdict(reviews, headSha) authorizedApprovalVerdict(reviews)
counts authorised, latest-decisive APPROVED, and commit_id equal to the PR's current head authorised, latest-decisive APPROVED — on whichever commit it was left
never counts dismissed · superseded · unauthorised account · stale sha · none dismissed · superseded · unauthorised account · non-decisive only · none
unreadable review list refuses, exit 4 refuses, exit 4 (unchanged)
staleApprovers bucket populated, printed on the refusal retired
API reads per governed PR 2 — PR head, then reviews 1 — the review list

Why staleApprovers goes away rather than becoming an inert printed reading (the card left the choice open, decided by reading what the refusal text and queue log did with it): under the new predicate every authorised latest-decisive approval lands in approvers, so the bucket could never be non-empty again. Its only renderer was a refusal line that can no longer print. An always-empty field is worse than an absent one — a reader takes its presence as evidence that this gate still measures staleness.

Why the pull-request head read goes with it. headSha reached the predicate through runGuard, which called fetchPullHead before fetchReviews purely to supply the sha to pin against. With no pin, that read feeds no verdict: it would be a cost, a second failure mode, and a verdict line naming traffic that no longer happens. makePullHeadReader is deleted and runGuard takes no fetchPullHead. The headSha on resolveEventContext is a different value and stays — it is the event's head, used to compute the git diff range, and no predicate reads it.

The accepted cost, stated out loud in the header rather than left to be discovered: a push after an approval is no longer re-reviewed by this gate, so an approved governed pull request can land carrying bytes its approver never read. The maintainer accepts that. The DRAFT remedy — dequeue, park as a draft, let the maintainer merge, which does read the final bytes — is still the one the refusal prints first.

Self-test: the flipped cases, and the count

--self-test printed count: 132 before, 132 after. Every case is a flip or a rename, plus exactly one replacement; nothing was added and nothing was dropped.

Flipped to the ruled direction:

  • an-authorized-approval-left-on-an-OLDER-commit-COUNTS-the-2026-09-04-ruling
  • an-authorized-approval-with-NO-commit_id-counts-a-human-record-is-not-a-sha
  • an-authorized-approval-on-an-UNPARSABLE-commit_id-counts-too-nothing-reads-a-sha-any-more
  • an-approval-left-on-an-OLDER-commit-CLEARS-the-merge-group-too
  • the-authorized-predicate-is-wired-end-to-end-an-approval-on-an-OLDER-commit-CLEARS
  • an-older-commit-approval-renders-as-CLEARED-and-nothing-in-the-output-calls-it-stale

Kept, and still refusing — the half that proves the flip did not widen the gate open:

  • an-unauthorized-approval-never-counts
  • a-DISMISSED-authorized-approval-never-counts
  • an-authorized-approval-later-superseded-by-CHANGES_REQUESTED-never-counts
  • no-reviews-at-all-is-unapproved-under-the-authorized-predicate-too
  • only-DECISIVE-states-carry-a-decision-COMMENTED-and-PENDING-are-not-approvals
  • an-unauthorized-approval-does-not-mask-an-authorized-one

One replacement: an-unreadable-pr-head-REFUSES-with-exit-4-and-never-reads-reviews becomes the-queue-leg-costs-exactly-ONE-review-lookup-per-governed-pr-the-head-read-is-retired. A mechanism a ruling removes has to be checked absent, not inferred from the code reading clean; exit 4 is still pinned by the throwing-review-reader case.

HEAD and OLD stay in the fixtures although no sha is compared any more: "the commit does not matter" is only measurable on cases where the commits actually differ. Grepping the file for commit_id therefore finds the evidence, never a surviving comparison — the header says so, so the hits do not read as leftovers.

Ablation — the flipped cases can actually fail

Committed first, then mutated. The sha pin was reinstated inside authorizedApprovalVerdict against the self-test's HEAD constant. Mutation confirmed on disk by grepping the exact text: injected marker ABLATION_PIN present 1 time, the original predicate line present 0 times (an editor's exit code proves nothing — a zero-hit anchor exits 0 too).

Predicted direction RED, and observed RED: FAIL check-governed-queue-guard self-test: 7 of 132 case(s) failed — exactly the six flipped cases plus the one replacement, with every "still never counts" case staying green.

Restore proven by state, not by exit code: git checkout HEAD -- ABSOLUTE_PATH, then git hash-object equal to the HEAD blob c79bb39a92bbc00b46c3661941bb5e01f520e699, git diff HEAD empty, and 0 residual ABLATION_PIN in the tree. Self-test on the restored tree back to 132 pass. The mutation script carried a trap on EXIT/INT/TERM with an absolute path.

Is this guard itself governed?

No — as the seat expected, and measured rather than assumed:

$ node scripts/check-governed-queue-guard.mjs --test scripts/check-governed-queue-guard.mjs
✅ NOT GOVERNED — 1 path(s) checked against 5 governed surface(s); none matched.

The same question asked of this PR's whole file list gives the same answer. .github/workflows/** is deliberately outside GOVERNED_SURFACES, so this guard does not govern its own workflow — the widening not taken, pinned in both the self-test and the vitest suite.

Gates — head sha a3f3035, clean tree

Every exit code captured by redirecting first and reading $? before any pipe; each row quotes the gate's own verdict line, never a bare exit status.

command exit verdict line
node scripts/check-governed-queue-guard.mjs --self-test 0 OK check-governed-queue-guard self-test: 132 cases pass (…)
node scripts/check-governed-queue-guard.mjs --test scripts/check-governed-queue-guard.mjs 0 ✅ NOT GOVERNED — 1 path(s) checked against 5 governed surface(s); none matched.
pnpm exec vitest run scripts/__tests__/check-governed-queue-guard.test.ts 0 Test Files 1 passed (1) · Tests 15 passed (15)
pnpm lint 0 Tasks: 47 successful, 47 total
pnpm lint:root 0 ✖ 33 problems (0 errors, 33 warnings) — pre-existing warning debt, 0 errors
pnpm check:control-bytes 0 ✅ check-control-bytes: OK (scanned 6227 tracked text file(s); skipped 85 binary).
node scripts/check-changeset-presence.mjs 0 ✅ No source or published contract of a released package changed in this range, so no changeset is owed.
pnpm check:esm-specifiers 0 Specifier leg: no un-ledgered package emits an extensionless relative specifier.
pnpm check:node-esm-load 0 only the by-design CSS-extension exclusions already in the ledger
pnpm type-check:scripts 0 tsc -p tsconfig.scripts.json, silent on success

No changeset, and that is the gate's verdict rather than a judgement call — quoted above. ⛔ No skip-changeset label: objectui has no such mechanism, check-changeset-presence.mjs is what decides.

The changed file is genuinely linted, checked rather than assumed: pnpm exec eslint --format json scripts/check-governed-queue-guard.mjs reports files linted: 1, 0 errors, 0 warnings. That matters because a zero-match lint run also exits 0.

Scope

One file: scripts/check-governed-queue-guard.mjs. Unchanged on purpose — GOVERNED_APPROVERS, the path test, the pull_request early-warning leg, the exit contract, the check-run name, and .github/workflows/governed-surface-guard.yml.

scripts/__tests__/check-governed-queue-guard.test.ts needed no edit, and that is a measurement: it pins the gate's wiring and the governed surface, and imports only CHECK_CONTEXT_NAME, CHECK_JOB_ID, CHECK_WORKFLOW, GOVERNED_SURFACES and governedPathsIn — it asserts no predicate at all, so the rename reaches nothing in it. It is green at 15/15 against the changed script, including its own case that shells out to --self-test.

Follow-up filed rather than folded in: #7605AGENTS.md and content/docs/guide/ci-cd-pipeline.md still describe the sha pin. AGENTS.md is a governed surface, so correcting it is a draft PR that parks for a human merge; folding it in here would have parked this ordinary PR too.

Authored by the objectui dev seat in session session_019RfFHiRCSs3JXLK4cwcfox, on branch claude/issue-7601-queue-guard-unpinned-approval — every seat here writes under a shared GitHub identity, so the branch is the identifying detail.


🤖 Generated with Claude Code

https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox


Generated by Claude Code

…e leg on any commit

The `merge_group` leg counted an authorised APPROVED review only when its
`commit_id` equalled the pull request's current head sha, so every push after an
approval turned it stale and reopened the refusal. Maintainer ruling of
2026-09-04, in the live PM chat, verbatim and untranslated:

  「你的门禁有问题,只需要有人工批准记录就行,不需要卡最新的提交。」

A human approval record suffices and is never pinned to the latest commit.

- `pinnedApprovalVerdict(reviews, headSha)` becomes
  `authorizedApprovalVerdict(reviews)`: an account in `GOVERNED_APPROVERS` whose
  latest-decisive review is APPROVED satisfies the leg on whichever commit that
  review was left. Dismissed, superseded, unauthorised, non-decisive and absent
  reviews still never count; an unreadable review list still fails closed.
- `staleApprovers` is retired outright rather than kept as an inert printed
  reading: under the new predicate every authorised approval lands in
  `approvers`, so the bucket could never be non-empty again and an always-empty
  field would read as evidence that staleness is still measured.
- The pull-request head read existed only to feed the pin, so it goes with it —
  `makePullHeadReader` is deleted and `runGuard` takes no `fetchPullHead`. One
  governed pull request now costs exactly one review lookup, and the verdict
  line no longer names a read that does not happen.
- The header's "What satisfies the queue leg", the sha-pin rationale, the
  refusal and cleared texts and the `--test` output are rewritten to the ruled
  predicate, quoting the ruling and stating its accepted cost out loud: a push
  after an approval is no longer re-reviewed here. The DRAFT remedy stays first.

Unchanged, deliberately: `GOVERNED_APPROVERS`, the path test, the `pull_request`
early-warning leg, the exit contract, the check-run name, and the workflow file.

The self-test cases that pinned the sha pin flip to pin the new predicate in both
directions; the printed count is unchanged at 132 (every case is a flip or a
rename, plus one replacement — the retired head read is now measured ABSENT
where its ordering used to be pinned).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
@os-steve
os-steve marked this pull request as ready for review September 4, 2026 06:54
@os-steve
os-steve added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 0d8fd7c Sep 4, 2026
31 checks passed
@os-steve
os-steve deleted the claude/issue-7601-queue-guard-unpinned-approval branch September 4, 2026 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants