docs(sdlc): give the review loop a termination condition - #606
Merged
Conversation
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.
…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.
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.
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.
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.
…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.
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.
Owner
Author
|
CROSS-MODEL-CLEARANCE Codex GPT-5.6 Sol, reasoning effort No findings. Verified independently: no tenth unbounded restatement in any of the four documents; the rule authorizes #598 round 17 (the first evidence invalidation spends the one-time exception) and terminates a loop where a second invalidation arrives from an otherwise clean round; the guard deletion removed exactly its comments, function and invocation — 137/0 at the pre-guard commit Disclosed: rounds 6 and 7-attempt-1 were launched by hand instead of through the launcher. Attempt 1 hung at 39 bytes with no {"confidence": 99, "reviewer": "codex-gpt-5.6-sol-high", "sha": "612297e5d2d6ff03c51b4fdb9991c67e39055952", "verdict": "YES"} |
Owner
Author
|
CROSS-MODEL-CLEARANCE Fable 5, fresh subagent, blind diff review plus one dialogue round. Ran both suites itself: doc-consistency 137/0, cowork-drift 30/0, skill copies byte-identical. Enumerated all nine live statements of the exception across the four documents and confirmed each carries the bound. Walked the motivating incident in both directions: round 16's first invalidation authorizes round 17; round 21's later evidence-only finding is filed, so round 22 is forbidden. Raised four P3s, all filed to #609 rather than fixed in-PR — fixing them here would reenact the pattern this PR exists to stop, the same reasoning that sent the guard to #608. On the dialogue round it disputed nothing about the deferral and decomposed its 92%: roughly half was findings sitting undispositioned, resolved by #609; the remainder was doubt about the #598 round-by-round narrative it could not independently audit, which it then ruled does not touch the deliverable — the rule's mechanics hold regardless of whether the illustrative round numbers are exact. {"confidence": 96, "reviewer": "fable-5-subagent", "sha": "612297e5d2d6ff03c51b4fdb9991c67e39055952", "verdict": "YES"} |
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
BaseInfinity
added a commit
that referenced
this pull request
Aug 15, 2026
…tly not happen (#620) * release: v1.98.0 — the review loop terminates, and a leg cannot silently not happen Nine consumer-affecting commits have been sitting on main since the v1.97.0 release commit, reaching nobody. release-drift.yml filed #618 for exactly this, and the v1.98.0 milestone has been closed with eleven issues and no tag. What consumers get that they did not have: #606 the review loop has a stated termination condition #590 a launcher for review legs, and a gate that requires it (#595 the owner-not-observer design, #596 falsify-before-review, #610 the gate lane that refuses a hand-typed leg) #613 the build is an input to a review verdict, not background noise #588 the commit gate stops matching `git commit` in prose #581 a JSON-escaped newline no longer hides an invocation from the gate #533 the review gate no longer makes the review protocol uncommittable #577 the review contract as one prose batch The two hook fixes are the ones worth reading twice: both REFUSED legitimate work on consumer machines. A commit whose message merely mentioned the phrase was blocked, and a command split across an escaped newline walked straight past. Consumers have been running both since v1.97.0. package.json and CHANGELOG.md only. Every item was certified on its own PR; this commit ships them. doc-consistency 137/0. release-workflow 14/0. release-drift 21/0. * fix(changelog): restore the 1.97.0 header this release deleted Review found it, and it was the worst kind of mistake to make in a release PR: the edit that inserted the 1.98.0 entry anchored on the `## [1.97.0]` header and replaced it, so the header vanished. Headers ran 1.98.0 -> 1.96.0, and the whole v1.97.0 entry — the Cowork classifier removal and everything beneath it — read as part of THIS release. A changelog that attributes a previous release's work to the current one is worse than a missing entry: it is confidently wrong, and it ships to every consumer as the record of what they just installed. Headers now read 1.98.0 (7), 1.97.0 (65), 1.96.0 (153), 1.95.0 (228). No suite checks that changelog version headers are present and ordered, which is why CI passed on it. Filed separately rather than fixed here — this PR's own review bar blocks on any file other than package.json and CHANGELOG.md changing, and a release PR is the wrong place to grow test surface. The same review verified every load-bearing claim in the new entry against its certified source and found each accurate verbatim, and confirmed no consumer-affecting change is missing from it. * fix(release): bump every version stamp, and correct two false CHANGELOG claims CI validate was red on 3e5d36a: test-self-update's version-parity check, twice. SDLC.md:123 enumerates nine locations the version string lives in; this release had bumped package.json and CHANGELOG.md only. Bumped the rest. The strings left at 1.97.0 are deliberate — transcripts of observed command output, and historical "removed in v1.97.0" references. Bumping those would falsify them. Two CHANGELOG claims were wrong and are corrected: - It said consumers receive scripts/run-review-leg.sh. They do not. `npm pack --dry-run | grep -c scripts/` is 0; package.json's files list has no scripts/ entry. The #610 gate lane ships in hooks/; the launcher it names is #594, still open. A "Not in this release" section now says so. - It said all three commit-gate defects blocked legitimate work. #588 and #533 did; #581 failed OPEN. Opposite failure directions, and reading one for the other gives a consumer the wrong idea of what the gate is for. The review bar itself was also wrong. Rule 3 admitted only package.json and CHANGELOG.md, which is mutually exclusive with CI's parity test — a release PR satisfying it cannot go green. Amended to permit the SDLC.md:123 stamps. * docs(sdlc): add the Cowork marketplace manifest to the version-bump checklist Both reviewers found this independently on the v1.98.0 release PR — Sol as a P1, Fable as a P3. SDLC.md:123 enumerates where the version string lives, and the amended release-PR rule points at that enumeration. It omitted cowork/.claude-plugin/marketplace.json, which every release must bump: test-cowork-drift.sh:89 enforces marketplace/plugin parity and :283 enforces Cowork/package parity, so moving package.json forces both Cowork manifests. So a release that satisfies the checklist still fails CI, and a release that passes CI is outside the checklist. Same shape as the rule-3 defect this PR already fixed, one level down. The bump was correct; the checklist was stale. Fixed the checklist. Also: ROADMAP.md's "Last release: vX.Y.Z" marker joins the list — this release had to bump it and it was not named — and the "no dedicated test yet" clause now excludes the Cowork marketplace, which test-cowork-drift does cover.
This was referenced Aug 15, 2026
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.
The defect
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, reached independently by both reviewers: "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.
The evidence from #598, and it is the whole argument:
The rejected first draft, and why this one looks different
The first attempt added a SECOND rule beside Convergence: two consecutive clean rounds. Cross-model review rejected it, and the argument is the reason for the fold — 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. The condition was unreachable. A document stating two round-accounting rules is worse than one stating neither.
It also carried a clause saying a defect the loop itself introduced does not reset the counter. That was wrong and is reversed here.
What ships
One counter, one continuation test, folded into Convergence:
Plus: 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.
Consistency
CLAUDE_CODE_SDLC_WIZARD.mdstated the old rule in four more places. All four now match, so the shipped set does not contradict itself. The Cowork copy is byte-identical.Not in scope, deliberately
Enforcement. The rule is stated, not checked — nothing in
handoff.jsonrecords or verifies the stop condition. Recorded as a known limit rather than built here, because building it in the same PR is the scope creep this rule exists to stop.Verification
doc-consistency 137/0, cowork-drift 30/0, docs-usability 29/0, compliance pass.