Skip to content

Commit c8bf50f

Browse files
yinlianghuiclaude
andauthored
fix(pm): stop the verify-lock VERDICT certifying a batch it cannot answer for (#12363)
`VERDICT command-exit N` is the line every dispatch brief tells a dev to quote as proof a suite passed. For `-c '<string>'` the string runs in a shell, and a shell's exit status is its last command's — so a failure in the MIDDLE of a batch was announced as `command-exit 0`. It failed in the green direction, on the one line a reviewer is instructed to trust. The run, the exit code and the wrapper's behaviour are unchanged; the WORD changes. A string whose exit can only be 0 if every part exited 0 (one command, or an `&&` chain) keeps `command-exit`. Anything else — `;`, a newline, a pipeline, `||`, `&` — prints `batch-last-exit`, which says on its own line that it is the last part's exit and names the repair. Aggregating per-command exits was rejected: this wrapper is handed one opaque string, so aggregating means parsing shell, and "worst" is undefined over exit codes that are not ordered by severity and include 99 = NOT MEASURED. Injecting `set -e`/an ERR trap was rejected for changing what the caller's command does, and for holes (a failure in a pipeline's non-final element never fires ERR). Refusing the batch outright was rejected on this file's own precedent: the declared-unlocked ruling prefers a loud declared degradation to a refusal, and a blanket refusal would block answerable work like `export CI=1; pnpm test`. Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 Co-authored-by: Claude <noreply@anthropic.com>
1 parent c48f9a0 commit c8bf50f

1 file changed

Lines changed: 298 additions & 4 deletions

File tree

scripts/pm/os-verify-lock.sh

Lines changed: 298 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,81 @@
125125
# exactly what they were.
126126
#
127127
# ---------------------------------------------------------------------------
128+
# WHAT `command-exit` CERTIFIES, AND WHY A BATCH GETS A DIFFERENT WORD
129+
#
130+
# Measured 2026-08-25 (#12288), against this wrapper, in this container:
131+
#
132+
# os-verify-lock.sh -c 'echo one; sh -c "exit 1"; echo three'
133+
# → VERDICT command-exit 0
134+
#
135+
# A failure in the MIDDLE of that batch is announced as a pass. Nothing is red
136+
# anywhere: the lock was held, the command ran, the verdict says 0. And
137+
# `VERDICT command-exit N` is the exact line every dispatch brief tells a dev to
138+
# quote as proof a suite passed -- so this instrument failed in the GREEN
139+
# direction, on the one line a reviewer is instructed to trust. Same family as
140+
# the restore-leg hazards (#11539, #11648, #12204): an instrument that reports
141+
# success while measuring the wrong thing.
142+
#
143+
# The mechanism is not a defect in the acquisition path. `-c '<string>'` runs
144+
# the string in a shell, and a shell's exit status is its LAST command's. The
145+
# wrapper reported the wrapped command's own exit code, exactly as the contract
146+
# at the top of this file says it does. What was wrong was the WORD:
147+
# `command-exit` reads as a verdict on everything the caller passed, and for a
148+
# `;`-sequenced string it is a verdict on the tail of it.
149+
#
150+
# The card offered three remedies. The two NOT taken are recorded here because
151+
# both look better than they are:
152+
#
153+
# AGGREGATE THE PER-COMMAND EXITS AND REPORT THE WORST -- rejected twice over.
154+
# This wrapper never sees per-command exits: it is handed ONE opaque string
155+
# and hands it to a shell, so aggregating means parsing shell, and a partial
156+
# shell parser that got `;` inside quotes or a `case` arm wrong would itself
157+
# be an instrument answering confidently and wrongly -- this card's own
158+
# defect, shipped by the fix for it. And "worst" is undefined: exit codes are
159+
# not ordered by severity (is 2 worse than 1? is 143 -- SIGTERM -- worse than
160+
# either?), and 99 here is not a failure at all but NOT MEASURED. A max() over
161+
# that set is a number with no meaning.
162+
#
163+
# MAKE THE STRING'S EXIT MEAN "ALL OF IT PASSED" -- inject `set -e`, or an ERR
164+
# trap, into the caller's string. Rejected: it changes what the caller's
165+
# command DOES (`set -e` truncates the batch at the first failure, so the rest
166+
# of a deliberate sequence never runs), and `set -e`'s own footguns -- an
167+
# arithmetic expression evaluating to 0, an assignment from a failing
168+
# substitution -- would turn correct green runs RED. An ERR trap avoids the
169+
# truncation but not the holes: a failure inside a pipeline's non-final
170+
# element never fires it, so it would certify the `pnpm test | tee log` case
171+
# while missing exactly the failure it claimed to catch. This wrapper does not
172+
# change the meaning of the command it is given.
173+
#
174+
# REFUSING THE BATCH OUTRIGHT (the card's remedy 3) was the closer call, and the
175+
# reason against it is precedent in this file rather than taste. Where this
176+
# wrapper cannot deliver its full guarantee, the maintainer's 2026-08-22 ruling
177+
# on declared unlocked mode chose a LOUD DECLARED DEGRADATION over a refusal
178+
# that leaves the agent with no route -- and the filter preflight below refuses
179+
# only what is CERTAINLY worthless (a command that will measure nothing at all),
180+
# fail-open in every other direction. A batch is not worthless: it runs real
181+
# verification, and only the one-line summary is uncertifiable. A blanket
182+
# refusal would also block correct work -- `-c 'export CI=1; pnpm --filter x
183+
# test'` is sequenced and perfectly answerable -- and this file's doctrine is
184+
# that a wrong refusal is worse than a miss.
185+
#
186+
# So: the run is unchanged, the exit code is unchanged, and the WORD changes.
187+
# A string whose exit status can only be 0 if every part of it exited 0 keeps
188+
# `command-exit`. Anything else prints `batch-last-exit`, which says on its own
189+
# line that it is the last part's exit and not a verdict on the whole, and names
190+
# the repair (`&&`). Neither line can be misquoted as the other -- which is what
191+
# makes the `--self-test` pin possible at all: a mid-batch failure must never
192+
# print `command-exit 0`.
193+
#
194+
# WHAT IS CERTIFIED, STATED NARROWLY SO IT CANNOT BE OVER-READ. `command-exit`
195+
# means: the string is one command, or a chain joined only by `&&`, so bash's
196+
# documented short-circuit semantics make a 0 impossible unless every element of
197+
# that chain exited 0. It does NOT mean no failure can hide anywhere. An element
198+
# that is itself a shell (`bash -c 'a; b'`), a subshell `(a; b)`, or a pipeline
199+
# inside an element still reports only its own last status, and this wrapper
200+
# cannot see inside it. That limit is declared here rather than papered over:
201+
# the whole defect being repaired was a line that claimed more than it measured.
202+
# ---------------------------------------------------------------------------
128203
# THE BASH 3.2 FLOOR, AND WHY A MISSING BUILTIN IS THE WORST FAILURE HERE
129204
#
130205
# `/usr/bin/env bash` is bash 3.2.57 on macOS. A bash 4+/5+ construct in this
@@ -219,6 +294,10 @@ SLOT_SLUG=""
219294
ARRIVAL_DEPTH=-1
220295
LEDGER_WRITTEN=0
221296
LABEL_FOR_LEDGER="?"
297+
# Which word this run's verdict may use. Decided once, from the command
298+
# string, before anything runs -- see the certification block at the top.
299+
VERDICT_WORD="command-exit"
300+
CERTIFIABLE_NOTE=""
222301

223302
log() { printf 'os-verify-lock: %s\n' "$*" >&2; }
224303

@@ -920,6 +999,131 @@ filter_preflight() {
920999
return 1
9211000
}
9221001

1002+
# --- can this exit code certify the whole command? ---------------------------
1003+
#
1004+
# Why this exists: see "WHAT `command-exit` CERTIFIES" at the top of this file.
1005+
# The question here is deliberately small, and it is the only one asked: can the
1006+
# exit status of the string we are about to hand to a shell be 0 ONLY IF every
1007+
# part of it succeeded? True for a single command, and for a chain joined by
1008+
# `&&` -- which short-circuits, so a 0 at the end proves every link ran and
1009+
# passed. False for `;` and for a newline (the exit is the tail's), for `|` (the
1010+
# exit is the right-hand end's, so `pnpm test | tee log` is green when the test
1011+
# is red), for `||` (the exit is the fallback's, which is the point of `||`),
1012+
# and for `&`.
1013+
#
1014+
# It is a SCANNER, not a shell parser, and that difference is what makes it
1015+
# trustworthy rather than clever. It tracks three states of quoting and a
1016+
# parenthesis depth, decides one question, and can only ever err toward
1017+
# `batch-last-exit`: over-labelling costs a warning on a run nobody had to act
1018+
# on, while under-labelling would be the very defect this repairs. So anything
1019+
# it cannot read with confidence -- a backtick substitution, an unbalanced quote
1020+
# -- comes out uncertified by construction rather than by analysis.
1021+
#
1022+
# Operators count only at parenthesis depth 0 and outside quotes, and that is
1023+
# correctness rather than leniency: a `;` inside `$(echo a; echo b)`, or inside
1024+
# a quoted argument, belongs to one ELEMENT of the chain and not to the chain,
1025+
# and the element's own exit status is what the chain sees either way.
1026+
#
1027+
# Sets CERTIFIABLE_NOTE to the reason when it answers no; returns 0 for yes.
1028+
exit_certifiable() {
1029+
local kind="$1" s="$2" n i ch nx sq=0 dq=0 depth=0
1030+
CERTIFIABLE_NOTE=""
1031+
1032+
# `-- argv` never reaches a shell: one command, its own exit, nothing to scan.
1033+
[[ "$kind" == argv ]] && return 0
1034+
1035+
n=${#s}
1036+
i=0
1037+
while ((i < n)); do
1038+
ch="${s:i:1}"
1039+
1040+
# Inside single quotes nothing is special but the closing quote.
1041+
if ((sq == 1)); then
1042+
[[ "$ch" == "'" ]] && sq=0
1043+
i=$((i + 1))
1044+
continue
1045+
fi
1046+
# Inside double quotes a backslash still escapes, so it is honoured here or
1047+
# a `\"` would be read as the end of the string.
1048+
if ((dq == 1)); then
1049+
case "$ch" in
1050+
'\') i=$((i + 2)) && continue ;;
1051+
'"') dq=0 ;;
1052+
esac
1053+
i=$((i + 1))
1054+
continue
1055+
fi
1056+
1057+
case "$ch" in
1058+
"'") sq=1 ;;
1059+
'"') dq=1 ;;
1060+
'\') i=$((i + 2)) && continue ;;
1061+
'`')
1062+
CERTIFIABLE_NOTE="it contains a backtick substitution, which this scanner does not read"
1063+
return 1
1064+
;;
1065+
'(') depth=$((depth + 1)) ;;
1066+
')') ((depth > 0)) && depth=$((depth - 1)) ;;
1067+
';')
1068+
((depth == 0)) && {
1069+
CERTIFIABLE_NOTE="its parts are sequenced with ';'"
1070+
return 1
1071+
}
1072+
;;
1073+
$'\n')
1074+
((depth == 0)) && {
1075+
CERTIFIABLE_NOTE="its parts are on separate lines"
1076+
return 1
1077+
}
1078+
;;
1079+
'&')
1080+
# `&&` is the one joiner that certifies, so it is consumed, not flagged.
1081+
nx="${s:i+1:1}"
1082+
if [[ "$nx" == '&' ]]; then
1083+
i=$((i + 2))
1084+
continue
1085+
fi
1086+
((depth == 0)) && {
1087+
CERTIFIABLE_NOTE="a part of it is backgrounded with '&'"
1088+
return 1
1089+
}
1090+
;;
1091+
'|')
1092+
if ((depth == 0)); then
1093+
nx="${s:i+1:1}"
1094+
if [[ "$nx" == '|' ]]; then
1095+
CERTIFIABLE_NOTE="its parts are joined with '||', so a 0 can be the fallback's"
1096+
else
1097+
CERTIFIABLE_NOTE="it is a pipeline, so the exit is the RIGHT-HAND end's"
1098+
fi
1099+
return 1
1100+
fi
1101+
;;
1102+
esac
1103+
i=$((i + 1))
1104+
done
1105+
1106+
# A string this scanner did not read to the end is not one it may certify.
1107+
if ((sq == 1 || dq == 1)); then
1108+
CERTIFIABLE_NOTE="it has an unbalanced quote, which this scanner does not read"
1109+
return 1
1110+
fi
1111+
return 0
1112+
}
1113+
1114+
# The ONE place the verdict word and its caveat become text, so the locked and
1115+
# the unlocked verdict lines cannot drift apart -- and so the caveat travels
1116+
# with the number rather than sitting on a line above it that a quote would
1117+
# leave behind.
1118+
verdict_head() {
1119+
if [[ "$VERDICT_WORD" == command-exit ]]; then
1120+
printf 'VERDICT command-exit %s' "$1"
1121+
return 0
1122+
fi
1123+
printf "VERDICT batch-last-exit %s · ⚠ NOT A VERDICT ON THE WHOLE COMMAND — %s, so this number is the LAST part's exit and a failure in an earlier part is NOT in it · join the parts with '&&' to get a verdict that covers all of them" \
1124+
"$1" "$CERTIFIABLE_NOTE"
1125+
}
1126+
9231127
# --- modes ------------------------------------------------------------------
9241128

9251129
usage() {
@@ -955,6 +1159,14 @@ inside a green count. If you are mid-ablation, the restore is your own
9551159
A `pnpm --filter <name>` that matches no project is refused BEFORE the lock is
9561160
taken, because pnpm exits 0 on it and the run would measure nothing (#10853).
9571161
OS_VERIFY_LOCK_NO_FILTER_CHECK=1 skips that check for one call.
1162+
1163+
A verdict says `command-exit` only when the exit status can CERTIFY the command:
1164+
one command, or a chain joined by `&&`. A string sequenced with `;` or newlines,
1165+
a pipeline, `||`, or `&` gets `batch-last-exit` instead — the run, the exit code
1166+
and this wrapper's behaviour are unchanged, but that number is the LAST part's,
1167+
so a failure earlier in the string is not in it and it must not be quoted as
1168+
proof the whole command passed (#12288). Join the parts with `&&` for a verdict
1169+
that covers all of them.
9581170
Exit 99 means this call never acquired the lock; every run prints a VERDICT line
9591171
— including the refusals: `lock-unusable` (this host cannot operate the lock at
9601172
all) and `usage-error`. A run that prints no verdict is a bug in this script.
@@ -1038,7 +1250,7 @@ run_unlocked() {
10381250
ended="$(now_s 2> /dev/null)"
10391251
case "$ended" in '' | *[!0-9]*) ended="$started" ;; esac
10401252
ran=$((ended - started))
1041-
log "VERDICT command-exit ${rc} · UNLOCKED (declared) · no usable \`${FLOCK_BIN}\` on this host, so the shared verify lock was NEVER taken and NOTHING was serialized · ran $(human_s "$ran") · declare it in the PR body · ${label}"
1253+
log "$(verdict_head "$rc") · UNLOCKED (declared) · no usable \`${FLOCK_BIN}\` on this host, so the shared verify lock was NEVER taken and NOTHING was serialized · ran $(human_s "$ran") · declare it in the PR body · ${label}"
10421254
ledger_append unlocked 0 "$ran" "$rc" "$label"
10431255
exit "$rc"
10441256
}
@@ -1226,6 +1438,26 @@ mode_run() {
12261438
exit 2
12271439
fi
12281440

1441+
1442+
# Which word this run's verdict may use, decided from the command string and
1443+
# said at second zero as well as in the verdict itself. The placement is
1444+
# chosen twice over: AFTER the refusals above, because a refused run prints a
1445+
# different verdict entirely and an instrument that predicts the wrong one is
1446+
# this card's own defect in miniature; and BEFORE the wait, because a dev
1447+
# about to spend the fleet's lock on a string whose result cannot be quoted
1448+
# should learn it while the repair still costs nothing.
1449+
VERDICT_WORD="command-exit"
1450+
if ! exit_certifiable "$kind" "$label"; then
1451+
VERDICT_WORD="batch-last-exit"
1452+
log "⚠ THIS COMMAND'S EXIT CODE CANNOT CERTIFY IT — ${CERTIFIABLE_NOTE}."
1453+
log "⚠ A shell string's exit status is its LAST part's, so a failure in an earlier"
1454+
log "⚠ part exits 0 and reads as a pass (#12288). If this run reaches a verdict it"
1455+
log "⚠ will therefore read batch-last-exit, not command-exit, and that number must"
1456+
log "⚠ NOT be quoted as proof that the whole command passed."
1457+
log "⚠ Join the parts with '&&' instead and the verdict certifies every one of them."
1458+
log "⚠ Nothing here refuses, truncates or alters your command — only the word changes."
1459+
fi
1460+
12291461
# No usable flock on this host: the declared unlocked run, and it never
12301462
# returns. Its position is chosen, not incidental. AFTER the filter preflight,
12311463
# because a command that would measure nothing is refused whether or not there
@@ -1483,8 +1715,8 @@ mode_run() {
14831715
held=$(($(now_s) - acquired_at))
14841716
HOLDING=0
14851717
rm -f "$HOLDER_FILE" 2> /dev/null || true
1486-
log "VERDICT command-exit ${rc} · held the lock $(human_s "$held") · waited $(human_s "$waited")"
1487-
ledger_append command-exit "$waited" "$held" "$rc" "$label"
1718+
log "$(verdict_head "$rc") · held the lock $(human_s "$held") · waited $(human_s "$waited")"
1719+
ledger_append "$VERDICT_WORD" "$waited" "$held" "$rc" "$label"
14881720
if ((held >= LONG_HOLD_WARN_S)); then
14891721
log "⚠ THIS RUN held the shared verify lock for $(human_s "$held"). Every sibling agent"
14901722
log "⚠ in this container queued behind it, and a long holder lengthens every cycle for"
@@ -2050,6 +2282,65 @@ mode_self_test() {
20502282
wait
20512283
st_case 'three staggered waiters acquire in arrival order' "$(< "$order")" ABC
20522284

2285+
# (g2) the verdict WORD: what an exit code may and may not claim (#12288).
2286+
#
2287+
# The defect these pin: a batch whose MIDDLE command failed printed
2288+
# `VERDICT command-exit 0` — a pass, on the one line every dispatch brief
2289+
# tells a dev to quote. It failed in the GREEN direction, which is why it
2290+
# survived: nothing anywhere was red.
2291+
#
2292+
# Pinned from BOTH directions on purpose, because only the pair is a pin. The
2293+
# batch must not print the certified word, AND an ordinary command must still
2294+
# print it — a "fix" that labelled every run a batch would pass the first
2295+
# case alone while destroying the meaning of the line for everyone.
2296+
local batchout batchrc bled
2297+
bled="${tmp}/batch.ledger"
2298+
rm -f "$bled"
2299+
batchout="$(OS_VERIFY_LOCK_LEDGER="$bled" bash "$SELF" -c 'echo one; sh -c "exit 1"; echo three' 2>&1)"
2300+
batchrc=$?
2301+
st_case 'a mid-batch failure does NOT print the certified verdict word' \
2302+
"$(printf '%s' "$batchout" | grep -c 'VERDICT command-exit 0')" 0
2303+
st_case 'it prints batch-last-exit instead, so the number names what it is' \
2304+
"$(printf '%s' "$batchout" | grep -c 'VERDICT batch-last-exit 0')" 1
2305+
st_case 'and the verdict LINE ITSELF says it is not a verdict on the whole command' \
2306+
"$([[ "$batchout" == *'NOT A VERDICT ON THE WHOLE COMMAND'* ]] && echo yes || echo no)" yes
2307+
st_case 'and names the repair, so the caller can get a certified verdict instead' \
2308+
"$([[ "$batchout" == *"join the parts with '&&'"* ]] && echo yes || echo no)" yes
2309+
st_case 'and warns BEFORE the lock is spent, not only after the run' \
2310+
"$([[ "$batchout" == *'CANNOT CERTIFY IT'* ]] && echo yes || echo no)" yes
2311+
# The three invariants that keep this a LABEL and not a behaviour change.
2312+
st_case 'the batch still ran in full — nothing is refused or truncated' \
2313+
"$([[ "$batchout" == *one* && "$batchout" == *three* ]] && echo yes || echo no)" yes
2314+
st_case "and the command's own exit code still passes through untouched" "$batchrc" 0
2315+
st_case 'and the ledger records the uncertified run as such, so it is findable later' \
2316+
"$(grep -c 'outcome=batch-last-exit' "$bled" 2> /dev/null || true)" 1
2317+
2318+
# The other direction, and the forms in between. Each one is a claim about
2319+
# what the exit status can prove, not about how the string looks.
2320+
st_case 'an ordinary single command still gets the certified word' \
2321+
"$(bash "$SELF" -c true 2>&1 | grep -c 'VERDICT command-exit 0')" 1
2322+
st_case 'an && chain is certified — short-circuit means a 0 proves every link passed' \
2323+
"$(bash "$SELF" -c 'true && true' 2>&1 | grep -c 'VERDICT command-exit 0')" 1
2324+
st_case 'an && chain that fails is certified too, and reports the failing exit' \
2325+
"$(bash "$SELF" -c 'sh -c "exit 3" && true' 2>&1 | grep -c 'VERDICT command-exit 3')" 1
2326+
st_case 'a pipeline is NOT certified — a red left-hand side exits 0 through it' \
2327+
"$(bash "$SELF" -c 'sh -c "exit 1" | cat' 2>&1 | grep -c 'VERDICT batch-last-exit 0')" 1
2328+
st_case 'nor is a || fallback, whose 0 can be the fallback speaking' \
2329+
"$(bash "$SELF" -c 'sh -c "exit 1" || true' 2>&1 | grep -c 'VERDICT batch-last-exit 0')" 1
2330+
st_case 'nor a newline-separated sequence, which is a `;` by another spelling' \
2331+
"$(bash "$SELF" -c 'sh -c "exit 1"
2332+
true' 2>&1 | grep -c 'VERDICT batch-last-exit 0')" 1
2333+
# Quoting decides whether an operator belongs to the CHAIN or to one element
2334+
# of it, and a scanner that got this wrong would refuse to certify ordinary
2335+
# commands — the over-labelling that would make the word meaningless.
2336+
st_case 'a ; inside quotes is an ARGUMENT, not a sequence, and stays certified' \
2337+
"$(bash "$SELF" -c 'printf "%s" "a; b" > /dev/null' 2>&1 | grep -c 'VERDICT command-exit 0')" 1
2338+
local subcmd='printf "%s" "$(echo x; echo y)" > /dev/null'
2339+
st_case 'a ; inside a substitution is one element, not a chain, and stays certified' \
2340+
"$(bash "$SELF" -c "$subcmd" 2>&1 | grep -c 'VERDICT command-exit 0')" 1
2341+
st_case 'argv mode never reaches a shell, so it is certified by construction' \
2342+
"$(bash "$SELF" -- true 2>&1 | grep -c 'VERDICT command-exit 0')" 1
2343+
20532344
# (h) the filter preflight (#10853). BOTH directions, because a guard shown
20542345
# only to red could be a guard that reds on everything -- which here would
20552346
# block the fleet's verification. The commands are `echo`, so what is measured
@@ -2079,7 +2370,10 @@ mode_self_test() {
20792370
# The other direction: a REAL package name must sail straight through, run,
20802371
# and report an ordinary command-exit verdict.
20812372
: > "${fp_ran}.live"
2082-
out="$(cd "$repo_root" && bash "$SELF" -c "echo pnpm --filter @objectstack/hono test; : > '${fp_ran}.liveran'" 2>&1)"
2373+
# `&&`, not `;`: this case asserts the CERTIFIED verdict word, and a `;` would
2374+
# make the fixture itself a batch (#12288). `echo` cannot fail, so the marker
2375+
# is written either way and what the case tests is unchanged.
2376+
out="$(cd "$repo_root" && bash "$SELF" -c "echo pnpm --filter @objectstack/hono test && : > '${fp_ran}.liveran'" 2>&1)"
20832377
st_case 'a filter naming a REAL package is not refused' "$?" 0
20842378
st_case 'and its command actually ran' \
20852379
"$([[ -e "${fp_ran}.liveran" ]] && echo ran || echo 'did not run')" ran

0 commit comments

Comments
 (0)