Skip to content

Refuse a governed enqueue with no head-pinned approval, in a PreToolUse hook - #14140

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-14059-governed-enqueue-precondition
Sep 1, 2026
Merged

Refuse a governed enqueue with no head-pinned approval, in a PreToolUse hook#14140
os-zhuang merged 3 commits into
mainfrom
claude/issue-14059-governed-enqueue-precondition

Conversation

@claude

@claude claude Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Part of #14059

Part of, not Fixes: the card asks for two things and this PR lands one of
them completely. The machine half — a PreToolUse hook that refuses an
enqueue-class tool call on a governed PR with no head-pinned approval — is here.
The landing-operations.md annotation is STOPPED on a measured ceiling, with
byte-exact floors below; that half routes to the decision box rather than being
funded by deleting live operational text.

The incident, in one paragraph

A governed PR (four skills/*/references/_index.md) was flipped ready and
enqueued with zero approvals. The merge-queue guard read the reviews on the
merge_group build and refused exactly as designed, so nothing wrong landed —
but the queue entry popped out red, and the approval the maintainer pinned to
head b25f061c6 at 01:53:29Z did not re-run it. GitHub's merge queue does
not retry a failed entry when an approval arrives afterwards. Cost: one burned
queue cycle, plus a red entry that reads to the maintainer as "I reviewed it and
it still blocked me".

So "enqueue first, get approved after" is not a slower route to the same place.
It is a route that always costs a cycle and always manufactures a false red.

Reconciling the triage note with this hook

The card quotes the queue guard's sibling design note: the enqueue threshold
「入队门槛机器上拦不住是 GitHub 模型限制」. That statement is true, and this PR
does not contradict it — it locates it:

  • GitHub cannot gate enqueue server-side. There is no server-side hook
    between a seat's enqueue call and the queue. Nothing here changes that, and
    the red light stays deliberately placed on the merge_group anomaly.
  • Our own tool-call surface can. The enqueue action reaches GitHub through
    a tool call this process makes, and PreToolUse sits in front of it. That is
    a client-side control, and it is the only place the wasted cycle can be saved.

⇒ The queue guard remains the hard correctness line. This hook is a
cost control, not a correctness one. It can only ever refuse what the queue
guard would also have refused, because it asks the same two predicates.

The triage note's conclusion 「为什么这里只能靠纪律」 is therefore superseded on
the client side only. Nothing about the server-side reasoning changes.

No second mechanism

The hook contains no path list and no approver list of its own. It asks the
two existing single sources and reports what they answer:

question asked of how
is this diff governed? scripts/pm/check-governed-merges.mjs --test --json — exit 3 governed, exit 0 clear
is it approved, pinned? scripts/pm/check-governed-queue-guard.mjs pinnedApprovalVerdict + GOVERNED_APPROVERS, imported

A register row or a ruling that moves either predicate reaches this guard for
free, and the two tools cannot answer differently about the same diff. The
self-test asserts this structurally: it fails if an approver login or a
governed-path literal ever appears in the hook's code.

The pure-regeneration class passes, and that is measured

Maintainer ruling 2026-09-01, verbatim and untranslated:

纯生成的指针行(spec 源变更后再生成的 references/_index.md) 不需要我审核吧

Because the governed question is answered by the register's own --test, the
exemption is inherited whole: a pure regeneration comes back exit 0 and is
ALLOWED before a single review is read. This guard must never be the thing
that re-closes a path the queue guard now clears.

Measured both ways on the incident's own file class (four
skills/*/references/_index.md, zero reviews):

  • toolchain present: register LIFTS them (--test exit 0, 1.2s) ⇒ hook ALLOWS.
  • toolchain absent: register fails closed (--test exit 3) ⇒ hook REFUSES,
    agreeing with the register rather than overriding it either way.

The self-test pins this as an agreement assertion, not a copied verdict —
see "What went wrong once" below.

The two feasibility premises, MEASURED

The 方向修订 named both as things to measure rather than assume. Both were
measured in this container class on 2026-09-01.

① The API read path. VERBATIM MEASUREMENT:

  • command -v gh → absent. There is no gh in this container class, so the
    gh pr merge spelling is defensive portability, not the live path here.
  • The repo-scoped REST channel is reachable. GET /repos/OWNER/REPO/pulls/N,
    .../files and .../reviews each answered HTTP 200, and answered 200
    again from a grandchild subprocess two levels down, which is the depth a
    hook runs at. Reachable for the sibling repo too: objectui PR reads answered
    200 (a nonexistent number answered 404, which is the control).
  • ⚠️ curl, NOT node's global fetch. GITHUB_TOKEN here is the 14-byte
    placeholder proxy-injected; the real credential is swapped in on the wire by
    the agent proxy. curl honours HTTPS_PROXY and gets 200. Node's fetch
    does not proxy by default, sends the placeholder verbatim, and gets 401
    measured side by side, same URL, same second. So the transport in the hook is
    curl, and node only ever receives already-fetched JSON as data. (The queue
    guard's own fetch-based readers are fine where they run: CI holds a real
    token.)
  • The PR is identified from the tool-call input: owner / repo /
    pullNumber for the MCP tools, and for Bash from the command text (gh pr merge with a number or a PR URL, gh api and curl against a
    /pulls/N/merge URL). gh pr merge with no PR argument names no PR and is
    allowed with a stated warning — the parse-confidently-or-allow line.

⇒ Premise ① holds. The hook half was NOT stopped.

② Fail-open on read failure. Implemented as the chosen semantic, and the
choice is stated where it can be re-examined: every branch the hook cannot
ANSWER — no token, an HTTP error, a truncated file list, an unparsable payload,
a predicate that will not run, an exception row with no checkout of the target
repo to recompute on — ALLOWS, with one warning line on stderr naming what could
not be read.

That is affordable for exactly one reason, and the header says so: the
correctness line is elsewhere. An unapproved governed PR that slips past this
hook still meets the queue guard on the merge_group build and is still
refused. Note this is the OPPOSITE of the queue guard's stance, deliberately —
that one is the last thing before main and fails closed; this one sits in
front of every enqueue attempt with a network dependency, and a guard that
blocks work whenever the API hiccups gets switched off, and then it guards
nothing.

Decision table

Every row is a self-test case in
.claude/hooks/guard-governed-enqueue.selftest.sh (49 cases, 0 failures).

situation verdict case name
governed, zero reviews REFUSE enable_pr_auto_merge on a governed PR with zero reviews · merge_pull_request on a governed PR with zero reviews
governed, authorized APPROVED pinned to current head allow governed + os-zhuang APPROVED at the current head
governed, approval pinned to an EARLIER head REFUSE a STALE approval (approved an earlier head) never counts
governed, APPROVED by an account outside the set REFUSE an APPROVED review from outside GOVERNED_APPROVERS never counts
governed, approval later DISMISSED REFUSE a later DISMISSED supersedes the same reviewer approval
pure regeneration only, zero reviews allow the hook agrees with the register about an exception-row candidate (+ a cleared predicate verdict exits before any review is read)
nothing governed in the diff allow an ordinary diff enqueues freely
Bash spellings of the same action same verdict gh pr merge N -R owner/repo · gh pr merge --auto --repo=... N · gh pr merge PR_URL · a REST PUT .../pulls/N/merge through curl · gh api -X PUT ... · reached through a separator
the command only MENTIONS a merge allow grep -n "gh pr merge" AGENTS.md · echo "never gh pr merge a governed PR"
unparseable / unidentifiable input allow a payload with no tool_name at all · an empty payload · an enqueue call naming no pull number · a non-numeric pull number · gh pr merge on the CURRENT branch (the PR is not named)
the API cannot be read allow + warn the API cannot be read at all · the fail-open names where correctness still lives
exception row, target repo has no checkout here allow + warn an exception-row path in a repo this container cannot resolve
deliberate exception allow OS_ALLOW_GOVERNED_ENQUEUE=1 on the blocking case
jq missing from PATH still REFUSES (no jq on PATH)

The refusal message is asserted, not just the exit code: it must carry
approve BEFORE enqueue, does NOT re-run on a later approval, the head sha
the approval has to pin, the governed hit, and the escape hatch.

Live end-to-end runs, against real PRs

Not just fixtures — the whole chain was exercised against the live API:

What went wrong once, recorded so it is not re-learned

The first revision of the self-test hard-coded expect allow against
.claude/workflows/docs-accuracy-audit.js, the one exception row that was cheap
to lift without a generator toolchain. That row was retired upstream the same
day
— the maintainer ruled that list off the governed surface entirely
(#13591, 「同意」) and its register row retired with it. The case then went red
over a register change the hook had nothing to do with.

The lesson is not "pick a more stable row". It is that a verdict copied out of
the register makes the matrix a second register — the exact shape the "⛔ do not
author a second mechanism" constraint exists to prevent, one level out. The case
now ASKS the register and requires the hook to answer the same way, whichever
way that is.

The prose half: STOPPED on a measured ceiling

The card also asks for one line in landing-operations.md's 落地前检 carrying
the three elements, cross-stating the pure-regeneration exemption. It is not
fundable, and the floors are byte-exact rather than estimated:

measurement value how
ceiling 80 lines check-skill-line-ratchet CEILINGS row
current count 80 lines same gate, headroom 0
reflow headroom 0 lines every block re-wrapped with the gate's OWN exported wrapLine at its own 120-byte cap reproduces the file exactly — the file is already optimally packed
shortest honest encoding 372 bytes → 4 lines the three elements + the exemption cross-statement + the hook's provenance, measured through the same wrapLine
fuller encoding 512 bytes → 5 lines same measurement
real deletion available ~70 bytes the one genuinely overlapping clause (漏判被队列守卫在 merge group 里拒收,白烧一轮队列), absorbable into the new sentence — under one line, so it funds 0 lines

⇒ 4 lines needed, 0 lines available, and no dead text to delete. Deleting live
operational text to fund an annotation would be a worse trade than stopping.
⛔ No ceiling was raised: this card carries no authorizing ruling, and raising
one requires a maintainer ruling quoted in the raising PR.

What is NOT lost. The three elements are written in full, with their
provenance, in the hook's own header — which is where the house pattern already
puts a guard's provenance (guard-shared-stash.sh carries its incident, its
alternatives and its exemptions the same way), and .claude/hooks/ deliberately
carries no ceiling. The seat also now meets the rule at the point of use: the
refusal message states the order, the reason, and the remedy at the moment the
enqueue is attempted, which is strictly better placement than a line in a file
that has to be recalled. What the stop leaves open is the protocol text's own
cross-reference; that is an annotation, not a defense, which is precisely the
demotion the 方向修订 ordered.

Registration

.claude/settings.json gains one PreToolUse entry matching
mcp__github__enable_pr_auto_merge|mcp__github__merge_pull_request, and the
hook is appended to the existing Bash chain. The self-test asserts the
registration exists and covers both MCP tool names — a hook that is not
registered guards nothing, and that failure is silent.

Sibling

The derivation-side half of this same guard — dispatch-gates.mjs listing
check-governed-queue-guard among the LOCAL gates, where it can only ever exit
1 outside a workflow run — is a separate card in another lane, #14004. It is not
addressed here and remains open; naming it so whoever implements either side
knows the other exists.

⛔ Also untouched, per the card's own limit: the queue guard's
dependency-install trade text, settled by its own card and only read here.

Verification

HEAD aa49e8385. The derived union was re-run after the last commit.

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands
over the 3-path change set, all exit codes captured before any pipe:

gate exit its own verdict line
check:doc-formula-expressions 0 ✓ … 9 @example(s) judged clean across 1095 packages/spec/src files
check:agent-test-spelling 0 ✓ check-agent-test-spelling: 0 violations — 428 file(s)
check:bash32-floor 0 ✓ … 26 tracked shell file(s) under scripts/**, .claude/hooks/**, .githooks/** name no bash 4+ construct
check:doc-authoring 0 ✓ doc authoring guard: 14317 customer-facing string(s) … clean
check:pm-governed-merges 0 ✓ check-governed-merges --self-test: 243 assertions
check:skill-frame-sync 0 ✓ … 4 copies of the decision frame are structurally isomorphic
check:nul-bytes 0 check-nul-bytes: OK (scanned 7729 text file(s) … no raw ASCII control bytes)
check:pm-skill-ratchet 0 ✓ check-skill-line-ratchet: … headroom 0 (added beyond the derived list: this PR makes a claim about that gate's numbers)

check:doc-formula-expressions first answered exit 3 — PREREQUISITE NOT MET,
which its own text says is "not a finding" and measures nothing. Building its
dependency closure (@objectstack/formula, then @objectstack/lint) turned it
into a real 0. Recorded because reading that 3 as a red gate is the mistake it
exists to prevent.

All five hook self-tests re-run as regression, 337 cases total:

  • guard-governed-enqueue 49 passed, 0 failed (new)
  • guard-main-checkout-bash 121 passed, 0 failed
  • guard-main-checkout 87 passed, 0 failed
  • guard-shared-stash 44 passed, 0 failed
  • guard-tree-enum 36 passed, 0 failed

Changeset: none — this publishes nothing from any package. The diff is three
.claude/** files. skip-changeset applied.

No skills/** file is in this diff, so the published-catalog budget reading
does not apply: the diff is .claude/hooks/ plus .claude/settings.json. The
published catalog appears only as a fixture path string inside the self-test.

Governed, and it walks its own rule

.claude/** ⇒ this PR is governed ⇒ draft only, human merge, review
requested from the authorized approvers. This PR is itself the shape the guard
describes, and the guard refuses its own enqueue until an approval is pinned to
its current head.

Generated by Claude Code


Generated by Claude Code

os-sam and others added 3 commits September 1, 2026 07:23
…n a PreToolUse hook

A governed PR was enqueued with zero approvals; the merge-queue guard refused as
designed, the entry popped out red, and the approval that arrived afterwards did
NOT re-run it. One queue cycle burned and a false red shown to the maintainer.

The queue guard stays the hard correctness line. This hook removes the wasted
cycle at the only place a machine can: our own tool-call surface. It intercepts
the enqueue-class calls, asks the two existing single sources -- the register's
`--test` predicate and `pinnedApprovalVerdict`/`GOVERNED_APPROVERS` -- and
refuses when the diff is governed and unpinned. No second mechanism, no second
path list, no second approver list. Pure regeneration clears untouched.

Fail-open on any unanswerable read, by design: the correctness line is elsewhere.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Msg17tAHJ3jVTYFgHydCm2
… not a copied verdict

The first revision hard-coded `expect allow` against the one exception row that
was cheap to lift. That row was retired upstream the same day -- its surface
left the governed fence entirely -- and the case went red over a register change
the hook had nothing to do with. A verdict copied out of the register makes the
matrix a second register.

It now asks the register and requires the hook to answer the same way, over the
incident's own file class: four `skills/*/references/_index.md`. Measured both
branches -- fail-closed with no toolchain (block, agreed) and lifted with the
toolchain present (allow with zero reviews, agreed).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Msg17tAHJ3jVTYFgHydCm2
@claude
claude Bot requested review from hotlong and os-zhuang September 1, 2026 07:32
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 1, 2026
@github-actions github-actions Bot added the size/l label Sep 1, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review September 1, 2026 07:33
@os-zhuang
os-zhuang enabled auto-merge September 1, 2026 07:33
@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Live self-demonstration: the guard refuses this PR's own enqueue

Run against the live API, no fixtures, at head aa49e8385 — this PR is governed (.claude/**) and carries no approval yet, which is the incident's exact shape:

$ printf '%s' '{"tool_name":"mcp__github__enable_pr_auto_merge","tool_input":{"owner":"objectstack-ai","repo":"objectstack","pullNumber":14140}}' \
    | .claude/hooks/guard-governed-enqueue.sh; echo "EXIT=$?"

⛔ Blocked: objectstack-ai/objectstack#14140 is GOVERNED and has no APPROVED review pinned to its current head —
   approve BEFORE enqueue; a failed queue entry does NOT re-run on a later approval.

   tool:      mcp__github__enable_pr_auto_merge
   head:      aa49e83850484de0f4e90d71c6dcde3cd70dc3ac
   governed:  .claude/hooks/guard-governed-enqueue.selftest.sh, .claude/hooks/guard-governed-enqueue.sh, .claude/settings.json
   approvals: reviews read: 0; authorized approvers (GOVERNED_APPROVERS): os-zhuang, hotlong

Enqueuing now does not fail safe, it fails EXPENSIVELY. The merge-queue guard
(scripts/pm/check-governed-queue-guard.mjs) reads the reviews on the merge_group
build and refuses, so nothing wrong lands — but GitHub does not retry that queue
entry when the approval arrives afterwards. The entry stays red, one queue cycle
is spent, and the red reads to the maintainer as "I approved it and it still
blocked me". That exact sequence was measured on 2026-09-01.

Do this instead:
  1. Leave the PR as a DRAFT and request review from an authorized approver.
  2. Wait for the APPROVED review to land on THIS head sha (aa49e83850484de0f4e90d71c6dcde3cd70dc3ac).
     A push after the approval unpins it — re-request, do not re-enqueue.
  3. Then enqueue. Or let the maintainer merge by hand: a human merge IS the
     review record for a governed PR, and it needs nothing from this guard.
  ⛔ Never approve a governed PR from an agent seat, under any account.

Pure regeneration is already exempt and never reaches this message: a diff whose
only governed paths byte-equal their own generator's output clears with zero
approvals, decided by the register (check-governed-merges.mjs), not here.

Verdict source: check-governed-merges.mjs --test (governed) +
pinnedApprovalVerdict/GOVERNED_APPROVERS from check-governed-queue-guard.mjs.
Deliberate exception (you know this one is right): OS_ALLOW_GOVERNED_ENQUEUE=1.
EXIT=2

The governed hits and the head sha are read live; the approver set is the imported GOVERNED_APPROVERS, not a copy. Once an authorized approval is pinned to the then-current head, the same call returns exit 0 — measured on #14070, which carries exactly such an approval and is allowed silently.

Generated by Claude Code


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants