Skip to content

Commit efb35b0

Browse files
os-samclaude
andcommitted
feat(devx): refuse a governed enqueue with no head-pinned approval, in 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
1 parent ec0c6c2 commit efb35b0

3 files changed

Lines changed: 887 additions & 0 deletions

File tree

Lines changed: 318 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,318 @@
1+
#!/usr/bin/env bash
2+
# Self-test for guard-governed-enqueue.sh — run it after touching that hook:
3+
#
4+
# .claude/hooks/guard-governed-enqueue.selftest.sh
5+
#
6+
# Feeds the hook the same JSON payload shape Claude Code delivers on PreToolUse
7+
# and asserts the block/allow verdict per case, plus the load-bearing sentences
8+
# of the refusal. Modelled on guard-shared-stash.selftest.sh; the two matrices
9+
# are kept in the same shape so neither drifts into its own idiom.
10+
#
11+
# NO NETWORK. The three GitHub reads come from `OS_GOVERNED_ENQUEUE_FIXTURE`
12+
# (documented in the hook's header as test-only injection): a directory holding
13+
# `pull.json` / `files.json` / `reviews.json`. What is NOT stubbed is the part
14+
# that matters — both predicates run for real, so this matrix fails if the hook
15+
# ever stops asking the register and the queue guard and starts deciding for
16+
# itself.
17+
#
18+
# Needs `jq` (to build fixtures) and `node` (the two real predicates run). No
19+
# pnpm install, no build: measured against a worktree with no `node_modules`.
20+
#
21+
# ⚠️ ONE COUPLING, STATED SO A FAILURE IS NOT MISREAD. The
22+
# `pure-regeneration-only ⇒ ALLOWED` case uses a REAL register-lifted path
23+
# (`.claude/workflows/docs-accuracy-audit.js`, the #9866 row) rather than a
24+
# stub, because the requirement under test is precisely "this guard must never
25+
# re-close the zero-approval path the register clears" and a stub cannot show
26+
# that. The consequence: the case needs that artifact to be in sync with its own
27+
# generator on the tree it runs against — which is what the required
28+
# `check:docs-audit-scope` gate keeps true. If this one case fails while the
29+
# rest pass, look there first; re-run
30+
# node scripts/pm/check-governed-merges.mjs --test .claude/workflows/docs-accuracy-audit.js
31+
# and read what the register says before touching this matrix or the hook.
32+
33+
set -uo pipefail
34+
35+
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
36+
hook="$here/guard-governed-enqueue.sh"
37+
repo_root="$(cd "$here/../.." && pwd)"
38+
pass=0
39+
fail=0
40+
41+
command -v jq >/dev/null 2>&1 || { echo "selftest needs jq to build payloads" >&2; exit 1; }
42+
command -v node >/dev/null 2>&1 || { echo "selftest needs node: both predicates run for real" >&2; exit 1; }
43+
[ -x "$hook" ] || { echo "hook is not executable: $hook" >&2; exit 1; }
44+
45+
HEAD_SHA=b25f061c6a1d4e2f3c9b8a7d6e5f4a3b2c1d0e9f
46+
OLD_SHA=0f9e8d7c6b5a4938271605f4e3d2c1b0a98877665
47+
48+
root="$(mktemp -d)"
49+
trap 'rm -rf "$root"' EXIT INT TERM
50+
51+
# fixture <name> <files-json> <reviews-json> -> prints the directory
52+
fixture() {
53+
local dir="$root/$1"
54+
mkdir -p "$dir"
55+
jq -nc --arg s "$HEAD_SHA" '{head:{sha:$s}}' > "$dir/pull.json"
56+
printf '%s' "$2" > "$dir/files.json"
57+
printf '%s' "$3" > "$dir/reviews.json"
58+
printf '%s' "$dir"
59+
}
60+
61+
files_of() { # files_of path... -> the /pulls/{n}/files body shape
62+
local out="[]" p
63+
for p in "$@"; do out="$(printf '%s' "$out" | jq -c --arg f "$p" '. + [{filename:$f}]')"; done
64+
printf '%s' "$out"
65+
}
66+
67+
approved_at() { # approved_at <login> <sha>
68+
jq -nc --arg l "$1" --arg c "$2" '[{state:"APPROVED",user:{login:$l},commit_id:$c}]'
69+
}
70+
71+
NO_REVIEWS='[]'
72+
GOVERNED_FILES="$(files_of AGENTS.md packages/spec/src/index.ts)"
73+
CLEAR_FILES="$(files_of packages/spec/src/index.ts README.md)"
74+
REGEN_FILES="$(files_of .claude/workflows/docs-accuracy-audit.js)"
75+
76+
F_UNAPPROVED="$(fixture governed-unapproved "$GOVERNED_FILES" "$NO_REVIEWS")"
77+
F_PINNED="$(fixture governed-pinned "$GOVERNED_FILES" "$(approved_at os-zhuang "$HEAD_SHA")")"
78+
F_STALE="$(fixture governed-stale "$GOVERNED_FILES" "$(approved_at os-zhuang "$OLD_SHA")")"
79+
F_OUTSIDER="$(fixture governed-outsider "$GOVERNED_FILES" "$(approved_at os-warren "$HEAD_SHA")")"
80+
F_DISMISSED="$(fixture governed-dismissed "$GOVERNED_FILES" \
81+
"$(jq -nc --arg c "$HEAD_SHA" '[{state:"APPROVED",user:{login:"os-zhuang"},commit_id:$c},{state:"DISMISSED",user:{login:"os-zhuang"},commit_id:$c}]')")"
82+
F_CLEAR="$(fixture not-governed "$CLEAR_FILES" "$NO_REVIEWS")"
83+
F_REGEN="$(fixture pure-regeneration "$REGEN_FILES" "$NO_REVIEWS")"
84+
F_EMPTY="$(fixture empty-diff '[]' "$NO_REVIEWS")"
85+
86+
mcp() { # mcp <tool> <pull> [owner] [repo]
87+
jq -nc --arg t "$1" --argjson n "$2" --arg o "${3:-objectstack-ai}" --arg r "${4:-objectstack}" \
88+
'{tool_name:$t,tool_input:{owner:$o,repo:$r,pullNumber:$n}}'
89+
}
90+
bash_call() { jq -nc --arg c "$1" '{tool_name:"Bash",tool_input:{command:$c}}'; }
91+
92+
AUTO=mcp__github__enable_pr_auto_merge
93+
MERGE=mcp__github__merge_pull_request
94+
95+
# The hook is the LAST element of the pipeline, so `$?` here is the HOOK's exit
96+
# status and not some downstream reader's. That is the only shape in which
97+
# reading a status after a pipe is safe, and it is why nothing is piped past it.
98+
run() { # run <payload> [env assignments…] -> allow | block | exitN
99+
local payload="$1"; shift
100+
local rc
101+
printf '%s' "$payload" | env "$@" "$hook" >/dev/null 2>&1
102+
rc=$?
103+
case "$rc" in
104+
0) printf 'allow' ;;
105+
2) printf 'block' ;;
106+
*) printf 'exit%s' "$rc" ;;
107+
esac
108+
}
109+
110+
stderr_of() { # stderr_of <payload> [env…]
111+
local payload="$1"; shift
112+
printf '%s' "$payload" | env "$@" "$hook" 2>&1 >/dev/null
113+
}
114+
115+
expect() { # expect <block|allow> <label> <payload> [env…]
116+
local want="$1" label="$2" payload="$3"; shift 3
117+
local got; got="$(run "$payload" "$@")"
118+
if [ "$got" = "$want" ]; then
119+
pass=$((pass + 1)); printf ' ok %-5s %s\n' "$got" "$label"
120+
else
121+
fail=$((fail + 1)); printf ' FAIL want=%s got=%s %s\n' "$want" "$got" "$label"
122+
fi
123+
}
124+
125+
expect_says() { # expect_says <needle> <label> <payload> [env…]
126+
local needle="$1" label="$2" payload="$3"; shift 3
127+
local out; out="$(stderr_of "$payload" "$@")"
128+
case "$out" in
129+
*"$needle"*) pass=$((pass + 1)); printf ' ok says %s\n' "$label" ;;
130+
*) fail=$((fail + 1)); printf ' FAIL missing "%s" %s\n' "$needle" "$label" ;;
131+
esac
132+
}
133+
134+
echo "== the incident's own shape: governed + no approval at all =="
135+
expect block 'enable_pr_auto_merge on a governed PR with zero reviews' \
136+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
137+
expect block 'merge_pull_request on a governed PR with zero reviews' \
138+
"$(mcp $MERGE 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
139+
140+
echo "== the refusal carries its one-line reason and its remedy =="
141+
expect_says 'approve BEFORE enqueue' 'the order is stated' \
142+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
143+
expect_says 'does NOT re-run on a later approval' 'the no-re-run reason is stated' \
144+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
145+
expect_says 'OS_ALLOW_GOVERNED_ENQUEUE=1' 'the deliberate exception is named' \
146+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
147+
expect_says "$HEAD_SHA" 'the head sha the approval must pin is named' \
148+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
149+
expect_says 'AGENTS.md' 'the governed hit is named' \
150+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
151+
152+
echo "== an AUTHORIZED approval PINNED to the current head is the pass =="
153+
expect allow 'governed + os-zhuang APPROVED at the current head' \
154+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_PINNED"
155+
156+
echo "== the three ways an approval does not count (pinnedApprovalVerdict, imported) =="
157+
expect block 'a STALE approval (approved an earlier head) never counts' \
158+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_STALE"
159+
expect block 'an APPROVED review from outside GOVERNED_APPROVERS never counts' \
160+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_OUTSIDER"
161+
expect block 'a later DISMISSED supersedes the same reviewer approval' \
162+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_DISMISSED"
163+
expect_says 'STALE approval' 'a stale approval is reported as stale, not as absent' \
164+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_STALE"
165+
expect_says 'outside the authorized set' 'an unauthorized approval is reported as such' \
166+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_OUTSIDER"
167+
168+
echo "== nothing governed in the diff: allowed, and no review is ever consulted =="
169+
expect allow 'an ordinary diff enqueues freely' \
170+
"$(mcp $AUTO 14070)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_CLEAR"
171+
172+
echo "== PURE REGENERATION clears with ZERO approvals — the register decides, not this hook =="
173+
# Real path, real lift (see the coupling note in this file's header): the #9866
174+
# row, byte-exact against its own generator recomputed on this tree. The hook
175+
# must never be what re-closes it (maintainer 2026-09-01: 纯生成的指针行 …
176+
# 不需要我审核吧).
177+
expect allow 'a governed-register path the generator certifies byte-exact, zero reviews' \
178+
"$(mcp $AUTO 14070)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_REGEN"
179+
180+
echo "== the Bash spellings reach the same decision =="
181+
expect block 'gh pr merge <n> -R owner/repo' \
182+
"$(bash_call 'gh pr merge 13794 -R objectstack-ai/objectstack --squash')" \
183+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
184+
expect block 'gh pr merge --auto --repo=owner/repo <n>' \
185+
"$(bash_call 'gh pr merge --auto --repo=objectstack-ai/objectstack 13794')" \
186+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
187+
expect block 'gh pr merge <html url>' \
188+
"$(bash_call 'gh pr merge https://github.com/objectstack-ai/objectstack/pull/13794 --squash')" \
189+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
190+
expect block 'a REST PUT .../pulls/<n>/merge through curl' \
191+
"$(bash_call 'curl -sS -X PUT https://api.github.com/repos/objectstack-ai/objectstack/pulls/13794/merge -d "{}"')" \
192+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
193+
expect block 'gh api -X PUT /repos/o/r/pulls/<n>/merge' \
194+
"$(bash_call 'gh api -X PUT /repos/objectstack-ai/objectstack/pulls/13794/merge')" \
195+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
196+
expect block 'reached through a separator' \
197+
"$(bash_call 'git fetch origin main && gh pr merge 13794 -R objectstack-ai/objectstack')" \
198+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
199+
expect allow 'the same Bash spelling on an approved PR' \
200+
"$(bash_call 'gh pr merge 13794 -R objectstack-ai/objectstack')" \
201+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_PINNED"
202+
203+
echo "== an UNQUOTED \\\" opens no quote, so the merge behind it is still seen =="
204+
# The #11738 class, carried by all three Bash-reading guards in this directory:
205+
# segmentation used to read the escaped `\"` as OPENING a region that never
206+
# closed, every separator behind it went inert, and the real command rode
207+
# through as an argument of something harmless.
208+
expect block 'echo \" ; gh pr merge <n>' \
209+
"$(bash_call 'echo \" ; gh pr merge 13794 -R objectstack-ai/objectstack')" \
210+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
211+
expect allow 'echo \" ; git status (precision twin: no manufactured block)' \
212+
"$(bash_call 'echo \" ; git status')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
213+
214+
echo "== writing ABOUT the ban must not trip the ban =="
215+
expect allow 'grep -n "gh pr merge" AGENTS.md' \
216+
"$(bash_call 'grep -n "gh pr merge" AGENTS.md')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
217+
expect allow 'echo "never gh pr merge a governed PR"' \
218+
"$(bash_call 'echo "never gh pr merge a governed PR"')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
219+
220+
echo "== unrelated tools and commands are untouched =="
221+
expect allow 'a Bash command that enqueues nothing' \
222+
"$(bash_call 'pnpm --filter @objectstack/spec test')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
223+
expect allow 'a non-enqueue MCP tool' \
224+
"$(jq -nc '{tool_name:"mcp__github__create_pull_request",tool_input:{owner:"objectstack-ai",repo:"objectstack",draft:true}}')" \
225+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
226+
expect allow 'an Edit call' \
227+
"$(jq -nc '{tool_name:"Edit",tool_input:{file_path:"AGENTS.md"}}')" \
228+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
229+
230+
echo "== parse-confidently-or-allow =="
231+
expect allow 'a payload with no tool_name at all' '{}' "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
232+
expect allow 'an empty payload' '' "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
233+
expect allow 'an enqueue call naming no pull number' \
234+
"$(jq -nc --arg t "$AUTO" '{tool_name:$t,tool_input:{owner:"objectstack-ai",repo:"objectstack"}}')" \
235+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
236+
expect allow 'a non-numeric pull number' \
237+
"$(jq -nc --arg t "$AUTO" '{tool_name:$t,tool_input:{owner:"objectstack-ai",repo:"objectstack",pullNumber:"nope"}}')" \
238+
"OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
239+
expect allow 'gh pr merge on the CURRENT branch (the PR is not named)' \
240+
"$(bash_call 'gh pr merge --auto --squash')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
241+
expect_says 'names no pull request' 'the unidentifiable form says why it was allowed' \
242+
"$(bash_call 'gh pr merge --auto --squash')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED"
243+
244+
echo "== FAIL-OPEN on a read failure, with the reason on stderr =="
245+
expect allow 'the API cannot be read at all' \
246+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" OS_GOVERNED_ENQUEUE_READFAIL=1
247+
expect_says 'ALLOWING' 'the fail-open says it is allowing' \
248+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" OS_GOVERNED_ENQUEUE_READFAIL=1
249+
expect_says 'merge-queue guard remains the hard line' 'the fail-open names where correctness still lives' \
250+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" OS_GOVERNED_ENQUEUE_READFAIL=1
251+
expect allow 'a PR reporting no changed files is not a governed answer' \
252+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_EMPTY"
253+
254+
echo "== a generated-exception row on a repo with no checkout to recompute against =="
255+
# objectstack-ai/cloud has no sibling checkout here, so the register cannot
256+
# recompute the row's provenance on the RIGHT tree. Judging one repo's paths
257+
# against another's files would be worse than not answering: fail open, say so.
258+
expect allow 'an exception-row path in a repo this container cannot resolve' \
259+
"$(mcp $AUTO 999 objectstack-ai cloud)" \
260+
"OS_GOVERNED_ENQUEUE_FIXTURE=$(fixture cross-repo-regen "$(files_of skills/objectstack-spec/references/_index.md)" "$NO_REVIEWS")"
261+
262+
echo "== the deliberate exception switch =="
263+
expect allow 'OS_ALLOW_GOVERNED_ENQUEUE=1 on the blocking case' \
264+
"$(mcp $AUTO 13794)" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" OS_ALLOW_GOVERNED_ENQUEUE=1
265+
266+
echo "== the jq-less fallback still identifies the call and still refuses =="
267+
nojq="$(mktemp -d)"
268+
# Every external the hook reaches for, MINUS jq. A missing one here reads as a
269+
# fail-open ("could not read …"), which is the hook behaving correctly on a
270+
# broken PATH — so keep this list complete or the case tests the harness.
271+
for b in bash env cat sed head tail grep printf node git curl mktemp rm tr cut wc dirname; do
272+
p="$(command -v "$b")" && ln -s "$p" "$nojq/$b" 2>/dev/null
273+
done
274+
printf '%s' "$(mcp $AUTO 13794)" \
275+
| env "PATH=$nojq" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" "$hook" >/dev/null 2>&1
276+
case "$?" in
277+
2) pass=$((pass + 1)); printf ' ok block (no jq on PATH)\n' ;;
278+
0) fail=$((fail + 1)); printf ' FAIL no-jq fallback ALLOWED a governed unapproved enqueue\n' ;;
279+
*) fail=$((fail + 1)); printf ' FAIL no-jq fallback exit%s\n' "$?" ;;
280+
esac
281+
rm -rf "$nojq"
282+
283+
echo "== the two predicates are the imported ones, not a local copy =="
284+
# A restatement of either predicate inside the hook is the failure this asserts
285+
# against: grep the hook for a second path list or a second approver list.
286+
if grep -q 'check-governed-merges.mjs' "$hook" && grep -q 'pinnedApprovalVerdict' "$hook"; then
287+
pass=$((pass + 1)); printf ' ok wired both single sources are invoked by name\n'
288+
else
289+
fail=$((fail + 1)); printf ' FAIL the hook no longer invokes both single sources\n'
290+
fi
291+
if grep -qE "os-zhuang|hotlong" "$hook"; then
292+
fail=$((fail + 1)); printf ' FAIL the hook spells out an approver login: GOVERNED_APPROVERS is the single source\n'
293+
else
294+
pass=$((pass + 1)); printf ' ok wired no approver login is spelled out in the hook\n'
295+
fi
296+
if grep -vE '^[[:space:]]*#' "$hook" | grep -qE 'AGENTS\.md|CLAUDE\.md|skills/|docs/adr/'; then
297+
fail=$((fail + 1)); printf ' FAIL a governed-surface path literal appears in the hook CODE: that is a second register\n'
298+
else
299+
pass=$((pass + 1)); printf ' ok wired no governed-path literal outside the header comments\n'
300+
fi
301+
302+
echo "== the hook is registered where Claude Code will actually run it =="
303+
settings="$repo_root/.claude/settings.json"
304+
if [ -f "$settings" ] && grep -q 'guard-governed-enqueue.sh' "$settings"; then
305+
pass=$((pass + 1)); printf ' ok wired .claude/settings.json registers the hook\n'
306+
else
307+
fail=$((fail + 1)); printf ' FAIL .claude/settings.json does not register the hook — it would guard nothing\n'
308+
fi
309+
for m in mcp__github__enable_pr_auto_merge mcp__github__merge_pull_request; do
310+
if [ -f "$settings" ] && grep -q "$m" "$settings"; then
311+
pass=$((pass + 1)); printf ' ok wired matcher covers %s\n' "$m"
312+
else
313+
fail=$((fail + 1)); printf ' FAIL no PreToolUse matcher covers %s\n' "$m"
314+
fi
315+
done
316+
317+
printf '\n%s passed, %s failed\n' "$pass" "$fail"
318+
[ "$fail" -eq 0 ]

0 commit comments

Comments
 (0)