diff --git a/devtools/benchmarks/terminal_bench/METHODOLOGY.md b/devtools/benchmarks/terminal_bench/METHODOLOGY.md index bfae87d84..b1bfdf46b 100644 --- a/devtools/benchmarks/terminal_bench/METHODOLOGY.md +++ b/devtools/benchmarks/terminal_bench/METHODOLOGY.md @@ -112,6 +112,15 @@ assumed in two places that used to hardcode TB2.1: own rules permit web access is run with the explicit `--allow-agent-web` flag, which prints a loud non-leaderboard-faithful warning for TB2.1 and is recorded in the manifest and the disclosure ledger — never a silent default. +- **Every configured reviewer row is declared (2026-09-02).** Task acceptance now executes + every triad row on its configured delivery (api packet, configured-subagent native episode, + or agent session), so `metadata.yaml` declares every row the container's panel carries: api + rows by model id, a session row by its opaque `harness[=model]` target under the role + `commit_review_triad_agent_session` (provider = the harness). Before this change only the + panel's non-retrieving api rows ran — and were declared — with the shipped defaults + substituted when none existed; runs whose panel carries retrieving rows are therefore not + comparable on the acceptance axis with earlier runs, and a leaderboard submission must be + read against its own `metadata.yaml`. - **Harbor version:** the pinned TB2.1 bench venv is harbor **0.18.0** (`~/ouro/venv-tb`). 0.20.0 is the current latest and is installed in a SEPARATE venv (`~/ouro/venv-fb`), reachable via `--harbor-bin` and leaving `venv-tb` frozen at 0.18.0 so published TB2.1 numbers keep their diff --git a/devtools/benchmarks/terminal_bench/run_tb.py b/devtools/benchmarks/terminal_bench/run_tb.py index 2845f7f8d..3ce4895fe 100644 --- a/devtools/benchmarks/terminal_bench/run_tb.py +++ b/devtools/benchmarks/terminal_bench/run_tb.py @@ -141,11 +141,14 @@ def _effective_helper_models( bound to an operator-roster subagent does not resolve there and is a typed refusal, never a declared-but-never-run model). Inside a Terminal-Bench task nothing commits: the panel reaches the run through task acceptance, which - projects the panel's NON-retrieving api rows into the legacy triad key and - falls back to the shipped defaults when none exist — so that projection, - not the raw row list, is what is declared here. Without a panel the legacy - comma keys apply (env override else the shipped config defaults). Returns - ordered (model_id, role) pairs, deduped by model id. + executes EVERY triad row on its configured delivery (owner R2, 2026-09-01) — + so every row is declared: an api row by its model id, a configured-subagent + row by its roster model id (a native inspection episode on that model), an + agent-session row by its opaque ``harness[=model]`` target under the role + ``commit_review_triad_agent_session`` (the harness, not an API provider, + serves it). Without a panel the legacy comma keys apply (env override else + the shipped config defaults). Returns ordered (model_id, role) pairs, + deduped by model id. """ from devtools.benchmarks.common.model_slots import single_model_subagents_setting from ouroboros.reviewer_slot_config import ( @@ -165,9 +168,11 @@ def _effective_helper_models( if structured: with roster_env_override(single_model_subagents_setting(measured_model)): panel = parse_reviewer_slots(structured) - api_triad = [row.target_id for row in panel.triad if not row.retrieves] - for model_id in api_triad or review_default.split(","): - ordered.append((model_id.strip(), "commit_review_triad")) + for row in panel.triad: + ordered.append(( + row.target_id.strip(), + "commit_review_triad_agent_session" if row.is_session else "commit_review_triad", + )) else: review = os.environ.get("OUROBOROS_REVIEW_MODELS", review_default) or review_default for m in review.split(","): @@ -212,8 +217,14 @@ def leaderboard_metadata( for model_id, role in _effective_helper_models( model, light_model, disable_agent_web=disable_agent_web, settings=settings, ): - provider = model_id.split("/", 1)[0] if "/" in model_id else "openrouter" - display = model_id.split("/", 1)[1] if "/" in model_id else model_id + if "/" in model_id: + provider, display = model_id.split("/", 1) + elif "agent_session" in role: + # An agent-session row: the harness serves the model (`harness[=model]`). + provider, _, display = model_id.partition("=") + display = display or provider + else: + provider, display = "openrouter", model_id lines.append(f" - model_name: {json.dumps(model_id)}") lines.append(f" model_provider: {json.dumps(provider)}") lines.append(f" model_display_name: {json.dumps(display)}") diff --git a/devtools/benchmarks/terminal_bench/test_run_tb_methodology.py b/devtools/benchmarks/terminal_bench/test_run_tb_methodology.py index 06c71c6e1..8f18fa975 100644 --- a/devtools/benchmarks/terminal_bench/test_run_tb_methodology.py +++ b/devtools/benchmarks/terminal_bench/test_run_tb_methodology.py @@ -348,10 +348,11 @@ _PANEL = { def test_metadata_declares_what_the_container_executes_from_the_structured_panel(monkeypatch): """The container runs the structured panel the adapter forwards (operator env, else the host settings file). Inside a TB task nothing commits: the - panel reaches the run through task acceptance, which executes the panel's - NON-retrieving api rows (else the shipped defaults) — so metadata declares - exactly that projection, never a session row the container never runs and - never a stale legacy comma key.""" + panel reaches the run through task acceptance, which executes EVERY triad + row on its own delivery (owner R2, 2026-09-01) — so metadata declares every + row: api rows by model id, a session row by its opaque harness target under + the agent-session role, never a stale legacy comma key and never a shipped + default the container does not run.""" from ouroboros.config import SETTINGS_DEFAULTS monkeypatch.delenv("OUROBOROS_WEBSEARCH_MODEL", raising=False) @@ -361,19 +362,29 @@ def test_metadata_declares_what_the_container_executes_from_the_structured_panel roles = dict(run_tb._effective_helper_models("openai/gpt-5.5", "google/gemini-3.5-flash", disable_agent_web=True)) assert "foreign/stale-triad" not in roles and "foreign/stale-scope" not in roles assert roles["openai/gpt-5.5"] == "agent+commit_review_triad" - assert "codex=gpt-5.6-sol" not in roles # a session row does not execute in a TB task today + # The session row executes in the acceptance panel now: declared by its + # harness target, under the role that names the delivery. + assert roles["codex=gpt-5.6-sol"] == "commit_review_triad_agent_session" # Scope review is a commit-time gate: it never fires inside a task, so its # rows are not declared (the same honesty rule as the advisory). assert "google/gemini-3.5-pro" not in roles and "scope_review" not in roles.values() assert roles["google/gemini-3.5-flash"] == "light_safety_post_task_synthesis" + meta = run_tb.leaderboard_metadata( + agent_name="Ouroboros", org_name="Ouroboros", model="openai/gpt-5.5", + light_model="google/gemini-3.5-flash", disable_agent_web=True, + ) + # A harness target is served by the harness, not by an API provider. + assert 'model_name: "codex=gpt-5.6-sol"' in meta + assert meta.count('model_provider: "codex"') == 1 and 'model_display_name: "gpt-5.6-sol"' in meta - # An all-retrieving triad projects to the shipped defaults — the models - # acceptance really runs on — not to an empty declaration. + # An all-retrieving triad declares exactly its rows — the shipped defaults + # no longer run anywhere in the task and are not declared. all_session = {**_PANEL, "triad": [_PANEL["triad"][1]]} monkeypatch.setenv("OUROBOROS_REVIEWER_SLOTS", json.dumps(all_session)) roles = dict(run_tb._effective_helper_models("openai/gpt-5.5", "google/gemini-3.5-flash", disable_agent_web=True)) + assert roles["codex=gpt-5.6-sol"] == "commit_review_triad_agent_session" for helper in SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"].split(","): - assert "commit_review_triad" in roles[helper] + assert helper not in roles # Settings-file fallback, exactly like the container adapter's env → settings order. monkeypatch.delenv("OUROBOROS_REVIEWER_SLOTS", raising=False) @@ -392,11 +403,13 @@ def test_metadata_parses_the_panel_under_the_container_roster(monkeypatch): panel = {**_PANEL, "triad": [{"slot_id": "t1", "subagent_id": BENCHMARK_SUBAGENT_ID}]} monkeypatch.setenv("OUROBOROS_REVIEWER_SLOTS", json.dumps(panel)) roles = dict(run_tb._effective_helper_models("openai/gpt-5.5", "google/gemini-3.5-flash", disable_agent_web=True)) - # The benchmark actor row RETRIEVES (native tool rounds): acceptance does not - # execute it today, so the defaults are what run — and what is declared. + # The benchmark actor row RETRIEVES (native tool rounds) on the measured + # model: acceptance executes it, so the measured model carries the triad + # role and no shipped default is declared. from ouroboros.config import SETTINGS_DEFAULTS - assert all("commit_review_triad" in roles[h] for h in SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"].split(",")) + assert roles["openai/gpt-5.5"] == "agent+commit_review_triad" + assert not any(h in roles for h in SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"].split(",")) # An operator-roster reference the container cannot resolve is a typed refusal. monkeypatch.setenv("OUROBOROS_REVIEWER_SLOTS", json.dumps({**panel, "triad": [{"slot_id": "t1", "subagent_id": "operator-critic"}]})) with pytest.raises(ValueError, match="operator-critic"): diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 862d588f0..9da914ab7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -182,7 +182,7 @@ server.py (Starlette+uvicorn) ← HTTP + WebSocket on configurable host:port (de ├── review_execution_projection.py ← Pure read-side presentation leaf shared by Skill Review, plan review, and task acceptance: the `executions[]` projection (admits only returned usage or a resolved delegated route; never invents execution, money, or profile facts; kinds `api` | `harness` | `native` — the native tool-round episode is an API execution with a different DELIVERY, projected as its own kind so the owner can tell a retrieving review from a packet review on the same model) and the bounded owner-facing finding rows (`projected_finding_row` + `MAX_PROJECTED_ACTOR_FINDINGS` — row-count bound, generous 2000-char string bound after redaction, unknown shapes ship as a disclosed JSON row). ├── preflight_runner.py ← Hermetic reviewed-change pytest gate: disposable git worktree, ONE hardened candidate capture (`git diff --binary --no-ext-diff --no-textconv --no-color --src-prefix=a/ --dst-prefix=b/ HEAD` applied as RAW BYTES, identically for every index state including an unfinished merge — whose unmerged entries the former staged+unstaged pair could only render as contentless stubs and `--cc` hunks `git apply` rejects or silently drops; capture/apply failure is the typed hard block PREFLIGHT_CANDIDATE_ASSEMBLY, never a test verdict; the honest bound is an exact tracked projection of the live worktree plus its safe non-ignored untracked entries), temp data/settings/pycache env, and live OUROBOROS_*/secret-class scrub so review tests cannot inherit operator behavior or mutate live repo/data. Runs the node web-tests lane (`preflight_node.py`) then CI's own two-pass split in that one worktree (parallel `not serial` with `-n auto --dist loadscope --max-worker-restart=0 --timeout=300`, then a flag-free `serial` pass) under ONE total budget, with `LANE_EXCLUSION_EXPR` as the marker-lane SSOT; a dead xdist worker and a missing xdist/timeout plugin are distinct named hard blocks, never a retry and never a silent serial fallback ├── preflight_node.py ← Node lane of the hermetic commit gate (called from `preflight_runner.run_hermetic_pytest` once the candidate is assembled, as the first consumer of the shared budget): when the candidate tree carries `web/tests/*.test.js`, runs that browser-module suite once with `node --test` from the candidate's `web/` (bundled signed node first, then PATH; version floor 20.11) inside the same ProcessContainer/reap/bounded-output pattern as the pytest passes. Content-keyed: candidates without web tests never require node; while active, a missing/unusable runtime is the typed PREFLIGHT_NODE_MISSING/PREFLIGHT_NODE_TOO_OLD hard block and a red suite is NODE_TESTS_FAILED — never a silent skip. Both CI jobs run the mirrored `cd web && node --test tests/*.test.js` step - ├── review_substrate.py ← Reviewer-slot coordinator used by task acceptance and planning helpers; duplicate model ids remain independent slots. Actor records keep transport status, parse status, semantic verdict, model/provider, role, coverage, quorum contribution, reason, enforcement impact, and review-binding hashes distinct; only a compact projection reaches task/event/UI records. Task acceptance enforces adaptive quorum, one substantive call and no more than two physical attempts per actor, metric-grounded criterion evidence, provenance, and a public-info-only anti-cheat boundary. Commit/triad/scope P3 orchestration remains a separate one-pass contract. (v6.87.21) Slot execution has ONE seam: `_run_slot` builds an immutable `ReviewAssignment` and binds it ONCE through `review_execution._review_route_executor` — the single place a transport is chosen (closed `ReviewRouteKind`: `api_chat` and `agent_session` — never a vendor/harness name), bound before the first send so the durable prompt record is written from the route's own lazily rendered projection; `_execute_slot_attempt` is the single physical-attempt seam that runs the already-bound executor, and the route's executor returns a typed `ReviewAttemptResult`. Attempt rails, persistence, parsing, actor projection and quorum stay above the seam and are route-agnostic; a route that cannot deliver raises the typed `ReviewRouteUnavailable` on its own slot instead of falling back to another transport. Prompt assembly lives BELOW the seam: `ApiChatReviewExecutor` renders the historical messages lazily and memoizes them, so the durable prompt record and both permitted physical sends share one byte-identical rendering (pinned by a golden digest test) and a non-API route never assembles an API pack. Everything below the seam — route vocabulary, assignment, attempt result, executors, and the api_chat prompt renderers — lives in `review_execution.py`, which never imports the coordinator back; `review_substrate` re-exports the historical renderer names for existing callers. (phase 5) `AgentSessionReviewExecutor` delivers a slot as ONE delegated read-only Claudexor session through the shared `run_delegated_review_session` nanny loop (custody, settlement, verified-cancel time cap, D7 full-artifact read; the delegated advisory rides the same loop). Its typed verdict follows D19: `outputSchema` is asked only when the route's own live manifest (`GET /v2/harnesses`) declares structured output — the agent-capability catalog's harness rows carry no such field at all, so reading it there answered False for every route — trusted only on the run's own `outputConformance == "passed"` (never run success); otherwise the strict parser first, then LIGHT-MODEL extraction canonicalizes narrative to the review's own contract by the surface's output SHAPE (`triad_review.review_output_shape`: `array` — bare `[]` or a findings array, so a session's clean verdict survives `empty_array_is_verified_clean` unchanged; `object` — the whole acceptance verdict; `report` — passed through verbatim, no schema asked), with every extraction-instead-of-schema landing disclosed as `capability_delta` (actor usage + durable event). Per-row delivery comes from `OUROBOROS_REVIEW_ROUTES` / `OUROBOROS_SCOPE_REVIEW_ROUTES` with the session target in `OUROBOROS_REVIEW_SESSION_ROUTE` (falling back to `OUROBOROS_SUBAGENT_HARNESS`, so an owner who configured ONE delegated route does not have to configure it twice; review/advisory sessions riding that subagent default also inherit the owner's Delegation account pin — `OUROBOROS_SUBAGENT_PROFILE` — with the same strict per-subject health on dispatch); task acceptance is pinned `api_chat` (D15); plan review follows each configured row's delivery kind (`api_chat` packet in-process, `agent_session` retrieving reviewer). The advisory row lives on the SAME shared row vocabulary (structured `OUROBOROS_REVIEWER_SLOTS.advisory`, or legacy `OUROBOROS_ADVISORY_REVIEW_ROUTE`): kind `api_chat` runs the bounded native inspection episode on the row's routed model (the retired legacy kind `api` — Claude-SDK spellings — still parses and migrates to the same model's routed id, force-disabling the row with a typed `disabled_reason` when unmappable, never silently swapping models), `agent_session` rides the delegated session executor, and a `subagent_id` reference resolves through the Available-subagents roster. Advisory availability is credentials-based on the resolved model (`advisory_model_credentials_missing` bypass), never a vendor-key special case. Scope session delivery is assembled by `tools/scope_review_session.py` from the SAME `build_scope_review_prompt` builder (retrieval pointers instead of packs, canonical docs as `generate_doc_nav_map` navigation maps); its coverage manifest is forensics, never a gate: `host_file_read_attestation: unobserved` is a non-blocking disclosed fact (the host does not see which files the session opened — a provenance limit, not a coverage finding), and the api-only ≥1M window floor does not apply to the agentic-delivery session mode, which BIBLE P3 admits as an ALTERNATE AUTHORITATIVE delivery mode once its window is sourced at ≥200K (D16). + ├── review_substrate.py ← Reviewer-slot coordinator used by task acceptance and planning helpers; duplicate model ids remain independent slots. Actor records keep transport status, parse status, semantic verdict, model/provider, role, coverage, quorum contribution, reason, enforcement impact, and review-binding hashes distinct; only a compact projection reaches task/event/UI records. Task acceptance enforces adaptive quorum, one substantive call and no more than two physical attempts per actor, metric-grounded criterion evidence, provenance, and a public-info-only anti-cheat boundary. Commit/triad/scope P3 orchestration remains a separate one-pass contract. (v6.87.21) Slot execution has ONE seam: `_run_slot` builds an immutable `ReviewAssignment` and binds it ONCE through `review_execution._review_route_executor` — the single place a transport is chosen (closed `ReviewRouteKind`: `api_chat` and `agent_session` — never a vendor/harness name), bound before the first send so the durable prompt record is written from the route's own lazily rendered projection; `_execute_slot_attempt` is the single physical-attempt seam that runs the already-bound executor, and the route's executor returns a typed `ReviewAttemptResult`. Attempt rails, persistence, parsing, actor projection and quorum stay above the seam and are route-agnostic; a route that cannot deliver raises the typed `ReviewRouteUnavailable` on its own slot instead of falling back to another transport. Prompt assembly lives BELOW the seam: `ApiChatReviewExecutor` renders the historical messages lazily and memoizes them, so the durable prompt record and both permitted physical sends share one byte-identical rendering (pinned by a golden digest test) and a non-API route never assembles an API pack. Everything below the seam — route vocabulary, assignment, attempt result, executors, and the api_chat prompt renderers — lives in `review_execution.py`, which never imports the coordinator back; `review_substrate` re-exports the historical renderer names for existing callers. (phase 5) `AgentSessionReviewExecutor` delivers a slot as ONE delegated read-only Claudexor session through the shared `run_delegated_review_session` nanny loop (custody, settlement, verified-cancel time cap, D7 full-artifact read; the delegated advisory rides the same loop). Its typed verdict follows D19: `outputSchema` is asked only when the route's own live manifest (`GET /v2/harnesses`) declares structured output — the agent-capability catalog's harness rows carry no such field at all, so reading it there answered False for every route — trusted only on the run's own `outputConformance == "passed"` (never run success); otherwise the strict parser first, then LIGHT-MODEL extraction canonicalizes narrative to the review's own contract by the surface's output SHAPE (`triad_review.review_output_shape`: `array` — bare `[]` or a findings array, so a session's clean verdict survives `empty_array_is_verified_clean` unchanged; `object` — the whole acceptance verdict; `report` — passed through verbatim, no schema asked), with every extraction-instead-of-schema landing disclosed as `capability_delta` (actor usage + durable event). Per-row delivery comes from `OUROBOROS_REVIEW_ROUTES` / `OUROBOROS_SCOPE_REVIEW_ROUTES` with the session target in `OUROBOROS_REVIEW_SESSION_ROUTE` (falling back to `OUROBOROS_SUBAGENT_HARNESS`, so an owner who configured ONE delegated route does not have to configure it twice; review/advisory sessions riding that subagent default also inherit the owner's Delegation account pin — `OUROBOROS_SUBAGENT_PROFILE` — with the same strict per-subject health on dispatch); task acceptance and plan review follow each configured row's delivery kind (`api_chat` packet in-process, `agent_session` retrieving reviewer, a configured-subagent api row as the native episode) — task acceptance through the same `reviewer_slot_config.triad_delivery_slots` builder as plan and skill review (owner R2, 2026-09-01; the former D15 api pin and its default-panel projection are gone). The advisory row lives on the SAME shared row vocabulary (structured `OUROBOROS_REVIEWER_SLOTS.advisory`, or legacy `OUROBOROS_ADVISORY_REVIEW_ROUTE`): kind `api_chat` runs the bounded native inspection episode on the row's routed model (the retired legacy kind `api` — Claude-SDK spellings — still parses and migrates to the same model's routed id, force-disabling the row with a typed `disabled_reason` when unmappable, never silently swapping models), `agent_session` rides the delegated session executor, and a `subagent_id` reference resolves through the Available-subagents roster. Advisory availability is credentials-based on the resolved model (`advisory_model_credentials_missing` bypass), never a vendor-key special case. Scope session delivery is assembled by `tools/scope_review_session.py` from the SAME `build_scope_review_prompt` builder (retrieval pointers instead of packs, canonical docs as `generate_doc_nav_map` navigation maps); its coverage manifest is forensics, never a gate: `host_file_read_attestation: unobserved` is a non-blocking disclosed fact (the host does not see which files the session opened — a provenance limit, not a coverage finding), and the api-only ≥1M window floor does not apply to the agentic-delivery session mode, which BIBLE P3 admits as an ALTERNATE AUTHORITATIVE delivery mode once its window is sourced at ≥200K (D16). ├── review_custody.py ← Process-local physical review-worker custody: independent slot deadlines, late-result settlement, stable retry identity, and duplicate-dispatch suppression; it is not durable scheduling. ├── review_owner_custody.py ← Causal bridge from the existing process-custody identity to the existing commit-attempt ledger. A paid attempt records its owner `(server session, pid)` before physical fan-out; only confirmed death of that pid, or a later server generation observing that prior pid already dead, may settle tokenless process-local rows. It adds no scheduler or process ledger. ├── review_execution.py ← (v6.87.21, phase 5) Review execution BELOW the substrate's seam: the closed route vocabulary (`ReviewRouteKind`: `api_chat` and `agent_session` — never a vendor/harness name) with THE delivery-class predicate beside it (`delivery_retrieves(route, subagent_id)`: a hosted session or a configured-subagent api row retrieves the subject itself and never receives the packet — `ReviewSlot.retrieves`, `ConfiguredReviewerSlot.retrieves`, admission, packet fit and the surfaces' request builders all call this one definition), the immutable `ReviewAssignment`, the per-route executors returning a typed `ReviewAttemptResult` (a route that cannot deliver raises the typed `ReviewRouteUnavailable` on its own slot, never a fallback to another transport), the api_chat prompt renderers (rendered lazily and memoized so the durable prompt record and both permitted physical sends share one byte-identical rendering), and `AgentSessionReviewExecutor` with the shared `run_delegated_review_session` nanny loop and the D19 typed-verdict order — see the `review_substrate.py` row above for the seam's coordinator side and the full phase-5 contract. One-way dependency: this module never imports the coordinator back; `review_substrate.py` re-exports the historical renderer names for existing callers. @@ -191,7 +191,7 @@ server.py (Starlette+uvicorn) ← HTTP + WebSocket on configurable host:port (de ├── review_session_custody.py ← Exact delegated-review recovery validation and the pre-POST durable invocation checkpoint, extracted from `review_execution.py` at the module-size gate; it adds no scheduler or state store and keeps the same delegate-custody authority. ├── review_slot_cancel.py ← Hosted-review slot poller's cancel-honesty helpers (extracted from `review_execution.py` at the module-size gate; `review_execution` imports them and the poll loop stays there): early-termination decision for a parked question (`_interaction_outlives_slot`), the verified slot cancel reporting only what it PROVED (`_slot_cancel_outcome` — outcome + state + the verify read's own `terminal_detail` when carried), honest attribution wording (`_cancel_honesty_clause` — "host-cancelled" only on a `confirmed` receipt whose state is the cancel's own; a confirmed `failed`/`interrupted` is attributed to the run's OWN terminal, BR2-2), and completion-wins consumption of a discovered natural success (`_natural_success_terminal` — the carried detail is used as-is, a re-read gets one bounded retry, and a still-unreadable detail raises the typed `ReviewSessionSucceededResultUnavailable` naming the settled custody row and the capture surfaces instead of "may still be live", BR2-1). ├── commit_admission.py ← Deterministic commit-admission preflights SSOT (Q3): release-metadata P9 checks + auto-sync, staged-Python syntax compile, and `run_tests_preflight_with_proof` — the ONE helper binding a green hermetic pytest preflight to the Q10 managed pre-commit proof; the advisory gate and the commit gate both delegate here so the run→proof coupling cannot drift - ├── reviewer_slot_config.py ← Structured reviewer-slot SSOT: stable slot ids, route targets, per-slot effort, legacy projections, save/runtime validation, and disclosure-only last-effective execution records. A row is EITHER an inline route OR a `subagent_id` reference into the Available-subagents roster (mutually exclusive; the reference materializes route/effort at load time — frozen for the run, next load sees roster edits; an unresolvable reference is a typed refusal). Derived per-row facts are split (`is_session` = transport, `retrieves` = delivery class: session rows and native-retrieving actor rows both read the tree themselves and get pointer packs instead of full packet assembly). The advisory row shares the same vocabulary (+`enabled`, `disabled_reason`), and so does the optional `deep_review` singleton (fixed id `deep_review_slot_1`, no `enabled`; `deep_review_slot()` returns the saved row or the packed api row synthesized from the legacy `OUROBOROS_MODEL_DEEP_SELF_REVIEW` key, whose own effort outranks `OUROBOROS_EFFORT_DEEP_SELF_REVIEW` only when set). Malformed configuration loudly refuses commit, scope, advisory, plan, skill and deep self-review; task acceptance deliberately retains the projected legacy/default API panel. + ├── reviewer_slot_config.py ← Structured reviewer-slot SSOT: stable slot ids, route targets, per-slot effort, legacy projections, save/runtime validation, and disclosure-only last-effective execution records. A row is EITHER an inline route OR a `subagent_id` reference into the Available-subagents roster (mutually exclusive; the reference materializes route/effort at load time — frozen for the run, next load sees roster edits; an unresolvable reference is a typed refusal). Derived per-row facts are split (`is_session` = transport, `retrieves` = delivery class: session rows and native-retrieving actor rows both read the tree themselves and get pointer packs instead of full packet assembly). The advisory row shares the same vocabulary (+`enabled`, `disabled_reason`), and so does the optional `deep_review` singleton (fixed id `deep_review_slot_1`, no `enabled`; `deep_review_slot()` returns the saved row or the packed api row synthesized from the legacy `OUROBOROS_MODEL_DEEP_SELF_REVIEW` key, whose own effort outranks `OUROBOROS_EFFORT_DEEP_SELF_REVIEW` only when set). Malformed configuration loudly refuses EVERY surface — commit, scope, advisory, plan, skill review, deep self-review and task acceptance (owner R3; the former legacy/default API panel residual is gone). `triad_delivery_slots` is THE triad-row builder: plan review, skill/commit review (as aligned vectors through `commit_triad_delivery`) and task acceptance all read the rows through it, so no surface reads a projection of the panel instead of the panel; the legacy comma keys remain a runtime projection of api model ids for legacy consumers only. ├── review_state.py ← Durable advisory pre-review state (advisory_review.json) ├── review_cycles.py ← Shared paid-review-cycle cap SSOT (`OUROBOROS_REVIEW_MAX_CYCLES`, string: positive int or `unlimited`; `review_max_cycles()` → Optional[int]): one number, four documented per-gate meanings — plan review (panel cycles per task), task acceptance (`improvement passes = cycles − 1`; the retired `OUROBOROS_ACCEPTANCE_MAX_IMPROVEMENT_PASSES` migrates into this key at settings load), the commit gate (paid triad+scope cycles per root task; independently, a byte-identical staged diff is refused free from the first verdict-block) and skill review (paid panel dispatches per root task / manual group lane, free replay of identical snapshots) — plus `emit_review_cycles_exhausted`, the typed D27 escalation on the existing events rail ├── review_dispatch.py ← Reviewer dispatch primitives (moved whole from `review_substrate.py` at the module-size gate): the row-identity mint (`slot_id_for_row` + surface prefixes; `review_substrate` re-exports the historical names) and the write-ahead PAID stamp seam — a gate that meters paid review cycles installs an idempotent `ReviewPaidStamp` on `ctx._review_paid_stamp`; the coordinator captures that exact object, session routes invoke it before replayable `START_REQUESTED`, and API routes bind it until the canonical attempt ledger durably reaches `dispatched` immediately before wire send, so typed pre-start refusals stay $0 while a late worker or crash cannot race away the durable paid fact. Task acceptance binds its strict exact-hash tree-wallet claim immediately before that API transition: route/candidate refusals remain free and retryable, a wallet/deadline/cancellation veto leaves the usage reservation released, and no reviewer transport proceeds. @@ -842,7 +842,7 @@ Connect is link-first and harness-agnostic. A typed disclosure renders the sign- Account status refresh runs immediately and on visible page/tab activation but does not make every hidden page pay for daemon round-trips. Entering Agents is also an explicit owner action: after the fresh read, an already-provisioned `stale` home is restarted through the existing wake endpoint, while `not_provisioned`, foreign-owned, and repair states remain behind Connect. Background polling stays read-only and never wakes the daemon. Job polling uses one request at a time, begins at the healthy cadence, backs off to a bounded delay on consecutive failures, and after ten consecutive failures stops with an honest unconfirmed state: lost contact does not prove either failure or settlement. One transition lock covers Start, Retry, and Dismiss. A new login begins only once release of the prior job is proven (`loginReleaseProven`): a terminal snapshot whose termination reason is not `termination_unconfirmed`, a reconciliation that found the setup empty, or a job proven absent by 404/410. A terminal `termination_unconfirmed` snapshot keeps the job fenced, and a successful (2xx) cancel response alone is not proof of release; a network or server failure retains the card and job id because dropping it could orphan a still-live server job. After each await the handler rechecks whether polling settled the job, so a stale cancel continuation cannot overwrite a terminal result. -Review lanes edits one structured reviewer configuration. Each triad, scope, optional advisory, or deep self-review row picks its reviewer from ONE flat select: the Available-subagents roster rows lead as references (facts-first labels), then the inline channels — API delivery or a coding-agent session — followed by its model, optional credential profile, and effort. The two multi-row categories are driven by one `CATEGORIES` table and the two single-row categories (advisory, deep self-review) by one renderer/binder parameterized by a `SINGLETONS` table; the deep self-review block states its one difference from the advisory where the owner picks — an API model there is ONE packed review (Atlas + memory), not an inspection episode — and, while the row is only synthesized from `OUROBOROS_MODEL_DEEP_SELF_REVIEW`, says so — an untouched synthesized (or empty) placeholder is OMITTED from the save payload, so an unrelated save never writes the key's value into the setting and a repair save beside a `config_error` (the endpoint still attaches the synthesized row there) succeeds without it; editing the row materializes it, and a blanked model box is hinted client-side and refused typed at save (owner fork 3 = A). The former Models-tab field is gone (R7). API models use free text with catalogue suggestions; agent-session models come from the selected harness. Saved choices that disappear from discovery remain visible as unavailable rather than silently changing to the first option. Capability labels configure nothing; the server-returned limits and last effective execution disclose what a saved row actually ran as, including capability deltas. An unloaded or unreachable view authors no replacement. A successfully loaded empty triad/scope is sent as shown so backend validation returns the real error instead of the browser falsely reporting that nothing changed. A row pinned to an account discovery no longer lists keeps its pin and is disclosed once, above the rows, as unavailable rather than silently rerouted — and only on the word of a facet that was actually read: an account pin answers to the `accounts` facet and a model to `catalog`, and while that facet is unread or failed the row says the pin was not checked instead of "not in discovery". The all-delegated disclosure is neutral routing information: commit, scope, plan, advisory, and skill review follow their configured rows and wait for subscription capacity rather than falling back to API spend; task acceptance alone retains the owner-approved API/default projection. +Review lanes edits one structured reviewer configuration. Each triad, scope, optional advisory, or deep self-review row picks its reviewer from ONE flat select: the Available-subagents roster rows lead as references (facts-first labels), then the inline channels — API delivery or a coding-agent session — followed by its model, optional credential profile, and effort. The two multi-row categories are driven by one `CATEGORIES` table and the two single-row categories (advisory, deep self-review) by one renderer/binder parameterized by a `SINGLETONS` table; the deep self-review block states its one difference from the advisory where the owner picks — an API model there is ONE packed review (Atlas + memory), not an inspection episode — and, while the row is only synthesized from `OUROBOROS_MODEL_DEEP_SELF_REVIEW`, says so — an untouched synthesized (or empty) placeholder is OMITTED from the save payload, so an unrelated save never writes the key's value into the setting and a repair save beside a `config_error` (the endpoint still attaches the synthesized row there) succeeds without it; editing the row materializes it, and a blanked model box is hinted client-side and refused typed at save (owner fork 3 = A). The former Models-tab field is gone (R7). API models use free text with catalogue suggestions; agent-session models come from the selected harness. Saved choices that disappear from discovery remain visible as unavailable rather than silently changing to the first option. Capability labels configure nothing; the server-returned limits and last effective execution disclose what a saved row actually ran as, including capability deltas. An unloaded or unreachable view authors no replacement. A successfully loaded empty triad/scope is sent as shown so backend validation returns the real error instead of the browser falsely reporting that nothing changed. A row pinned to an account discovery no longer lists keeps its pin and is disclosed once, above the rows, as unavailable rather than silently rerouted — and only on the word of a facet that was actually read: an account pin answers to the `accounts` facet and a model to `catalog`, and while that facet is unread or failed the row says the pin was not checked instead of "not in discovery". The standing note states the rule, never the situation: every review surface — commit, scope, plan, advisory, skill review and task acceptance — follows its configured rows and waits for subscription capacity rather than falling back to API spend (owner R2 retired the task-acceptance API pin and its default-panel disclosure). **Available subagents** is the single task-actor editor. Its list-level Enabled flag and at most ten stable rows are the saved `OUROBOROS_SUBAGENTS` intent. The owner sees numbered rows and authors one prose field, Description (`recommended_use`), alongside the structured API-model or Agent-session route, optional effort and optional session account pin. Identity is the stable internal `subagent_id` plus the route facts derived live from the row; the legacy display `name` is retired — parse accepts and drops it, nothing mints or fabricates one — and a changing visual ordinal never becomes durable identity. The editor shares only neutral route/model/account/status primitives with Review lanes; reviewer quorum, roles and schema stay separate. Empty session pin means Claudexor's compatible-account rotation. A saved route, model or pin that disappears from discovery remains visible and editable, labeled unavailable or not checked according to the exact status facet rather than silently rewritten. @@ -1265,7 +1265,7 @@ Review delivery has two closed route kinds in `review_execution.py`: `api_chat` The hosted-agent executor instead starts one read-only delegated session through the shared Claudexor nanny. The session receives route-owned instructions and retrieval pointers and uses its own tools; it does not assemble the API review pack. A conforming structured result is preferred when the live harness manifest supports it. Otherwise strict parsing runs first and a light extractor canonicalizes the already-collected transcript. Extraction never launches a second hosted session. Custody, cancellation, full-artifact recovery, capability deltas, and settlement stay on the same delegated transport contract. -Advisory availability is evaluated from the current configured slot and route, not inferred from a stale stored verdict. A disabled advisory slot is an audited bypass; an `api_chat` row requires provider credentials for its RESOLVED model (same-model payable-spelling fallback included), while `agent_session` requires a resolvable session route. If the commit advisory is unavailable, the commit gate runs its compensating hermetic preflight only when tests remain independently applicable: the caller did not explicitly skip them and the diff is not documentation-only. Other bypasses record why tests were skipped. Optional skill advisory remains fail-open with disclosure. Malformed structured slot configuration is refused at save and becomes a typed loud review-time failure for commit triad, scope, advisory, plan, skill review — and deep self-review (`deep_review_slot()` raises on the malformed value and `run_deep_self_review` returns the typed `deep_self_review_unavailable` result instead of a report). Task acceptance retains the explicit owner-approved residual: it uses the projected legacy/default API panel when that structured configuration is malformed. No surface silently chooses the opposite route. +Advisory availability is evaluated from the current configured slot and route, not inferred from a stale stored verdict. A disabled advisory slot is an audited bypass; an `api_chat` row requires provider credentials for its RESOLVED model (same-model payable-spelling fallback included), while `agent_session` requires a resolvable session route. If the commit advisory is unavailable, the commit gate runs its compensating hermetic preflight only when tests remain independently applicable: the caller did not explicitly skip them and the diff is not documentation-only. Other bypasses record why tests were skipped. Optional skill advisory remains fail-open with disclosure. Malformed structured slot configuration is refused at save and becomes a typed loud review-time failure for commit triad, scope, advisory, plan, skill review — and deep self-review (`deep_review_slot()` raises on the malformed value and `run_deep_self_review` returns the typed `deep_self_review_unavailable` result instead of a report). Task acceptance refuses the same way (a typed DEGRADED panel, `reviewer_slot_config_invalid`; owner R3). No surface silently chooses the opposite route or a default panel. ### Usage ledger substrate vs. accounting policy @@ -2893,8 +2893,8 @@ Runtime floors: | OUROBOROS_USER_FILES_ROOT | "" (home) | **Env-only operational override** (NOT a `settings.json`/UI carrier — like `OUROBOROS_DATA_DIR`; deliberately absent from `SETTINGS_DEFAULTS`/`apply_settings_to_env`, whose pop-on-absent would erase an injected value). Filesystem base for the `user_files` resource root, read directly by `tool_access._user_files_root`. Defaults to the owner's real home; a jailed or isolated runtime sets a scratch dir so a task cannot read the owner's real home (e.g. secret files), and unnamed deliverables then derive under that jail (`tool_access._deliverables_root`). Any unusable value falls back to home (fail-safe). | | OUROBOROS_OBSERVABILITY_KEEP_RAW | unset | **Env-only operator debug override** (NOT a `settings.json`/UI carrier — deliberately absent from `SETTINGS_DEFAULTS`/`apply_settings_to_env` so a self-change or non-owner save can NEVER enable secret logging). When set, persist the RAW LLM/tool payload as the authoritative observability blob. Default OFF: the authoritative blob is REDACTED (secret values masked, structure/route/non-secret text preserved per BIBLE P1) so no secret lands on disk; `full_payload_redacted` declares it honestly. | | OUROBOROS_GENERATIVE_PROBE | 1 (on) | Enables the generative context-window probe machinery (`capability_evidence.probe(allow_generative=True)`): when provider metadata gives no window, an over-window request can empirically confirm ≥1M from a FREE pre-inference reject; `OUROBOROS_GENERATIVE_PROBE_CHARS` (default 5,000,000) sizes the padding, and a 200 (possibly-paid accept) never auto-confirms — it routes to owner-ack. Since the settings-time Max gate retirement no production surface passes `allow_generative=True` (the scope-slot ack path probes metadata-only), so this toggle governs dormant machinery kept for tests and future explicit owner probes. | -| OUROBOROS_REVIEW_MODELS | google/gemini-3.7-flash,openai/gpt-5.6-terra,anthropic/claude-opus-5 | Legacy ordered triad roster shared by commit/plan/task/skill review when `OUROBOROS_REVIEWER_SLOTS` is absent; duplicate model IDs are independent slots. With the structured setting present, commit/plan/skill review read its exact per-row delivery while this key is only the runtime api_chat projection retained for API-pinned task acceptance — never a second write | -| OUROBOROS_REVIEWER_SLOTS | (empty) | (6.1) Structured reviewer-slot SSOT (`reviewer_slot_config.py`): JSON `{triad[], scope[], advisory, deep_review?}`; each row `{slot_id, route:{kind: api_chat\|agent_session, target_id}, effort}` with a STABLE owner-assigned slot_id (never an array index); the optional `deep_review` singleton carries the same row keys minus `slot_id` (fixed `deep_review_slot_1`) and, absent, is synthesized as the packed api row from `OUROBOROS_MODEL_DEEP_SELF_REVIEW`. Empty = read the legacy comma keys + phase-5 route envs as the migration source. Malformed value refuses typed at save AND at review time on every surface except task acceptance (owner-approved residual: acceptance still reads the projected legacy/default env keys); env-apply logs and leaves legacy keys unprojected | +| OUROBOROS_REVIEW_MODELS | google/gemini-3.7-flash,openai/gpt-5.6-terra,anthropic/claude-opus-5 | Legacy ordered triad roster shared by commit/plan/task/skill review when `OUROBOROS_REVIEWER_SLOTS` is absent; duplicate model IDs are independent slots. With the structured setting present, every review surface (commit/plan/skill review and task acceptance) reads the exact per-row delivery; this key is then only a runtime projection of the panel's api model ids for legacy consumers (the external review script's key ordering, benchmark manifests) — never a second write and never a review input | +| OUROBOROS_REVIEWER_SLOTS | (empty) | (6.1) Structured reviewer-slot SSOT (`reviewer_slot_config.py`): JSON `{triad[], scope[], advisory, deep_review?}`; each row `{slot_id, route:{kind: api_chat\|agent_session, target_id}, effort}` with a STABLE owner-assigned slot_id (never an array index); the optional `deep_review` singleton carries the same row keys minus `slot_id` (fixed `deep_review_slot_1`) and, absent, is synthesized as the packed api row from `OUROBOROS_MODEL_DEEP_SELF_REVIEW`. Empty = read the legacy comma keys + phase-5 route envs as the migration source. Malformed value refuses typed at save AND at review time on every surface, task acceptance included (owner R3); env-apply logs and leaves legacy keys unprojected | | OUROBOROS_SUBSCRIPTION_PRESET_VERSION | (empty) | One-shot INSTALL-TIME marker recording which generation of the agent-subscription preset (`subscription_install_presets.py`) this install received. Written only by `POST /api/onboarding/complete`, beside the preset it records; endpoint-authored and DISK-ONLY (`config.ENDPOINT_AUTHORED_SETTINGS`), so the generic save's merge skip blocks the request body while the loader and the environment projection keep the key out of `os.environ` in both directions — an environment-only marker used to be persisted by an ordinary settings POST. Its ABSENCE authorizes nothing — every install that predates presets lacks it too, which is why install time is proved by all three of: no recorded completion, no preset generation, and no `settings.json` yet | | OUROBOROS_SUBAGENT_PRESET_RECEIPT | (empty) | Endpoint-authored, disk-only receipt for the compiled Available-subagent/reviewer preset, including source/fingerprint and disclosed diagnostics. It is evidence, never an alternate actor setting. | | OUROBOROS_ONBOARDING_COMPLETED_AT | (empty) | The durable "onboarding finished here" timestamp, written by EVERY completion of `POST /api/onboarding/complete` — including one that connected no subscription and one that skipped the preset. Same endpoint-authored disk-only treatment as the preset marker: an environment timestamp alone once answered `not_install_time` on a genuinely fresh install and closed the window with no preset installed. It exists because `has_startup_ready_provider` being false is a state an OLD install reaches whenever its provider key stops working: without a recorded completion, that alone re-opened the install-time window and wrote presets over the owner's own reviewer/subagent configuration | diff --git a/ouroboros/acceptance_dialogue.py b/ouroboros/acceptance_dialogue.py index ca06eb8e2..3a734ed79 100644 --- a/ouroboros/acceptance_dialogue.py +++ b/ouroboros/acceptance_dialogue.py @@ -35,7 +35,7 @@ import time from typing import Any, Dict, List, Optional, Tuple from ouroboros import task_pacing -from ouroboros.config import adaptive_quorum, resolve_effort +from ouroboros.config import adaptive_quorum from ouroboros.delivery_protocol import extract_plain_text_from_content from ouroboros.outcomes import ( ACCEPTANCE_ACCEPTED, @@ -898,8 +898,8 @@ def _execute_task_acceptance_panel(ctx: Any) -> Any: HARDNESS_ADVISORY_VISIBLE, ReviewRequest, ReviewRunResult, - reviewer_slots, run_review_request, + triad_delivery_slots, ) from ouroboros.review_dispatch import ( TaskAcceptanceDispatchUnavailable, @@ -908,8 +908,23 @@ def _execute_task_acceptance_panel(ctx: Any) -> Any: task_acceptance_preclaim_refusal, ) + def _refused(reason: str) -> Any: + return ReviewRunResult( + request={"surface": "task_acceptance", "task_id": str(ctx.task_id)}, + actors=[], parsed_findings=[], aggregate_signal="DEGRADED", degraded=True, + degraded_reasons=[reason], + ) + evidence = ctx.evidence or _build_host_acceptance_evidence(ctx) - slots = reviewer_slots(effort=resolve_effort("review"), role_hint="task acceptance") + try: + # R2: the SAME triad rows every other triad surface reads — each with + # its own delivery, effort, credential pin, actor binding and stable + # id. R3: a malformed structured value refuses typed here exactly as + # it does for plan and skill review; the silently projected default + # panel is gone. + slots = triad_delivery_slots(role_hint="task acceptance") + except ValueError as exc: + return _refused(f"reviewer_slot_config_invalid: {exc} (no reviewer was called)") request = ReviewRequest( surface="task_acceptance", goal=( @@ -930,14 +945,7 @@ def _execute_task_acceptance_panel(ctx: Any) -> Any: task_id=ctx.task_id, retry_key=f"task_acceptance:{task_acceptance_evidence_revision(evidence)}", ) if not slots: - return ReviewRunResult( - request={"surface": "task_acceptance", "task_id": str(ctx.task_id)}, - actors=[], - parsed_findings=[], - aggregate_signal="DEGRADED", - degraded=True, - degraded_reasons=["no_review_slots"], - ) + return _refused("no_review_slots") # Budget admission for the whole acceptance wave (v6.69.0): a wave that # cannot fit the remaining root budget is declined up front as a terminal # DEGRADED (no-quorum semantics) instead of dying mid-wave. The estimate @@ -958,17 +966,10 @@ def _execute_task_acceptance_panel(ctx: Any) -> Any: prompt_chars=_prompt_chars, ) if _admission is not None: - return ReviewRunResult( - request={"surface": "task_acceptance", "task_id": str(ctx.task_id)}, - actors=[], - parsed_findings=[], - aggregate_signal="DEGRADED", - degraded=True, - degraded_reasons=[ - "review_wave_budget_insufficient: estimated " - f"~${_admission.get('estimated_wave_usd')} > remaining " - f"${_admission.get('remaining_usd')} (no reviewer was called)" - ], + return _refused( + "review_wave_budget_insufficient: estimated " + f"~${_admission.get('estimated_wave_usd')} > remaining " + f"${_admission.get('remaining_usd')} (no reviewer was called)" ) free_result = _free_dispatch( request, slots, drive_root=ctx.drive_root or ctx.tools._ctx.drive_root, usage_ctx=ctx.tools._ctx) @@ -989,11 +990,7 @@ def _execute_task_acceptance_panel(ctx: Any) -> Any: usage_ctx=usage_ctx, ) except TaskAcceptanceDispatchUnavailable as exc: - return ReviewRunResult( - request={"surface": "task_acceptance", "task_id": str(ctx.task_id)}, - actors=[], parsed_findings=[], aggregate_signal="DEGRADED", degraded=True, - degraded_reasons=[f"{exc} (no reviewer was called)"], - ) + return _refused(f"{exc} (no reviewer was called)") duration_sec = round(time.monotonic() - started, 3) try: from ouroboros.review_cycles import review_max_cycles, review_max_cycles_source diff --git a/ouroboros/gateway/settings.py b/ouroboros/gateway/settings.py index 34468723c..f26b03d77 100644 --- a/ouroboros/gateway/settings.py +++ b/ouroboros/gateway/settings.py @@ -1173,8 +1173,8 @@ def _check_reviewer_slots_against_incoming_roster(body: dict) -> str: roster-only save re-validates the STORED slots so a still-referenced actor cannot be removed out from under them. An EXPLICITLY cleared slots value ('' present in the body) is a clear, not a fallback to the stored - value — presence and emptiness are tracked separately. Returns the D4 - fallback warning ('' when none); raises ValueError on malformed.""" + value — presence and emptiness are tracked separately. Returns the + save-time disclosure ('' when none); raises ValueError on malformed.""" subagents_key = "OUROBOROS_SUBAGENTS" slots_key = "OUROBOROS_REVIEWER_SLOTS" roster_changed = subagents_key in body @@ -1253,10 +1253,10 @@ def _api_settings_post_locked(request: Request, body: Any) -> JSONResponse: return unsaved_error(str(exc), 400) body = dict(body) body[subagents_key] = canonical_subagents - # Reviewer-slot SSOT (6.1): 400 on malformed; D4 fallback disclosed; + # Reviewer-slot SSOT (6.1): 400 on malformed; save-time disclosure returned; # validated against the roster THIS save produces (S4 — see helper). try: - _reviewer_fallback_warning = _check_reviewer_slots_against_incoming_roster(body) + _reviewer_slots_warning = _check_reviewer_slots_against_incoming_roster(body) except ValueError as exc: return unsaved_error(str(exc), 400) parsed_budget: dict[str, float] = {} @@ -1381,8 +1381,8 @@ def _api_settings_post_locked(request: Request, body: Any) -> JSONResponse: # Tolerate stubbed side effects returning None (test harnesses). warnings = list(side_effect_warnings or []) - if _reviewer_fallback_warning: - warnings.append(_reviewer_fallback_warning) + if _reviewer_slots_warning: + warnings.append(_reviewer_slots_warning) if provider_defaults_changed: change_kind = classify_runtime_provider_change(old_effective_settings, current) if change_kind == "direct_normalize": diff --git a/ouroboros/review_substrate.py b/ouroboros/review_substrate.py index 18d506dcf..383511f2b 100644 --- a/ouroboros/review_substrate.py +++ b/ouroboros/review_substrate.py @@ -965,9 +965,9 @@ from ouroboros.review_dispatch import ( # noqa: E402,F401 — re-exports ) -# reviewer_slots() lives in reviewer_slot_config (altitude, P7); re-exported -# because acceptance surfaces and tests import it from here. -from ouroboros.reviewer_slot_config import reviewer_slots # noqa: F401,E402 +# reviewer_slots()/triad_delivery_slots() live in reviewer_slot_config (altitude, +# P7); re-exported because acceptance surfaces and tests import them from here. +from ouroboros.reviewer_slot_config import reviewer_slots, triad_delivery_slots # noqa: F401,E402 def scope_reviewer_slots( diff --git a/ouroboros/reviewer_slot_config.py b/ouroboros/reviewer_slot_config.py index 36c0c8c79..69eff6e05 100644 --- a/ouroboros/reviewer_slot_config.py +++ b/ouroboros/reviewer_slot_config.py @@ -31,15 +31,19 @@ direct row carries (route drift after admission changes later waves only). An ``api_model`` actor delivers as bounded native tool rounds — retrieval, never the assembled ``api_chat`` packet. -MIGRATION (D15: "старый читается, если новых нет"): when the structured key is +MIGRATION ("старый читается, если новых нет"): when the structured key is absent, the legacy comma-lists (``OUROBOROS_REVIEW_MODELS`` / ``OUROBOROS_SCOPE_REVIEW_MODELS``) plus the phase-5 per-row route lists are read into rows, and the global Review / Scope Review efforts are copied into each row. There is NO permanent double-write: once the structured key is saved, the comma keys become a derived runtime projection -(``project_reviewer_slots_into_env``) for legacy consumers. Task acceptance -stays API-only by owner decision (D15); commit, scope, plan, advisory, and skill -review follow their configured delivery rows. +(``project_reviewer_slots_into_env``) for legacy consumers only. EVERY review +surface — commit, scope, plan, advisory, skill review and task acceptance — +follows its configured delivery rows; the triad rows reach plan review, skill +review and task acceptance through ONE builder (``triad_delivery_slots``), so +no surface reads a projection of the panel instead of the panel (owner +decision R2, 2026-09-01: the former task-acceptance API pin and its +default-panel fallback are gone). Malformed configuration RAISES: mapping a typo to ``api_chat`` would silently spend the API money the owner configured the row to move off of, and mapping @@ -750,24 +754,70 @@ def structured_scope_review_slots() -> Optional[list]: """ if not structured_reviewer_slots_present(): return None + return [ + _delivery_slot(row, effort_surface="scope_review", role_hint="scope reviewer") + for row in commit_scope_rows() + ] + + +def _delivery_slot( + row: ConfiguredReviewerSlot, *, effort_surface: str, role_hint: str, + default_effort: str = "", **slot_fields: Any, +) -> Any: + """ONE configured row as the substrate's ``ReviewSlot``, carrying its own + delivery: the route kind, the opaque session target and credential pin, and + the configured-subagent binding the route seam turns into a native episode.""" from ouroboros.config import review_model_uses_local from ouroboros.review_execution import ReviewRouteKind from ouroboros.review_substrate import ReviewSlot + return ReviewSlot( + slot_id=row.slot_id, + model=row.target_id, + effort=row_effort(row, effort_surface, default=default_effort), + role_hint=role_hint, + use_local=review_model_uses_local(row.target_id), + route=(ReviewRouteKind.AGENT_SESSION if row.is_session + else ReviewRouteKind.API_CHAT), + session_target=row.session_target, + session_profile=row.profile_id, + subagent_id=row.subagent_id, + **slot_fields, + ) + + +def triad_delivery_slots( + *, + role_hint: str = "", + default_effort: str = "", + config: Optional["ReviewerSlotConfig"] = None, + **slot_fields: Any, +) -> List[Any]: + """The configured triad rows as ``ReviewSlot`` objects — THE builder for + plan review, skill/commit review (through ``commit_triad_delivery``'s + aligned vectors) and task acceptance (owner R2: acceptance reads the same + rows every other triad surface reads, with each row's own effort, session + target, credential pin, configured-subagent binding and stable slot id). + + Every row rides its own delivery: an ``api_chat`` row receives the + assembled packet, an ``agent_session`` row is a delegated retrieving + reviewer, a ``subagent_id`` api row is a native retrieving episode — the + substrate's route seam decides from the slot fields carried here. Effort is + the row's explicit value, else a compound Cursor/Agy route's encoded value, + else ``default_effort``, else the configured Review effort. Slot ids are the + rows' own: owner-assigned on a structured config, ``slot_N`` from the one + mint on legacy. ``slot_fields`` are the caller's per-surface ReviewSlot + properties (timeout, output budget, temperature). A malformed structured + value RAISES ValueError — every surface turns that into its typed refusal + (R3); no surface has a silently projected default panel to fall back to. + """ + rows = (config if config is not None else load_reviewer_slot_config()).triad return [ - ReviewSlot( - slot_id=row.slot_id, - model=row.target_id, - effort=row_effort(row, "scope_review"), - role_hint="scope reviewer", - use_local=review_model_uses_local(row.target_id), - route=(ReviewRouteKind.AGENT_SESSION if row.is_session - else ReviewRouteKind.API_CHAT), - session_target=row.session_target, - session_profile=row.profile_id, - subagent_id=row.subagent_id, + _delivery_slot( + row, effort_surface="review", role_hint=role_hint, + default_effort=default_effort, **slot_fields, ) - for row in commit_scope_rows() + for row in rows ] @@ -779,14 +829,15 @@ def reviewer_slots( id_prefix: str = "", route_env_key: str = "", ) -> List[Any]: - """The configured reviewer rows, each carrying its DELIVERY route. + """Reviewer rows from an explicit (or legacy comma-key) MODEL LIST. Moved here from ``review_substrate`` for module altitude (P7); the - substrate re-exports it. ``route_env_key`` names the surface's per-row - route list (plan 5.1): the commit triad and scope pass theirs, so a row - can be an api_chat call or a delegated agent session. Surfaces that stay - on the API by owner decision (task acceptance — D15) pass NOTHING, which - pins every row to ``api_chat`` explicitly rather than by accident. + substrate re-exports it. ``route_env_key`` names the caller's per-row + route list (plan 5.1): the legacy scope path passes its own, so a row can + be an api_chat call or a delegated agent session; a caller that passes + NOTHING gets every row pinned to ``api_chat`` explicitly rather than by + accident. Surfaces that follow the configured triad rows do not come + here — they use ``triad_delivery_slots``. """ from ouroboros.config import get_review_models, review_model_uses_local from ouroboros.review_execution import ReviewRouteKind, configured_review_routes @@ -807,31 +858,30 @@ def reviewer_slots( def commit_triad_delivery() -> Dict[str, Any]: - """Aligned per-row delivery vectors for the commit triad, one call. + """Aligned per-row delivery vectors for the commit triad and skill review. - The commit surface consumes rows as parallel lists (models for display and - slot construction, routes for delivery, efforts/session targets/ids as row - properties); deriving them HERE keeps the surface at its size gate and - keeps the vectors impossible to misalign. Raises ValueError on a malformed + Those surfaces consume rows as parallel lists (models for display and slot + construction, routes for delivery, efforts/session targets/ids as row + properties); projecting them from ``triad_delivery_slots`` keeps the + surfaces at their size gates, keeps the vectors impossible to misalign, + and keeps ONE reader of the triad rows. Raises ValueError on a malformed configuration — the caller turns that into its typed infra block. """ from ouroboros.review_execution import ReviewRouteKind config = load_reviewer_slot_config() - rows = list(config.triad) + slots = triad_delivery_slots(config=config, role_hint="multi-model review") return { - "models": [row.target_id for row in rows], - "routes": [ - ReviewRouteKind.AGENT_SESSION if row.is_session else ReviewRouteKind.API_CHAT - for row in rows - ], - "efforts": [row_effort(row, "review") for row in rows], - "session_targets": [row.session_target for row in rows], - "session_profiles": [row.profile_id for row in rows], - "slot_ids": [row.slot_id for row in rows], - "subagent_ids": [row.subagent_id for row in rows], + "models": [slot.model for slot in slots], + "routes": [slot.route for slot in slots], + "efforts": [slot.effort for slot in slots], + "session_targets": [slot.session_target for slot in slots], + "session_profiles": [slot.session_profile for slot in slots], + "slot_ids": [slot.slot_id for slot in slots], + "subagent_ids": [slot.subagent_id for slot in slots], "legacy_skill_fingerprint": ( - config.source == "legacy" and all(not row.is_session for row in rows) + config.source == "legacy" + and all(slot.route is ReviewRouteKind.API_CHAT for slot in slots) ), } @@ -867,81 +917,26 @@ def row_effort( # --------------------------------------------------------------------------- -# Runtime projection for the API-pinned surfaces (D15). +# Save-time validation and the legacy comma-key projection. # --------------------------------------------------------------------------- -def api_fallback_disclosure(config: "ReviewerSlotConfig") -> Dict[str, Any]: - """What API-pinned task acceptance runs with no triad api_chat row (D4/D15). - - The review gates follow configured delivery rows. Task acceptance remains - API-only, so an all-session triad makes its legacy projection fall back to - shipped defaults. That real paid substitution stays owner-visible. - """ - from ouroboros.config import SETTINGS_DEFAULTS - - out: Dict[str, Any] = {} - # An actor row never feeds the API-only acceptance projection (its api form - # is retrieval delivery, not an api_chat packet model), so a triad of - # sessions and actors falls back exactly like an all-session one. - if config.triad and not any( - not r.retrieves for r in config.triad - ): - out["triad"] = str(SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"]).split(",") - return out - - -def reviewer_slot_api_fallback_warning(raw: Optional[str] = None) -> str: - """Save-time warning for the all-delegated substitution, or '' when none. - - ``raw`` lets the save handler pass the INCOMING structured value (before it - is stored); with none it reads the currently stored value. A malformed - value returns '' (the strict parser reports that separately).""" - raw = structured_reviewer_slots_raw() if raw is None else str(raw or "").strip() - if not raw: - return "" - try: - disclosure = api_fallback_disclosure(parse_reviewer_slots(raw)) - except ValueError: - return "" - return _fallback_warning_text(disclosure) - - -def _fallback_warning_text(disclosure: Dict[str, Any]) -> str: - """The owner-facing all-delegated routing disclosure, or '' when none. - - Deliberately NOT advice. It used to end "keep at least one API reviewer row - to avoid the fallback", which told the owner to undo the ratified default - (D-3: with a subscription connected, everything that can run on a - subscription does, and a triad is never half API and half subscription). - What remains true is a routing FACT worth stating once: which surfaces this - configuration moved off the API, which ones the API still serves, and which - models they will use when they run.""" - if not disclosure: - return "" - models = sorted({m for row in disclosure.values() for m in row}) - return ( - "Every commit, plan, and skill-review triad row runs on an agent " - "subscription, so those reviews spend subscription windows instead of " - "API budget and never fall back to API spend. Task acceptance remains " - f"API-only and uses the shipped default models ({', '.join(models)})." - ) - - def reviewer_slot_save_check(raw: str, *, subagents_raw: Optional[str] = None) -> str: - """Validate an incoming structured value and return the fallback warning. + """Validate an incoming structured value; return the save-time disclosure. Raises ValueError (row-precise) on a malformed value so the save handler - turns it into a 400; returns the all-delegated API-fallback warning ('' when - none) otherwise. ``subagents_raw`` threads the roster the SAME save + turns it into a 400. ``subagents_raw`` threads the roster the SAME save produces (S4 atomicity) through a context-local override — actor - references validate against it without any process-env mutation.""" + references validate against it without any process-env mutation. Returns + '' when there is nothing to disclose; the former all-delegated API-fallback + warning described a task-acceptance substitution that no longer exists + (acceptance follows the rows, owner R2).""" if subagents_raw is None: - disclosure = api_fallback_disclosure(parse_reviewer_slots(raw)) - return _fallback_warning_text(disclosure) + parse_reviewer_slots(raw) + return "" with roster_env_override(subagents_raw): - disclosure = api_fallback_disclosure(parse_reviewer_slots(raw)) - return _fallback_warning_text(disclosure) + parse_reviewer_slots(raw) + return "" @_contextlib.contextmanager @@ -958,33 +953,21 @@ def roster_env_override(subagents_raw: str): _ROSTER_ENV_OVERRIDE.reset(token) -def _record_api_fallback_substitution(disclosure: Dict[str, Any]) -> None: - """Durable half of the D4 disclosure: a config projection that silently - substituted default models leaves a record, not just a log line.""" - from ouroboros.utils import utc_now_iso, write_text_atomic - - path = _last_execution_path().parent / "reviewer_slot_api_fallback.json" - try: - path.parent.mkdir(parents=True, exist_ok=True) - write_text_atomic(path, json.dumps({ - "ts": utc_now_iso(), - "reason": "all_commit_rows_delegated_api_surfaces_fell_back_to_defaults", - "substituted": disclosure, - }, ensure_ascii=False, indent=1)) - except OSError: - pass - - def project_reviewer_slots_into_env() -> None: - """Project the structured config into the legacy env keys, at env-apply time. + """Project the structured config into the legacy comma keys, at env-apply time. - Task acceptance stays on the API by owner decision (D15) and keeps reading - ``get_review_models()``; when the owner's triad mixes in delegated rows it - must see ONLY the API rows. Commit, plan, and skill review read structured - rows directly, including both delivery kinds. - This is a runtime DERIVATION, not a second write: settings.json holds the - structured key alone, and a stale comma value there is overwritten here - rather than winning silently. + No review surface reads these keys while the structured key is present — + commit, scope, plan, skill review and task acceptance all read the + structured rows directly, both delivery kinds — but legacy consumers still + do: the external review script's key ordering, benchmark manifests, and + ``get_review_models()`` callers with no panel of their own. Only api_chat + rows project (a session row's target is a ``harness[=model]`` spec, not a + model id); a configured-subagent api row's target IS its roster model id + and projects like any other api row. An all-session triad therefore + leaves the comma key at the shipped default for those legacy readers — + never for a review surface. This is a runtime DERIVATION, not a second + write: settings.json holds the structured key alone, and a stale comma + value there is overwritten here rather than winning silently. Also owns the historical default-if-empty floor for both comma keys (moved verbatim from ``apply_settings_to_env`` so the tail behavior is one place). @@ -1009,43 +992,17 @@ def project_reviewer_slots_into_env() -> None: REVIEWER_SLOTS_ENV, exc_info=True, ) else: - # D15: only DIRECT api_chat rows project into the legacy comma keys - # API-only task acceptance reads. A configured-subagent api row is - # retrieval delivery — projecting its model here would silently - # hand acceptance a packet reviewer the owner configured as an - # actor (and its model id may not even be an api_chat catalog id). - api_triad = [ - r.target_id for r in config.triad if not r.retrieves - ] - api_scope = [ - r.target_id for r in config.scope if not r.retrieves - ] + api_triad = [r.target_id for r in config.triad if not r.is_session] + api_scope = [r.target_id for r in config.scope if not r.is_session] if api_triad: os.environ["OUROBOROS_REVIEW_MODELS"] = ",".join(api_triad) else: - # An all-delegated triad leaves API-only task acceptance (D15) - # on shipped defaults rather than on zero reviewers. os.environ.pop("OUROBOROS_REVIEW_MODELS", None) if api_scope: os.environ["OUROBOROS_SCOPE_REVIEW_MODELS"] = ",".join(api_scope) else: os.environ.pop("OUROBOROS_SCOPE_REVIEW_MODELS", None) os.environ.pop("OUROBOROS_SCOPE_REVIEW_MODEL", None) - # D4: the substitution about to happen at the floor below is not - # silent — it lands loudly in the log and a durable record. The - # save-time warning (reviewer_slot_api_fallback_warning) is the - # third disclosure surface the owner actually reads. - disclosure = api_fallback_disclosure(config) - if disclosure: - import logging - - logging.getLogger(__name__).warning( - "reviewer slots: every %s row is delegated; API-only task " - "acceptance falls back to shipped default models %s and " - "spends API budget", - " and ".join(disclosure), disclosure, - ) - _record_api_fallback_substitution(disclosure) if not os.environ.get("OUROBOROS_REVIEW_MODELS"): os.environ["OUROBOROS_REVIEW_MODELS"] = str(SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"]) if not os.environ.get("OUROBOROS_SCOPE_REVIEW_MODELS") and not os.environ.get("OUROBOROS_SCOPE_REVIEW_MODEL"): @@ -1183,20 +1140,20 @@ __all__ = [ "ConfiguredReviewerSlot", "ReviewerSlotConfig", "advisory_slot_config", - "api_fallback_disclosure", "commit_scope_rows", "commit_triad_rows", "deep_review_slot", "synthesized_deep_review_slot", - "reviewer_slot_api_fallback_warning", "load_reviewer_slot_config", "parse_reviewer_slots", "reviewer_slot_config_error", "project_reviewer_slots_into_env", "record_reviewer_slot_executions", "reviewer_slot_last_executions", + "reviewer_slot_save_check", "row_effort", "structured_reviewer_slots_present", "structured_scope_review_slots", "structured_reviewer_slots_raw", + "triad_delivery_slots", ] diff --git a/ouroboros/tools/plan_review_runtime.py b/ouroboros/tools/plan_review_runtime.py index 5edcdabe8..dad5a6329 100644 --- a/ouroboros/tools/plan_review_runtime.py +++ b/ouroboros/tools/plan_review_runtime.py @@ -19,7 +19,6 @@ import logging import pathlib from typing import Any, Dict, List, Optional -from ouroboros.config import review_model_uses_local from ouroboros.deadline_utils import parse_deadline_ts, utc_now from ouroboros.llm import LLMClient from ouroboros.review_execution_projection import review_executions_from_actor_usage @@ -208,36 +207,19 @@ def record_raw_plan_request_attempt( def plan_review_slots() -> list: - """The configured commit-triad rows as plan-review ``ReviewSlot`` objects. - - Both kinds ride: an ``api_chat`` row is one in-process call over the lean - packet; an ``agent_session`` row is a delegated retrieving reviewer - (``session_target``/``session_profile`` carried per row). Effort is the row's - explicit value, else a compound Cursor/Agy route's encoded value, else - ``PLAN_REVIEW_EFFORT``. Slot ids are the rows' own (structured: - owner-assigned; legacy: ``slot_N`` from the one mint). + """The configured commit-triad rows as plan-review ``ReviewSlot`` objects: + the shared ``triad_delivery_slots`` builder (one reader of the triad rows + for plan, skill and acceptance review) with plan review's own slot + properties — timeout, output budget, temperature, and ``PLAN_REVIEW_EFFORT`` + as the effort default. Both delivery kinds ride; slot ids are the rows' own. """ - from ouroboros.review_execution import ReviewRouteKind - from ouroboros.review_substrate import ReviewSlot - from ouroboros.reviewer_slot_config import load_reviewer_slot_config, row_effort + from ouroboros.reviewer_slot_config import triad_delivery_slots - return [ - ReviewSlot( - slot_id=row.slot_id, - model=row.target_id, - effort=row_effort(row, "review", default=PLAN_REVIEW_EFFORT), - timeout_sec=PLAN_REVIEW_SLOT_TIMEOUT_SEC, - max_tokens=PLAN_REVIEW_MAX_TOKENS, - temperature=0.2, - role_hint="plan reviewer", - use_local=review_model_uses_local(row.target_id), - route=ReviewRouteKind.AGENT_SESSION if row.is_session else ReviewRouteKind.API_CHAT, - session_target=row.session_target, - session_profile=row.profile_id, - subagent_id=row.subagent_id, - ) - for row in load_reviewer_slot_config().triad - ] + return triad_delivery_slots( + role_hint="plan reviewer", default_effort=PLAN_REVIEW_EFFORT, + timeout_sec=PLAN_REVIEW_SLOT_TIMEOUT_SEC, max_tokens=PLAN_REVIEW_MAX_TOKENS, + temperature=0.2, + ) def slot_is_session(slot: Any) -> bool: diff --git a/ouroboros/tools/review.py b/ouroboros/tools/review.py index 4289fa542..50e834325 100644 --- a/ouroboros/tools/review.py +++ b/ouroboros/tools/review.py @@ -156,7 +156,7 @@ def _handle_task_acceptance_review( rationale: str = "", obligation_dispositions: Optional[list] = None, ) -> str: - from ouroboros.config import get_task_review_mode, resolve_effort + from ouroboros.config import get_task_review_mode from ouroboros.review_evidence import ( build_task_acceptance_evidence, task_acceptance_evidence_revision, @@ -297,8 +297,8 @@ def _handle_task_acceptance_review( ReviewRequest, build_improvement_capsule, dissent_findings, - reviewer_slots, run_review_request, + triad_delivery_slots, ) request = ReviewRequest( @@ -317,9 +317,27 @@ def _handle_task_acceptance_review( }, task_id=str(getattr(ctx, "task_id", "") or ""), retry_key=f"task_acceptance:{task_acceptance_evidence_revision(evidence)}", ) - # Task acceptance alone stays API-only by owner decision (D15); configured - # rows now also route commit, scope, advisory, plan, and Skill Review. - slots = reviewer_slots(effort=resolve_effort("review"), role_hint="task acceptance") + # Child-task and `off`-mode acceptance is advisory evidence, never the root + # verdict, and buys no retrieving panel (owner R2; plan roast item 12): it + # runs the configured triad's PACKET rows only, refusing typed when none + # remain — never a silently projected default panel (R3). + try: + slots = [ + slot for slot in triad_delivery_slots(role_hint="task acceptance") + if not getattr(slot, "retrieves", False) + ] + except ValueError as exc: + return json.dumps({ + "status": "not_dispatched", + "error": f"invalid reviewer-slot configuration blocks task acceptance: {exc}", + }, ensure_ascii=False) + if not slots: + return json.dumps({ + "status": "not_dispatched", "reason": "no_packet_reviewer_rows", + "detail": "every configured triad row retrieves (agent session or configured " + "subagent); child/off task acceptance runs packet rows only, so no " + "reviewer was called", + }, ensure_ascii=False) request.policy["min_successful_slots"] = _cfg.adaptive_quorum(len(slots)) result = run_review_request(request, slots=slots, drive_root=pathlib.Path(ctx.drive_root), usage_ctx=ctx) # Agent self-call (auto): lead with the compact improvement capsule (the diff --git a/tests/test_acceptance_delivery.py b/tests/test_acceptance_delivery.py new file mode 100644 index 000000000..3c2c873a3 --- /dev/null +++ b/tests/test_acceptance_delivery.py @@ -0,0 +1,242 @@ +"""Task acceptance on the configured triad rows (owner decisions R0/R2/R3, +2026-09-01, Ф2 of the agentic-review sprint). + +ONE builder — ``reviewer_slot_config.triad_delivery_slots`` — turns the triad +rows into ``ReviewSlot`` objects for plan review, skill/commit review (as the +aligned vectors of ``commit_triad_delivery``) and task acceptance, so acceptance +carries every row's own delivery, effort, credential pin, configured-subagent +binding and stable slot id instead of an api-pinned projection. A malformed +structured configuration refuses acceptance typed (DEGRADED) exactly as it +refuses plan and skill review; a legacy comma-key config reproduces today's +panel byte for byte; child-task and ``off``-mode acceptance buy no retrieving +row. +""" + +import json +from types import SimpleNamespace + +import pytest + +from ouroboros.review_execution import ReviewRouteKind +from ouroboros.reviewer_slot_config import REVIEWER_SLOTS_ENV, triad_delivery_slots + +_ROSTER = { + "enabled": True, + "items": [{ + "subagent_id": "api-critic", + "name": "API critic", + "recommended_use": "Exact recursive API reviewer.", + "route": {"kind": "api_model", "target_id": "openai/gpt-5.6-terra"}, + "effort": "medium", + }], +} + +_TRIAD = { + "triad": [ + {"slot_id": "t_api", "route": {"kind": "api_chat", "target_id": "openai/gpt-5.6-luna"}, + "effort": "high"}, + {"slot_id": "t_sess", + "route": {"kind": "agent_session", "target_id": "codex=gpt-5.6-sol", "profile_id": "acct-1"}, + "effort": "xhigh"}, + {"slot_id": "t_actor", "subagent_id": "api-critic"}, + ], + "scope": [{"slot_id": "s1", "route": {"kind": "api_chat", "target_id": "openai/gpt-5.6-terra"}}], +} + + +@pytest.fixture() +def structured_env(monkeypatch): + monkeypatch.setenv("OUROBOROS_SUBAGENTS", json.dumps(_ROSTER)) + monkeypatch.setenv(REVIEWER_SLOTS_ENV, json.dumps(_TRIAD)) + for key in ("OUROBOROS_REVIEW_MODELS", "OUROBOROS_REVIEW_ROUTES", "OUROBOROS_REVIEW_SESSION_ROUTE"): + monkeypatch.delenv(key, raising=False) + return monkeypatch + + +def _acceptance_ctx(tmp_path, *, evidence=None, task_metadata=None, **tool_ctx_fields): + """A root acceptance context whose wallet claim can be exercised for real.""" + from ouroboros import loop as loop_mod + from ouroboros.contracts.task_contract import build_task_contract + from ouroboros.review_substrate import build_review_binding + from ouroboros.task_results import STATUS_RUNNING, write_task_result + + contract = build_task_contract({"budget_profile": {"max_improvement_passes": 0}}) + metadata = { + "root_task_id": "root-delivery", "delegation_role": "root", + "budget_drive_root": str(tmp_path), "task_contract": contract, + **(task_metadata or {}), + } + tool_ctx = SimpleNamespace( + task_id="root-delivery", drive_root=tmp_path, budget_drive_root=str(tmp_path), + task_contract=contract, task_metadata=metadata, pending_events=[], + **tool_ctx_fields, + ) + write_task_result( + tmp_path, "root-delivery", STATUS_RUNNING, root_task_id="root-delivery", + delegation_role="root", task_contract=contract, + ) + evidence = evidence if evidence is not None else {"evidence": "complete"} + return loop_mod._TaskAcceptanceContext( + tools=SimpleNamespace(_ctx=tool_ctx), content="deliverable", task_id="root-delivery", + task_type="task", llm_trace={"tool_calls": []}, drive_root=tmp_path, + messages=[{"role": "system", "content": "policy"}, {"role": "user", "content": "goal"}], + emit_progress=lambda _text: None, mode="required", subtree_statuses=[], + budget_profile=contract["budget_profile"], passes_done=0, evidence=evidence, + review_binding=build_review_binding( + candidate="deliverable", evidence=evidence, fence_token_or_state="delivery-test", + ), + ) + + +def _capture_panel(monkeypatch): + """Stub the substrate call and the wave gate; return the captured (request, kwargs).""" + import ouroboros.review_substrate as rs + from ouroboros.tools import review_helpers + + captured = [] + + def _run(request, **kwargs): + captured.append((request, kwargs)) + return SimpleNamespace(aggregate_signal="PASS", actors=[]) + + monkeypatch.setattr(rs, "run_review_request", _run) + monkeypatch.setattr(review_helpers, "review_wave_budget_gate", lambda *_a, **_k: None) + return captured + + +# --------------------------------------------------------------------------- +# One builder for plan review, skill/commit vectors and task acceptance. +# --------------------------------------------------------------------------- + + +def test_triad_delivery_slots_is_the_one_builder_shared_by_plan_and_commit_vectors(structured_env): + from ouroboros.reviewer_slot_config import commit_triad_delivery + from ouroboros.tools.plan_review_runtime import ( + PLAN_REVIEW_EFFORT, + PLAN_REVIEW_MAX_TOKENS, + plan_review_slots, + ) + + acceptance = triad_delivery_slots(role_hint="task acceptance") + plan = plan_review_slots() + identity = lambda s: (s.slot_id, s.model, s.route, s.session_target, s.session_profile, s.subagent_id) # noqa: E731 + assert [identity(s) for s in plan] == [identity(s) for s in acceptance] + assert [s.slot_id for s in acceptance] == ["t_api", "t_sess", "t_actor"] + # Plan review keeps its own slot properties on the shared rows. + assert all(s.role_hint == "plan reviewer" and s.max_tokens == PLAN_REVIEW_MAX_TOKENS for s in plan) + assert all(s.role_hint == "task acceptance" for s in acceptance) + # Effort: explicit row → row; compound/none → the caller's default (plan) or the + # roster row's own effort (actor row). + assert [s.effort for s in plan] == ["high", "xhigh", "medium"] + assert plan[0].effort != PLAN_REVIEW_EFFORT or PLAN_REVIEW_EFFORT == "high" + # The commit/skill vectors are a projection of the same slots. + vectors = commit_triad_delivery() + assert vectors["slot_ids"] == [s.slot_id for s in acceptance] + assert vectors["models"] == [s.model for s in acceptance] + assert vectors["routes"] == [s.route for s in acceptance] + assert vectors["session_profiles"] == ["", "acct-1", ""] + assert vectors["subagent_ids"] == ["", "", "api-critic"] + assert vectors["legacy_skill_fingerprint"] is False + + +def test_acceptance_panel_carries_each_rows_identity_effort_pin_and_binding(structured_env, tmp_path): + from ouroboros import loop as loop_mod + from ouroboros.config import adaptive_quorum + + captured = _capture_panel(structured_env) + result = loop_mod._execute_task_acceptance_panel(_acceptance_ctx(tmp_path)) + assert result.aggregate_signal == "PASS" + (request, kwargs), = captured + slots = kwargs["slots"] + assert [s.slot_id for s in slots] == ["t_api", "t_sess", "t_actor"] # owner ids, not slot_N + assert [s.route for s in slots] == [ + ReviewRouteKind.API_CHAT, ReviewRouteKind.AGENT_SESSION, ReviewRouteKind.API_CHAT, + ] + assert [s.effort for s in slots] == ["high", "xhigh", "medium"] # per-row, not one global effort + assert slots[1].session_target == "codex=gpt-5.6-sol" and slots[1].session_profile == "acct-1" + assert slots[2].subagent_id == "api-critic" and slots[2].native_retrieval + assert request.policy["min_successful_slots"] == adaptive_quorum(3) + + +def test_malformed_structured_config_refuses_acceptance_typed(structured_env, tmp_path): + """R3: the same typed refusal plan and skill review give — never the silently + projected default panel the retired residual used to run.""" + import ouroboros.review_substrate as rs + from ouroboros import loop as loop_mod + + structured_env.setenv(REVIEWER_SLOTS_ENV, "{broken") + structured_env.setattr( + rs, "run_review_request", + lambda *_a, **_k: (_ for _ in ()).throw(AssertionError("no reviewer may be called")), + ) + result = loop_mod._execute_task_acceptance_panel(_acceptance_ctx(tmp_path)) + assert result.aggregate_signal == "DEGRADED" and result.degraded + assert any( + r.startswith("reviewer_slot_config_invalid:") and "no reviewer was called" in r + for r in result.degraded_reasons + ) + assert result.actors == [] + + +def test_legacy_comma_config_reproduces_todays_api_panel(monkeypatch, tmp_path): + """The GAIA/CLB/SWE-Pro class: no structured key, a comma list — the panel is + the same three api rows with the legacy `slot_N` ids and the configured + Review effort, exactly what the projection used to hand acceptance.""" + from ouroboros import loop as loop_mod + from ouroboros.config import resolve_effort + + monkeypatch.delenv(REVIEWER_SLOTS_ENV, raising=False) + monkeypatch.delenv("OUROBOROS_REVIEW_ROUTES", raising=False) + monkeypatch.setenv("OUROBOROS_REVIEW_MODELS", "openai/a,openai/b,openai/c") + captured = _capture_panel(monkeypatch) + loop_mod._execute_task_acceptance_panel(_acceptance_ctx(tmp_path)) + (_request, kwargs), = captured + slots = kwargs["slots"] + assert [(s.slot_id, s.model, s.route) for s in slots] == [ + ("slot_1", "openai/a", ReviewRouteKind.API_CHAT), + ("slot_2", "openai/b", ReviewRouteKind.API_CHAT), + ("slot_3", "openai/c", ReviewRouteKind.API_CHAT), + ] + assert all(s.effort == resolve_effort("review") and not s.retrieves for s in slots) + + +def test_child_and_off_acceptance_run_packet_rows_only(structured_env, tmp_path): + """Child-task and `off`-mode acceptance is advisory evidence: it buys no + retrieving panel (no agent session, no native episode) — it runs the + configured PACKET rows, and refuses typed when none remain.""" + import ouroboros.review_substrate as rs + from ouroboros import review_evidence as re_mod + from ouroboros.tools.review import _handle_task_acceptance_review + + calls = [] + structured_env.setattr(re_mod, "collect_turn_diff", lambda ctx, **kwargs: "") + structured_env.setattr(rs, "build_improvement_capsule", lambda _result: "") + structured_env.setattr(rs, "dissent_findings", lambda _result: []) + + def fake_run(request, **kwargs): + calls.append([s.slot_id for s in kwargs["slots"]]) + return SimpleNamespace(aggregate_signal="PASS", actors=[], parsed_findings=[]) + + structured_env.setattr(rs, "run_review_request", fake_run) + structured_env.setenv("OUROBOROS_TASK_REVIEW_MODE", "off") + ctx = SimpleNamespace( + drive_root=str(tmp_path), task_id="root", root_task_id="root", + task_metadata={"root_task_id": "root"}, task_contract={}, + ) + # Mixed triad: only the api row is dispatched; the session and the actor + # row are dropped without being called. + json.loads(_handle_task_acceptance_review(ctx, claim="root done")) + assert calls == [["t_api"]] + + # All-retrieving triad: a typed not_dispatched result, no reviewer called. + all_retrieving = {**_TRIAD, "triad": _TRIAD["triad"][1:]} + structured_env.setenv(REVIEWER_SLOTS_ENV, json.dumps(all_retrieving)) + payload = json.loads(_handle_task_acceptance_review(ctx, claim="root done")) + assert payload["status"] == "not_dispatched" and payload["reason"] == "no_packet_reviewer_rows" + assert calls == [["t_api"]] + + # Malformed configuration: the same typed refusal, never a default panel. + structured_env.setenv(REVIEWER_SLOTS_ENV, "{broken") + payload = json.loads(_handle_task_acceptance_review(ctx, claim="root done")) + assert payload["status"] == "not_dispatched" and "invalid reviewer-slot configuration" in payload["error"] + assert calls == [["t_api"]] diff --git a/tests/test_loop_misc.py b/tests/test_loop_misc.py index 2b983e57b..fbc472d47 100644 --- a/tests/test_loop_misc.py +++ b/tests/test_loop_misc.py @@ -537,7 +537,7 @@ def test_task_acceptance_agent_tool_is_advisory_before_auto_host_gate(monkeypatc ) panel_state = {"calls": 0, "reviewed_at_dispatch": None} monkeypatch.setattr(loop_mod, "get_task_review_mode", lambda: "auto") - monkeypatch.setattr(rs, "reviewer_slots", lambda **_kwargs: [object(), object(), object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **_kwargs: [object(), object(), object()]) ctx = SimpleNamespace( _task_acceptance_reviewed=False, is_direct_chat=True, @@ -698,7 +698,7 @@ def _exercise_owner_followup_during_acceptance_panel(monkeypatch, tmp_path, *, d return clean monkeypatch.setattr(loop_mod, "get_task_review_mode", lambda: "auto") - monkeypatch.setattr(rs, "reviewer_slots", lambda **_kwargs: [object(), object(), object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **_kwargs: [object(), object(), object()]) monkeypatch.setattr(rs, "run_review_request", panel) trace = {"tool_calls": [{"tool": "write_file", "args": {"path": "x.py"}}]} messages = [{"role": "system", "content": ""}, {"role": "user", "content": "goal"}] @@ -765,7 +765,7 @@ def test_task_acceptance_required_feeds_back_capsule(monkeypatch, tmp_path): import ouroboros.review_substrate as rs monkeypatch.setattr(loop_mod, "get_task_review_mode", lambda: "required") - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [object(), object(), object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [object(), object(), object()]) # (a) CONTRACT-VALID solved PASS (a non-empty completion_coach, as the required # contract demands) with no actionable findings -> still NO injection, finalize. @@ -879,7 +879,7 @@ def test_required_review_blocked_commit_does_not_surface_prior_head(monkeypatch, request = {"surface": "task_acceptance"} monkeypatch.setattr(rs, "run_review_request", lambda *a, **k: _FakeResult()) - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [object(), object(), object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [object(), object(), object()]) captured = {} diff --git a/tests/test_owner_hurry_s3.py b/tests/test_owner_hurry_s3.py index 3c20fdac8..b0ae116d1 100644 --- a/tests/test_owner_hurry_s3.py +++ b/tests/test_owner_hurry_s3.py @@ -378,7 +378,7 @@ def test_acceptance_panel_skips_with_typed_reason_and_zero_reviewer_calls(tmp_pa monkeypatch.setattr(loop_mod, "get_task_review_mode", lambda: "required") reviewer_calls = [] monkeypatch.setattr( - rs, "reviewer_slots", + rs, "triad_delivery_slots", lambda **kw: reviewer_calls.append(kw) or [object()], ) oh.record_requested(tmp_path, "t-acc", request_id="rq", attempt=1) diff --git a/tests/test_review_agent_session_route.py b/tests/test_review_agent_session_route.py index 24033bf60..c37c4f40b 100644 --- a/tests/test_review_agent_session_route.py +++ b/tests/test_review_agent_session_route.py @@ -2225,13 +2225,23 @@ def test_mixed_scope_fanout_sends_each_row_over_its_own_route(tmp_path, monkeypa ], dispatched -def test_acceptance_rows_stay_api_even_when_triad_routes_delegate(monkeypatch): - """D15: task acceptance is pinned to the API (plan review follows each configured - row's delivery since the spec-gate redesign). The triad's route list must not - leak into surfaces that pass no route_env_key.""" - monkeypatch.setenv(TRIAD_REVIEW_ROUTES_ENV, "agent_session,agent_session,agent_session") - rows = reviewer_slots(["m1", "m2"], effort="high", role_hint="task acceptance") - assert all(row.route is ReviewRouteKind.API_CHAT for row in rows) +def test_acceptance_rows_follow_the_configured_triad_delivery(monkeypatch): + """Owner R2 (2026-09-01): task acceptance reads the SAME triad rows every other + triad surface reads — on a legacy config that includes the per-row route list — + instead of an api-pinned projection of them. The generic model-list builder + keeps its explicit pin for callers that pass no route list (a caller's own + statement, never a surface default).""" + from ouroboros.reviewer_slot_config import triad_delivery_slots + + monkeypatch.delenv("OUROBOROS_REVIEWER_SLOTS", raising=False) + monkeypatch.setenv("OUROBOROS_REVIEW_MODELS", "m1,m2") + monkeypatch.setenv(TRIAD_REVIEW_ROUTES_ENV, "agent_session,api_chat") + rows = triad_delivery_slots(role_hint="task acceptance") + assert [row.route for row in rows] == [ReviewRouteKind.AGENT_SESSION, ReviewRouteKind.API_CHAT] + assert [row.slot_id for row in rows] == ["slot_1", "slot_2"] # the one legacy mint + assert all(row.role_hint == "task acceptance" for row in rows) + pinned = reviewer_slots(["m1", "m2"], effort="high", role_hint="task acceptance") + assert all(row.route is ReviewRouteKind.API_CHAT for row in pinned) def test_agent_slot_without_session_task_refuses_the_api_pack(tmp_path, fake_route): diff --git a/tests/test_review_prompt_caching.py b/tests/test_review_prompt_caching.py index b7619fe66..5048cc764 100644 --- a/tests/test_review_prompt_caching.py +++ b/tests/test_review_prompt_caching.py @@ -1527,7 +1527,7 @@ def test_acceptance_panel_declines_wave_on_insufficient_budget(monkeypatch, tmp_ from ouroboros.tools import review_helpers calls = {"panel": 0} - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [SimpleNamespace(model="m1"), SimpleNamespace(model="m2")]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [SimpleNamespace(model="m1"), SimpleNamespace(model="m2")]) def _boom(*a, **k): calls["panel"] += 1 raise AssertionError("reviewer must not be called") diff --git a/tests/test_review_substrate_v2.py b/tests/test_review_substrate_v2.py index 5055e7848..f42084f5a 100644 --- a/tests/test_review_substrate_v2.py +++ b/tests/test_review_substrate_v2.py @@ -435,7 +435,7 @@ def test_acceptance_review_evidence_diff_is_host_owned(monkeypatch, tmp_path): return NS(aggregate_signal="PASS") monkeypatch.setattr(rs, "run_review_request", _fake_run) - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [ReviewSlot(slot_id="a", model="m")]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [ReviewSlot(slot_id="a", model="m")]) ctx = NS( drive_root=str(tmp_path), task_id="t", @@ -467,7 +467,7 @@ def test_acceptance_review_empty_host_diff_does_not_fall_back_to_agent(monkeypat return NS(aggregate_signal="PASS") monkeypatch.setattr(rs, "run_review_request", _fake_run) - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [ReviewSlot(slot_id="a", model="m")]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [ReviewSlot(slot_id="a", model="m")]) ctx = NS( drive_root=str(tmp_path), task_id="t", @@ -496,7 +496,7 @@ def test_acceptance_review_records_agent_disposition(monkeypatch, tmp_path): return NS(aggregate_signal="PASS", actors=[], parsed_findings=[]) monkeypatch.setattr(rs, "run_review_request", _fake_run) - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [ReviewSlot(slot_id="a", model="m")]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [ReviewSlot(slot_id="a", model="m")]) monkeypatch.setattr(rs, "build_improvement_capsule", lambda _result: "") ctx = NS( @@ -534,7 +534,7 @@ def test_root_acceptance_tool_defers_to_host_without_model_calls(monkeypatch, tm ) monkeypatch.setattr( rs, - "reviewer_slots", + "triad_delivery_slots", lambda **kwargs: (_ for _ in ()).throw(AssertionError("review slots must not resolve")), ) ctx = NS( @@ -595,7 +595,7 @@ def test_typed_retry_root_defers_self_review_and_is_host_eligible( ) monkeypatch.setattr( rs, - "reviewer_slots", + "triad_delivery_slots", lambda **kwargs: (_ for _ in ()).throw( AssertionError("normalized root self-call must not resolve review slots") ), @@ -668,7 +668,7 @@ def test_retry_root_markers_must_agree_before_acceptance_authority( calls = [] monkeypatch.setattr( rs, - "reviewer_slots", + "triad_delivery_slots", lambda **kwargs: [ReviewSlot(slot_id="legacy", model="m")], ) monkeypatch.setattr(rs, "build_improvement_capsule", lambda _result: "") @@ -822,7 +822,7 @@ def test_off_mode_root_and_auto_mode_child_keep_existing_model_review(monkeypatc calls = [] monkeypatch.setattr(re_mod, "collect_turn_diff", lambda ctx, **kwargs: "") - monkeypatch.setattr(rs, "reviewer_slots", lambda **kwargs: [ReviewSlot(slot_id="a", model="m")]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **kwargs: [ReviewSlot(slot_id="a", model="m")]) monkeypatch.setattr(rs, "build_improvement_capsule", lambda _result: "") monkeypatch.setattr(rs, "dissent_findings", lambda _result: []) @@ -876,7 +876,7 @@ def test_stale_parent_lineage_cannot_trigger_a_second_host_panel(monkeypatch, tm monkeypatch.setattr(re_mod, "collect_turn_diff", lambda ctx, **kwargs: "") monkeypatch.setattr( rs, - "reviewer_slots", + "triad_delivery_slots", lambda **kwargs: [ReviewSlot(slot_id="a", model="m")], ) monkeypatch.setattr(rs, "build_improvement_capsule", lambda _result: "") diff --git a/tests/test_review_verification_v6544.py b/tests/test_review_verification_v6544.py index fbebe5769..4bae30ce6 100644 --- a/tests/test_review_verification_v6544.py +++ b/tests/test_review_verification_v6544.py @@ -513,7 +513,7 @@ def _acceptance_harness(monkeypatch, tmp_path, review_result, *, enforcement="bl monkeypatch.setattr(loop_mod, "get_task_review_mode", lambda: "required") monkeypatch.setattr(loop_mod, "get_review_enforcement", lambda: enforcement) - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [object(), object(), object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [object(), object(), object()]) monkeypatch.setattr(rs, "run_review_request", lambda *a, **k: review_result) meta = {} if deadline_remaining is not None: @@ -994,7 +994,7 @@ def test_agent_tool_payload_carries_dissent_noted(monkeypatch, tmp_path): ], parsed_findings=[], aggregate_signal="PASS", ) - monkeypatch.setattr(rs, "reviewer_slots", lambda **k: [object(), object(), object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **k: [object(), object(), object()]) monkeypatch.setattr(rs, "run_review_request", lambda *a, **k: result) monkeypatch.setattr( "ouroboros.review_evidence.build_task_acceptance_evidence", diff --git a/tests/test_reviewer_slot_actor_rows.py b/tests/test_reviewer_slot_actor_rows.py index b1f99aca8..a7ea8c5b3 100644 --- a/tests/test_reviewer_slot_actor_rows.py +++ b/tests/test_reviewer_slot_actor_rows.py @@ -4,8 +4,9 @@ A reviewer row may reference an ``OUROBOROS_SUBAGENTS`` roster row instead of carrying an inline route. Resolution happens once at load/admission from the APPLIED env; the resolved slot carries the actor id as identity/provenance and the roster row's execution facts. An api_model actor is the RETRIEVES class -(bounded native tool rounds) and must never enter the assembled-packet plane: -not the pack-assembly predicate, not the D15 acceptance projection. +(bounded native tool rounds) and must never enter the assembled-packet plane +(the pack-assembly predicate); its roster model id still projects into the +legacy comma key, which no review surface reads once the structured key exists. """ import json @@ -112,29 +113,38 @@ def test_empty_subagent_id_refuses(roster_env): parse_reviewer_slots(_payload([{"slot_id": "t1", "subagent_id": " "}])) -def test_actor_rows_never_project_into_api_only_acceptance(roster_env): - """D15: only DIRECT api_chat rows feed the legacy comma-key projection.""" +def test_api_rows_of_both_forms_project_their_model_ids_into_the_legacy_key(roster_env): + """The legacy comma key is a projection of api MODEL IDS for legacy readers + (external review tooling, benchmark manifests) — an actor row's roster model + id is one, a session row's `harness[=model]` target is not. No review surface + reads the key while the structured key exists (owner R2 retired the acceptance + pin that used to filter actor rows out of it).""" roster_env.setenv(REVIEWER_SLOTS_ENV, _payload([ {"slot_id": "t1", "subagent_id": "api-critic"}, {"slot_id": "t2", "route": {"kind": "api_chat", "target_id": "openai/gpt-5.5"}}, + {"slot_id": "t3", "subagent_id": "session-critic"}, ])) project_reviewer_slots_into_env() - assert roster_env is not None import os - assert os.environ["OUROBOROS_REVIEW_MODELS"] == "openai/gpt-5.5" + assert os.environ["OUROBOROS_REVIEW_MODELS"] == "openai/gpt-5.6-terra,openai/gpt-5.5" -def test_actor_only_triad_discloses_acceptance_fallback(roster_env): - """A triad of actors and sessions leaves API-only acceptance on defaults.""" - from ouroboros.reviewer_slot_config import api_fallback_disclosure +def test_actor_and_session_triad_reaches_acceptance_as_configured(roster_env): + """A triad of a native-retrieving actor and a session row IS the acceptance + panel (R0/R2): no API-default substitution, no disclosure of one, both + rows carried with their actor binding and pin.""" + from ouroboros.reviewer_slot_config import triad_delivery_slots roster_env.setenv(REVIEWER_SLOTS_ENV, _payload([ {"slot_id": "t1", "subagent_id": "api-critic"}, {"slot_id": "t2", "subagent_id": "session-critic"}, ])) - disclosure = api_fallback_disclosure(load_reviewer_slot_config()) - assert "triad" in disclosure + slots = triad_delivery_slots(role_hint="task acceptance") + assert [slot.slot_id for slot in slots] == ["t1", "t2"] + assert slots[0].native_retrieval and slots[0].subagent_id == "api-critic" + assert slots[1].route.value == "agent_session" and slots[1].session_profile == "profile-1" + assert all(slot.retrieves for slot in slots) def test_commit_triad_delivery_carries_actor_vector(roster_env): diff --git a/tests/test_reviewer_slot_config.py b/tests/test_reviewer_slot_config.py index 4b76b1595..12fcde8f9 100644 --- a/tests/test_reviewer_slot_config.py +++ b/tests/test_reviewer_slot_config.py @@ -1,8 +1,10 @@ """Reviewer-slot SSOT (phase 6.1): structured parse, legacy migration, projection. The one structured setting supersedes the comma-lists; the comma keys survive -only as a runtime projection for the API-pinned surfaces (D15). Malformed -configuration REFUSES typed — an unknown token must never silently pick a +only as a runtime projection for legacy consumers (external review tooling, +benchmark manifests) — no review surface reads them once the structured key +exists (owner R2 retired the task-acceptance API pin). Malformed configuration +REFUSES typed on every surface — an unknown token must never silently pick a transport, in either direction. """ import json @@ -480,7 +482,7 @@ def test_legacy_bad_route_token_still_refuses(monkeypatch): # --------------------------------------------------------------------------- -# Runtime projection for the API-pinned surfaces (D15). +# Runtime projection into the legacy comma keys (legacy consumers only). # --------------------------------------------------------------------------- @@ -494,7 +496,13 @@ def test_projection_exposes_only_api_rows(monkeypatch): assert os.environ["OUROBOROS_SCOPE_REVIEW_MODELS"] == "openai/gpt-5.6-terra" -def test_projection_all_delegated_triad_floors_to_defaults(monkeypatch): +def test_all_delegated_triad_projects_no_api_model_and_acceptance_follows_the_rows(monkeypatch): + """An all-session triad has no api model id to project: the comma key keeps + the shipped default for its LEGACY readers only (never a stale comma value), + while task acceptance — like every review surface — reads the session row + itself (owner R2; the former API-default substitution is gone).""" + from ouroboros.reviewer_slot_config import triad_delivery_slots + payload = json.loads(json.dumps(_STRUCTURED)) payload["triad"] = [payload["triad"][1]] _set_structured(monkeypatch, payload) @@ -504,9 +512,11 @@ def test_projection_all_delegated_triad_floors_to_defaults(monkeypatch): from ouroboros.config import SETTINGS_DEFAULTS - # The API-only task-acceptance surface falls back to shipped defaults, - # never to zero reviewers or a stale comma key. assert os.environ["OUROBOROS_REVIEW_MODELS"] == str(SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"]) + slots = triad_delivery_slots(role_hint="task acceptance") + assert [(s.slot_id, s.route.value, s.session_target, s.effort) for s in slots] == [ + ("t_sess", "agent_session", "codex=gpt-5.6-sol", "xhigh"), + ] def test_projection_malformed_leaves_legacy_keys_and_floors(monkeypatch): @@ -520,6 +530,12 @@ def test_projection_malformed_leaves_legacy_keys_and_floors(monkeypatch): assert os.environ["OUROBOROS_REVIEW_MODELS"] == "owner/comma-value" with pytest.raises(ValueError): commit_triad_rows() + # Task acceptance refuses on the same parse (R3) instead of reading the + # comma key its projection left in place. + from ouroboros.reviewer_slot_config import triad_delivery_slots + + with pytest.raises(ValueError): + triad_delivery_slots(role_hint="task acceptance") # --------------------------------------------------------------------------- @@ -801,20 +817,20 @@ def test_legacy_session_row_with_no_shared_route_is_empty_not_a_model(monkeypatc assert session_row.session_target == "" -def test_all_delegated_commit_surface_discloses_the_api_fallback(monkeypatch): - """When every triad row is delegated, API-only task acceptance falls back - to default models and spends budget — disclosed, never silent, while all - configured review gates keep their session rows. +def test_all_delegated_triad_writes_no_fallback_record_and_reaches_acceptance(monkeypatch): + """Owner R2: when every triad row is delegated, task acceptance RUNS those + rows — there is no API-default substitution to disclose, no durable + fallback record, and the retired disclosure apparatus is gone from the + module. The save check still validates (400 on malformed) and stays quiet.""" + import importlib + import pathlib - The disclosure is NEUTRAL routing information, not advice: an - all-subscription triad IS the ratified default (D-3), so the sentence must - not tell the owner to keep an API row and undo it.""" - from ouroboros.config import SETTINGS_DEFAULTS + from ouroboros import reviewer_slot_config as rsc + from ouroboros.config import DATA_DIR from ouroboros.reviewer_slot_config import ( - api_fallback_disclosure, - parse_reviewer_slots, project_reviewer_slots_into_env, - reviewer_slot_api_fallback_warning, + reviewer_slot_save_check, + triad_delivery_slots, ) payload = { @@ -822,42 +838,24 @@ def test_all_delegated_commit_surface_discloses_the_api_fallback(monkeypatch): "scope": [{"slot_id": "s1", "route": {"kind": "agent_session", "target_id": "codex"}}], "advisory": {"enabled": True, "route": {"kind": "agent_session", "target_id": "codex"}}, } - disclosure = api_fallback_disclosure(parse_reviewer_slots(json.dumps(payload))) - assert disclosure["triad"] == str(SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"]).split(",") - assert set(disclosure) == {"triad"} - + for retired in ("api_fallback_disclosure", "reviewer_slot_api_fallback_warning", + "_fallback_warning_text", "_record_api_fallback_substitution"): + assert not hasattr(importlib.reload(rsc), retired), retired + assert reviewer_slot_save_check(json.dumps(payload)) == "" _set_structured(monkeypatch, payload) - warning = reviewer_slot_api_fallback_warning() - assert warning and "commit, plan, and skill-review" in warning - # It names both halves of the routing fact: what moved to subscriptions, - # and which surfaces the API still serves with which models. - assert "agent subscription" in warning - assert "Task acceptance remains API-only" in warning - assert str(SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"]).split(",")[0] in warning - # The retired advice: telling the owner to keep an API reviewer row - # contradicts the ratified all-subscription default. - assert "Keep at least one API" not in warning - assert "coding agent" not in warning - # The projection still keeps the reviewers WORKING (defaults present), and - # writes the durable record — disclose-and-continue, never a block. - import os as _os project_reviewer_slots_into_env() - assert _os.environ["OUROBOROS_REVIEW_MODELS"] == str(SETTINGS_DEFAULTS["OUROBOROS_REVIEW_MODELS"]) - import pathlib - - from ouroboros.config import DATA_DIR - record = pathlib.Path(DATA_DIR) / "state" / "reviewer_slot_api_fallback.json" - assert record.is_file() + assert not (pathlib.Path(DATA_DIR) / "state" / "reviewer_slot_api_fallback.json").exists() + slots = triad_delivery_slots(role_hint="task acceptance") + assert [(s.slot_id, s.route.value, s.session_target) for s in slots] == [("t1", "agent_session", "codex")] + with pytest.raises(ValueError, match="triad needs at least one slot"): + reviewer_slot_save_check(json.dumps({**payload, "triad": []})) -def test_one_api_row_avoids_the_fallback_and_the_warning(monkeypatch): - """The mutation that proves the disclosure is scoped: a single surviving API - row in each group means no fallback and no warning.""" - from ouroboros.reviewer_slot_config import ( - api_fallback_disclosure, - parse_reviewer_slots, - reviewer_slot_api_fallback_warning, - ) +def test_mixed_triad_reaches_acceptance_in_row_order(monkeypatch): + """A session row beside an api row: acceptance carries BOTH, in the owner's + order, each with its own delivery — the mutation that proves no row is + filtered out of the panel any more.""" + from ouroboros.reviewer_slot_config import triad_delivery_slots payload = { "triad": [ @@ -867,9 +865,12 @@ def test_one_api_row_avoids_the_fallback_and_the_warning(monkeypatch): "scope": [{"slot_id": "s1", "route": {"kind": "api_chat", "target_id": "openai/gpt-5.6-terra"}}], "advisory": {"enabled": True, "route": {"kind": "api"}}, } - assert api_fallback_disclosure(parse_reviewer_slots(json.dumps(payload))) == {} _set_structured(monkeypatch, payload) - assert reviewer_slot_api_fallback_warning() == "" + slots = triad_delivery_slots(role_hint="task acceptance") + assert [(s.slot_id, s.route.value, s.model) for s in slots] == [ + ("t1", "agent_session", "codex"), ("t2", "api_chat", "openai/gpt-5.6-luna"), + ] + assert [s.retrieves for s in slots] == [True, False] def test_advisory_disabled_is_a_standing_owner_decision(monkeypatch): diff --git a/tests/test_v664_acceptance_planning.py b/tests/test_v664_acceptance_planning.py index 4a2dcbd65..491cfe580 100644 --- a/tests/test_v664_acceptance_planning.py +++ b/tests/test_v664_acceptance_planning.py @@ -148,7 +148,7 @@ def test_acceptance_panel_persists_timing_to_canonical_root(tmp_path, monkeypatc ) monkeypatch.setattr(evidence_mod, "build_task_acceptance_evidence", lambda *_a, **_k: {}) monkeypatch.setattr( - substrate, "reviewer_slots", + substrate, "triad_delivery_slots", lambda **_k: [SimpleNamespace(model="test-reviewer")], ) monkeypatch.setattr(review_helpers, "review_wave_budget_gate", lambda *_a, **_k: None) @@ -242,7 +242,7 @@ def _allow_acceptance_wave(monkeypatch): monkeypatch.setattr( substrate, - "reviewer_slots", + "triad_delivery_slots", lambda **_kwargs: [ReviewSlot(slot_id="slot", model="review-model")], ) monkeypatch.setattr( diff --git a/tests/test_v678_acceptance_state.py b/tests/test_v678_acceptance_state.py index 6c4591fd3..3866965c9 100644 --- a/tests/test_v678_acceptance_state.py +++ b/tests/test_v678_acceptance_state.py @@ -791,7 +791,7 @@ def test_deadline_reserve_writer_and_reader_move_together(monkeypatch, tmp_path) monkeypatch.setattr(loop_mod, "get_task_review_mode", lambda: "required") monkeypatch.setattr(loop_mod, "get_review_enforcement", lambda: "blocking") - monkeypatch.setattr(rs, "reviewer_slots", lambda **_k: [object()]) + monkeypatch.setattr(rs, "triad_delivery_slots", lambda **_k: [object()]) monkeypatch.setenv("OUROBOROS_FINALIZATION_GRACE_SEC", "120") now = datetime.now(timezone.utc) ctx = SimpleNamespace( diff --git a/web/modules/onboarding_agents_step.js b/web/modules/onboarding_agents_step.js index 71c268b8a..8c12e0bdc 100644 --- a/web/modules/onboarding_agents_step.js +++ b/web/modules/onboarding_agents_step.js @@ -16,9 +16,10 @@ // an API LLM client; a plan cannot run it. The ladder says so in the same // breath as the benefit, because a wizard that implies otherwise sends the // owner to a first run that refuses to start. -// * "Rides a plan" is not "free", and NOT every reviewer moves. Commit triad, -// scope, advisory, plan, and skill review follow their configured delivery; -// task acceptance stays API-only (D15). The footnote carries both. +// * "Rides a plan" is not "free", and what moves is exactly what is ROUTED: +// commit triad, scope, advisory, plan, skill review and task acceptance all +// follow their configured delivery rows (owner R2, 2026-09-01 — the former +// task-acceptance API pin is gone). The footnote carries both. // // The rotation artwork is a STATIC SVG — inline, no library, no animation // (no comparable product animates this, and motion here would be noise). It is @@ -77,7 +78,7 @@ export const VALUE_LADDER = [ title: 'Add one agent plan', body: 'Delegated subagents run inside that plan instead of billing your API key ' + 'per call. Claude Code, Codex, and Cursor plans can also move commit, plan, ' - + 'and skill review; ' + + 'and skill review and task acceptance; ' + 'task-only plans such as Antigravity do not change reviewer routes. The main ' + 'agent keeps using the API key or local model you configured above: a ' + 'plan cannot run it.', @@ -93,8 +94,10 @@ export const VALUE_LADDER = [ export const LADDER_FOOTNOTE = 'Riding a plan is not free — it moves that work onto a subscription you already ' - + 'pay for instead of adding per-call API charges. Task acceptance stays on the API ' - + 'key; plan and skill review follow each configured triad row.'; + + 'pay for instead of adding per-call API charges. What moves is exactly what you ' + + 'route: commit, plan, skill review and task acceptance each follow their configured ' + + 'triad row, so an all-subscription triad also puts each substantive task\'s ' + + 'acceptance panel on the subscription.'; // --------------------------------------------------------------------------- // Pure helpers. diff --git a/web/modules/reviewer_slots.js b/web/modules/reviewer_slots.js index 22818e11b..9d6315b17 100644 --- a/web/modules/reviewer_slots.js +++ b/web/modules/reviewer_slots.js @@ -589,9 +589,11 @@ export function renderReviewerSlotsSection() {
Rows routed to a subscription never fall back to API spend: if every eligible window - is exhausted, the review waits for capacity. Commit, plan, scope, advisory, and skill - review follow their configured rows. Task acceptance remains API-only: it uses - configured API rows, or the shipped defaults when none remain. + is exhausted, the review waits for capacity. Commit, plan, scope, advisory, skill + review and task acceptance all follow their configured rows — task acceptance runs + the triad rows on their own delivery (API packet, configured-subagent inspection + episode, or agent session), so an all-subscription triad puts every substantive + task's acceptance panel on the subscription as well.
diff --git a/web/tests/onboarding_agents_step.test.js b/web/tests/onboarding_agents_step.test.js index 9fddf8840..799f4d0d7 100644 --- a/web/tests/onboarding_agents_step.test.js +++ b/web/tests/onboarding_agents_step.test.js @@ -65,7 +65,7 @@ test('the ladder is three rungs and states the launch gate honestly', () => { // Rung 2: the benefit and the D-1 limit in the same breath — a plan moves // delegated work and configured review rows, and CANNOT run the main agent. assert.match(better.body, /delegated subagents/i); - assert.match(better.body, /commit, plan, and skill review/i); + assert.match(better.body, /commit, plan, and skill review and task acceptance/i); assert.match(better.body, /main\s+agent keeps using the API key or local model/i); assert.match(better.body, /a plan cannot run it/i); // Rung 3: rotation, in the owner's own terms. @@ -81,9 +81,11 @@ test('the ladder is three rungs and states the launch gate honestly', () => { test('the footnote refuses both easy lies: "free", and "every reviewer moves"', () => { assert.match(LADDER_FOOTNOTE, /not free/i); assert.match(LADDER_FOOTNOTE, /already\s+pay for/i); - // Task acceptance is the one API-pinned residual; plan + skill follow rows. - assert.match(LADDER_FOOTNOTE, /Task acceptance stays on the API key/i); - assert.match(LADDER_FOOTNOTE, /plan and skill review follow each configured triad row/i); + // Owner R2 (2026-09-01): task acceptance follows the triad rows too — the + // footnote states the RULE (what is routed moves), never "everything moves". + assert.match(LADDER_FOOTNOTE, /task acceptance each follow their configured\s+triad row/i); + assert.match(LADDER_FOOTNOTE, /acceptance panel on the subscription/i); + assert.doesNotMatch(LADDER_FOOTNOTE, /stays on the API|API-only/i); assert.doesNotMatch(LADDER_FOOTNOTE, /all reviewers|every reviewer/i); }); diff --git a/web/tests/reviewer_slots.test.js b/web/tests/reviewer_slots.test.js index 62c9335d5..925edaa82 100644 --- a/web/tests/reviewer_slots.test.js +++ b/web/tests/reviewer_slots.test.js @@ -181,9 +181,11 @@ test('the standing note states the POLICY, never the current routing', () => { // The unconditional claim, in the shapes it could come back as. assert.doesNotMatch(markup, /review runs? on subscriptions/i); assert.doesNotMatch(markup, /reviews? run on your subscription/i); - assert.match(markup, /skill\s+review follow their configured rows/i); - assert.match(markup, /Task acceptance remains API-only/i); - assert.match(markup, /uses\s+configured API rows, or the shipped defaults when none remain/i); + assert.match(markup, /skill\s+review and task acceptance all follow their configured rows/i); + // Owner R2 (2026-09-01): task acceptance follows the triad rows on their own + // delivery — the former "remains API-only" pin must not come back in any spelling. + assert.match(markup, /task acceptance runs\s+the triad rows on their own delivery/i); + assert.doesNotMatch(markup, /API-only|shipped defaults when none remain/i); });