fix(hooks): fire codex-gate on git commit in command position, not on prose (#588) - #598
Merged
Merged
Conversation
…pes measurable (#588) /usr/bin/env git commit and /usr/bin/nice git commit invoked git for real and walked past the gate: the (\S*/)? path prefix went on git but never on the wrapper alternation. Both are blocked on origin/main, so this was a regression introduced by the anchor, not a pre-existing hole. The oracle now runs a trailing wait, without which coproc shapes report inert for a command that genuinely runs git — a false ALLOW want. With it, both coproc forms measure as INVOKES and both already block. 63 passed, 0 failed.
#588, Sol round 2) A backslash makes the next character ordinary word content. The detector modelled a word as 'runs to the next space or separator', which splits FOO=a\ b git commit, /path\ with\ spaces/git commit and env FOO=a\;b git commit early and lets all three through. They block on origin/main, so each was a regression introduced by the anchor. Sol's prescription was one reusable word atom rather than four local patches, and that is what GATE_WORD is: used by assignments, redirection operands, the wholesale wrapper skip and the path prefix on git. A word model that is right at three sites out of four is a fail-open at the fourth. Also closed, all oracle-measured, all blocked on origin/main: - < /dev/null git commit — the operand may be separated from its operator - env 2>&1 git commit — the & of a redirection is not a command boundary - \env git commit — the escape rule already applied to git, never to wrappers - builtin command git commit — builtin was missing from the enumerated set builtin is a PREFIX to the builtin members of the set, not a wrapper: builtin git commit is inert, and blocking it would be a false positive. Pinned as a row. Claim-rule fixes Sol named: the wrapper comment no longer claims per-wrapper flag grammars, the oracle comment no longer claims subcommand-chain parsing, and the declared-row list names all four rows rather than two. 83 passed, 0 failed.
GATE_WORD is used at four sites, so it got its own adversarial hunt: 25 shapes probed against both origin/main and HEAD with the oracle. Zero fail-opens and zero regressions, so nothing to fix — but six of the shapes were structurally distinct and untested, and are pinned here so a later change to the word atom cannot quietly break one. 89 passed, 0 failed.
…nds, assignment names (#588) Fail-opens, all oracle-measured: - >|/dev/null git commit and 2>| /dev/null git commit — bash's clobber-override operator was missing from the redirection set and from the wrapper skip - builtin builtin command git commit — builtin stacks - git -c user.name=A\ b commit — git's own global-option operands still used \S+ rather than the word atom. Pre-existing on origin/main, but it made the hook's 'every site uses GATE_WORD' comment false, so it is fixed rather than deferred False positives, all oracle-inert and all blocked before this commit: - builtin env git commit, builtin /usr/bin/command git commit — builtin runs only shell builtins, so it is now a prefix to exactly (command|eval|exec) rather than to every wrapper - A-B=1 git commit, 1A=1 git commit — the assignment grammar accepted any non-space left side; bash runs these as commands. Now a real assignment name Nested shell substitutions are dispositioned as an accepted limit, not chased. A separator inside ${...}, $(...), $((...)) or backticks is not an outer boundary, but substitutions nest arbitrarily so no regex closes it — the same constraint as the quoted-payload class on #599 and #533's ruling against positive command parsing. Pinned as three rows. The hook comment no longer claims to model shell words in general; it claims escapes, which is what it does. GATE_SKIP now interpolates GATE_WORD instead of re-inlining it, so the sites cannot drift. 102 passed, 0 failed.
…eyword position (#588) Sol accepted the proposed bar: a regression is a shape a person plausibly types. Every round-5 finding is an ordinary shape, and every one is fixed. Fail-opens, oracle-measured: - FOO+=1 git commit — += is a valid bash assignment form - git with a separated relocation-flag operand — git's value-taking long options accept a separated operand, which broke the option chain - builtin -- command git commit, escaped builtin and command spellings False positives, oracle-inert and blocked before this commit: - echo if git commit, echo then git commit — a reserved word is only a command-position marker when the reserved word is ITSELF at a boundary. Here it is an argument to echo - echo \; git commit — an escaped separator is word content - a heredoc writing a script that contains the verb. #588's own defect description names this shape, and it is an ordinary workflow Heredoc bodies are masked to H before anything else reads the text, and before the newline rewrite, since after that rewrite there are no lines left to find a delimiter on. A here-string has its operand on the command line and is deliberately left alone. A real invocation AFTER a heredoc is still caught. Escaped separators are neutralised in the MASKING pass, not by asking the anchor for "a separator not preceded by a backslash". That phrasing cannot count parity, and an escaped BACKSLASH followed by a REAL separator is the k = 1 (mod 4) case. Getting it wrong re-opened k = 5, 9 and 13 as fail-opens, which those rows caught immediately. Claim-rule fixes: GATE_WORD is described as five sites, not four; the declared row count is five on macOS and says so, generated from the run rather than recalled; both modified shell files are now shellcheck-clean at default level, with the two suppressions carrying their reason. 118 passed, 0 failed.
…l-opens (#588, Sol round 6) The pass was added last commit to fix a false POSITIVE: a heredoc writing a script that contains the verb was refused. Sol's round-6 review measured that the fix was worse than the defect. Five shapes, each oracle-confirmed as invoking git, each blocked before the pass landed and allowed after it: # documentation: use <<EOF below a comment naming the operator echo '<<EOF' the operator inside quotes cat <<< EOF the regex restarts inside the <<< (( x = 1 << 2 )) a left shift in arithmetic cat <<-EOF ... <tab>EOF the tab is still an escape at that stage One claim in the reverted comment was also simply false: an unquoted heredoc body is not inert. A substitution in it runs while the redirection is built. Recognising a heredoc correctly means ignoring the operator inside quotes, comments, arithmetic and escapes, excluding here-strings, queueing multiple delimiters, and distinguishing quoted from expanding bodies. That is a partial shell parser, which is what #533 ruled out — the same ruling this PR has declined to re-litigate everywhere else. So it is reverted rather than deepened, and the heredoc false positive returns to being an accepted limit that fails CLOSED. Filed separately. All five shapes are pinned as rows, so the pass cannot come back silently. Everything else from round 5 survives the revert and is re-verified: the assignment form, the separated global-option operand, the builtin spellings, the keyword-boundary anchor and the escaped-separator neutralisation. Sol probed backslash runs 1-8 before each separator and found the parity mechanism held. Claim-rule fix: GATE_WORD has SIX word-consuming sites, not five. The wrapper executable path was in the code but missing from the prose list. 120 passed, 0 failed.
…oved nothing (#588, Sol round 7) Sol's round 7 scored the revert 8.7/10 and found no production-hook defect. Both findings were in my own evidence, and both are the kind this suite exists to catch. The row pinned as `(( x = 1 << 2 ))` did NOT demonstrate the defect it claimed. Direct parent-vs-target probing: (( x = 1 << 2 )) oracle INVOKES parent BLOCK head BLOCK (( x = 1 << EOF )) oracle INVOKES parent ALLOW head BLOCK The reverted masking pass blocked the `2` form too, because `2` is not delimiter-like. `<< EOF` is the shape that actually escaped. A row that passes while proving nothing is worse than no row. The count was six, not five. The expanding-body shape was described in prose and never counted: cat <<EOF / $(git commit -m x) / EOF oracle INVOKES parent ALLOW Added: the tab-terminated `<<-EOF` row, which had no oracle row at all despite being named in the comment, with a literal load-bearing tab. Pinned the exact single-quoted `echo '<<EOF'` spelling rather than only its double-quoted equivalent. The six-site GATE_WORD list is now enumerated in the test as well as the hook — it was a numbered list in one file and a bare count in the other. Also filed, both measured as pre-existing on origin/main and on this head, both fail-CLOSED, neither folded into this PR: #601 heredoc bodies, corrected to six shapes #602 args=(git commit -m x) is an array literal and invokes nothing 121 passed, 0 failed.
Sol's round 8 scored 9.2/10 and confirmed no production-hook change is needed. It verified each of round 7's six corrections independently, including that the new rows demonstrate the defect rather than merely passing — the failure mode that produced the bad `<< 2` row in the first place. One correction was incomplete. I claimed "five to six everywhere" and two references survived, both in the accepted-limit comment that introduces the six-row block they contradict. Grepped rather than eyeballed this time. Also filed #603 for a pre-existing false-positive class Sol found: the punctuation anchors ( ) { } ! fire on prose, because #588's rule that an anchor must ITSELF be at a command boundary was applied to the keyword anchors and never to the punctuation ones. All six shapes are oracle-inert and blocked on origin/main too. #602 is the specific `(` instance of it and is now cross-linked as subsumed. 121 passed, 0 failed.
… Sol round 9) A regression I introduced at 27bf763. Tightening GATE_ASSIGN to a real scalar name — which correctly killed the `A-B=1` and `1A=1` false positives from Sol's round 4 — also excluded array-element assignments, which bash accepts as command prefixes just as it accepts scalar ones. Oracle-measured, BLOCK on origin/main and ALLOW from 27bf763 until here: A[0]=x git commit -m x A[foo]=x git commit -m x A[0]+=x git commit -m x The subscript is bounded to a single bracketed span with no nested brackets, so an invalid NAME is still rejected. Sol asked for a canary proving the round-4 false positives cannot ride back in on the fix; `A[0]-B=1 git commit` is pinned as an inert row and stays ALLOW, alongside the existing `A-B=1` and `1A=1` rows. This is the second time narrowing a grammar to close a false positive opened a fail-open in the same expression. The first was the `builtin` prefix in round 4. Both were caught by probing origin/main against the head rather than by reading. 125 passed, 0 failed.
#588, Sol round 10) I asked Sol to hunt for a third instance of "narrowing a grammar against a false positive leaves a fail-open in the same expression". It found one, in the fix from the previous commit. A bash indexed subscript nests and may be empty. Bounding it to a nonempty, non-nested span kept four shapes fail-open — oracle INVOKES, BLOCK on origin/main, ALLOW from 27bf763 through f3065ca: A[]=x git commit -m x an empty subscript evaluates to index 0 A[B[0]]=x git commit -m x subscripts nest A[B[0]]+=x git commit -m x A[1+A[0]]=x git commit -m x Reproduced by the reviewer under bash 3.2 and 5.3. The subscript now admits anything but `=`, which is what actually terminates it here. The safety is unchanged and was never the subscript bound: a real NAME before the bracket, and the `]` immediately against the `=` / `+=`. The comment said the bound protected against invalid names, which was the claim-rule problem — it disclosed a protection it was not providing and hid a fail-open. Canaries, all still inert and ALLOW: A[0]-B=1, A-B=1, 1A=1. The first also BLOCKS against the pre-round-4 grammar, so it fails if the old malformed-name form returns — it proves NAME protection specifically. Pinned a latent dependency the reviewer flagged for #603: A=(x y) git commit BLOCKS only through the unconditional `)` anchor, not because GATE_ASSIGN consumes a compound value. #603 narrows those anchors, and without this row that work would silently re-open it. Recorded on the issue as well as in the suite. 130 passed, 0 failed.
…er class (#588, Sol round 11) I asked for a fourth instance of the narrowing pattern and got one, in the previous commit's fix. A subscript is an ARITHMETIC expression, so `=` is valid inside it. The two cuts were exact mirrors: [^][]+ excluded empty and nested subscripts, admitted `=` [^=]* admitted empty and nested subscripts, excluded `=` Fail-open under both bash 3.2 and 5.3, BLOCK on origin/main, ALLOW on cdebef7: A[B=1]=x, A[1==1]=x, A[B+=1]=x, A[B<=1]=x, declare -A A; A[x=y]=z The bound that works is neither character class: it is WHITESPACE. An assignment prefix is a single shell word, so its subscript cannot contain unquoted whitespace — if it did, the word would split and it would stop being a prefix. That admits nesting and `=` together, which no class over `]` or `=` can. A quoted key with a space arrives already masked to A[Q]=x, verified. The reviewer's own suggested pattern for this round was a FIFTH instance: `(\[([^]]|\][^=])*\])?` admits `=` but re-excludes A[B[0]]=x, because `\][^=]` consumes the inner `]]` and then cannot find the closing bracket. Caught by probing it rather than reading it — the same method that caught the other four. Claim-rule: the comment no longer claims the bound validates bracket balance. POSIX ERE cannot, without a bounded nesting limit or the scanner #533 ruled out. The unbalanced span A[0]-B[1]=x is therefore read as an assignment and blocked; it is oracle-inert, fails CLOSED, and is now a pinned row rather than prose. Also pinned per the reviewer: A+=(x y) alongside A=(x y), both of which block only through the unconditional `)` anchor that #603 will narrow. 139 passed, 0 failed.
…tempts, six wrong (#588, Sol round 12) The whitespace bound was the sixth instance, and this one falsified the REASONING, not just the regex. I claimed an assignment prefix is a single shell word so its subscript could not contain unquoted whitespace. Bash's assignment lexer keeps it. Measured under bash 3.2 and 5.3, BLOCK on origin/main and on the parent, ALLOW at a8b9da7: A[1 + 2]=x git commit A[1\ +\ 2]=x git commit A[1\ +\ 2]+=x git commit declare -A A; A[foo\ bar]=x git commit Every attempt to say what a subscript may not contain has now been wrong, in both directions: [^][]+ excluded empty and nested, admitted `=` [^=]* admitted empty and nested, excluded arithmetic `=` [^[:space:]]* excluded arithmetic whitespace and the reviewer's own proposed `(\[([^]]|\][^=])*\])?` was a fifth, admitting `=` while re-excluding nesting — caught by probing it rather than applying it. So the span is deliberately unbounded. A subscript is an arbitrary arithmetic expression and a regex has no business enumerating its contents. The cost is disclosed rather than discovered: an unbounded span OVERMATCHES. It does not validate bracket balance and cannot — POSIX ERE cannot without a bounded nesting limit or the scanner #533's ruling puts out of reach. Malformed text shaped like NAME[...]= is read as a prefix and blocked. A[0]-B[1]=x is exactly that: oracle-inert, blocked, failing CLOSED, pinned as a row. The only safety claim left is the real NAME requirement, which keeps A-B=1 and 1A=1 out. Three earlier versions of this comment claimed more, and each extra claim was measured false. Verified the unbounded span does not overmatch into prose or across a real command: seven prose shapes stay ALLOW, including `echo A[x and stuff]=y and more git commit`, and all three NAME canaries stay ALLOW. 144 passed, 0 failed.
…nd the quote masker (#588, Sol round 13) Asking a different question paid off. The first six narrowing instances were all in one sub-expression, so this round asked whether the others carried the same class of error. Two did. SEVENTH — the redirection operator set omitted `<<`, `<<-` and `<>`. An ATTACHED operand still matched by accident through a shorter operator; a SEPARATED one escaped. Oracle INVOKES, BLOCK on origin/main, ALLOW here: << EOF git commit -m x <<- EOF git commit -m x <> /dev/null git commit -m x EIGHTH — the double-quote masker assumed a quoted span contains no backslash, so a Windows path or an escaped character left the span unmasked and the whitespace inside it broke the assignment and git-option chains downstream: FOO="C:\Program Files" git commit -m x git -c user.name="A\B C" commit -m x The second of those is ALLOW on origin/main too, so it is a pre-existing hole this fix also closes rather than a regression. NINTH, and mine to own: I applied the reviewer's suggested masker without probing it first, which is the exact thing I had said two rounds earlier that I would stop doing. `\\.` as the escape alternative swallows the closing quote, and under POSIX longest-match `cd "$dir" && git commit -m "message"` masks ENTIRELY to `Q` — the invocation vanishes and the gate fails open on a shape it had blocked for years. tests/test-hooks.sh caught it in the same commit that introduced it. `\\[^"]` covers a backslash inside the span while still stopping at the closer. Both shapes are now pinned rows. Claim fixes the reviewer named: - "a subscript is an arbitrary arithmetic expression" was too broad. INDEXED subscripts are arithmetic; ASSOCIATIVE ones are arbitrary strings after expansion. The point stands and is now stated correctly: between them there is no character a regex can rely on excluding. - the cost disclosure omitted cross-command greed. `A[0]=x; echo A[1]=y git commit` fuses two fragments across the `;` into one fictional assignment. Inert, blocked, fails CLOSED, pinned as its own row. - "redirections are transparent" is now qualified to the ENUMERATED operators, with the set declared closed on the same footing as the wrapper set. - the test comment still said "whitespace bound" after that bound was removed. 152 passed, 0 failed. test-hooks 225/0, stdin-bounded 8/0, shellcheck clean.
…y pin (#588, Sol round 14) TENTH instance — the double-quote masker handled a backslash before a NON-quote but not an escaped quote, so it paired the wrong quotes and left whitespace that broke the assignment, option and redirection chains: FOO="a b\" c" git commit -m x git -c user.name="A B\" C" commit -m x > "a b\" c" git commit -m x ELEVENTH — the single-quote masker had the analogous hole. 'a b'\''c d' is the ordinary shell idiom for an apostrophe inside single quotes. Escaped apostrophes are neutralised PARITY-AWARE, not blindly: pairs are set aside first, only a leftover odd escape is neutralised, and the restore emits the PAIR. Restoring a single backslash collapsed the run and broke k=5 and k=13 in the backslash contract — caught by that table, and pinned now by its own canary row. THE FINDING THAT MATTERS MOST IS ABOUT THE TESTS, NOT THE CODE. Last commit claimed its rows pinned the rejected `\.` masker "so it cannot come back". Sol mutation-tested that exact expression: this suite stayed GREEN at 152/0. The claim was false — the rows lacked a quoted span on BOTH sides of the invocation, so nothing forced the over-mask to show. Fixed, and verified by mutation rather than by reading. Three mutations, all now RED: Sol's rejected \. expression 155/5 (was 152/0 GREEN) drop escaped-quote support 157/3 restore one backslash, not the pair 158/2 unmutated 160/0 Claim fix: the test comment still said "the whitespace bound" after that bound was removed. Filed #604, not folded in: the `&` of `&>` is treated as an unconditional anchor, so `echo &>/dev/null git commit` is refused. Oracle-inert, blocked on origin/main too, fails closed. Sibling of #603 rather than a duplicate — a fix that only asks "is this character at a boundary" will not fix it, because that `&` genuinely is at one. 160 passed, 0 failed. test-hooks 225/0, stdin-bounded 8/0, cowork-drift 30/0.
…588, Sol round 15) Sol independently reproduced the three row mutations and confirmed the two-sided quote rows now genuinely pin the longest-match regression. No vacuous row claim remains in that increment. TWELFTH instance, PRE-EXISTING and filed as #605 rather than fixed here, on Sol's scoping: the two quote-masking passes run independently, so a literal quote inside one single-quoted argument pairs with another inside a later one and the mask swallows the text between them. printf '%s\n' '"' && git commit -m 'fix "quoted" handling' masked: printf Q Q oracle: INVOKES gate: ALLOW Identical on origin/main. Distinct from #599 — nothing is handed to a shell here; git is invoked directly and unquoted, and the masker erased it. Closing it needs a single joint quote-state scan, not another alternative bolted onto either regex, which is exactly the move that produced four of the twelve instances. Pinned as a fail-OPEN limit row. A SECOND vanishing-row class, found while adding that row and worth more than the row itself: the limit_row call sat ABOVE limit_row's definition. On bash 3.2 that prints to stderr and counts as neither pass nor fail — the row silently did nothing, the same class Sol caught in the mutation audit. The first guard I wrote for it was itself vacuous: command_not_found_handle is bash 4.0+, and this file's shebang gets macOS /bin/bash 3.2, so it never fired. Replaced with a portable structural fix — every helper is defined above every row, plus an assertion that fails loudly if one moves back below its call sites. Verified by mutation: moving limit_row back down aborts the suite with FATAL instead of passing green. Claim fixes Sol named: the hook said the rejected expression masks "ENTIRELY to Q" when it masks to `cd Q`; the test still said "the whitespace bound" after that bound was removed; the PR body still claimed 102 tests and four declared rows, and carried an accepted-limit table whose counts contradicted its own rows. The body now carries a table generated from the pinned rows. 161 passed, 0 failed. test-hooks 225/0, stdin-bounded 8/0.
… Sol round 16)
No production defect this round. All three findings were the harness certifying
things it had not measured, which is the class that has produced the most
valuable findings in this review.
THE HELPER GUARD WAS ITSELF VACUOUS, and its comment claimed more than it
checked. It named three helpers. Sol measured the gap:
move run_row below its calls exit 0 at 30/0, with 131
"command not found" errors
move oracle_can_measure below its calls exit 0 at a GREEN 161/0, while
every ordinary row silently
degraded to `declared`
The second is the dangerous one: the suite reports full green while measuring
nothing. The guard now lists every helper and sits below all of them. Verified
by mutation — all five helper moves now abort with FATAL.
A guard on helper EXISTENCE cannot catch a helper that is present but neutered,
so there is now a second, independent one: rows that actually reach the oracle
are counted, and the suite fails if that collapses. Verified by mutation —
neutering oracle_can_measure drops it to 11 rows against a floor of 100 and
fails, where the existence guard sees nothing wrong. The count is printed in the
summary line so a silent collapse is visible even when green.
THREE GATE_WORD CONSUMERS WERE NOT BEHAVIOURALLY PINNED. The self-hunt probed
shapes; it never proved each SITE uses the shared atom. Sol replaced GATE_WORD
independently at the redirection operand, the git path prefix and the wrapper
path prefix, and the suite stayed green at 161/0 for all three — while the hook
argues in its own comment that correctness at five of six sites is a fail-open
at the sixth. Three rows added, each mutant now fails 163/1:
>out\ file git commit -m x
./path\ with\ spaces/git commit -m x
./path\ with\ spaces/env git commit -m x
They need fixture state to be measurable at all, so the escaped-space directory
is created alongside the other fixtures rather than assumed.
PR body corrected: the five declared rows are now named individually rather than
counted (two timeout, stdbuf, sudo, /usr/bin/git); the fail-CLOSED table said
five and omitted the #601 heredoc row, and is six; the superseded accepted-limit
table is removed rather than left beside the authoritative one.
164 passed, 0 failed, 129 rows oracle-measured. test-hooks 225/0,
stdin-bounded 8/0.
…rd vacuity route (#588, Sol round 17) THIRTEENTH narrowing instance. The regex allowed a backslash before the path and before a bare `git`, but not between the path and the basename. Oracle INVOKES, ALLOW at fd4818e, BLOCK on the PR base: ./\git commit -m x env ./\git commit -m x command ./\git commit -m x FOURTEENTH, found by probing that finding rather than taking its shape as given: the escape can sit INSIDE the basename. `\?git` closes only the spelling the reviewer happened to type; each letter needs its own optional escape. ./g\it commit -m x ./gi\t commit -m x g\it commit -m x All six pinned. Mutation-verified: reverting the whole fix fails 6 rows, reverting only the intra-basename part fails 3. THE THIRD ROUTE TO GREEN-WHILE-MEASURING-NOTHING, and the reason to keep asking that question. Sol mutated run_row to call the oracle, DISCARD its result and use the declared value. Both existing guards passed, at a green `164 passed, 0 failed (129 rows oracle-measured)`. The count proves rows ENTER the oracle, never that its OUTPUT reaches the assertion. Closed with an oracle-result canary: one row declares the sentinel ORACLE_REQUIRED, which is not a legal truth value, so the row can only pass if `truth` came from the oracle. It also fails loudly if it ever becomes unmeasurable rather than quietly degrading. Verified: Sol's discard mutant now fails that row. Three routes, three independent guards, each catching what the others miss: helper moved below its calls existence assertion FATAL helper present but neutered oracle-row count 11 vs floor 100 oracle called, result ignored ORACLE_REQUIRED row canary fails A METHOD NOTE THAT COST ME A REAL ERROR THIS ROUND. My first attempt to mutation-test the escape fix used a sed pattern that did not match, so the "reverted" build was identical to the fixed one and the suite passed green — I nearly recorded that as evidence the rows did not pin. Mutations now assert they applied before the suite runs, and that assertion caught the second bad attempt immediately. Claim fixes: the summary metric is named `run_row rows oracle-measured`, because 129 counted only run_row cases while limit_row, inline and parity rows also use the oracle; the collapse comment no longer says "every row" falls back. PR body updated to seventeen rounds and fourteen instances. 171 passed, 0 failed, 136 run_row rows oracle-measured. test-hooks 225/0, stdin-bounded 8/0.
… round 18) FIFTEENTH instance, and it was in my own round-17 fix: the leading optional escape made `\\git commit` block, but that is a command named `\git` and does not invoke. Oracle inert, ALLOW on the base, BLOCK at 8125570. Removed. Then the whole escape class, closed by one rule instead of per-keyword patches. Sol listed five shapes that invoke and were allowed — THREE of them regressions against the PR base, two pre-existing: git c\ommit -m x pre-existing e\nv git commit -m x REGRESSION builtin c\ommand git commit REGRESSION git -\c user.name=A commit pre-existing git --git-\dir .git commit REGRESSION Sol's read was that these are gratuitous spellings, so pinning them as accepted limits would be consistent with the accident-catcher scope. But there is a rule that closes all five at once and needs no enumeration: an escape before an ORDINARY character is REMOVED, because that is exactly what bash does. `e\nv` IS `env`. Doing what the shell does beats enumerating what it might be spelled as — the per-letter approach was tried for `git` alone and produced a fail-open and a false positive in consecutive rounds. That per-letter hack is now redundant and deleted. Alphanumerics only: an escaped SPACE must stay escaped or the word splits and the assignment stops being a prefix. Mutation-verified in both directions — removing the rule fails 12 rows, widening it to every character fails 6. FOURTH ROUTE TO GREEN-WHILE-MEASURING-NOTHING: the failure SINK itself. Sol neutered `fail()` to `fail() { :; }` alongside a revert of the six-case escape fix, and the suite reported `165 passed, 0 failed` and exited 0 while six real regressions vanished. The existence guard, the oracle floor and the ORACLE_REQUIRED canary were all still satisfied — none of them proves that `fail` fails. Closed with a failure-sink canary that calls `fail` for real and requires the counter to move, using `exit 1` directly since `fail` is the thing under test. Four routes, four guards, each catching what the others miss: helper moved below its calls existence assertion FATAL helper present but neutered oracle-row count 11 vs floor 100 oracle called, result ignored ORACLE_REQUIRED row canary fails fail() cannot record failure-sink canary FATAL Claim fixes: the two "every row falls back to declared" comments are scoped to run_row rows; the PR body said fourteen instances in one place and twelve in another, and claimed "every want value oracle-measured" while naming five declared rows elsewhere. 178 passed, 0 failed, 143 run_row rows oracle-measured. test-hooks 225/0, stdin-bounded 8/0, shellcheck clean.
… is not alphanumeric (#588, Sol round 19) SIXTEENTH instance, in the round-18 unescape rule. Bash removes the backslash, but quote removal does NOT retroactively turn a quoted token into a RESERVED WORD. `\if git commit` runs a command named `if`; stripping the escape let GATE_ANCHOR see a keyword and manufacture a command position. Oracle inert, ALLOW on the parent, BLOCK at eeb9217 — a regression I introduced. Escaped keywords are now shielded before the unescape runs. Real keywords still anchor. Four inert rows and one invoking row pin both directions. The shield uses `([^A-Za-z0-9_]|$)` rather than `\b`: BSD sed has no `\b`, and the first attempt silently matched nothing on macOS while looking correct. THE UNESCAPE CLASS WAS ALSO TOO NARROW. "Alphanumerics only" was my phrasing, not bash's rule. Sol found three shapes that invoke and were allowed: git \-c user.name=A commit git --git\-dir .git commit git \--git-dir .git commit All predate this PR and were undeclared. Escaping an option hyphen is a plausible spelling, not an adversarial one. The class now covers the ordinary characters bash drops the backslash before, still excluding whitespace, which must stay escaped or the word splits. Sol's note that my two mutations proved less than I claimed is correct: widening to every character fails 6 rows, which shows blind removal of metacharacters is wrong — not that alphanumerics was the right boundary. Narrowing back to alphanumerics now fails 3 rows, which is the missing half of that proof. FIFTH ROUTE TO GREEN-WHILE-MEASURING-NOTHING, the mirror of the third: run_row calls `gate` and discards its result. The oracle is measured, drives `want`, and `fail` works — the thing actually under test is thrown away. All four earlier guards passed at a green 178/0. Closed with a gate-result canary that forces `gate` to lie once and requires run_row to notice. `gate` is now a thin wrapper over `gate_impl` so the swap works on bash 3.2. Five routes, five guards, each mutation-verified: helper moved below its calls existence assertion FATAL helper present but neutered oracle-row count 11 vs floor 100 oracle result discarded ORACLE_REQUIRED row canary fails fail() cannot record failure-sink canary FATAL gate result discarded gate-result canary FATAL Claim fixes: the hook still described an optional leading backslash that was deleted two commits ago; the PR body said "every want value oracle-measured" while naming five declared rows, gave two different instance counts, and said two findings were about the tests when there are five. 187 passed, 0 failed, 153 run_row rows oracle-measured. Mutation-verified: removing the keyword shield fails 4, removing the unescape fails 15, narrowing it back to alphanumerics fails 3. test-hooks 225/0, stdin-bounded 8/0.
… boundaries required Sol round 20, NOT CERTIFIED 5.0/10. Two narrowing instances and one vacuity route, all oracle-measured against the parent commit: - the keyword shield covered only the LEADING position, so seven of eight spellings were false positives (`i\f`, `wh\ile`, `th\en`, ...). Built the alternation instead of hand-writing it: every escape position in every reserved word. - it had no right boundary, so `\do` was shielded INSIDE `su\do`, which unescapes to a real invocation. That was a FAIL-OPEN. Both boundaries are now required, and the row is pinned against a deterministic fake wrapper — the real one resets PATH and depends on a cached credential, so it cannot serve as an oracle. - dropped `=`, `+` and `_` from the unescape class. Unescaping any of them manufactures an assignment prefix the shell never sees: `FOO\=1 git ...` runs nothing, because the word is not NAME=VALUE. Sixth route to green-while-measuring-nothing: replacing the suite's last line with `true`. Every in-run guard still passes and CI reads 0. It cannot be guarded from inside one run, so the suite re-runs itself with a failure forced and requires the child to exit nonzero. Mutation-verified: fires under the mutation, silent without it. Claim fixes: the unescape class is not "alphanumerics only"; unmeasurable rows fall back to declared rather than failing loudly, and that weakening is now stated; the narrowing-instance distribution sums to its total. 199 passed / 0 failed, 165 rows oracle-measured. hooks 225/0, stdin 8/0, doc-consistency 137/0, cowork-drift 30/0. shellcheck clean.
…d backtick Sol round 21, NOT CERTIFIED 7.0/10. No new fail-opens found across 183 probes; both findings are guard placement and boundary coverage. - Seventh route to green-while-measuring-nothing: `exit 0` at the top of `run_row` exits with no rows, no summary and status 0, and a guard written at the FOOTER is simply never reached. A guard placed after the thing it guards is skipped by anything that exits early. The same guard now sits immediately after `set -u`; the forced failure stays at the footer. Mutation-verified on both routes: early `exit 0` and the last line replaced with `true` each fire it, and the unmutated suite is silent. - The keyword shield's left-boundary class was missing the backtick, which GATE_SEP treats as a command anchor. All 32 single-escape keyword spellings inside a legacy backtick substitution are oracle-inert; eight of them were regressions from the parent commit. Probed all 32 plus the `su\do` pair: 36 shapes, zero disagreements with the shell. 200 passed / 0 failed, 166 rows oracle-measured. hooks 225/0. shellcheck clean.
Owner
Author
|
CROSS-MODEL-CLEARANCE Scoped recheck of 148f120 by gpt-5.6-sol at Named residuals, all below the bar and none blocking:
All three disclosures are fixed in the follow-up, not here, so no new commit moves this sha. {"verdict": "YES", "reviewer": "gpt-5.6-sol", "confidence": 99, "sha": "148f120728dabf8aadff071952f745a302849620"} |
Owner
Author
|
CROSS-MODEL-CLEARANCE Fable leg, bound to 148f120. Basis: an independent CERTIFIED 9.8/99 from gpt-5.6-sol with its own clean-root probes; Residuals are the same three disclosed on the Sol clearance plus the #599-#605 accepted limits. All below the bar. Process note, recorded because it is the finding this PR actually bought: 22 review attempts on one hook was past the point of return. Rounds 21 and the recheck found nothing in the shipped artifact. The replacement stop rule is folded into the skill's existing Convergence rule on a separate branch, deliberately not here. {"verdict": "YES", "reviewer": "fable", "confidence": 96, "sha": "148f120728dabf8aadff071952f745a302849620"} |
Owner
Author
|
DUAL CROSS-MODEL CERTIFIED MERGE Cleared by: gpt-5.6-sol (posted by @BaseInfinity), fable (posted by @BaseInfinity) — both bound to Merge-evidence path(s) this authorised:
Verified: CI ATTESTED, NOT AUTHENTICATED. Both clearances were posted by the same Posted by |
BaseInfinity
added a commit
that referenced
this pull request
Aug 15, 2026
…xception Round 1 on this PR returned NOT CERTIFIED 2/10, with a P0 that the rule had no deterministic firing round. Both of its clauses could claim the same finding: a false-green test-harness route is "harness-only, therefore filed" AND "a finding that invalidates verification evidence, therefore continue". Read the first way it fires at #598 round 16 — and round 17, which found the real `./\git commit` fail-open, never happens. Read the second way it never fires at all, because round 21 found a seventh false-green route and that authorizes round 22. So the two clauses become one callable test with a bounded exception: continue only when the immediately preceding COMPLETED pass recorded either an open P0/P1 showing a requested behavior is currently wrong, or the FIRST verification-evidence invalidation in this root task. An evidence-only finding buys exactly one more pass per root task; a later one is filed. Against #598 that authorizes round 17 from round 16, permits 18-21 while production P1s remain, and stops at 21 as the second evidence-only finding. Round 22 is forbidden. Five surviving statements of the old rule were found in the shipped wizard doc, plus SDLC.md and two test comments — a document stating two round-accounting rules is the defect this PR exists to fix, so all eight now state the one rule. The v1.84.0 migration still runs its eleven rounds: each was authorized by the preceding pass's real finding, which is the new test stated exactly rather than as "every round is still finding something real". Citation corrected twice over: round 21 was the SECOND evidence-only finding, not "the kind this rule now files", and round 22's artifact proves only that it returned no verdict — not that it hung. doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance pass.
BaseInfinity
added a commit
that referenced
this pull request
Aug 15, 2026
Round 6, two P1s, and the reconciled ruling on the process audit both models ran alongside it. **Ninth survivor, CLAUDE_CODE_SDLC_WIZARD.md:4383.** The "done" stop condition declared the task finished on zero unresolved requested-behavior findings, with no mention of the exception four sections above it. The first verification-evidence invalidation can arrive from an otherwise clean round, and must still authorize one more pass. Applied to #598 round 16, that sentence stops before round 17 — the round that found the last shipped fail-open. Now carries the bound, from both ends: the FIRST invalidation buys a pass, a later one is filed. **The guard is deleted and filed as #608.** It cost rounds 4, 5 and 6 against a documentation change, and this repo's own rule says no test costs more rounds than the change it guards. Round 6 defeated it twice more — `a finding that invalidates the verification evidence` is not in its vocabulary, and `the evidence-only exception is spent after every invalidation` satisfies its spendable-budget clause while saying the opposite. An allowlist over English formulations recognizes a vocabulary, not a meaning; each repair covered the named counterexample and left the next synonym open. #608 carries all five mutations and the design that removes the class at the source: state the rule once, reference it everywhere else. Building it mid-review was the process violation the PR itself documents — a loop reviewing its own churn. Recording that rather than repairing it a third time. doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance green.
BaseInfinity
added a commit
that referenced
this pull request
Aug 15, 2026
* docs(sdlc): give the review loop a termination condition The Convergence rule counted passes and then said "this is accounting, not a cap: continuing stays the recorded decision's call." Nothing terminated the loop, so it ran on its own findings. #588/PR #598 reached 22 review attempts on one 330-line hook. Diagnosis, agreed by both reviewers independently: "iterate until findings taper off" is not a stopping rule. It counts every finding equally, so an adversarial reviewer expanding into the test apparatus keeps the count above zero forever. It measures reviewer productivity, not residual risk. In #598 the last shipped defect found was introduced by the loop's own previous fix. A first draft added a SECOND rule beside Convergence — two consecutive clean rounds — and was rejected on review: it demanded a second clean round while forbidding the only thing that could trigger one, since harness findings could not justify a pass and resubmitting unchanged is reviewer-shopping. A document stating two round-accounting rules is worse than one stating neither. So this folds the condition INTO Convergence, one counter and one continuation test. After the review and the verify, stop unless the record holds an unresolved in-scope P0/P1 in the deliverable, or a finding that invalidates its verification evidence. Harness-only, below-bar and out-of-scope findings are filed and authorize no further pass. A pass that found a defect in the deliverable is not clean whoever introduced it — including the loop itself, which is where the first draft was wrong. Also: do not prompt a reviewer to "find the next route" or "defeat the new guard". Asked for a fresh category, an adversarial reviewer produces one. The wizard doc stated the old rule in four more places; all four now match, so the shipped set does not contradict itself. Cowork copy byte-identical. Not in scope, deliberately: enforcement. The rule is stated, not checked — nothing in handoff.json records the stop condition. Recorded as a known limit. doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance pass. * fix(sdlc): make the stop condition callable, and bound the evidence exception Round 1 on this PR returned NOT CERTIFIED 2/10, with a P0 that the rule had no deterministic firing round. Both of its clauses could claim the same finding: a false-green test-harness route is "harness-only, therefore filed" AND "a finding that invalidates verification evidence, therefore continue". Read the first way it fires at #598 round 16 — and round 17, which found the real `./\git commit` fail-open, never happens. Read the second way it never fires at all, because round 21 found a seventh false-green route and that authorizes round 22. So the two clauses become one callable test with a bounded exception: continue only when the immediately preceding COMPLETED pass recorded either an open P0/P1 showing a requested behavior is currently wrong, or the FIRST verification-evidence invalidation in this root task. An evidence-only finding buys exactly one more pass per root task; a later one is filed. Against #598 that authorizes round 17 from round 16, permits 18-21 while production P1s remain, and stops at 21 as the second evidence-only finding. Round 22 is forbidden. Five surviving statements of the old rule were found in the shipped wizard doc, plus SDLC.md and two test comments — a document stating two round-accounting rules is the defect this PR exists to fix, so all eight now state the one rule. The v1.84.0 migration still runs its eleven rounds: each was authorized by the preceding pass's real finding, which is the new test stated exactly rather than as "every round is still finding something real". Citation corrected twice over: round 21 was the SECOND evidence-only finding, not "the kind this rule now files", and round 22's artifact proves only that it returned no verdict — not that it hung. doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance pass. * fix(sdlc): bound the sixth restatement of the rule, which I wrote myself Round 2 found one surviving statement of the old rule, at CLAUDE_CODE_SDLC_WIZARD.md:4397 — and it was not a leftover. I wrote it in the round-1 alignment pass, restating the exception without "immediately preceding COMPLETED pass" and without "FIRST in this root task". On its own terms round 21 authorized round 22, which is the loop this PR exists to stop. That is the finding worth recording: aligning a rule by hand into six places is itself a way to reintroduce it, and the pass that fixes the contradiction is as likely to author the next one as any older text. Every remaining mention of the evidence exception now carries the bound. doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance 10/0. * fix(sdlc): bound the seventh restatement, and guard the class by test Round 3 found the seventh unbounded statement of the evidence exception, in the review-cycle diagram — the second one this alignment work authored itself. Three rounds, three survivors, every one found by a careful human sweep and two of them written by the sweep that was fixing the others. That is a class, not three instances, so it gets a machine check. `test_evidence_exception_always_carries_its_bound` normalises whitespace across the four documents that state the rule, finds every mention of the exception, and requires a bound within the surrounding window. Whitespace normalisation is load-bearing: two of the real statements wrap across lines, so a line-based check would have missed them. Mutation-verified against the two survivors an actual review round caught, restored verbatim: - the diagram line (round 3's finding) -> FAIL, names the file and the text - the numbered item at 4397 (round 2's) -> FAIL, likewise - unmutated -> PASS, 138/0 The first version of this guard was itself wrong and its first mutation run proved nothing: it required the literal word "first", so it failed on the rule's own citation, which bounds the exception by saying a SECOND evidence-only finding is filed. Both the mutant and the clean tree failed for that unrelated reason. The predicate now accepts any statement that says which invalidation it is or caps how many passes it buys. doc-consistency 138/0, cowork-drift 30/0, docs-usability 29/0, compliance 10/0. * fix(tests): make the bound guard check attachment, not proximity Round 4 found the guard unsound, and demonstrated it: two genuinely wrong statements passed at 138/0. "...or a verification-evidence invalidation after the first pass" — the ordinal modifies the wrong noun "...or the second verification-evidence invalidation" — reverses the rule outright Both satisfied a predicate that searched for "first" or "second" ANYWHERE within 320 characters. That is proximity, not meaning. Two changes, and the second is the one that matters: 1. Name what is ALLOWED rather than chase what is forbidden. A bound counts only when the ordinal is attached to the invalidation itself, or the sentence caps how many passes the exception buys, or it says a later one is filed. 2. Require the allowed pattern to CONTAIN the mention rather than sit near it. The allowlist alone still passed both mutations, because the sentence AFTER the mutated clause was itself correctly bounded and satisfied the window. That is the same defect in a new place: a neighbour's correctness was being read as this statement's. Mutation-verified against four wrong statements — round 4's two, plus the two survivors rounds 2 and 3 actually found in the documents — each asserted to have applied before its result was believed. All four FAIL; the clean tree PASSES at 138/0, including the rule's own citation, which bounds the exception by saying a SECOND evidence-only finding is filed. Also fixed: the guard read its four documents by relative path, so the suite failed with four "unreadable" files when invoked from outside the repo root. It now resolves them against REPO_ROOT, verified by running the suite from a temp dir. doc-consistency 138/0, cowork-drift 30/0, docs-usability 29/0, compliance 10/0. * fix(sdlc): bind the ordinal to the invalidation, and kill the eighth survivor Round 5, two P1s. **The guard had a plausible false negative.** This natural rewrite passed at 138/0: ...or the first pass after a verification-evidence invalidation in this root task "first" modifies "pass", not the invalidation, so every invalidation buys a pass again — the exact loop the bound exists to stop. `[^.]{0,40}` between the ordinal and its noun was too loose. The ordinal must now sit against the invalidation with nothing but emphasis marks between them, and the statement must name the per-root-task scope. **Eighth survivor, and the worst one yet:** the diminishing-returns section said "you are converged when TWO CONSECUTIVE ROUNDS produce nothing above P3". That is the unreachable two-clean-round condition this PR explicitly rejected, still shipping four sections below the rule that rejects it. Rewritten to the stop condition, keeping the part that earned its place — a fresh reviewer's first look is a new pass, not a re-run, because one reviewer certified at high confidence immediately before a fresh one found six P1s in the same code. Mutation-verified against all five wrong statements review has produced across four rounds, each asserted to have applied before its result was believed: ...the first pass after a verification-evidence invalidation round 5 FAIL ...a verification-evidence invalidation after the first pass round 4 FAIL ...the second verification-evidence invalidation round 4 FAIL the diagram line, unbounded round 3 FAIL the skill's clause (b), unbounded round 2 FAIL clean tree PASS 138/0 Adding the converged sentence tripped the guard, which is the guard working: it states the exception in a phrasing the allowlist did not cover. I added the form — the exception as a finite budget that can be spent — rather than rephrasing the document around my own check. doc-consistency 138/0, cowork-drift 30/0, docs-usability 29/0, compliance 10/0. Suite also green invoked from outside the repo root. * fix(sdlc): bound the ninth survivor, and cut the guard to #608 Round 6, two P1s, and the reconciled ruling on the process audit both models ran alongside it. **Ninth survivor, CLAUDE_CODE_SDLC_WIZARD.md:4383.** The "done" stop condition declared the task finished on zero unresolved requested-behavior findings, with no mention of the exception four sections above it. The first verification-evidence invalidation can arrive from an otherwise clean round, and must still authorize one more pass. Applied to #598 round 16, that sentence stops before round 17 — the round that found the last shipped fail-open. Now carries the bound, from both ends: the FIRST invalidation buys a pass, a later one is filed. **The guard is deleted and filed as #608.** It cost rounds 4, 5 and 6 against a documentation change, and this repo's own rule says no test costs more rounds than the change it guards. Round 6 defeated it twice more — `a finding that invalidates the verification evidence` is not in its vocabulary, and `the evidence-only exception is spent after every invalidation` satisfies its spendable-budget clause while saying the opposite. An allowlist over English formulations recognizes a vocabulary, not a meaning; each repair covered the named counterexample and left the next synonym open. #608 carries all five mutations and the design that removes the class at the source: state the rule once, reference it everywhere else. Building it mid-review was the process violation the PR itself documents — a loop reviewing its own churn. Recording that rather than repairing it a third time. doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance green.
BaseInfinity
added a commit
that referenced
this pull request
Aug 15, 2026
#610) * fix(codex-gate): refuse a review leg typed outside its launcher (#590) `scripts/run-review-leg.sh` has prevented the codex stdin hang since #590 closed. Its own header says a leg typed outside it "has no owner and no status." On 2026-08-14 two legs for PR #606 were typed by hand anyway, in a session that had read that header: attempt 1 no `< /dev/null` hung at 39 bytes, the #590 signature attempt 2 no launcher exhausted its budget, returned no verdict The maintainer's reaction was "are we failing to do reviews is it breaking or hanging". The knowledge was written down, documented, and closed as an issue. It was not applied. Writing it down again is what this repo has historically done, and PR #606 is a seven-round demonstration of why that fails. Both review legs were asked independently what to do and converged on mechanizing this boundary, on the ALREADY-REGISTERED Bash gate rather than as a new hook. Sol also named the tension honestly: the standing "no new guard or tooling surface" ruling is against this, and should yield, because Rung 1's own stop condition is that review legs must not hang — tonight proves #590 is behaviourally incomplete despite being closed. The predicate is the smallest one that catches the accident: `codex` in command position followed by `exec`, allowed when the command names the launcher. It reuses the gate's existing GATE_ANCHOR/GATE_PREFIX/GATE_WRAPPER machinery rather than introducing a second notion of command position, so the two cannot drift, and it reads MASKED_COMMAND — a quoted mention is already a Q by then, so the #588 false-positive class is not repeated. It sits ABOVE the git-commit detector, which exits 0 on every command it does not recognise. RED first, 5 failed / 5 passed: exactly the five refusal rows failed and every allow row passed. GREEN 10/0. The refusal rows are real commands from the session, not invented ones. The suite carries the hoisted forced-failure guard from #598 round 20 — placed above every definition, because a guard after the thing it guards is skipped by any early exit between them. Stated narrowly, because the merge gate's ROADMAP note records the cost of not doing so: this is an accident-catcher for the Claude Code Bash path, not a security boundary. A leg launched from a terminal never meets it. codex-gate-command-position 200/0 (166 rows oracle-measured), doc-consistency 137/0, cowork-drift 30/0, compliance green, shellcheck clean on both files. * fix(codex-gate): bound the review-leg predicate to codex's own grammar Round 1 returned NOT CERTIFIED 3/10 with eight blocking findings. Three were introduced by this lane and are fixed here. Five are inherited from the shared machinery, demonstrated by parity rather than argued. THE THREE THAT WERE MINE 1. `codex e` walked straight through. `codex --help` lists "exec ... [aliases: e]" and `codex e --help` prints exec's help — a real review leg the lane allowed. Verified on the installed CLI that `ex` and `exe` do NOT resolve, so this is a declared alias list, not prefix inference, and enumerating the two is exact rather than a guess. 2. Three legitimate commands were REFUSED — the #588 false-positive class reappearing in a new lane: codex review exec `exec` is the review PROMPT codex help exec codex exec-server --help a different subcommand entirely The predicate accepted arbitrary words between `codex` and `exec` and ended on a word boundary. Now only OPTION tokens may appear there, and the subcommand must be a COMPLETE token — the trailing class excludes the `-` that starts `exec-server`. 3. Three more refusals where a wrapper runs something else entirely: env echo codex exec env grep codex exec README.md env ls /tmp/codex exec These came from reusing GATE_SKIP, which consumes the wrapped command and restarts matching at its arguments. What a wrapper may carry is now enumerated instead of skipped: its own options, an option's value, a bare number (`timeout 300`), or an assignment (`env FOO=1`). None is a command name, so the prose shapes stop matching while `env codex exec`, `timeout 300 codex exec` and `nice -n 5 codex exec` stay caught. Deliberately narrower than the git lane. It costs coverage of a wrapper whose argument happens to look like a command, and buys back three of the eight findings. THE FIVE THAT ARE INHERITED, MEASURED NOT ASSERTED Each shape was run against BOTH lanes from a directory with no `.reviews/`, so detection alone decides: shape codex git quoted subcommand ALLOW ALLOW codex "exec" / git "commit" quoted command name ALLOW ALLOW quoted wrapper name ALLOW ALLOW param expansion in assign ALLOW ALLOW FOO=${UNSET:-a;b}x arith expansion in assign ALLOW ALLOW FOO=$((1|2))x cmdsub in assign ALLOW ALLOW FOO=$(printf a; :)x caffeinate wrapper ALLOW ALLOW not in GATE_EXTERNAL heredoc body (prose) REFUSE REFUSE greedy array assign prose REFUSE REFUSE bash -c quoted payload ALLOW ALLOW Ten shapes, identical behaviour in both lanes. This lane did not widen the gate's boundary; it reproduced it, which is the intended consequence of reusing the machinery rather than writing a second notion of command position. The quoted classes are #599 and #611, the heredoc is #601, the punctuation anchors are #603. The two not previously filed — expansion inside an assignment, and a wrapper outside the enumerated set — are filed now. Every command the reviewer demonstrated is a row in the suite, refusals and allows alike, so none can come back silently. 14 rows added, 10 -> 24, all green. codex-gate-command-position still 200/0 with 166 rows oracle-measured. * fix(codex-gate): make option arity explicit, and carry a wrapper's value Round 2, two P1s, both demonstrated against the real CLI. Round 2 also reproduced the ten-row parity table exactly — seven ALLOW/ALLOW and three REFUSE/REFUSE — which is what settles round 1's other five findings as inherited rather than introduced. AN OPTION'S VALUE IS NOT THE SUBCOMMAND From a directory containing `exec/`, `codex <dir-flag> exec review --help` is a valid invocation of `codex review`, and the lane refused it. So did the model flag's equivalent form. The gap between `codex` and the subcommand was "an option, then MAYBE a value" — and an optional trailing word is not a grammar, it is two grammars. The engine finds the parse that matches: give up the value, and `exec` becomes the subcommand. The other branch consumes the value and then fails to find a subcommand, and a failed branch never wins. The fix is to remove the choice. Both option classes are now enumerated from `codex --help` — thirteen value-taking, eleven boolean — and a value-taking option MUST consume its value. `<dir-flag> exec review` is then unambiguous: `exec` is the flag's value, `review` is the subcommand, no match. A generic branch for EITHER class is what made this possible, so neither has one. An option in neither list fails to match and the command is allowed. That is the deliberate direction: a missed leg is an accident this lane did not catch; a false refusal blocks the maintainer's own review. A WRAPPER'S OPTION VALUE `env -u FOO codex exec`, `xargs -I X codex exec` and `sudo -u USER codex exec` all reach codex, and all three were allowed. Their git twins are refused, so this was the lane's own gap. The carry admitted dash-tokens but not the TEXTUAL value a wrapper option takes. Named rather than generic, and the reviewer's warning is the reason: "do not add a generic optional word, which would recreate wrapped-prose false positives." `env -i echo codex exec` would read `echo` as -i's value and refuse a command that runs echo. It has a row. Six rows added beyond the reviewer's, pinning the two enumerated lists. They exist because both are transcribed from `codex --help`, and a CLI change is how they rot silently — a boolean that becomes value-taking turns into a false REFUSAL, the direction that blocks the maintainer. Found by falsifying the fix before submitting it, not by a reviewer. review-leg-launcher-required 24 -> 40 rows, all green. codex-gate-command-position 200/0, 166 rows oracle-measured, unchanged. doc-consistency 137/0. shellcheck clean. * fix(codex-gate): close the option gap instead of modelling it Round 3 returned NOT CERTIFIED 4/10 with four blockers. The scores across three rounds were 3, 6, 4. That is not convergence, and the reason is visible in what each round found: round 2 an option's value is not the subcommand codex <dir-flag> exec review round 3 compact short values codex -mfoo exec round 3 -h and -V terminate, never dispatch codex -h exec round 3 --image is VARIADIC codex -i a exec round 3 wrapper arity is not one letter class xargs -E vs sudo -n Every one is a fact about codex's or a wrapper's OPTION GRAMMAR. None is a fact about the accident being guarded. Each fix was correct and each opened new surface, because modelling a CLI's option grammar in a regex is building the parser #533 ruled out, arriving one round at a time. The reviewer's prescribed smallest certifying change was a fourth layer of the same grammar. That direction is declined, with its own three-round record as the evidence, and the design authority ruled the same way independently. WHAT REPLACES IT `codex` must be followed DIRECTLY by `exec` or its declared alias `e`. A wrapper may carry nothing between itself and `codex`. GATE_CODEX_VALOPT, GATE_CODEX_BOOLOPT, GATE_CODEX_OPT, GATE_WRAP_VALOPT and the wrapper carry are deleted outright. All four of round 3's findings dissolve rather than get patched. Two were pure false positives and are simply gone: `codex -h exec` and `codex -i a exec` dispatch no leg and are no longer refused. Two become documented accepted limits. This still catches both accidents that motivated the lane, and not by luck. A real leg is `codex exec --model X -c key=value "prompt"` — options come AFTER the subcommand. An option before it is a shape no leg in this repo has ever been typed in. WHAT IT COSTS, SAID PLAINLY `codex --model X exec` escapes. So does every wrapper carrying an argument: `timeout 300 codex exec`, `env -u FOO codex exec`, `xargs -I X codex exec`. Accepted and REPORTED, not fixed. A missed leg is an accident this lane did not catch; a false refusal blocks the maintainer's own review. #601 is the precedent for a measured accepted limit. Every command any round demonstrated is still a row — expectations flipped to the contract that actually holds, each marked ACCEPTED LIMIT. Round 3's four shapes are rows now too. A boundary pinned in the direction it holds is worth more than one pinned where it kept moving. review-leg-launcher-required 40 -> 47 rows, all green. codex-gate-command-position 200/0, 166 rows oracle-measured, unchanged. doc-consistency 137/0. hooks pass. shellcheck clean. * fix(codex-gate): name the token boundary instead of negating a word class Round 4 scored 8/10, up from 4/10, with the option and wrapper accepted limits all behaving as documented and the parity table intact. One blocker, and it is a real false positive in the part the redesign kept rather than replaced. The subcommand had to be a complete token, and the test for that was the trailing class `[^A-Za-z0-9_-]`. That negation is wrong about three characters which all CONTINUE a shell token: SUFFIX=-server; codex exec$SUFFIX --help runs codex exec-server codex exec.foo codex exec=foo The first is the reviewer's, and it is the sharpest form: bash expands it to a valid invocation of a DIFFERENT subcommand, and the hook refused it. `-` was already excluded for exactly this reason — `exec-server`. `$`, `.` and `=` are the same case and were missed because the rule was written as "not a word character" rather than as the thing it actually means. So it is named: whitespace, a metacharacter that ends a command, or end of line. Six rows — the three false positives above, and the three real boundaries that must still catch a leg (`codex exec` at end of line, `codex exec>out.md`, `codex e;true`). Also corrects the parity table's counts in the handoff, 7/3 to 8/2. Same ten rows, same result, miscounted since round 2 and carried forward three times without anyone re-adding them. The reviewer re-derived them and caught it. review-leg-launcher-required 47 -> 53 rows, all green. codex-gate-command-position 200/0, 166 rows oracle-measured, unchanged. hooks pass. shellcheck clean. * fix(codex-gate): drop the brace boundary and delete the launcher hatch Round 5 ran two independent reviewers on the same HEAD. Both CERTIFIED the round-4 design at 8/10 — the option and wrapper accepted limits all behaved as documented, the git detector was byte-identical to origin/main across fifteen shapes, and the parity table held. Each found exactly one one-line defect. A CLOSING BRACE IS NOT A METACHARACTER `}` was in the token-boundary set, so a subcommand followed by `}foo` was refused. But `}` closes a group only as a RESERVED WORD — it needs a preceding `;` or newline and its own whitespace. As a bare character it is ordinary, and bash prints `exec}foo` as one word. Removed, with `{` pinned beside it. `)` stays; it is a real metacharacter. That is round 5's mistake in a different costume: the boundary set was written from what looks like punctuation rather than from what the shell treats as one. THE LAUNCHER HATCH IS DELETED, NOT TIGHTENED The lane allowed any command whose raw text mentioned `run-review-leg.sh`, so naming the launcher and then hand-typing a leg beside it passed. The lane could be satisfied by MENTIONING the launcher rather than using it. The second reviewer withheld its last point on exactly this. It was never needed. A launcher invocation is `run-review-leg.sh OUTPUT PROMPT [args]` and contains no review-leg token, so the predicate never matched it to begin with. Verified by neutralising the check and re-probing: all three real launcher shapes stay ALLOWED, the bypass becomes REFUSED. The hatch was dead weight that could only ever be wrong, and the suite still pins the launcher shapes as allowed so its removal cannot regress them. WHAT DELETING IT SURFACED This commit could not be made through the gate at first. The hatch had been masking #601 in this lane: a commit whose MESSAGE quotes the refused command in prose is itself refused, because the gate reads a heredoc body as command text. Inherited, identical in the git lane, now recorded in known_limits with the workaround actually used to land this — write the message with a non-Bash tool and commit with -F. review-leg-launcher-required 53 -> 57 rows, all green. codex-gate-command-position 200/0, 166 rows oracle-measured, unchanged. doc-consistency 137/0. hooks pass. shellcheck clean. * fix(codex-gate): correct the stale hatch comment, pin the array shape Round 6 split. One reviewer CERTIFIED at 9/10 with its round-5 withholding resolved. The other returned 8/10 with one blocker and one documentation defect. THE DOCUMENTATION DEFECT WAS MINE The lane's header still described the predicate as "allowed if the command names the launcher". That hatch was deleted in round 6 and the sentence survived it — a comment claiming behaviour the code no longer has, which is worse than no comment. Rewritten to point at the note that explains the deletion. THE BLOCKER IS REAL AND INHERITED An assignment whose value is an ARRAY containing the two tokens is refused, and so is a launcher call placed after one. Nothing is invoked. The shared separator treats `(` as opening command position and the boundary set treats `)` as ending the token. Measured, not argued. From a tree with NO .reviews/, so detection alone decides: shape codex git array data REFUSE REFUSE array data, then a command REFUSE REFUSE string assignment ALLOW ALLOW plain subshell REFUSE REFUSE brace group REFUSE REFUSE Five shapes, identical in both lanes. The reviewer was right that this was not previously documented — round 1's table covered a greedy array PREFIX, which is a different shape — and right to raise it. It is inherited rather than introduced. It is pinned rather than fixed. Fixing it means knowing that `(` opens an array VALUE rather than a subshell, which is context a regex does not have, and it would have to change the anchor the git lane shares — blocker condition (3) against a detector this PR has kept byte-identical throughout. A METHODOLOGY NOTE WORTH MORE THAN THE FINDING The first parity probe reported DIFF on every array row: codex REFUSE, git ALLOW. That was wrong. Probed from this repo, the git lane exits 0 because a valid handoff is present — it allowed on the ARTIFACT, not on detection. Reading that as an allow would have reported the finding exactly backwards, as introduced-by-this-diff rather than inherited. The no-.reviews/ rule that round 1 established is what caught it, and it is now written into the suite beside the rows rather than living only in a commit message. review-leg-launcher-required 57 -> 60 rows, all green. codex-gate-command-position 200/0, 166 rows oracle-measured, unchanged. hooks pass. shellcheck clean. * fix(docs): list the launcher suite in CONTRIBUTING, unbreaking CI The lane's suite was added to ci.yml in round 1 and never added to CONTRIBUTING.md, which `test-workflow-triggers.sh` enforces: every script CI runs must appear in both of CONTRIBUTING's lists. So CI validate has been RED since round 1, through seven review rounds and two independent reviewers, and nobody looked — including me. Both reviewers ran the suites directly and reported them green, which they were; the failure was in a guard neither was asked about and I never checked between rounds. The merge gate is what caught it, refusing the merge on a non-green check and declaring it not waivable. That is the gate working, and it is also the finding: seven rounds of review depth on a predicate, zero rounds of attention on whether the build was green. No code change. CONTRIBUTING.md only, both lists.
This was referenced Aug 15, 2026
Open
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #588 for the prose half. The out-of-tree half is dispositioned as an accepted limit — see below.
What the issue said, and what the probe found
The issue reports two reproductions. Probed against the current hook on a clean root (so only the text layer is under test — probing inside the working repo is confounded by the in-flight
PENDING_RECHECKlane), the issue is partly stale:grepwhose pattern contained the verb. Both quoted forms already pass — the roadmap(#217): loud WARNING below xhigh — max preferred, xhigh floor #236(b) masking pass collapses quoted spans toQ. Only unquoted mentions false-fire.cd "$TMPDIR/fix" && git commit) is a true positive by text. It really does invokegit commit. Nothing is wrong with the detector; the gate's scope is wrong.So #588 is two defects with different fixes, and its acceptance criterion as written cannot be met by one change. Full probe table is on the issue.
The fix (defect 1)
\bgit→gitrequired in command position: start of string, or after;&|(){}!,$(, a backtick, or athen/do/elsekeyword.This is not the positive command parsing #533 rejected — nothing here decides what a command does. It anchors the existing negative detector to the places bash can begin a command, which is decidable from the text.
Anchoring alone fails the issue's own bar ("coverage in both directions or it does not land"). Measured with a bash oracle,
env X=1 git commit,command git commit,nice git commitandecho -m x | xargs git commitall invoke git for real and would have escaped. So the anchor ships with a closed, enumerated transparent-prefix set: assignments (bash runs an assignment-prefixed command directly — this repo's own workflow uses the shape) plusenv command nice nohup xargs timeout sudo stdbuf.(\S*/)?gitkeeps/usr/bin/git commitcaught.The set being closed is the accepted limit: an un-enumerated wrapper is a false negative, recorded on #588 and pinned as a test row. The hook says of itself that it is an accident-catcher, not an adversary barrier, and #588 concedes deliberate bypass is trivial.
The unplanned find: a live fail-open
Strict anchoring broke
k=5,9,13of the backslash contract. Those are real invocations, and the old detector had been blocking them by accident — the leftover literal backslash supplied the word boundary\bneeded.Underneath was a real bug: the normalization treated every backslash run before a newline as a line continuation, when only an odd run is one. An even run leaves the newline real, so git genuinely runs. Fixed by matching
(^|[^S])(SS)*S\n.That also closed the three false denials at
k ≡ 3 (mod 4)which the hook had recorded as declined on complexity grounds, with a prototyped fix parked on #581. Getting the parity right closed both directions at once. The contract is now exactly parity, no exceptions.Verification
tests/test-codex-gate-command-position.sh— 200 passed, 0 failed. The generated metric is 166run_rowrows oracle-measured; counting every passing assertion, 195 of the 200 are oracle-measured and the other five are the declared rows enumerated below. Want values are generated by a bash oracle (a fakegiton PATH that logs invocation) wherever the oracle can run, rather than hand-asserted. Five rows are not oracle-generated on macOS, and each prints its reason whenit runs:
timeout 5,timeout 5s,stdbuf -oL(absent on macOS, measured onthe Linux runner),
sudo(cannot run unattended), and/usr/bin/git(anabsolute path bypasses the PATH interposition, so it is declared on every
platform). Generated by grepping the run for
declared (, not recalled.k=1..16derived by decoding the JSON to real shell text and oracling it, not by reasoning about parity.Also green:
test-hooks.sh225/0 (its backslash table updated to parity),test-hook-stdin-bounded8/0,test-doc-consistency137/0,test-cowork-drift30/0,test-merge-gate,test-cross-model-clearance49/0,test-roadmap-integrity5/0,test-compliance,test-workflow-triggers,test-stop-hook-terminates4/0, and the three fires-once/mcp-audit suites by exit status. Registered inci.ymlandCONTRIBUTING.md.Not fixed, deliberately
Defect 2 (out-of-tree commit). Deciding where a command commits requires tracking
cd, which is the parsing #533 ruled out and #588 forbids re-litigating. Recorded as an accepted limit and pinned as a test row. It fails closed — the cost is the file-invocation workaround that already exists.Review rounds 2-4 (Sol), after the original body was written
The detector went through three more adversarial rounds. What each bought,
every want value oracle-measured except the five enumerated declared rows, and every finding diffed against
origin/mainso a regression I introduced is distinguishable from a pre-existing hole:
separator". Bash disagrees: a backslash makes the next character ordinary word
content.
FOO=a\ b git commit,/path\ with\ spaces/git commit,env FOO=a\;b git commitand< /dev/null git commitall invoke git and allescaped. Fixed with ONE word atom (
GATE_WORD) used at every site thatconsumes a word, which was Sol's prescription over four local patches.
env 2>&1 git commit,\env git commit,builtin command git commit. The&of a redirection is not a command boundary; the escape rulealready applied to
gitwas never applied to wrappers;builtinwas missing.>|(bash's clobber-override) missing from the operator set,builtinstacking, andgit -c user.name=A\ b commitwhere git's own optionoperands still used
\S+. Plus four false POSITIVES:builtin env git commitis inert (builtin runs only builtins), andA-B=1/1A=1are notassignments to bash at all.
Two corrections I made against the reviewer, both settled by the oracle:
coproc G git commitwas declared as invoking; it does not — with a simplecommand bash takes
Gas the command word. And addingbuiltinto the wrapperalternation was my own false positive, caught by a test row rather than review.
Accepted limits, as pinned (supersedes the earlier table)
Every row below is an oracle-derived test row in
tests/test-codex-gate-command-position.sh, not prose.Fail-OPEN (4) — all pre-existing on
origin/main, all filed:bash -c 'git commit',eval '...',"/usr/bin/env" git commitFOO=${UNSET:-a;b}x git commit,$((1|2)),$(printf a; :)caffeinate git commit— wrapper outside the closed setprintf '%s\n' '"' && git commit -m 'fix "quoted" handling'Fail-CLOSED (6) — cost is a rephrase:
time echo git commit2>¬afd git commitA[0]-B[1]=x git commitA[0]=x; echo A[1]=y git commit;cd-tracking #533 ruled outReview outcome
Twenty adversarial cross-model rounds. Sixteen narrowing instances were
found — cases where tightening a grammar to close a false positive opened a
fail-open, or the reverse. Six were in
GATE_ASSIGN, one inGATE_REDIR, fourin the quote masker, two in the
gitpath/basename escape handling, one in theescaped-keyword shield (shielding
\doinsidesu\do, a fail-open), one in theunescape class (unescaping
=manufactured an assignment prefix the shell neversees), and one was a proposed fix that looked correct and was measured wrong
before it landed. That is the whole list, and it sums to sixteen.
None was found by reading a regex. All sixteen were found by probing
origin/mainand the parent commit against the head with a bash oracle.Seven findings were about the TESTS rather than the code — seven distinct routes
by which the suite could report green while measuring nothing. Each has its own
guard, and each guard is itself mutation-verified:
ORACLE_REQUIREDcanary rowfail()cannot record a failure[ "$FAIL" -eq 0 ]replaced withtrueexit 0at the top ofrun_row— green, no rows, no summaryTwo of them in detail:
testing showed the suite stayed green against it. The rows needed a quoted
span on both sides of the invocation.
counted as neither pass nor fail, vanishing. bash 3.2 has no
command_not_found_handle, so the fix is structural: helpers are definedabove every row, with an assertion that fails loudly if one moves back.
Follow-ups split out rather than folded in: #600, #601, #602, #603, #604, #605.