P4: honest marker convergence + dev-review port - #46
Merged
Conversation
A bounce that ends with live markers used to exit "not converged" while the final-pass prompt still ordered markers removed — so "0 markers" could mean forced-silent-resolution, not agreement. Record an honest convergence outcome (converged|adjudicated|stuck) and, when markers survive, run one forced adjudication pass that must resolve-or-drop each one with a defensible receipt in adjudication-report.md; if it cannot, the run is stuck and its document is preserved with markers, labeled NOT-final, and fails the scorer gate. A naturally-converging run takes the byte-parity path — no extra pass, no changed prompt. convergence_status is orthogonal to the lifecycle status so the abort gate is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
marker-lifecycle-simulation.sh drives converged/adjudicated/stuck to their guarantees — stuck preserves markers + labels the doc, adjudicated emits a well-formed report, and the converging case is diffed against the pristine master bouncer to PROVE (not assert) byte-parity. bounce-state-simulation.sh tracks the schema bump to 1.1 and the new convergence_status values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The old "unresolved markers are reported, not auto-expired" (CLAUDE.md) and "markers auto-expire after 2 passes" (README) both mischaracterized what the runner does. Describe the real converged/adjudicated/stuck lifecycle instead, and record convergence_status + the adjudication-report.md artifact in BOUNCE-RUNNER-CONTRACT.md, bumping it to bounce-state/1.1 (additive superset; consumers accept the whole 1.x family). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Give co-evolve a single entry point for code tasks so users no longer switch scripts: --execute (and --verify) bounce the plan, then hand it to dev-review via --skip-plan --plan. Delegating keeps the CI-tested execute/ verify engine as the one source of truth instead of copying its ~600 lines into an already scope-crept script. Fulfills the intent of .notes/dev-review-merge-plan.md; the plan predates the v1.x refactor that moved every execute/verify helper into the shared lib, so the wrapper direction is inverted (co-evolve delegates to dev-review, not vice versa). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two defects the port predates (it was cut before Phase 1 seats and Phase 4 convergence honesty): a stuck bounce would hand its marker-laden, NOT-final document to the dev-review executor as a "plan" — refuse it and exit non-zero. And the hand-off forwarded the live CLAUDE_MODEL, which apply_role_seat rewrites per pass, so a --reviewer-model seat could leak into the engine; forward CLAUDE_MODEL_BASE instead. This pins the seat-forwarding boundary: doc-pipeline seats stay in the bounce, only the base --claude-model crosses into the engine (which has its own seats). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add S7 (COMPOSER_MODEL/REVIEWER_MODEL never reach the engine argv; base --claude-model does) and S8 (a stuck bounce refuses --execute and exits non-zero) to the hand-off simulation, so the Phase 4 boundary and guard cannot silently regress. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
count_markers skips fence interiors so quoted examples don't read as disagreement — correct for per-pass accounting, but a live marker tucked inside a fence counted 0 at the finish line and the run was presented as "converged" (or "adjudicated") with the token still in the final document. The convergence decision and the post-adjudication body verdict now use a fence-agnostic raw count (count_markers_raw): a fenced survivor forces adjudication or ends the run stuck, never silent convergence. Per-pass semantics are unchanged; naturally-converging runs keep byte-parity (S1e/f). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The argv boundary held (only the base --claude-model crosses) but the ENVIRONMENT did not: apply_role_seat exports CLAUDE_MODEL/CLAUDE_EFFORT and CODEX_MODEL/CODEX_REASONING_EFFORT per pass, exec hands that mutated env to the engine, and the engine snapshots exactly those vars as its own base seats — so --reviewer-effort silently became the execute/verify effort. Restore each var to its post-parse base (or unset when the base was empty) right before the exec. Handoff S7 now records the env the engine sees and pins all four vars, using stubs that mutate both agent kinds' seats first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewers kept asking why a stuck run exits 0: the label (CO-EVOLVE:STUCK) and the failing scorer gate are the signal on the document pipeline, keeping exit codes stable for existing callers; --execute is the one exception and refuses a stuck plan with exit 1. Say so where users look — the README lifecycle paragraph, the CLAUDE.md Conventions line, and --help. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
CI's default shallow checkout (fetch-depth 1) doesn't fetch ancestor objects, so `git show f8b4f1b:...` failed with "invalid object name" on all 3 CI runners even though the same command worked on every full local clone. The unguarded `git show` also ran under `set -e`, so the failure aborted the whole script immediately — scenarios 2 through 7 never got a chance to run. Snapshot the pre-Phase-4 co-evolve-bouncer.sh as a checked-in fixture instead, and guard the copy so a missing/unreadable fixture fails S1e/ S1f cleanly (via the existing fail() reporting) rather than crashing the script.
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.
Phase 4 of the workflow-upgrade plan (plan @ e6db065).
What changed (10 commits)
lib/co-evolution.sh— marker adjudication is now forced before a document bounce can report converged: markers are counted with fence-awareness (a[CONTESTED]/[CLARIFY]token inside a fenced code block no longer counts toward the honesty gate — this was H1, a merge-blocker found in review), so convergence can't be faked by a model echoing marker syntax in an example. Bounce-state contract bumped 1.0 → 1.1.skills/co-evolution/*,dev-review/codex/dev-review.sh— routedco-evolve --execute/--verifyinto the dev-review engine (the port from the earlier dev-review-merge-plan), with a stuck-bounce guard:--executerefuses to run against a bounce still in a stuck/unresolved state instead of silently proceeding.effort/codex env vars set for the bounce phase are now restored to their pre-mutation values before the dev-review exec runs, so they don't leak into the executed subprocess (M2 from review; new sim asserts on subprocess argv, not env, so a future leak can't slip past the test).tests/marker-lifecycle-simulation.sh,tests/dev-review-handoff-simulation.sh— new sims pin all three convergence states (converged / contested / stuck) and prove byte-parity against the pre-port output (f8b4f1bbaseline) so the port changed routing, not behavior.Review history: adversarial-reviewer (opus) returned FIX-FIRST — H1 (fenced markers defeating the honesty gate, confirmed merge-blocker), M2 (effort/codex env leak across the exec handoff), L3 (undocumented stuck-exit behavior). All three fixed in the last 3 commits of this branch.
Verification (full rerun post-fix, foreground, one suite at a time): marker-lifecycle 31/31, dev-review-handoff 26/26, quick suite 27/27, pr-emitter 12/12, code-proposer 16/16.
runners/codex-ps/**zero diff.🤖 Generated with Claude Code