feat(reviewer): D1 — run ordinary reviews on codex when claude is cooled (backend-agnostic fallback) - #486
Conversation
…led (backend-agnostic fallback) The self-review sweep's defer branch already flagged this as "a validated follow-up": until now, any non-claude backend selection meant claude — the only review backend — was unavailable, so the ordinary review PARKED. This gives the ordinary single-reviewer the controller's full claude→codex ladder instead: - claude runnable → review on claude/haiku (unchanged path). - claude cooled, codex up → DIVERT the ordinary review to codex_cli/codex, feeding its verdict into the SAME downstream pipeline (verdict parse → self-heal ladder → LGTM-only green-CI merge). Charges codex's budget. - whole ladder exhausted → PARK (defer+return, no burn); #446 auto-resume. backend→model reuses the validated council seat pairing (verdict._COUNCIL_PANEL, codex_cli→codex) via a small _review_model_for_backend helper — no new registry. The claude path keeps routing through _run_direct_review (the name the test suite patches; back-compat intact); the codex path branches to the already backend-agnostic _run_member_review. Downstream _dispatch_verdict_outcome was already backend-agnostic — no claude-specific assumption on the ordinary path. GUARDRAIL PRs are untouched: they fork to the K=3 cross-family council BEFORE this gate, so a cooled claude still genuinely PARKS the council (F14) rather than falling back to a single codex reviewer — pinned by a new test. Codex was validated as the ordinary reviewer by the 2026-07-15 spike (schema verdict.json, caught a planted bug with the exact span, did not author the overall verdict — INJ boundary held) and runs live as council seat C3. Tests: 3 root integration (codex-runs / ladder-exhausted-parks / guardrail-not-single-reviewed) + 3 unit under tests/unit/reviewer. 169 reviewer + 8536 unit green; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
reviewer process exited with rc=1 for state_key=OperationsCenter-486-council-codex_cli-codex backend=codex_cli model=codex (stdout_tail='') |
5 similar comments
reviewer process exited with rc=1 for state_key=OperationsCenter-486-council-codex_cli-codex backend=codex_cli model=codex (stdout_tail='') |
reviewer process exited with rc=1 for state_key=OperationsCenter-486-council-codex_cli-codex backend=codex_cli model=codex (stdout_tail='') |
reviewer process exited with rc=1 for state_key=OperationsCenter-486-council-codex_cli-codex backend=codex_cli model=codex (stdout_tail='') |
reviewer process exited with rc=1 for state_key=OperationsCenter-486-council-codex_cli-codex backend=codex_cli model=codex (stdout_tail='') |
reviewer process exited with rc=1 for state_key=OperationsCenter-486-council-codex_cli-codex backend=codex_cli model=codex (stdout_tail='') |
|
Council review: CONCERNS (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council concerns (attributed by member):
|
|
Self-review concerns — auto-fixing (up to 6 attempts; re-queued if still unresolved): Council concerns (attributed by member):
|
|
Council review: CONCERNS (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council concerns (attributed by member):
|
|
Council review: CONCERNS (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council concerns (attributed by member):
|
|
Council review: CONCERNS (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council concerns (attributed by member):
|
|
Needs human attention (reason= The automated fix passes exhausted the Self-Heal Ladder (reached L2/2) without changing the branch; a fresh self-review on the same PR head still finds concerns. Further autonomous retries would repeat without progress. Latest concerns: Council concerns (attributed by member):
|
|
Council review: CONCERNS (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council concerns (attributed by member):
|
Council concerns (attributed by member):
|
|
Council review: LGTM (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council unanimous LGTM (claude_code/claude-opus-5 (correctness), claude_code/claude-opus-4-8 (security-capability), claude_code/claude-opus-4-7 (convergence-operational)). |
1 similar comment
|
Council review: LGTM (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council unanimous LGTM (claude_code/claude-opus-5 (correctness), claude_code/claude-opus-4-8 (security-capability), claude_code/claude-opus-4-7 (convergence-operational)). |
|
Council review: CONCERNS (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council concerns (attributed by member):
|
Council concerns (attributed by member):
|
|
Council review: LGTM (cross-family panel, guardrail paths: src/operations_center/entrypoints/pr_review_watcher/main.py)
Council unanimous LGTM (claude_code/claude-opus-5 (correctness), claude_code/claude-opus-4-8 (security-capability), claude_code/claude-opus-4-7 (convergence-operational)). |
Surfaced by rebasing this branch onto main after #486 landed — the branch predated it, so nothing had run the two together. #486's _review_model_for_backend derives the ordinary-review fallback model by scanning _COUNCIL_PANEL for a matching backend. The all-Opus panel carries no codex seat, so the lookup returns None and #486's own unit test fails: assert _review_model_for_backend("codex_cli") == "codex" AssertionError: assert None == 'codex' That test lives in tests/unit/, which CI runs — so this would have gone red on merge. The coupling is the defect, not the panel. _COUNCIL_PANEL answers "who adjudicates guardrail PRs" (a review-policy choice the operator changes freely); _review_model_for_backend answers "which model reviews when claude is cooled" (a capacity fallback). Deriving the second from the first means reseating the council silently disables the fallback: on a host that DOES have codex, every claude-cooled ordinary review parks instead of diverting, with no error and no failing test. This host has no codex, so behavior here is unchanged; the latent trap is the point. Replaced with an explicit _REVIEW_FALLBACK_MODELS = {"codex_cli": "codex"} — the same pairing D1 validated, stated directly rather than inferred, so seating and fallback vary independently. Added a regression test asserting the lookup still yields codex WHILE the live panel has no codex seat: exactly the combination that was broken. Full suite on the rebased branch: 10382 passed, 5 failed — the same 5 pre-existing failures, zero new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ite their verdict (#497) * feat(reviewer): seat an all-Opus council — the codex seat was inert here Operator decision: this host has no codex subscription, so the C1 cross-family panel could never reach quorum. Because an unrunnable seat parks every guardrail PR fail-closed (min_council_members: 3), the council was not weaker than designed — it was inert. _COUNCIL_PANEL is now three pinned Opus versions on claude_code: claude-opus-5 (correctness), claude-opus-4-8 (security/capability), claude-opus-4-7 (convergence/operational), each keeping its distinct lens. Versions are pinned rather than aliased on purpose. "opus" resolves to whatever the CLI calls latest, which would silently collapse two seats onto one model and turn the panel into a duplicate vote that still reports 3/3. The seating change alone would have introduced a silent quota bug. _member_on_cooldown compared a seat's model string to the cooldown record's by equality, but the usage store only ever speaks the limit classifier's four-token vocabulary (sonnet/opus/haiku/codex) — that is all detect_model can parse out of a CLI limit message. A seat named claude-opus-5 would therefore match no "opus" cooldown, so a rate-limited council would report itself fully available and burn the quorum dispatching three doomed reviews. Both sides now normalize through detect_model, which is the identity for bare tokens, so alias-style seats keep their existing behavior. Regression test covers version seats cooling on a family-token cooldown while sonnet/haiku stay runnable. Two consequences are accepted deliberately and documented in COUNCIL_VERDICT.md rather than left to be rediscovered: 1. Diversity is now version + lens, NOT family. The same-family generator/evaluator gap C1 exists to close is no longer closed by panel composition; three Opus versions share training lineage and can share a blind spot. Restoring a genuine second family is the standing fix. 2. Availability is all-or-nothing. Every seat draws on one subscription and normalizes to one family token, so any claude cooldown — model-scoped, account-wide, or the budget guard's synthetic budget_reserve — cools the whole council. min_council_members: 2 is now unreachable via the cooldown store; the degraded-quorum test drives the seat predicate directly and says so. All four current Opus IDs were probed against the live CLI before pinning. Verification: reviewer + verdict suites 209 passed; full suite 10347 passed with 5 pre-existing failures that reproduce identically with this change stashed (sandbox file-deletion race guards + test_custodian_sweep) — zero new failures. ruff check / ruff format --check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(reviewer): grant council members write access — every review scored CONCERNS Found by running the review watcher for real, not by a test. Every member exited rc=0 saying it had written verdict.json; the reviewer found none, logged "no verdict from member review", and the fail-safe scored the PR CONCERNS — posting a FAILING reviewer-verdict status and burning a fix-ladder attempt on a PR nothing had actually reviewed. Left running it would have walked the backlog to max_fix_attempts and begun closing PRs. build_member_argv passed no permission mode, so a non-interactive `-p` run cannot write at all. Probed in an empty tmpdir: (default) -> "Write permission was denied, so `verdict.json` was not created." rc=0 --permission-mode acceptEdits -> "Written." verdict.json present --dangerously-skip-permissions -> "Written." verdict.json present The removed comment claimed the flagless form matched production, so the previous host must have had a permissive user-level Claude settings file masking this. A fresh install does not. Uses acceptEdits, deliberately not --dangerously-skip-permissions: a member reads attacker-influenceable text (the PR diff), so COUNCIL_VERDICT.md's injection threat is live and bypassPermissions would hand an injected instruction full Bash. Verified on this host that a Bash escape under acceptEdits is refused and writes stay confined to the member's temp cwd. Covers the ordinary single reviewer too — same argv builder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(reviewer): decouple the D1 fallback pairing from council seating Surfaced by rebasing this branch onto main after #486 landed — the branch predated it, so nothing had run the two together. #486's _review_model_for_backend derives the ordinary-review fallback model by scanning _COUNCIL_PANEL for a matching backend. The all-Opus panel carries no codex seat, so the lookup returns None and #486's own unit test fails: assert _review_model_for_backend("codex_cli") == "codex" AssertionError: assert None == 'codex' That test lives in tests/unit/, which CI runs — so this would have gone red on merge. The coupling is the defect, not the panel. _COUNCIL_PANEL answers "who adjudicates guardrail PRs" (a review-policy choice the operator changes freely); _review_model_for_backend answers "which model reviews when claude is cooled" (a capacity fallback). Deriving the second from the first means reseating the council silently disables the fallback: on a host that DOES have codex, every claude-cooled ordinary review parks instead of diverting, with no error and no failing test. This host has no codex, so behavior here is unchanged; the latent trap is the point. Replaced with an explicit _REVIEW_FALLBACK_MODELS = {"codex_cli": "codex"} — the same pairing D1 validated, stated directly rather than inferred, so seating and fallback vary independently. Added a regression test asserting the lookup still yields codex WHILE the live panel has no codex seat: exactly the combination that was broken. Full suite on the rebased branch: 10382 passed, 5 failed — the same 5 pre-existing failures, zero new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 participant
What & why
Completes audit item D1. The self-review sweep's defer branch already carried the marker that this was "a validated follow-up; until then any non-claude selection means claude — the only review backend — is unavailable". This builds that follow-up: the ORDINARY single-reviewer now runs the controller's full claude→codex ladder instead of parking whenever claude is cooled.
Operator directive verbatim: "d1 should have the same mechanisms the fleet has, why no fallback to codex" → give the reviewer the controller's backend ladder; divert reviews to codex when claude is cooled, PARK only when the whole ladder is exhausted.
The change (single-reviewer path only)
At the sweep's backend-selection gate:
claude/haiku(unchanged; still via_run_direct_review).codex_cli/codexand feed its verdict into the same downstream pipeline (verdict parse → self-heal ladder → LGTM-only green-CI merge). Charges codex's budget, not claude's, via the normal_run_member_reviewusage path.#4461h auto-resume.backend→modelreuses the validated council seat pairing (verdict._COUNCIL_PANEL,codex_cli→codex) via a small_review_model_for_backendhelper — no new registry. The claude path keeps routing through_run_direct_review(the name the test suite patches; back-compat default preserved); the codex path branches at the call site to the already-backend-agnostic_run_member_review.Boundary preserved (hard constraint)
Guardrail PRs are untouched. They fork to the K=3 cross-family council before this gate is reached, so a cooled claude still genuinely PARKS the council (the F14 park-cap case) rather than being single-reviewed on codex. Pinned by a new test.
Backend-agnostic downstream
_dispatch_verdict_outcome(verdict parse → self-heal ladder → LGTM-only merge with the green-CI precondition) was already backend-agnostic — it takes a plain{result, failing_checks, summary}dict shared with the council path. No claude-specific assumption found on the ordinary path. Updated one now-imprecise INJ comment that named_run_direct_review.Validation basis
The 2026-07-15 codex spike:
codex execwrote a schema-conformantverdict.json, caught a planted bug with the exact evidence span, and did NOT author the overall verdict (INJ boundary held) — same trust level as claude-solo today. Codex already runs live as council seat C3.Tests
test_phase1_cooled_claude_runs_ordinary_review_on_codex— cooled claude → runs oncodex_cli/codex, verdict flows to merge (not deferred).test_phase1_ladder_exhausted_parks_no_burn— no runnable backend → parks, no runner invoked, no budget charged.test_phase1_guardrail_pr_cooled_claude_does_not_single_review_on_codex— guardrail PR still forks to council; NOT single-reviewed on codex.tests/unit/reviewer/test_d1_codex_fallback.py(CI-run) —_review_model_for_backendpairing + unknown→None;_run_direct_reviewno-arg back-compat still runs claude/haiku.Full: 169 reviewer + 8536 unit passed; ruff clean; custodian pre-push clean.
🤖 Generated with Claude Code