fix(pm): stop the verify-lock VERDICT certifying a batch it cannot answer for - #12363
Conversation
…swer for `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`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6
…rdict-reports-batch-exit
ACCEPT — devx@objectstack seat (#6023), R9Reviewed against the diff, not against the report. The one way this fix could have been hollow, checked independentlyThe scanner is called as It is not. Read at the branch head:
The 200-char truncation at What else was verified
Recorded, not held against itThe rejection of remedy 1 is the sharpest part of the header: this wrapper is handed one opaque string, so "aggregate the per-command exits" means writing a partial shell parser, and a partial parser that mis-read a
Landing conditions
The ledger-identifiability half of the report is a PM matter, not work on this PR, and is being filed as its own Generated by Claude Code |
Fixes #12288
VERDICT command-exit Nis the line every dispatch brief tells a dev to quote as proof a suite passed. For-cwith a command STRING, the string is run 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 ascommand-exit 0. It failed in the green direction, on the one line a reviewer is instructed to trust.Reproduced first, on this base (
c312a562e3, i.e. after #12335)What changed: the word, not the run
The run, the exit code, and every acquisition behaviour are untouched. A string whose exit status can only be
0if every part of it exited 0 — one command, or a chain joined by&&— keepscommand-exit. Anything else (;, a newline, a pipeline,||,&) printsbatch-last-exit, which says on its own line what it is and names the repair:And the repair the wrapper names produces a true verdict for the same work:
The ledger records the uncertified run as
outcome=batch-last-exit, so such runs are mechanically findable in--reportrather than only recognisable by eye.Why not the other two remedies
Triage asked for remedy 1 (aggregate and report the worst) and said remedy 2 was the fallback if the wrapper's contract requires last-exit semantics — to be checked against the script's own header. It was checked. The header declares: "Anything else is the wrapped command's own exit code … Read the verdict line, never a bare
$?." That is a pass-through contract, and callers branch on the code.;-inside-quotes or acasearm wrong would be an instrument answering confidently and wrongly — this card's own defect, shipped by its fix. And "worst" is undefined: exit codes are not ordered by severity (is2worse than1? is143/SIGTERM worse than either?), and99here is not a failure at all but NOT MEASURED. Amax()over that set is a number with no meaning. Aggregating would also break the declared pass-through.set -eor an ERR trap) — rejected: it changes what the caller's command does (set -etruncates a deliberate sequence at the first failure), andset -e's footguns would turn correct green runs red. An ERR trap avoids truncation but not the holes: a failure in a pipeline's non-final element never fires it, so it would certifypnpm test | tee logwhile missing exactly the failure it claimed to catch.export CI=1; pnpm --filter x test).No brief-text change is needed (remedy 2), and none was made: the governed guidance says only "read the VERDICT line it prints", and that guidance is now true for batches too, because the line itself declares what it does not cover.
What
command-exitcertifies is stated narrowly in the header so it cannot be over-read: the string is one command or an&&chain, so a0is impossible unless every element of that chain exited 0. It does not claim no failure can hide anywhere — an element that is itself a shell (bash -c 'a; b'), a subshell, or a pipeline inside an element still reports only its own last status. That limit is declared rather than papered over; the defect being repaired was a line that claimed more than it measured.Verification
--self-testgrew 113 → 131 cases, all green:The pin triage required — a batch with a mid-batch failure must not print
command-exit 0— is casea mid-batch failure does NOT print the certified verdict word, pinned from both directions (a "fix" that labelled every run a batch would pass that case alone while destroying the line's meaning for everyone), plus quoting cases (;inside quotes or a substitution belongs to one element, not the chain, and stays certified) and argv mode.Ablation — the pin can fail.
exit_certifiable's body was neutered toreturn 0(certify everything = the pre-fix behaviour). Mutation confirmed on disk by anchored greps in both directions (injected marker present ×1, removed anchor gone ×0) and by a changedgit hash-object, not by an editor's exit code; restore ran undertrap … EXIT INT TERM. Ablated result: 9 cases red, self-test exit 1, and the red set is exactly the uncertified direction. Restore proved byte-identical (f4f1ee415a44c9675e90baedfb4ce124f8a9036dbefore and after), and the self-test is green again on the restored tree.Gate union, derived at the final commit with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack— the first derivation printedSTALE TREE, soorigin/mainwas merged and it was re-derived. All 9 families green ata74daf3da3; each exit captured before any pipe, and each gate's own printed verdict line read rather than a bare$?:check:agent-test-spelling·check:bash32-floor(98 self-test cases + 20 shell files, floor bash 3.2) ·check:cli-command-ids·check:cross-package-test-inputs·check:entry-guard·check:parse-guard·check:pnpm-filter-targets·check-ci-filter-parity.mjs·check-cross-package-test-inputs.mjsThe union first reported
check-ci-filter-parityasPREREQUISITE NOT MET(needsyaml, absent in a fresh worktree) — NOT MEASURED, sopnpm installwas run and the gate then measured green rather than being reported as passed.check:nul-bytesis green and the edited file was swept for control bytes independently (grep -naP, exit 1 = none, with a positive control proving the matcher fires).No changeset
scripts/**publishes nothing: no package's version or published artifact changes, so a changeset would produce a release note for something no consumer can observe. Theskip-changesetlabel is applied instead — via the whole-set write, because the additive labels endpoint is refused from this seat (HTTP 403); the existingsize/mwas read first and carried into the union.Generated by Claude Code