diff --git a/.console/log.md b/.console/log.md index de09164c..b91aee9f 100644 --- a/.console/log.md +++ b/.console/log.md @@ -1,3 +1,24 @@ +## 2026-07-14 — feat(reviewer): budget/cooldown-aware review — defer, don't burn (audit D1 pt1) + +The reviewer is part of the fleet but was claude-ONLY and consulted NO budget: +it burned claude reviewing PRs even when over the 25% reserve (observed live +2026-07-14 — reviewing my own PRs during a budget crunch pushed the account to +the hard cap). Now `_process_self_review` calls `_select_review_backend` +(reuses the controller's `select_worker_backend` ladder) BEFORE the direct +`claude -p` verdict call: if claude is cooled or over the budget_reserve +(`selected_backend != "claude_code"`), it DEFERS the sweep — no claude spawn, no +budget charge, no needs-human escalation — and retries when the window drains +(~5h). Fail-open: any selection/store error → proceed on claude (today's +behavior); `dynamic_worker_backend_selection=False` → operator opt-out honored. +Verdict parsing untouched (already backend-agnostic, file-based verdict.json). +3 new tests + 150 existing reviewer tests green; ruff+ty clean. + +This is D1 PART 1 (stop the over-budget burn, park smart). PART 2 = actually +review on CODEX when claude is cooled (needs live validation that codex writes a +schema-conformant verdict.json in the empty-dir/`-p` contract — the one unknown +from the scoping pass; until then non-claude selection = defer). See +audit-remediation-plan memory. Next: D2 council. + ## 2026-07-14 — feat(budget): operator budget signal `operations-center.sh budget` (audit D1) Voluntary operator readout (D1 part 3). A human session can't be hard-gated, so diff --git a/src/operations_center/entrypoints/pr_review_watcher/main.py b/src/operations_center/entrypoints/pr_review_watcher/main.py index 8214347d..b52eaad4 100644 --- a/src/operations_center/entrypoints/pr_review_watcher/main.py +++ b/src/operations_center/entrypoints/pr_review_watcher/main.py @@ -522,6 +522,33 @@ def _record_close_receipt( # ── pr review pipeline ──────────────────────────────────────────────────────── +def _select_review_backend(settings, *, usage_store=None, now=None): + """Pick the review backend via the shared fleet ladder (audit D1). + + The reviewer is part of the fleet, so it must respect the same claude→codex + cooldown/budget ladder the controller uses instead of burning claude + unconditionally. Returns the ``WorkerBackendSelection`` (``selected_backend`` + is ``claude_code`` when claude is runnable, another backend / ``None`` when + claude is cooled or over the 25% budget reserve), or ``None`` if selection + couldn't run — in which case the caller proceeds on claude (today's + behavior). The merge-gatekeeper must never crash here. + """ + try: + from operations_center.backends.worker_backend_selector import select_worker_backend + from operations_center.execution.usage_store import UsageStore + + team = getattr(settings, "team_executor", None) + return select_worker_backend( + preferred_backend="claude_code", + usage_store=usage_store or UsageStore(), + dynamic_enabled=bool(getattr(team, "dynamic_worker_backend_selection", True)), + now=now, + ) + except Exception as exc: # noqa: BLE001 — never block the gate on a store read + logger.warning("pr_review_watcher: backend selection failed, proceeding on claude: %s", exc) + return None + + def _run_direct_review( oc_root: Path, goal_text: str, @@ -3221,6 +3248,26 @@ def _phase1( state.setdefault("backend_error_passes", 0) state.setdefault("no_verdict_escalation_count", 0) + # D1: consult the shared fleet ladder BEFORE spawning a claude review. If + # claude is on cooldown or over the 25% budget reserve, don't burn it — + # defer this sweep and retry when it returns (degrade-never-halt applied to + # review; the budget window drains within ~5h). No claude spawn, no budget + # charge, no needs-human escalation for a transient wait. (Codex-fallback + # for the review itself is a validated follow-up; until then any non-claude + # selection means claude — the only review backend — is unavailable.) + _selection = _select_review_backend(settings) + if _selection is not None and _selection.selected_backend != "claude_code": + _resets = [r for r in _selection.cooldowns.values() if r is not None] + _reset_at = max(_resets) if _resets else None + logger.info( + "pr_review_watcher: PR #%d review DEFERRED — claude unavailable " + "(selected=%s, reset≈%s); not burning budget, will retry when it returns.", + pr_number, + _selection.selected_backend, + _reset_at.isoformat() if _reset_at else "unknown", + ) + return + try: verdict = _run_direct_review(oc_root, goal_text, state_key) except OCSourceTreeUncleanError as exc: diff --git a/tests/test_pr_review_watcher.py b/tests/test_pr_review_watcher.py index 782028d0..1f95e9d5 100644 --- a/tests/test_pr_review_watcher.py +++ b/tests/test_pr_review_watcher.py @@ -3468,3 +3468,66 @@ def test_branch_protection_gate_refuses_on_api_error() -> None: assert not watcher._branch_protection_ok( gh, "o", "r", "main", _settings_require_protection(True) ) + + +# ── D1: reviewer respects the fleet backend ladder (budget/cooldown aware) ───── + + +def _ladder_settings(dynamic: bool = True): + from types import SimpleNamespace + + return SimpleNamespace( + team_executor=SimpleNamespace(dynamic_worker_backend_selection=dynamic) + ) + + +def _cool_claude(store, now): + from datetime import timedelta + + store.record_worker_backend_cooldown( + worker_backend="claude_code", + reset_at=now + timedelta(hours=2), + now=now, + limit_kind="session_5h", # account-wide → claude fully cooled + model=None, + ) + + +def test_select_review_backend_available_when_no_cooldown(monkeypatch, tmp_path): + from datetime import datetime, timezone + + from operations_center.execution.usage_store import UsageStore + + monkeypatch.setenv("OPERATIONS_CENTER_EXECUTION_USAGE_PATH", str(tmp_path / "u.json")) + sel = watcher._select_review_backend( + _ladder_settings(), usage_store=UsageStore(), now=datetime.now(timezone.utc) + ) + assert sel is not None and sel.selected_backend == "claude_code" + + +def test_select_review_backend_defers_when_claude_over_budget(monkeypatch, tmp_path): + from datetime import datetime, timezone + + from operations_center.execution.usage_store import UsageStore + + monkeypatch.setenv("OPERATIONS_CENTER_EXECUTION_USAGE_PATH", str(tmp_path / "u.json")) + now = datetime.now(timezone.utc) + store = UsageStore() + _cool_claude(store, now) + sel = watcher._select_review_backend(_ladder_settings(), usage_store=store, now=now) + # claude cooled/over-budget → not selected → the reviewer will DEFER instead of burning it + assert sel is not None and sel.selected_backend != "claude_code" + + +def test_select_review_backend_respects_dynamic_disabled(monkeypatch, tmp_path): + from datetime import datetime, timezone + + from operations_center.execution.usage_store import UsageStore + + monkeypatch.setenv("OPERATIONS_CENTER_EXECUTION_USAGE_PATH", str(tmp_path / "u.json")) + now = datetime.now(timezone.utc) + store = UsageStore() + _cool_claude(store, now) + sel = watcher._select_review_backend(_ladder_settings(dynamic=False), usage_store=store, now=now) + # operator opted out of the ladder globally → always the preferred backend + assert sel is not None and sel.selected_backend == "claude_code"