Uh oh!
There was an error while loading. Please reload this page.
fix(pm): os-verify-lock reads a redirection's & as a redirection, not as a background operator - #12635
Merged
Merged
Conversation
os-litant
marked this pull request as ready for review
August 27, 2026 02:41
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#12518
exit_certifiable()'s scanner read the&in a2>&1redirection as a background operator, so a correctly&&-joined command was downgraded fromVERDICT command-exittoVERDICT batch-last-exit. The banner it printed told the caller to "join the parts with&&" — a repair the command had already made, so no action could clear it.The scanner now carries one character of look-behind: the redirection operator it just passed. Look-behind, because the character that disambiguates these tokens is the one before them — in
2>&1the&is followed by1, exactly as a backgrounding&before a next command would be. Nothing else about the scanner's semantics moves:;, newline,|,||and genuine backgrounding all keep their verdicts.Before / after — the card's two commands, verbatim
Before (at
0043c922):After (at
f55ee39d):The warning block is gone entirely — it is not merely reworded.
The implementation was measured, not assumed
The card suggested looking back for an unescaped
>. That is the right direction but not the whole grammar, so the forms were enumerated against bash's own parser rather than guessed:declare -fre-prints a function body from the parsed AST, so it reports what bash actually did with each token (bash 5.2.21;bash -cis what runs the string here, so bash's grammar and not POSIX sh's is the authority).Three readings follow, and each is why this is scanner state rather than a look-back into the raw string at
i-1:>must be unescaped and unquoted.echo \>&trueandprintf "a>"&trueboth background, and in both the character ati-1is a>. Only the scanner's own quote/escape state separates them, so every path that skips a character clears the look-behind as it goes.&>must be adjacent.true &> logredirects;true & > logbackgrounds. The test is the next character, never the next token.&must not swallow a later backgrounding one —true 2>&1& truecarries both, in that order.|&needed no new case: bash rewrites it to2>&1 |, so it really is a pipeline and the existing|branch already answers it correctly.Full verdict matrix, before and after
Driven through the real script, one isolated lock file, at
0043c922andf55ee39d:Two rows are beyond the
&fix as the card framed it, and both are the same defect class — a redirection operator's second character read as a control operator, in the same scan loop, repaired by the same state:<&input duplication. The card names only>;<&0and<&-are real bash redirections and were misread identically.>|noclobber override. Read as a pipeline, not as backgrounding. Guarded on>alone, because<|is a bash syntax error and so has no input form to admit.One form is deliberately left uncertified: the append-both redirection
check:bash32-floorwent red on the first draft of this change, and it was right. An&before a doubled>appends both streams on bash 4.0+, but this file is held to a bash 3.2 floor (/usr/bin/env bashis 3.2.57 on macOS, andbash -cis what runs the string, so the host's bash decides). On 3.2 that spelling parses as&then>>— it really is backgrounded there.Certifying it would have been an under-label on a 3.2 host, which is the one direction this scanner's doctrine forbids. Its meaning is a property of the host rather than of the string, which is the definition of what this scanner "cannot read with confidence", so it takes the uncertified exit reserved for exactly that — with its own note naming the portable spelling, which this change certifies:
Unlike the defect being fixed, that banner's remedy is not already satisfied — it is actionable, and the command it recommends comes out
command-exit.There is deliberately no
st_casefor it: writing the token in this repo's shell is itself acheck:bash32-floorviolation, and the gate fires on quoted occurrences too (measured — the two flagged lines were anst_caselabel and a single-quoted payload handed tobash -c). So the gate that forbids the spelling is also what makes the pin unwritable. The reading is recorded in full in the comment block aboveexit_certifiable. The same reason removed a|&case from the draft:|&is bash 4.0 as well.Tests
Regression pins — 17 new
st_casecases in the existing self-test suite, in a new(g3)block beside the(g2)verdict-word pins, pinned from both directions:The card asked for two pins specifically; both are present —
true > /dev/null 2>&1 && trueis the first case, andsleep 1 &is "a REAL background & is still uncertified".Ablation — the pins are shown to fail. Run from the committed state at
f55ee39d, mutating only the three repaired guards toif false, with the mutation proven on disk by anchored text counts (not by an edit tool's exit code) and the restore proven byte-identical against the HEAD blob:All ten certified cases go red; the seven negative-direction cases and every pre-existing case stay green. A "fix" that merely stopped flagging
&would have passed the ten while deleting the check they sit beside — which is why both directions are pinned.An earlier ablation attempt refused to run rather than reporting a reading, because the anchors had moved with the implementation ("MUTATION DID NOT APPLY — this ablation did NOT run; reading is void"). Recorded because a zero-match edit that reports success is the failure mode this check exists for.
Derived gate family, run at
f55ee39dunder the shared verify lock (VERDICT command-exit 0 · held the lock 29s). Families derived bynode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, which reads the change set from git rather than from a hand-written list:check:bash32-flooris quoted at its green state; its red state on the first draft is the section above.Scope
One file,
scripts/pm/os-verify-lock.sh(the scanner and its ownst_casesuite) — 184 insertions, 3 deletions, no other path touched. No changeset:scripts/pm/**is internal PM-loop tooling and publishes nothing, so this PR carries theskip-changesetlabel.Filed while here, out of scope and not fixed in this PR: #12634 —
check-bash32-floor's construct table has the append-both entry but no|&entry, though both operators arrived in bash 4.0. Found because the gate caught one of them in this diff and let the other through.Generated by Claude Code