Skip to content

Commit 6530f75

Browse files
claude[bot]claude
andauthored
fix(pm): an authorised APPROVED review satisfies the queue guard on any commit (2026-09-04 ruling) (#15203)
* fix(pm): an authorised APPROVED review satisfies the queue guard on any commit The maintainer ruled on 2026-09-04, in the live PM chat, while a governed PR that an authorised approver had approved three times kept falling out of the merge queue (verbatim, untranslated): 你的门禁有问题,只需要有人工批准记录就行,不需要卡最新的提交。 So the `merge_group` leg of `check-governed-queue-guard.mjs` now passes a governed PR iff an account in `GOVERNED_APPROVERS` holds a latest-decisive APPROVED review — on ANY commit, with `commit_id` unread by the decision. This supersedes the sha-pin half of the 2026-08-27 predicate and nothing else: that ruling's approver set stands, DISMISSED and superseded approvals still never count, an unauthorised APPROVED still never counts, and an empty or unreadable review list still fails closed. The accepted cost is 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. It is the same shape of cost the 2026-08-12 ruling on the retired per-PR gate accepted when it dropped the identity proxy. `staleApprovers` is kept as a PRINTED READING and nothing more — authorised approvals given on a commit that is no longer the head, which are also in `approvers` because they count. A cleared queue log therefore names the commit each approval was given on, so the accepted cost is visible at the moment it is paid. The PR head is still read for that line but decides nothing, so an unreadable head is no longer a refusal: a read that decides nothing may not block a landing. The review list, which IS the predicate's input, still refuses. The `pull_request` early-warning leg, the path test, the exit codes, the check-run name, `GOVERNED_APPROVERS` and the generated-surface register are all untouched, and the early-warning leg's rendered output stays byte-identical. Self-test: 133 cases pass. The cases that pinned the sha pin now pin the ruling in both directions — an authorised approval on an OLDER commit passes, while dismissed, superseded, unauthorised and empty still refuse. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox * fix(agents,pm): carry the 2026-09-04 unpinned-approval ruling into the governed surfaces The guard half landed in the previous commit; this is the half that lives in governed files, and the two only work together. Without it the CI job "Claude hook guard self-tests" is red on main, because the seat-side enqueue hook's self-test pinned the sha pin the maintainer retired. `.claude/hooks/guard-governed-enqueue.selftest.sh` — the fixture that was `governed-stale` is now `governed-approved-on-an-earlier-commit`, and the case that asserted it BLOCKS asserts it ALLOWS. It is flipped rather than deleted on purpose: the hook holds no predicate of its own, so that direction arrives entirely through the imported predicate, and flipping the guard back would flip this case red. Adds `expect_lacks`, because a retired RULE leaves its sentence behind in the text a seat reads long after the predicate stopped enforcing it, and only an assertion that a phrase is ABSENT can catch that. `.claude/hooks/guard-governed-enqueue.sh` — the blocking message no longer tells a seat that a push unpins the approval; it quotes the ruling instead. The `approvalsOnEarlierCommits` line is dropped from the rendered detail, because since the ruling that bucket can only be non-empty when the verdict is `approved`, and the detail string is only ever printed on the blocking path. Both renames land here, with their only importer, in one commit: `pinnedApprovalVerdict` becomes `authorizedApprovalVerdict` and `staleApprovers` becomes `approvalsOnEarlierCommits`. A bucket still named for a refusal is one a later reader re-derives a refusal from. Renaming either without this hook in the same diff would have turned the hook into a silent fail-open: it catches the failure and ALLOWS, one warning line on stderr, every governed enqueue through. `AGENTS.md` and `.claude/skills/pm-dispatch/SKILL.md` — the two prose sites that stated the retired pin as normative now state the ruled predicate, with the ruling quoted verbatim and the accepted cost named once. The skill rewrite is line-for-line so the line ratchet is unmoved. The sentence that enumerates the governed surfaces is untouched. Hook self-test: 50 passed, 0 failed. Guard self-test: 133 cases pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox * style(agents,pm): re-wrap the rewritten governed-landing paragraphs to the 120-byte line budget `check:pm-skill-ratchet` prices a per-session token read by counting lines, so a long line is an unmetered tax. The rewrite in the previous commit put AGENTS.md L313 at 136B and two SKILL.md lines at 121B and 164B. Re-wrapped at legal break points with the content otherwise unchanged, and the verbatim ruling kept whole on one line in both files — a soft line break inside 「」 renders as a space, and a quotation with a space in it is a quotation that was edited. AGENTS.md is 1161 lines against its 1162 ceiling; the pm-dispatch skill is 971 against 1005. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8ab926b commit 6530f75

5 files changed

Lines changed: 333 additions & 163 deletions

File tree

.claude/hooks/guard-governed-enqueue.selftest.sh

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ REGEN_FILES="$(files_of $REGEN_PATHS)"
7979

8080
F_UNAPPROVED="$(fixture governed-unapproved "$GOVERNED_FILES" "$NO_REVIEWS")"
8181
F_PINNED="$(fixture governed-pinned "$GOVERNED_FILES" "$(approved_at os-zhuang "$HEAD_SHA")")"
82-
F_STALE="$(fixture governed-stale "$GOVERNED_FILES" "$(approved_at os-zhuang "$OLD_SHA")")"
82+
F_OLDER="$(fixture governed-approved-on-an-earlier-commit "$GOVERNED_FILES" "$(approved_at os-zhuang "$OLD_SHA")")"
8383
F_OUTSIDER="$(fixture governed-outsider "$GOVERNED_FILES" "$(approved_at os-warren "$HEAD_SHA")")"
8484
F_DISMISSED="$(fixture governed-dismissed "$GOVERNED_FILES" \
8585
"$(jq -nc --arg c "$HEAD_SHA" '[{state:"APPROVED",user:{login:"os-zhuang"},commit_id:$c},{state:"DISMISSED",user:{login:"os-zhuang"},commit_id:$c}]')")"
@@ -135,6 +135,19 @@ expect_says() { # expect_says <needle> <label> <payload> [env…]
135135
esac
136136
}
137137

138+
# The other direction, and it earns its place: a retired RULE leaves its
139+
# sentence behind in the text a seat actually reads, long after the predicate
140+
# stopped enforcing it. `expect_says` cannot catch that — only an assertion that
141+
# a phrase is ABSENT can.
142+
expect_lacks() { # expect_lacks <needle> <label> <payload> [env…]
143+
local needle="$1" label="$2" payload="$3"; shift 3
144+
local out; out="$(stderr_of "$payload" "$@")"
145+
case "$out" in
146+
*"$needle"*) fail=$((fail + 1)); printf ' FAIL still says "%s" %s\n' "$needle" "$label" ;;
147+
*) pass=$((pass + 1)); printf ' ok lacks %s\n' "$label" ;;
148+
esac
149+
}
150+
138151
echo "== the incident's own shape: governed + no approval at all =="
139152
expect block 'enable_pr_auto_merge on a governed PR with zero reviews' \
140153
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
@@ -148,24 +161,31 @@ expect_says 'does NOT re-run on a later approval' 'the no-re-run reason is state
148161
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
149162
expect_says 'OS_ALLOW_GOVERNED_ENQUEUE=1' 'the deliberate exception is named' \
150163
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
151-
expect_says "$HEAD_SHA" 'the head sha the approval must pin is named' \
164+
expect_says "$HEAD_SHA" 'the current head sha is named so the reader knows which PR state this is' \
165+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
166+
expect_says 'does NOT have to sit on the' 'the remedy states the 2026-09-04 predicate, not the retired sha pin' \
167+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
168+
expect_lacks 'unpins it' 'the retired advice (a push unpins the approval) is gone from the refusal' \
152169
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
153170
expect_says 'AGENTS.md' 'the governed hit is named' \
154171
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
155172

156-
echo "== an AUTHORIZED approval PINNED to the current head is the pass =="
173+
echo "== an AUTHORIZED APPROVED review is the pass, on the head or on any commit =="
157174
expect allow 'governed + os-zhuang APPROVED at the current head' \
158175
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_PINNED"
159-
160-
echo "== the three ways an approval does not count (pinnedApprovalVerdict, imported) =="
161-
expect block 'a STALE approval (approved an earlier head) never counts' \
162-
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_STALE"
176+
# ⭐ 2026-09-04, verbatim: 只需要有人工批准记录就行,不需要卡最新的提交。 Under the
177+
# retired sha pin this SAME fixture was the block case, which is exactly why it
178+
# is pinned in the ruled direction here rather than deleted. The hook holds no
179+
# predicate of its own, so this direction arrives entirely through the imported
180+
# `authorizedApprovalVerdict` — flipping it back there flips this case red.
181+
expect allow 'governed + os-zhuang APPROVED on an EARLIER commit still counts' \
182+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_OLDER"
183+
184+
echo "== the two ways an approval does not count (authorizedApprovalVerdict, imported) =="
163185
expect block 'an APPROVED review from outside GOVERNED_APPROVERS never counts' \
164186
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_OUTSIDER"
165187
expect block 'a later DISMISSED supersedes the same reviewer approval' \
166188
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_DISMISSED"
167-
expect_says 'STALE approval' 'a stale approval is reported as stale, not as absent' \
168-
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_STALE"
169189
expect_says 'outside the authorized set' 'an unauthorized approval is reported as such' \
170190
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_OUTSIDER"
171191

@@ -321,7 +341,7 @@ rm -rf "$nojq"
321341
echo "== the two predicates are the imported ones, not a local copy =="
322342
# A restatement of either predicate inside the hook is the failure this asserts
323343
# against: grep the hook for a second path list or a second approver list.
324-
if grep -q 'check-governed-merges.mjs' "$hook" && grep -q 'pinnedApprovalVerdict' "$hook"; then
344+
if grep -q 'check-governed-merges.mjs' "$hook" && grep -q 'authorizedApprovalVerdict' "$hook"; then
325345
pass=$((pass + 1)); printf ' ok wired both single sources are invoked by name\n'
326346
else
327347
fail=$((fail + 1)); printf ' FAIL the hook no longer invokes both single sources\n'

.claude/hooks/guard-governed-enqueue.sh

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,12 @@
5252
# the very predicate a seat runs before flipping ready. Exit 3 = governed,
5353
# exit 0 = clear. Its register (`GOVERNED_SURFACES`) is repo-agnostic, so
5454
# the same call answers for every repo configured in `GOVERNED_REPOS`.
55-
# 2. "is it approved, pinned?" → `pinnedApprovalVerdict` +
56-
# `GOVERNED_APPROVERS` imported from `check-governed-queue-guard.mjs` —
55+
# 2. "is it approved by an authorized account?" → `authorizedApprovalVerdict`
56+
# + `GOVERNED_APPROVERS` imported from `check-governed-queue-guard.mjs` —
5757
# the same function the queue build decides on, applied to the same review
58-
# list shape. The authorized set is never spelled out here.
58+
# list shape. The authorized set is never spelled out here, and neither is
59+
# the rule: the 2026-09-04 ruling (an authorized approval counts on ANY
60+
# commit) reached this hook by changing that function and nothing in here.
5961
#
6062
# A register row or a ruling that moves either predicate reaches this guard for
6163
# free, and the two tools cannot answer differently about the same diff — the
@@ -475,7 +477,7 @@ fi
475477

476478
hits="$(json_read "$work/test.json" hits)"
477479

478-
# ── predicate 2: an authorized approval pinned to THIS head ──────────────────
480+
# ── predicate 2: an authorized APPROVED review, on any commit (2026-09-04) ───
479481

480482
api_get "/repos/$owner/$repo/pulls/$pull/reviews?per_page=100" "$work/reviews.json" \
481483
|| { warn "could not read the reviews of $pr_ref"; exit 0; }
@@ -492,14 +494,15 @@ const { OS_GUARD_MODULE: modPath, OS_GUARD_REVIEWS: reviewsPath, OS_GUARD_HEAD:
492494
const { readFileSync } = await import("node:fs");
493495
const m = await import(modPath);
494496
const reviews = JSON.parse(readFileSync(reviewsPath, "utf8"));
495-
const v = m.pinnedApprovalVerdict(reviews, headSha);
497+
const v = m.authorizedApprovalVerdict(reviews, headSha);
496498
// Line 1 is the verdict word the shell branches on; line 2 is the human detail.
497499
// Rendered HERE rather than in the shell so there is one reading of this object.
500+
// ⚠️ `v.approvalsOnEarlierCommits` is deliberately NOT rendered here. Since
501+
// 2026-09-04 it can only be non-empty when the verdict is `approved`, and this
502+
// string is only ever printed on the blocking path below — so a line for it
503+
// would be dead code that reads like a live refusal reason.
498504
const detail = [
499505
`reviews read: ${v.reviewsRead}`,
500-
v.staleApprovers.length
501-
? `STALE approval(s), pinned to an earlier head: ${v.staleApprovers.map((s) => `${s.login}@${String(s.commitId).slice(0, 9)}`).join(", ")}`
502-
: null,
503506
v.unauthorizedApprovers.length
504507
? `APPROVED by account(s) outside the authorized set (never counts): ${v.unauthorizedApprovers.join(", ")}`
505508
: null,
@@ -512,7 +515,7 @@ console.log(detail);
512515
verdict_rc=$?
513516

514517
if [ "$verdict_rc" -ne 0 ] || [ -z "$verdict" ]; then
515-
warn "the pinned-approval predicate did not run: $(tr '\n' ' ' < "$work/verdict.err" | cut -c1-200)"
518+
warn "the authorized-approval predicate did not run: $(tr '\n' ' ' < "$work/verdict.err" | cut -c1-200)"
516519
exit 0
517520
fi
518521

@@ -522,7 +525,7 @@ detail="$(printf '%s\n' "$verdict" | tail -n +2)"
522525
[ "$state" = "approved" ] && exit 0
523526

524527
cat >&2 <<EOF
525-
⛔ Blocked: $pr_ref is GOVERNED and has no APPROVED review pinned to its current head
528+
⛔ Blocked: $pr_ref is GOVERNED and has no APPROVED review from an authorized approver
526529
approve BEFORE enqueue; a failed queue entry does NOT re-run on a later approval.
527530
528531
tool: $tool
@@ -539,8 +542,9 @@ blocked me". That exact sequence was measured on 2026-09-01.
539542
540543
Do this instead:
541544
1. Leave the PR as a DRAFT and request review from an authorized approver.
542-
2. Wait for the APPROVED review to land on THIS head sha ($head_sha).
543-
A push after the approval unpins it — re-request, do not re-enqueue.
545+
2. Wait for the APPROVED review to land. It does NOT have to sit on the
546+
current head ($head_sha), and a later push does not expire it — maintainer,
547+
2026-09-04: 只需要有人工批准记录就行,不需要卡最新的提交。
544548
3. Then enqueue. Or let the maintainer merge by hand: a human merge IS the
545549
review record for a governed PR, and it needs nothing from this guard.
546550
⛔ Never approve a governed PR from an agent seat, under any account.
@@ -550,7 +554,7 @@ only governed paths byte-equal their own generator's output clears with zero
550554
approvals, decided by the register (check-governed-merges.mjs), not here.
551555
552556
Verdict source: check-governed-merges.mjs --test (governed) +
553-
pinnedApprovalVerdict/GOVERNED_APPROVERS from check-governed-queue-guard.mjs.
557+
authorizedApprovalVerdict/GOVERNED_APPROVERS from check-governed-queue-guard.mjs.
554558
Deliberate exception (you know this one is right): OS_ALLOW_GOVERNED_ENQUEUE=1.
555559
EOF
556560
exit 2

.claude/skills/pm-dispatch/SKILL.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -724,10 +724,11 @@ dev**:先试 SendMessage 复活,不可用才走接手协议(四条增量见 runb
724724
(objectos 指令面 PR 照样 draft/人工合并)。路径面**一条命中** ⇒ ACCEPT 换终局四件套:
725725
① 复核结论照常写在 issue
726726
上(不能合 ≠ 不复核;技能面 PR 的复核席须跑在契约复审档位,档位单源见条款②闸门);② PR
727-
留给维护者,**看得见地悬着**,终局两条:**人工直合即审核记录**(兜底);或**授权人工批准钉
728-
head ⇒ 队列放行**(2026-08-27 裁定)(批准人集合与判定单源
729-
= 队列守卫常量 `GOVERNED_APPROVERS`(`scripts/pm/check-governed-queue-guard.mjs`):APPROVED`commit_id`
730-
= PR 当前 head 才算,批准后再推提交即过期转红)。未钉批准 ⛔ 不翻 ready、不入队、不挂
727+
留给维护者,**看得见地悬着**,终局两条:**人工直合即审核记录**(兜底);或**授权人工批准
728+
⇒ 队列放行**(2026-09-04 裁定,逐字「只需要有人工批准记录就行,不需要卡最新的提交。」
729+
——取代 2026-08-27 的钉 head 半边,批准人集合不变)(判定单源 = 队列守卫常量 `GOVERNED_APPROVERS`
730+
(`scripts/pm/check-governed-queue-guard.mjs`):授权账户 APPROVED 即算,⛔ 不卡 `commit_id`,
731+
批准后再推提交也不过期)。无批准 ⛔ 不翻 ready、不入队、不挂
731732
auto-merge;③ 在 draft PR 上**向两个授权批准账户 `os-zhuang``hotlong` 都 request review**
732733
(2026-08-27 裁定)—— 主动推,永不等被发现;通道出处(2026-08-19 裁定),
733734
当日实测推送到达(「推送到了」);

AGENTS.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,12 +307,13 @@ localStorage / auth gotchas.
307307
governed surface, ⛔ never merge it, ⛔ never add it to the merge queue, ⛔ never call `enable_pr_auto_merge`,
308308
⛔ never flip it out of draft to make any of those possible. Judge it on the PR's **file list**, not on its
309309
description, and a **mixed diff is not a proportion question** — one path hit is enough; if the rest needs to
310-
land, split the governed files into their own PR. **Those four lift only for an authorized approval pinned to the
311-
current head** (2026-08-27; guard 2026-08-28): APPROVED, by an account in `GOVERNED_APPROVERS`
312-
(`scripts/pm/check-governed-queue-guard.mjs`), `commit_id` = that sha — then the queue lands it, re-validating as
313-
it merges, and any later push expires it. Hand-authored governed content needs that approval; a PR whose only
314-
governed paths are register rows the queue leg regenerates byte-exact clears with zero approvals (2026-09-01) — an
315-
uncertified recompute, drift or a hand-authored sibling keeps it governed. Unpinned, the maintainer's bypass direct
310+
land, split the governed files into their own PR. **Those four lift only for an authorized APPROVED review** — by
311+
an account in `GOVERNED_APPROVERS` (`scripts/pm/check-governed-queue-guard.mjs`), on ANY commit; the queue then
312+
lands it. Approver set 2026-08-27; pin retired 2026-09-04:
313+
「只需要有人工批准记录就行,不需要卡最新的提交。」 A later push no longer expires it, and
314+
this gate does not re-review it. Hand-authored governed content needs that approval; a PR whose only governed paths
315+
are register rows the queue leg regenerates byte-exact clears with zero approvals (2026-09-01) — an uncertified
316+
recompute, drift or a hand-authored sibling keeps it governed. Unapproved, the bypass direct
316317
merge (人工直合) is the only landing. ⛔ **No agent seat submits an approving review on a governed-surface PR,
317318
under any account** — an authorized account is agent-operated too. Nothing else substitutes: under #13 an accepted
318319
ADR *is* the decision, so merging one is the act of adopting a governance position — the one class of change about

0 commit comments

Comments
 (0)