From 867e6d14745a32f9985ab2cbfb2ca8e31c742432 Mon Sep 17 00:00:00 2001 From: Ouroboros Date: Mon, 21 Sep 2026 01:55:48 +0300 Subject: [PATCH] Name roster rows by a route-derived handle, never a stored label A subagent row was identified everywhere by its stored `subagent_id`. Presets mint that id by position (`primary-builder`, `fast-scout`, ...) and Duplicate inherited it (`fast-scout_copy_pls35j`), so once the owner re-pointed a row the id lied to the model and to the owner exactly like the display `name` retired on 2026-08-30 did. The stored id is now a hidden join key (reviewer references, snapshots, custody and history keep following the row through it) and every model- and owner-facing surface names a row by its HANDLE: the route target as stored plus the facets set on that row (effort, non-default access, `@pin`, processing), e.g. `codex=gpt-6-astra/xhigh`. It is a pure function of ONE row, so a new sibling never renames a neighbour and a snapshot or history record recomputes its own name without the live roster. - One resolver (`subagent_runtime.resolve_configured_row`) serves `schedule_subagent` and `delegate_start`: a handle, else a stored id as a permanent silent alias; a string naming two rows is a typed `subagent_selector_conflict`. The actor-first start resolves its argument before comparing it with the bound snapshot, so a handle accepted at scheduling is no longer refused at the physical start. - Saves refuse two rows with an identical engine (Settings save, onboarding draft and completion; effective defaults, so an omitted session access IS `full`). Reads stay tolerant: twins saved earlier still load and are told apart as `~`. In the editor Duplicate mints a neutral key and is born a draft whose card names its twin until an engine field changes. - The model catalog, unavailable-route alternatives, the startup receipt, the start result and the dated execution facts carry handles; history is named from each record's OWN identity (an old record without one shows its recorded route target), never mapped through today's roster. - The Review-lanes picker and its "ran as" line show the handle instead of `#`; a JS twin of the projection is pinned to the Python one by a shared parity table, and a browser test opens the real picker at 1 and 10 rows, desktop and phone width. Co-authored-by: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com> --- docs/DESIGN.md | 5 +- .../03-web-ui-pages-and-buttons.md | 4 +- docs/architecture/06-agent-core.md | 2 +- docs/development/06-rules-by-change-class.md | 6 +- ouroboros/configured_subagents.py | 83 ++++- ouroboros/context_runtime_facts.py | 10 +- ouroboros/gateway/onboarding.py | 2 + ouroboros/gateway/settings.py | 7 +- ouroboros/subagent_bootstrap.py | 4 +- ouroboros/subagent_history.py | 23 ++ ouroboros/subagent_runtime.py | 131 ++++--- prompts/SYSTEM.md | 14 +- tests/test_available_subagents_catalog.py | 11 +- tests/test_builtin_refusal_results.py | 4 +- tests/test_context_runtime_section.py | 3 +- tests/test_docs_sync.py | 2 +- tests/test_roster_handles_browser.py | 108 ++++++ tests/test_subagent_handles.py | 328 ++++++++++++++++++ tests/test_subagent_history_attempt_facts.py | 6 +- web/modules/reviewer_slots.js | 42 +-- web/modules/route_editor_primitives.js | 39 +++ web/modules/subagents_settings.js | 50 +-- .../fixtures/subagent_handle_parity.json | 61 ++++ web/tests/reviewer_slots.test.js | 23 +- web/tests/subagent_handles.test.js | 74 ++++ web/tests/subagents_settings.test.js | 43 ++- 26 files changed, 957 insertions(+), 128 deletions(-) create mode 100644 tests/test_roster_handles_browser.py create mode 100644 tests/test_subagent_handles.py create mode 100644 web/tests/fixtures/subagent_handle_parity.json create mode 100644 web/tests/subagent_handles.test.js diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 53a8c8c56..fe81c5620 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -804,7 +804,10 @@ stored spellings (`provider::model`, `claudexor::source=model`, `harness=model`) are serialization authored by the editor: never required from the owner, never a field placeholder or help-text instruction, never the primary displayed value; the exact stored id may appear in a meta line or -tooltip. The route identity chip names the source (API · OpenAI, Codex · model, +tooltip. A configured-subagent reference is the one place a stored spelling +names a thing: a roster row is labelled by its handle — its route target plus +the facets set on that row — because a friendlier stored label rots as soon as +the owner re-points the row. The route identity chip names the source (API · OpenAI, Codex · model, Claude Code · agent), not the channel alone. A last-run receipt is shown against the route that produced it: when the row's route changed since, the line says so and names the earlier route. diff --git a/docs/architecture/03-web-ui-pages-and-buttons.md b/docs/architecture/03-web-ui-pages-and-buttons.md index 5d1c81aa3..74c279ea8 100644 --- a/docs/architecture/03-web-ui-pages-and-buttons.md +++ b/docs/architecture/03-web-ui-pages-and-buttons.md @@ -289,9 +289,9 @@ Account status refresh runs immediately and on visible page/tab activation; hidd #### Review lanes and Available subagents -Review lanes edits one structured reviewer configuration (contract: §7 Reviewer slots). Each triad, scope, optional advisory or deep self-review row picks its reviewer from ONE flat select: Available-subagents roster rows lead as references, then the inline channels — API delivery or a coding-agent session — followed by model, optional credential profile and effort. Scope and deep self-review always retrieve: an API row runs bounded native inspection, a session row reads through its own harness. The scope note states that recorded reads are diagnostic and that scope runs in every context mode; no reviewer row asks for a context-window acknowledgement. The deep-review block describes native inspection with the memory whitelist inline, or a delegated session, and labels a row synthesized from `OUROBOROS_MODEL_DEEP_SELF_REVIEW`. An untouched synthesized (or empty) placeholder is OMITTED from the save payload, so an unrelated save, including repair beside `config_error`, never writes the key's value into the setting; editing the row materializes it, and a blanked model box is refused typed at save. Saved choices that disappear from discovery stay visible as unavailable rather than silently changing; capability labels configure nothing, and server-returned limits plus the last effective execution disclose what a saved row actually ran as. The `CATEGORIES` and `SINGLETONS` tables share renderers and binders across the respective row groups. The serializer preserves a loaded empty triad/scope draft for local validation before POST; the backend independently enforces the same configuration contract, and an unloaded view authors no replacement. A row pinned to an account discovery no longer lists keeps its pin, disclosed once above the rows and only on the word of a facet actually read (account pins answer to `accounts`, models to `catalog`; an unread or failed facet yields "not checked", not "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. An agent-session row's meta line carries the live availability sentence the Available-subagents cards compute (`sessionRouteVerdict`): the row, never the model option, says whether an account can run the selected model. +Review lanes edits one structured reviewer configuration (contract: §7 Reviewer slots). Each triad, scope, optional advisory or deep self-review row picks its reviewer from ONE flat select: Available-subagents roster rows lead as references named by handle, then the inline channels — API delivery or a coding-agent session — followed by model, optional credential profile and effort. Scope and deep self-review always retrieve: an API row runs bounded native inspection, a session row reads through its own harness. The scope note states that recorded reads are diagnostic and that scope runs in every context mode; no reviewer row asks for a context-window acknowledgement. The deep-review block describes native inspection with the memory whitelist inline, or a delegated session, and labels a row synthesized from `OUROBOROS_MODEL_DEEP_SELF_REVIEW`. An untouched synthesized (or empty) placeholder is OMITTED from the save payload, so an unrelated save, including repair beside `config_error`, never writes the key's value into the setting; editing the row materializes it, and a blanked model box is refused typed at save. Saved choices that disappear from discovery stay visible as unavailable rather than silently changing; capability labels configure nothing, and server-returned limits plus the last effective execution disclose what a saved row actually ran as. The `CATEGORIES` and `SINGLETONS` tables share renderers and binders across the respective row groups. The serializer preserves a loaded empty triad/scope draft for local validation before POST; the backend independently enforces the same configuration contract, and an unloaded view authors no replacement. A row pinned to an account discovery no longer lists keeps its pin, disclosed once above the rows and only on the word of a facet actually read (account pins answer to `accounts`, models to `catalog`; an unread or failed facet yields "not checked", not "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. An agent-session row's meta line carries the live availability sentence the Available-subagents cards compute (`sessionRouteVerdict`): the row, never the model option, says whether an account can run the selected model. -**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 compact cards (`docs/DESIGN.md` §6 row anatomy) and authors one prose field, Description (`recommended_use`), beside the structured API-model or Agent-session route, optional effort and optional managed-model/session account pin (empty pin = Claudexor's compatible-account rotation). Each card's status dot composes two axes — intent (saved / draft / generated) and availability (an API-model route reads Checked at start) — with the tone the worse of the two (`subagent_status_primitives.sessionRouteVerdict` / `rowStatus`). Identity is the stable internal `subagent_id` plus live route facts; parse drops a legacy display `name`, and a visual ordinal never becomes durable identity. Agent-session rows also select native Access: Full system access (default) or Working files. A saved lower choice is explicit; rows saved without the field default to full for new tasks, while running tasks keep their captured profile and read-only assignments stay read-only. The editor shares only neutral route/model/account/status primitives with Review lanes; Add and Duplicate reveal the new entry through the shared `ui_helpers.revealNewRow`. A fresh entry is not an error: its meta line carries a neutral hint until a save attempt (`noteSaveAttempt`; `validate()` stays pure), after which the offending card is tinted and names its error. +**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 compact cards (`docs/DESIGN.md` §6 row anatomy) and authors one prose field, Description (`recommended_use`), beside the structured API-model or Agent-session route, optional effort and optional managed-model/session account pin (empty pin = Claudexor's compatible-account rotation). Each card's status dot composes two axes — intent (saved / draft / generated) and availability (an API-model route reads Checked at start) — with the tone the worse of the two (`subagent_status_primitives.sessionRouteVerdict` / `rowStatus`). A row is NAMED by its handle — its route target plus the facets set on that row (effort, non-default access, `@account`, processing), e.g. `codex=gpt-6-astra/xhigh` — one pure projection in Python and JS (`configured_subagents.subagent_handle`, `route_editor_primitives.subagentHandle`; one parity table), because a stored label rots once the owner re-points the row: the display `name` did (parse drops it), then the role-shaped preset ids. `subagent_id` is a hidden stored join key, no surface offers a name or id input, and a visual ordinal is never durable identity. Two rows never SAVE with one engine: Duplicate makes a draft whose card names its twin until an engine field changes; earlier twins still load as `~`. Agent-session rows also select native Access: Full system access (default) or Working files. A saved lower choice is explicit; rows saved without the field default to full for new tasks, while running tasks keep their captured profile and read-only assignments stay read-only. The editor shares only neutral route/model/account/status primitives with Review lanes; Add and Duplicate reveal the new entry through the shared `ui_helpers.revealNewRow`. A fresh entry is not an error: its meta line carries a neutral hint until a save attempt (`noteSaveAttempt`; `validate()` stays pure), after which the offending card is tinted and names its error. Saved intent and live evidence are separate axes: row status distinguishes saved/generated/draft intent from current availability, and a status/catalog/accounts failure annotates rows, never erases them. The existing Accounts payload also carries dated API/session helper history, separate from the availability dot. Its requested and observed model/account remain distinct; changing route, pin, effort, processing or session access labels older execution settings, while changing Description does not. Outcome and date wrap on narrow screens. Historical failure neither disables a row nor ranks alternatives (§6 Route health). Settings GET may offer an unsaved migration/default candidate when no canonical value exists; the editor materializes it only on Save, and a late status or preview response may replace a still-clean generated baseline, never an owner-edited draft. Generic Settings save strictly validates and canonicalizes the materialized value before the serialized off-event-loop owner transaction; a running task keeps its immutable start snapshot, and the save response says changes apply from the next task. diff --git a/docs/architecture/06-agent-core.md b/docs/architecture/06-agent-core.md index fa3100fd7..711b9e5bd 100644 --- a/docs/architecture/06-agent-core.md +++ b/docs/architecture/06-agent-core.md @@ -256,7 +256,7 @@ Ordinary delegation requests no extra engine review panel; new ordinary runs on Children coordinate through `tree_note` and `tree_read`; read-only children can read/list the project-scoped knowledge store without writing its index; only the parent may use `override_delegation_constraint`, and a `review_requested` note carries an exact evidence reference/hash and wakes the parent without starting a paid cycle. Read-only and acting children alike hold the descendant-scoped `forward_to_worker`, `peek_task`, `cancel_task` and `discard_child_result` controls, and recursive delegation never widens filesystem, budget, depth, deadline, commit or owner authority. `delegation_budget` governs descendants (`may_delegate`, `may_fan_out`, additive depth provenance; a free-form intent note is never authority); persisted admission facts outrank later Settings changes, and a lower permitted depth is reported `capability_reduced`, never a silent flat tree. -**Registry.** `OUROBOROS_SUBAGENTS` is the active task-actor SSOT: a strict `{enabled, items}` value of at most ten `ConfiguredSubagent` rows — stable `subagent_id`, owner-authored English `recommended_use`, one normalized route (`api_model` or `agent_session`), optional effort, session credential pin and native access (`full` by default, or an owner-selected `workspace_write` ceiling; `configured_subagents.py`; legacy env keys are fail-closed migration inputs). New session snapshots store effective access explicitly; older snapshots without it retain workspace_write. The description is selection context only — host code never parses, ranks or maps its words to task text — so API models and session harnesses occupy one LLM-selectable list without pretending to share a topology, and an exact owner selection either starts that route or reports why not: never a keyword router or an automatic API substitution. +**Registry.** `OUROBOROS_SUBAGENTS` is the task-actor SSOT: a strict `{enabled, items}` value of at most ten `ConfiguredSubagent` rows — a hidden stored `subagent_id` join key behind a route-derived handle (why: §3 Available subagents), owner-authored `recommended_use` in any language, one normalized route (`api_model` or `agent_session`), optional effort, session credential pin and native access (`full` by default, or an owner-selected `workspace_write` ceiling; `configured_subagents.py`; legacy env keys are fail-closed migration inputs). Snapshots store effective access; older ones retain workspace_write. Tools take a handle, else a stored id; saves refuse identical engines, reads stay tolerant; history is named from its own record. Host code never parses, ranks or maps the description to task text, so both route kinds share one LLM-selectable list and an exact selection starts that route or reports why not: no keyword router, no automatic API substitution. **Scheduling.** `schedule_subagent` requires `subagent_id`, a focused `objective` and `expected_output`; the remaining public fields describe child-local context, constraints, capability needs, write surface, narrower deadline, delegation budget and acceptance claims. `access=inherit` (default or omitted) preserves the configured session access; `readonly|workspace_write` can only lower it. API-model rows ignore this field with a result disclosure; `write_surface` remains the read/write authority. There is no model-visible lane/executor axis and no public `effort` override: the selected row is the complete execution choice, lineage/bounds/route/budget are host-derived, and omission never inherits the parent's acceptance claims. `subagent_runtime.select_subagent_snapshot` copies an immutable snapshot of the exact enabled row into the child task — an `api_model` row becomes an ordinary recursive API child on that exact model/effort, an `agent_session` row an ordinary recursive Ouroboros nanny on that exact external route. Burst cash (each sibling launched before the first sibling's first response pays its own prefix write on cache-write-priced routes) is disclosed in the tool description as an affordance, never scheduled by the host. The old lane/executor resolver serves only old durable records: for those historical lane envelopes `schedule_subagent` reports the requested lane only — effective facts remain on the dispatched child record — and a task carrying `configured_subagent` goes straight to `subagent_runtime`, so legacy policy cannot reinterpret an active selection. diff --git a/docs/development/06-rules-by-change-class.md b/docs/development/06-rules-by-change-class.md index 042e8fc7e..0d65346d3 100644 --- a/docs/development/06-rules-by-change-class.md +++ b/docs/development/06-rules-by-change-class.md @@ -462,9 +462,9 @@ Settings, accounts and shared controls. Tests: `test_owner_settings_write_seam.p - One capability, one section: the task-actor story lives in Agents → Available subagents (`web/modules/subagents_settings.js`), editing one canonical `OUROBOROS_SUBAGENTS` object (list-level Enabled, at most ten - stable rows, one prose field `recommended_use`; id and compatibility name - automatic and hidden). Never derive durable identity from the visual - ordinal, and never render a second control over the same settings key + rows, one prose field `recommended_use`; the stored id is a hidden join + key). Name a row by its route-derived handle, never by a stored label or + the ordinal, and never render a second control over the same settings key (`OUROBOROS_MAX_WORKERS` stays in Advanced because it sizes the process pool). Share only neutral route/model/account/effort/status primitives with reviewer rows (`route_editor_primitives.js`): task routes serialize diff --git a/ouroboros/configured_subagents.py b/ouroboros/configured_subagents.py index 35f2cbfb9..0ce73a1d2 100644 --- a/ouroboros/configured_subagents.py +++ b/ouroboros/configured_subagents.py @@ -5,6 +5,7 @@ from __future__ import annotations import hashlib import json import re +from collections import Counter from dataclasses import dataclass from typing import Any, Mapping, Optional, Sequence @@ -61,12 +62,15 @@ ALTERNATIVE_RECOMMENDATION = ( @dataclass(frozen=True) class ConfiguredSubagent: + # Hidden stored join key (reviewer-slot references, snapshots, custody and + # history follow the ROW through it). What minds and owners are shown is + # `subagent_handle`, a projection of the route: a role-shaped id rotted the + # same way the retired `name` did once the owner re-pointed the row. subagent_id: str # Retired semantic field (owner decision 1=A, 2026-08-30): a second # human-facing label beside recommended_use rotted against route edits # (the shipped "Fast scout" incident). The parser accepts legacy values - # and drops them; identity everywhere is the neutral subagent_id plus - # DERIVED route facts, and recommended_use is the ONE semantic field. + # and drops them; recommended_use is the ONE semantic field. name: str = "" recommended_use: str = "" route: RouteSpec = None # type: ignore[assignment] @@ -256,6 +260,76 @@ def configured_subagents_fingerprint(config: ConfiguredSubagents) -> str: return hashlib.sha256(serialize_configured_subagents(config).encode("utf-8")).hexdigest() +def engine_identity(row: ConfiguredSubagent) -> dict[str, str]: + """Execution-affecting facts of ONE saved row, with its EFFECTIVE access. + + Same shape as ``subagent_history.execution_identity``, which reads frozen + snapshots and therefore defaults a missing access to ``workspace_write``; a + saved row's default is ``full`` (already applied by the parser). + """ + return { + "kind": row.route.kind, + "target_id": row.route.target_id, + "credential_profile_id": row.route.credential_profile_id, + "effort": row.effort, + "processing_preference": row.processing_preference, + **({"access": row.access} if row.route.is_session else {}), + } + + +def engine_handle(identity: Mapping[str, Any]) -> str: + """The name shown for an engine: its route target plus its OWN set facets. + + A pure function of one identity (a saved row, a frozen snapshot or a + history record), so a neighbour row can never rename it and the past is + never relabelled from the live roster. Facets, in fixed order: effort, + session access when not the default ``full``, account pin as ``@``, + processing preference. Compared, never parsed. + """ + access = str(identity.get("access") or "") if identity.get("kind") == ROUTE_KIND_AGENT_SESSION else "" + pin = str(identity.get("credential_profile_id") or "") + facets = ( + str(identity.get("effort") or ""), + "" if access == "full" else access, + f"@{pin}" if pin else "", + str(identity.get("processing_preference") or ""), + ) + return "/".join(part for part in (str(identity.get("target_id") or ""), *facets) if part) + + +def subagent_handle(row: ConfiguredSubagent) -> str: + return engine_handle(engine_identity(row)) + + +def roster_handles(config: ConfiguredSubagents) -> dict[str, str]: + """Stored id -> the handle the LIVE roster shows and the tools accept. + + Save-time uniqueness keeps handles distinct; rows that still share one + (twins saved before that rule) are told apart by their stored key as + ``~`` — never by list order, which is not durable. + """ + base = {row.subagent_id: subagent_handle(row) for row in config.items} + counts = Counter(base.values()) + return { + row_id: f"{handle}~{row_id}" if counts[handle] > 1 else handle + for row_id, handle in base.items() + } + + +def validate_unique_engines(config: ConfiguredSubagents) -> None: + """SAVE-path rule: two rows may not run an identical engine (reads stay tolerant).""" + seen: dict[tuple, int] = {} + for index, row in enumerate(config.items): + key = tuple(sorted(engine_identity(row).items())) + if key in seen: + raise ValueError( + f"{SUBAGENTS_SETTING}: items[{index}] runs the same engine as items[{seen[key]}] " + f"({subagent_handle(row)}); change its model, effort, access, account or " + "processing, or remove it" + ) + seen[key] = index + + def _materialized_source( settings: Mapping[str, Any], config: ConfiguredSubagents, ) -> str: @@ -585,10 +659,15 @@ __all__ = [ "SUBAGENTS_SETTING", "configured_subagents_dict", "configured_subagents_fingerprint", + "engine_handle", + "engine_identity", "make_configured_subagents", "normalize_configured_subagents", "parse_configured_subagents", "resolve_configured_subagents", "resolve_settings_subagent_candidate", + "roster_handles", "serialize_configured_subagents", + "subagent_handle", + "validate_unique_engines", ] diff --git a/ouroboros/context_runtime_facts.py b/ouroboros/context_runtime_facts.py index e032b81ed..af84c6e5c 100644 --- a/ouroboros/context_runtime_facts.py +++ b/ouroboros/context_runtime_facts.py @@ -161,6 +161,7 @@ def _delegation_capability_fact() -> Optional[Dict[str, Any]]: """ try: from ouroboros.reviewer_slot_config import reviewer_slot_last_executions + from ouroboros.subagent_history import recorded_handle from ouroboros.subagents import subagent_last_delegation def _observed_label(ts: Any) -> str: @@ -215,15 +216,20 @@ def _delegation_capability_fact() -> Optional[Dict[str, Any]]: last_fact["requested_profile"] = str(last["requested_profile"]) if last.get("applied_profile"): last_fact["applied_profile"] = str(last["applied_profile"]) + # Model-facing actor names are handles computed from each record's + # OWN facts; the stored key stays in the durable receipt file. if last.get("selected_subagent_id"): - last_fact["selected_subagent_id"] = str(last["selected_subagent_id"]) + last_fact["selected_subagent_id"] = recorded_handle(last) for key in ("outcome", "failure_code", "reset_at", "occurred_at", "observed_at"): if key in last: last_fact[key] = last[key] delegation["subagent_last_delegation"] = last_fact rows = last.get("latest_by_subagent") if isinstance(rows, dict) and rows: - delegation["subagents_last_executions"] = list(rows.values()) + delegation["subagents_last_executions"] = [ + {**row, "selected_subagent_id": recorded_handle(row)} + for row in rows.values() if isinstance(row, dict) + ] if len(delegation) == 1: return None return delegation diff --git a/ouroboros/gateway/onboarding.py b/ouroboros/gateway/onboarding.py index c45e7cbe3..96b377bd4 100644 --- a/ouroboros/gateway/onboarding.py +++ b/ouroboros/gateway/onboarding.py @@ -44,6 +44,7 @@ from ouroboros.configured_subagents import ( ConfiguredSubagents, configured_subagents_dict, normalize_configured_subagents, + validate_unique_engines, ) from ouroboros.gateway.owner_settings import ( @@ -667,6 +668,7 @@ def _configured_owner_draft( return None, "" try: config, _canonical = normalize_configured_subagents(body.get(SUBAGENTS_SETTING)) + validate_unique_engines(config) except ValueError as exc: return None, str(exc) return config, "" diff --git a/ouroboros/gateway/settings.py b/ouroboros/gateway/settings.py index 85757c222..991053a79 100644 --- a/ouroboros/gateway/settings.py +++ b/ouroboros/gateway/settings.py @@ -1205,11 +1205,14 @@ def _api_settings_post_locked(request: Request, body: Any) -> JSONResponse: # not the stale process env (see the check helper below). subagents_key = "OUROBOROS_SUBAGENTS" if subagents_key in body and body.get(subagents_key) not in (None, ""): - from ouroboros.configured_subagents import normalize_configured_subagents + from ouroboros.configured_subagents import ( + normalize_configured_subagents, validate_unique_engines, + ) try: - _subagents, canonical_subagents = normalize_configured_subagents( + subagents, canonical_subagents = normalize_configured_subagents( body.get(subagents_key) ) + validate_unique_engines(subagents) except ValueError as exc: return unsaved_error(str(exc), 400) body = dict(body) diff --git a/ouroboros/subagent_bootstrap.py b/ouroboros/subagent_bootstrap.py index 9e46abf45..11b1d1cbe 100644 --- a/ouroboros/subagent_bootstrap.py +++ b/ouroboros/subagent_bootstrap.py @@ -7,6 +7,7 @@ from hashlib import sha256 from pathlib import Path from typing import Any, Mapping +from ouroboros.subagent_history import snapshot_handle from ouroboros.subagent_work_order import compile_external_work_order @@ -679,7 +680,8 @@ def _prepare_actor_first_bootstrap( "zero_run_evidence_unknown" if zero_run_evidence_gaps and not durable_zero_run else "pending" ), - "selected_subagent_id": str(snapshot.get("selected_subagent_id") or ""), + # Model-facing name: the snapshot's own handle, never the stored key. + "selected_subagent_id": snapshot_handle(snapshot), "route": route_id, "work_order_fingerprint": work_order_fingerprint, "work_order_chars": work_order_chars, diff --git a/ouroboros/subagent_history.py b/ouroboros/subagent_history.py index 207258e08..f1b14c414 100644 --- a/ouroboros/subagent_history.py +++ b/ouroboros/subagent_history.py @@ -40,6 +40,29 @@ def execution_identity(snapshot: Mapping[str, Any]) -> dict[str, str]: if route.get("kind") == "agent_session" else {})} +def snapshot_handle(snapshot: Mapping[str, Any]) -> str: + """The handle of the engine a frozen snapshot names, from its own facts.""" + from ouroboros.configured_subagents import engine_handle + + return engine_handle(execution_identity(snapshot)) + + +def recorded_handle(row: Mapping[str, Any]) -> str: + """Name the engine a history row ran, from the row's OWN recorded facts. + + A typed ``identity`` yields its handle; an older row without one yields its + recorded route target. Never mapped through the live roster: the row an id + points at today may be a different engine, and that would relabel the past. + """ + from ouroboros.configured_subagents import engine_handle + + identity = row.get("identity") + if isinstance(identity, Mapping) and identity.get("target_id"): + return engine_handle(identity) + route, model = str(row.get("route") or ""), str(row.get("requested_model") or "") + return model if route == "api_model" else route + ("=" + model if route and model else "") + + def record_last_delegation(*, route: str, requested_model: str, applied_model: str, run_id: str, selected_subagent_id: str = "", requested_profile: str = "", applied_profile: str = "", diff --git a/ouroboros/subagent_runtime.py b/ouroboros/subagent_runtime.py index 06dbd9687..30c233383 100644 --- a/ouroboros/subagent_runtime.py +++ b/ouroboros/subagent_runtime.py @@ -16,6 +16,7 @@ from typing import Any, Mapping, Optional from ouroboros.configured_subagents import ( ConfiguredSubagent, + ConfiguredSubagents, ConfiguredSubagentsResolution, SESSION_ACCESS_PROFILES, SESSION_ACCESS_LOWERING, @@ -24,6 +25,7 @@ from ouroboros.configured_subagents import ( SOURCE_UNDECIDED, configured_subagents_fingerprint, resolve_configured_subagents, + roster_handles, ) from ouroboros.delegate_shared import delegate_payload from ouroboros.route_spec import route_spec_dict @@ -81,8 +83,9 @@ def model_visible_subagent_catalog(settings: Mapping[str, Any]) -> dict[str, Any """Project saved, dispatchable rows as facts, without probing or ranking them. Facts only: how to choose among rows is the mind's own prose - (``prompts/SYSTEM.md`` §Delegation), never a host-authored sentence here; - the list fingerprint and provenance stay host-side in snapshots. + (``prompts/SYSTEM.md`` §Delegation), never a host-authored sentence here. + ``subagent_id`` carries the row's handle — the value the tools accept; the + stored key and the list fingerprint stay host-side in snapshots. """ resolution = resolve_configured_subagents(settings) @@ -95,15 +98,18 @@ def model_visible_subagent_catalog(settings: Mapping[str, Any]) -> dict[str, Any ): return {} + handles = roster_handles(config) rows: list[dict[str, Any]] = [] for row in config.items: session = row.route.is_session + handle = handles[row.subagent_id] projected: dict[str, Any] = { - "subagent_id": row.subagent_id, + "subagent_id": handle, "route_class": "Agent session" if session else "API model", "requested_effort": row.effort or "(not explicitly set)", - "requested_target" if session else "requested_model": row.route.target_id, } + if row.route.target_id != handle: # a bare handle already IS the target + projected["requested_target" if session else "requested_model"] = row.route.target_id if session: projected["mutating_access"] = row.access if row.route.credential_profile_id: @@ -255,6 +261,33 @@ def _legacy_matches( return rows +def resolve_configured_row(config: ConfiguredSubagents, selector: str) -> ConfiguredSubagent: + """The one ``subagent_id`` argument resolver: a handle, else a stored id. + + Stored ids stay accepted forever, silently (cached prompts, old habits). A + selector that is one row's handle AND a different row's stored id is + refused naming both — never a silent pick. + """ + handles = roster_handles(config) + named = next((row for row in config.items if handles[row.subagent_id] == selector), None) + stored = next((row for row in config.items if row.subagent_id == selector), None) + if named is not None and stored is not None and named is not stored: + raise SubagentSelectionError( + "subagent_selector_conflict", + f"{selector!r} is ambiguous: it is the handle of the row stored as " + f"{named.subagent_id!r} and the stored id of the row whose handle is " + f"{handles[stored.subagent_id]!r}; pass one of those two values instead.", + ) + row = named or stored + if row is None: + raise SubagentSelectionError( + "unknown_subagent_id", + f"No configured subagent is named {selector!r}. Available: " + + ", ".join(repr(handle) for handle in handles.values()) + ".", + ) + return row + + def select_subagent_snapshot( settings: Mapping[str, Any], *, @@ -285,12 +318,7 @@ def select_subagent_snapshot( assert config is not None used_legacy = False if selected_id: - matches = [row for row in config.items if row.subagent_id == selected_id] - if not matches: - raise SubagentSelectionError( - "unknown_subagent_id", f"No configured subagent has id {selected_id!r}." - ) - row = matches[0] + row = resolve_configured_row(config, selected_id) else: lane = str(legacy_model_lane or "auto").strip().lower() or "auto" executor = str(legacy_executor or "auto").strip().lower() or "auto" @@ -533,9 +561,10 @@ def current_subagent_alternatives(exclude_id: str = "") -> list[dict[str, Any]]: if config is None or not config.enabled: return [] excluded = str(exclude_id or "") + handles = roster_handles(config) return [ { - "subagent_id": row.subagent_id, + "subagent_id": handles[row.subagent_id], "recommended_use": row.recommended_use, "route_kind": row.route.kind, "target_id": row.route.target_id, @@ -788,9 +817,10 @@ def exact_start(ctx: Any, prompt: str, spec: Optional[dict[str, Any]] = None) -> _mark_actor_physical_start(ctx, result) payload = delegate_payload(result) if isinstance(selected_snapshot, dict): - payload["selected_subagent_id"] = str( - selected_snapshot.get("selected_subagent_id") or "" - ) + # Model-facing name: the snapshot's own handle; custody keeps the stored key. + from ouroboros.subagent_history import snapshot_handle + + payload["selected_subagent_id"] = snapshot_handle(selected_snapshot) payload["config_fingerprint"] = str( selected_snapshot.get("config_fingerprint") or "" ) @@ -829,6 +859,30 @@ def _mark_actor_physical_start(ctx: Any, result: "ToolResult") -> None: ctx._nanny_physical_activity_seed = True +def _names_bound_actor(selector: str, bootstrap: Mapping[str, Any]) -> bool: + """Whether a ``subagent_id`` argument names the actor this episode is bound to. + + The bound row is the frozen snapshot, so its stored id and its own handle + (the one the startup receipt shows) always name it; any other value goes + through the same resolver as ``schedule_subagent`` against the live roster. + """ + from ouroboros.subagent_history import snapshot_handle + + expected_id = str(bootstrap.get("selected_subagent_id") or "") + snapshot = bootstrap.get("snapshot") if isinstance(bootstrap.get("snapshot"), dict) else {} + if selector in {expected_id, snapshot_handle(snapshot)}: + return True + try: + from ouroboros.config import runtime_settings + + config = _resolution( + effective_runtime_subagent_settings(runtime_settings()), allow_undecided_legacy=False, + ).config + return config is not None and resolve_configured_row(config, selector).subagent_id == expected_id + except SubagentSelectionError: + return False + + def delegate_start_entry(ctx: Any, prompt: str, _resolved_binding: Any = None, **params: Any) -> "ToolResult": # Actor-first configured sessions bind every fresh start to the immutable # snapshot captured before the episode. The model supplies only an advisory @@ -846,15 +900,22 @@ def delegate_start_entry(ctx: Any, prompt: str, _resolved_binding: Any = None, * record_start_blocked(ctx, str(getattr(ctx, "task_id", "") or ""), reason) - if isinstance(bootstrap, dict) and not retry_of: + if isinstance(bootstrap, dict): + # A fresh start and a retry alike stay bound to the frozen snapshot: an + # argument the bound start cannot honor is refused typed, never + # silently discarded. expected_id = str(bootstrap.get("selected_subagent_id") or "") requested_id = str(params.get("subagent_id") or "").strip() - if requested_id and requested_id != expected_id: + if retry_of and params.get("access") is not None: + _blocked("retry_selector_conflict") + return _fail("delegate_start", "retry_selector_conflict", + "A retry replays its recorded access; omit access.") + if requested_id and not _names_bound_actor(requested_id, bootstrap): _blocked("configured_actor_route_mismatch") return _fail( "delegate_start", "configured_actor_route_mismatch", - "This actor-first turn is bound to its scheduled configured session; " - "select another actor with schedule_subagent instead.", + "This configured session is bound to its scheduled actor, fresh start and " + "retry alike; select another actor with schedule_subagent instead.", selected_subagent_id=expected_id, requested_subagent_id=requested_id, host_fallback=False, @@ -863,11 +924,12 @@ def delegate_start_entry(ctx: Any, prompt: str, _resolved_binding: Any = None, * _blocked("configured_actor_resource_mismatch") return _fail( "delegate_start", "configured_actor_resource_mismatch", - "An actor-first configured session may start only its assigned route, " - "not a skill-payload resource.", + "A configured session starts only its assigned route, fresh start and " + "retry alike, never a skill-payload resource.", selected_subagent_id=expected_id, host_fallback=False, ) + if isinstance(bootstrap, dict) and not retry_of: canonical_work_order = str(bootstrap.get("canonical_work_order") or "") if not canonical_work_order: _blocked("configured_work_order_unavailable") @@ -893,34 +955,7 @@ def delegate_start_entry(ctx: Any, prompt: str, _resolved_binding: Any = None, * bound["_resolved_binding"] = _resolved_binding return exact_start(ctx, canonical_work_order, bound) if retry_of and isinstance(bootstrap, dict): - # Retry replays the stored canonical request byte-for-byte - so an - # argument the replay cannot honor is refused typed, never silently - # discarded (the fresh-start refusals, mirrored). - expected_id = str(bootstrap.get("selected_subagent_id") or "") - if params.get("access") is not None: - _blocked("retry_selector_conflict") - return _fail("delegate_start", "retry_selector_conflict", - "A retry replays its recorded access; omit access.") - requested_id = str(params.get("subagent_id") or "").strip() - if requested_id and requested_id != expected_id: - _blocked("configured_actor_route_mismatch") - return _fail( - "delegate_start", "configured_actor_route_mismatch", - "A configured retry replays its own scheduled session; it cannot " - "be redirected to another actor.", - selected_subagent_id=expected_id, - requested_subagent_id=requested_id, - host_fallback=False, - ) - if any(str(params.get(key) or "").strip() for key in ("root", "bucket", "skill_name")): - _blocked("configured_actor_resource_mismatch") - return _fail( - "delegate_start", "configured_actor_resource_mismatch", - "A configured retry replays its assigned route, not a " - "skill-payload resource.", - selected_subagent_id=expected_id, - host_fallback=False, - ) + # Retry replays the stored canonical request byte-for-byte. from ouroboros import delegate_custody as custody invocation = custody.invocation_record(custody.custody_root(ctx), retry_of) or {} diff --git a/prompts/SYSTEM.md b/prompts/SYSTEM.md index 93dcdbb0b..f78623b57 100644 --- a/prompts/SYSTEM.md +++ b/prompts/SYSTEM.md @@ -81,12 +81,14 @@ silently with my own work. set, as facts: the host neither ranks rows nor substitutes actors. I choose by my human's words in `recommended_use` plus the route facts. Agent-session rows ride my human's subscriptions — no incremental API dollars, but shared quota — -while API rows bill per token; weighing that is mine. An unavailable row -returns a typed refusal and I choose the next action; if the block is absent, -no configured actor is available and I invent no id. When I edit the roster in -settings, I rewrite that row's `recommended_use` in the same change. -`write_surface` says what a child may DO; the row says WHO runs — its route -facts, not its description, are its identity. +while API rows bill per token; weighing that is mine. A row's `subagent_id` +there is its handle: its route plus the facets explicitly set on it. An +unavailable row returns a typed refusal and I choose the next action; if the +block is absent, no configured actor is available and I invent no id. In saved +settings `subagent_id` is a hidden stored key instead: editing the roster, I +match rows by route, keep their keys, and rewrite the row's `recommended_use` +in the same change. `write_surface` says what a child may DO; the row says WHO +runs. An API model row is an ordinary recursive Ouroboros child. An Agent session row makes me a nanny: the host starts the exact snapshotted leaf BEFORE my first diff --git a/tests/test_available_subagents_catalog.py b/tests/test_available_subagents_catalog.py index 309614cf9..f7062d075 100644 --- a/tests/test_available_subagents_catalog.py +++ b/tests/test_available_subagents_catalog.py @@ -67,8 +67,10 @@ def test_catalog_projects_every_saved_row_in_owner_order_verbatim(): # Facts only: selection prose lives in prompts/SYSTEM.md, and the list # fingerprint and provenance stay host-side (snapshots carry them). assert set(catalog) == {"rows"} + # Rows are named by their handle - the route plus the row's own set facets. assert [row["subagent_id"] for row in catalog["rows"]] == [ - "api-scout", "auto-session", "pinned-session", + "google/gemini-3.7-flash/low", "claude=claude-fable-5/high", + "cursor=cursor-grok-4.6-high/@cursor-owner", ] assert catalog["rows"][0]["recommended_use"] == verbatim assert list(catalog["rows"][0])[-1] == "recommended_use" @@ -81,7 +83,6 @@ def test_catalog_projects_every_saved_row_in_owner_order_verbatim(): assert "credential_profile_id" not in catalog["rows"][1] assert catalog["rows"][2]["requested_effort"] == "(not explicitly set)" assert catalog["rows"][2]["credential_profile_id"] == "cursor-owner" - # An explicit pin is the only account fact; the policy boilerplate is gone. assert not any("account_policy" in row for row in catalog["rows"]) @@ -196,14 +197,16 @@ def test_catalog_is_semi_stable_while_dated_history_stays_dynamic(tmp_path, monk assert blocks[2]["text"] == core.dynamic_text assert "cache_control" not in blocks[2] assert catalog["rows"][0]["recommended_use"] == owner_text - assert '"subagent_id": "builder"' in core.semi_stable_text + assert '"subagent_id": "codex=gpt-5.6-sol/high"' in core.semi_stable_text + assert "builder" not in core.semi_stable_text, "the stored key is not model-facing" assert "2026-08-18T01:02:03+00:00" not in core.semi_stable_text assert "reviewer_slots_last" not in core.semi_stable_text assert "subagent_last_delegation" not in core.semi_stable_text assert "2026-08-18T01:02:03+00:00" in core.dynamic_text assert "reviewer_slots_last" in core.dynamic_text assert "subagent_last_delegation" in core.dynamic_text - assert '"selected_subagent_id": "builder"' in core.dynamic_text + # An old receipt without a typed identity shows its recorded route target. + assert '"selected_subagent_id": "codex=gpt-5.6-sol"' in core.dynamic_text for profile in ( "review-requested", "review-applied", "delegate-requested", "delegate-applied", ): diff --git a/tests/test_builtin_refusal_results.py b/tests/test_builtin_refusal_results.py index cabc7abc5..f27674af5 100644 --- a/tests/test_builtin_refusal_results.py +++ b/tests/test_builtin_refusal_results.py @@ -744,7 +744,9 @@ def test_exact_start_decorates_the_native_result_without_losing_its_class( assert result.code == code payload = json.loads(result.text) - assert payload["selected_subagent_id"] == "session-builder" + # The actor is named by the snapshot's own handle (an older snapshot without + # a captured access keeps workspace_write); custody keeps the stored key. + assert payload["selected_subagent_id"] == "some-route=weak/low/workspace_write" assert payload["config_fingerprint"] == "cfg-v1" assert payload["work_order_source_request"] == {"schema": 1, "kind": "source_request"} assert payload["status"] == ("started" if produced == "started" else "refused") diff --git a/tests/test_context_runtime_section.py b/tests/test_context_runtime_section.py index 6d5131ca2..843c4e4e4 100644 --- a/tests/test_context_runtime_section.py +++ b/tests/test_context_runtime_section.py @@ -452,7 +452,8 @@ def test_delegation_fact_carries_historical_rows_and_profile_evidence(tmp_path, assert last["applied_model"] == "claude-opus-5" assert last["requested_profile"] == "requested-delegate-profile" assert last["applied_profile"] == "applied-delegate-profile" - assert last["selected_subagent_id"] == "builder" + # Named from the record's own facts (no typed identity here: its route target). + assert last["selected_subagent_id"] == "claudexor=opus-5" assert last["observed"] == "last observed at 2026-08-18T02:00:00+00:00" assert "historical" not in rows["triad_1"]["observed"] # The prompt-visible note teaches the semantics ONCE: rows are history, live diff --git a/tests/test_docs_sync.py b/tests/test_docs_sync.py index 519266cd8..2343590e1 100644 --- a/tests/test_docs_sync.py +++ b/tests/test_docs_sync.py @@ -496,7 +496,7 @@ PROMPT_NON_TOOL_IDENTIFIERS = frozenset({ "skill_payload", "subagent_projects", "system_repo", "task_drive", "user_files", "write_root", "write_surface", # tool parameters named as cross-tool policy - "project_id", "project_name", "recommended_use", "review_rebuttal", + "project_id", "project_name", "recommended_use", "review_rebuttal", "subagent_id", # typed outcomes / statuses / runtime-context keys "needs_manual_target", "started_uncustodied", "owner_client", # safety policy class names (ouroboros/safety.py TOOL_POLICY values) and diff --git a/tests/test_roster_handles_browser.py b/tests/test_roster_handles_browser.py new file mode 100644 index 000000000..fd74a9e1f --- /dev/null +++ b/tests/test_roster_handles_browser.py @@ -0,0 +1,108 @@ +"""Roster rows are named by their handle in a REAL render: the Review-lanes picker +at 1 row and at 10 rows, desktop and phone width, and the Duplicate draft in the +Available-subagents editor. Controlled API responses; no runtime, no model calls.""" +from __future__ import annotations + +import json + +import pytest +from tests import test_subscription_role_routes_browser as roles + +pytestmark = [pytest.mark.ui_browser, pytest.mark.serial] +subscription_ui = roles.subscription_ui +role_ui = roles.role_ui + +EFFORTS = ("", "low", "medium", "high", "xhigh") + + +def _roster(count): + """Stored ids deliberately carry the rotten role labels the owner must never see.""" + rows = [] + for index in range(count): + effort = EFFORTS[index % len(EFFORTS)] + row = { + "subagent_id": "fast-scout" if not index else f"fast-scout_copy_{index}", + "recommended_use": f"Notes {index}", + "route": ({"kind": "agent_session", "target_id": f"codex=gpt-test-{index}"} if index % 2 == 0 + else {"kind": "api_model", "target_id": f"openai::gpt-api-{index}"}), + } + if effort: + row["effort"] = effort + rows.append(row) + return rows + + +def _handle(row): + return row["route"]["target_id"] + (f"/{row['effort']}" if row.get("effort") else "") + + +def _configure(ui, count): + roles.configure_mixed(ui) + rows = _roster(count) + slots = { + "triad": [{"slot_id": "triad_1", "route": {"kind": "api_chat", "target_id": "openai::gpt-api"}}], + "scope": [{"slot_id": "scope_1", "subagent_id": "fast-scout"}], + "advisory": {"enabled": False}, + } + ui["settings"].update(OUROBOROS_SUBAGENTS={"enabled": True, "items": rows}, + OUROBOROS_REVIEWER_SLOTS=json.dumps(slots)) + ui["fixture"]["preview"]["reviewer_slots"] = slots + return rows + + +@pytest.mark.parametrize("width", [1360, 390]) +@pytest.mark.parametrize("count", [1, 10]) +def test_the_review_lanes_picker_names_rows_by_handle_at_any_roster_size(role_ui, count, width): + ui = role_ui + rows = _configure(ui, count) + ui["page"].set_viewport_size({"width": width, "height": 900}) + page = roles.open_agents(ui) + picker = page.locator('[data-slot-id="scope_1"] [data-slot-route]') + assert picker.input_value() == "subagent:fast-scout", "the stored id stays the option VALUE" + labels = picker.locator('optgroup[label="Available subagents"] option').all_text_contents() + + assert labels == [f"{_handle(row)} — {row['recommended_use']}" for row in rows] + # Scale invariant: the first row reads the same with one row and with ten. + assert labels[0] == "codex=gpt-test-0 — Notes 0" + for label in labels: + assert "#" not in label and "fast-scout" not in label and "_copy_" not in label + picker.scroll_into_view_if_needed() + assert page.evaluate("document.documentElement.scrollWidth <= window.innerWidth") + roles.capture(page, f"roster-handles-picker-{count}-rows-{width}") + + +def test_duplicate_is_a_draft_whose_card_names_its_twin_until_the_engine_changes(role_ui): + ui = role_ui + _configure(ui, 1) + page = roles.open_agents(ui) + page.locator("[data-subagent-duplicate]").first.click() + cards = page.locator("[data-subagent-row]") + assert cards.count() == 2 + copy = cards.nth(1) + # Named on the card BEFORE any Save click, tinted as an error; the source stays clean. + meta = copy.locator("[data-subagent-meta]") + assert "Subagent 2 runs the same engine as Subagent 1" in meta.text_content() + assert meta.get_attribute("data-tone") == "error" + assert copy.get_attribute("data-invalid") is not None + assert cards.nth(0).get_attribute("data-invalid") is None + roles.capture(page, "roster-handles-duplicate-draft") + + posts_before = len([1 for path, _ in ui["posts"] if path == "/api/settings"]) + page.locator("#btn-save-settings").click() + assert "runs the same engine as Subagent 1" in page.locator("#settings-status").text_content() + assert len([1 for path, _ in ui["posts"] if path == "/api/settings"]) == posts_before, "a twin is never sent" + + copy.locator('[data-subagent-field="effort"]').select_option("low") + assert "same engine" not in (copy.locator("[data-subagent-meta]").text_content() or "") + assert copy.get_attribute("data-invalid") is None + with page.expect_response("**/api/settings"): + page.locator("#btn-save-settings").click() + saved = [body for path, body in ui["posts"] if path == "/api/settings"][-1]["OUROBOROS_SUBAGENTS"]["items"] + assert [row["route"]["target_id"] for row in saved] == ["codex=gpt-test-0"] * 2 + assert saved[1]["effort"] == "low" + assert saved[1]["subagent_id"].startswith("subagent_") and "fast-scout" not in saved[1]["subagent_id"] + # The reviewer picker now offers both engines under their own names. + labels = page.locator( + '[data-slot-id="scope_1"] [data-slot-route] optgroup[label="Available subagents"] option' + ).all_text_contents() + assert labels == ["codex=gpt-test-0 — Notes 0", "codex=gpt-test-0/low — Notes 0"] diff --git a/tests/test_subagent_handles.py b/tests/test_subagent_handles.py new file mode 100644 index 000000000..04ad845f6 --- /dev/null +++ b/tests/test_subagent_handles.py @@ -0,0 +1,328 @@ +"""Roster handles: a row is NAMED by a projection of its route, never by a stored label. + +The stored ``subagent_id`` stays the hidden join key (reviewer references, +snapshots, custody, history). These tests pin the projection, the one argument +resolver, the save-time engine uniqueness and the facts-only model catalog — +each guard in both directions. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from ouroboros.configured_subagents import ( + engine_identity, + parse_configured_subagents, + roster_handles, + subagent_handle, + validate_unique_engines, +) + +PARITY = json.loads( + (Path(__file__).resolve().parents[1] / "web" / "tests" / "fixtures" + / "subagent_handle_parity.json").read_text(encoding="utf-8") +)["rosters"] + + +def _config(*rows): + return parse_configured_subagents({"enabled": True, "items": list(rows)}) + + +def _settings(*rows): + return {"OUROBOROS_SUBAGENTS": json.dumps({"enabled": True, "items": list(rows)})} + + +def _api(row_id, target="x-ai/grok-4.6", **extra): + return {"subagent_id": row_id, "recommended_use": f"use {row_id}", + "route": {"kind": "api_model", "target_id": target}, **extra} + + +def _session(row_id, target="codex=gpt-6-astra", pin="", **extra): + route = {"kind": "agent_session", "target_id": target} + if pin: + route["credential_profile_id"] = pin + return {"subagent_id": row_id, "recommended_use": f"use {row_id}", "route": route, **extra} + + +def _duplicate_of(config): + """Index pairs a save refuses, read off the validator's own message.""" + try: + validate_unique_engines(config) + except ValueError as exc: + text = str(exc) + later, earlier = (int(part.split("]")[0]) for part in text.split("items[")[1:3]) + return later, earlier + return None + + +@pytest.mark.parametrize("roster", PARITY, ids=[item["case"] for item in PARITY]) +def test_the_shared_table_pins_handles_roster_labels_and_refused_twins(roster): + config = _config(*roster["items"]) + labels = roster_handles(config) + refused = _duplicate_of(config) + first_twin = next( + ((index, want["same_engine_as"]) for index, want in enumerate(roster["expected"]) + if want["same_engine_as"] is not None), None) + assert refused == first_twin + for row, want in zip(config.items, roster["expected"]): + assert subagent_handle(row) == want["handle"] + assert labels[row.subagent_id] == want["roster"] + + +def test_a_handle_is_a_function_of_one_row_so_a_new_sibling_never_renames_it(): + alone = _config(_api("one")) + crowded = _config(_api("one"), _api("two", effort="low"), _session("three")) + assert subagent_handle(alone.items[0]) == subagent_handle(crowded.items[0]) == "x-ai/grok-4.6" + assert roster_handles(crowded)["one"] == "x-ai/grok-4.6" + + +def test_row_identity_and_snapshot_identity_are_one_shape(): + """The save-time identity and the frozen-snapshot identity must not drift: + the same row compared through either reader yields the same facts.""" + from ouroboros.subagent_history import execution_identity, snapshot_handle + from ouroboros.subagent_runtime import select_subagent_snapshot + + rows = (_session("s", pin="koshak", effort="xhigh", access="workspace_write"), + _api("a", effort="low", processing_preference="fast")) + config = _config(*rows) + for row in config.items: + snapshot, _legacy = select_subagent_snapshot(_settings(*rows), subagent_id=row.subagent_id) + assert execution_identity(snapshot) == engine_identity(row) + assert snapshot_handle(snapshot) == subagent_handle(row) + + +@pytest.mark.parametrize("facet", [ + {"effort": "low"}, {"access": "workspace_write"}, {"processing_preference": "economy"}, +]) +def test_identical_engines_are_refused_and_one_differing_facet_is_accepted(facet): + base = _session("first", effort="high") + twin = _session("second", effort="high", recommended_use="different words, same engine") + with pytest.raises(ValueError, match=r"items\[1\] runs the same engine as items\[0\]"): + validate_unique_engines(_config(base, twin)) + validate_unique_engines(_config(base, {**twin, **facet})) + validate_unique_engines(_config(base, _session("second", pin="other-account", effort="high"))) + validate_unique_engines(_config(base, _session("second", target="codex=gpt-5.6-sol", effort="high"))) + + +def test_uniqueness_uses_effective_defaults_and_reads_stay_tolerant(): + """A session row without ``access`` IS a ``full`` row; and a roster saved + before the rule still parses, resolves and projects — only a SAVE is refused.""" + from ouroboros.configured_subagents import resolve_configured_subagents + from ouroboros.subagent_runtime import model_visible_subagent_catalog, select_subagent_snapshot + + rows = (_session("implicit"), _session("explicit", access="full")) + with pytest.raises(ValueError, match="same engine"): + validate_unique_engines(_config(*rows)) + settings = _settings(*rows) + assert resolve_configured_subagents(settings).config is not None + labels = [row["subagent_id"] for row in model_visible_subagent_catalog(settings)["rows"]] + assert labels == ["codex=gpt-6-astra~implicit", "codex=gpt-6-astra~explicit"] + for label, stored in zip(labels, ("implicit", "explicit")): + assert select_subagent_snapshot(settings, subagent_id=label)[0]["selected_subagent_id"] == stored + + +def test_the_argument_resolves_by_handle_then_by_stored_id_and_refuses_a_cross_row_collision(): + from ouroboros.subagent_runtime import SubagentSelectionError, select_subagent_snapshot + + settings = _settings(_session("primary-builder", effort="xhigh"), _api("fast-scout")) + by_handle, _ = select_subagent_snapshot(settings, subagent_id="codex=gpt-6-astra/xhigh") + by_stored, _ = select_subagent_snapshot(settings, subagent_id="primary-builder") + assert by_handle["selected_subagent_id"] == by_stored["selected_subagent_id"] == "primary-builder" + assert by_handle["route"] == by_stored["route"] + + with pytest.raises(SubagentSelectionError) as unknown: + select_subagent_snapshot(settings, subagent_id="codex=gpt-6-astra") # facets are part of the name + assert unknown.value.code == "unknown_subagent_id" + assert "'codex=gpt-6-astra/xhigh'" in unknown.value.detail and "'x-ai/grok-4.6'" in unknown.value.detail + assert "primary-builder" not in unknown.value.detail, "the refusal lists handles, not stored keys" + + # A bare session target is a legal stored id too: one string, two rows. + collision = _settings(_session("first", target="codex"), _api("codex", target="openai/gpt-5.6-sol")) + with pytest.raises(SubagentSelectionError) as conflict: + select_subagent_snapshot(collision, subagent_id="codex") + assert conflict.value.code == "subagent_selector_conflict" + assert "'first'" in conflict.value.detail and "'openai/gpt-5.6-sol'" in conflict.value.detail + # Both named values still reach their rows; the same string on ONE row is no conflict. + assert select_subagent_snapshot(collision, subagent_id="first")[0]["route"]["target_id"] == "codex" + assert select_subagent_snapshot( + collision, subagent_id="openai/gpt-5.6-sol")[0]["selected_subagent_id"] == "codex" + same_row = _settings(_session("codex", target="codex")) + assert select_subagent_snapshot(same_row, subagent_id="codex")[0]["selected_subagent_id"] == "codex" + + +def test_the_model_catalog_is_facts_only_and_keyed_by_handle(): + from ouroboros.subagent_runtime import model_visible_subagent_catalog + + verbatim = "Любой язык.\nKeep punctuation: a/b, quotes, and cost $0." + catalog = model_visible_subagent_catalog(_settings( + {**_api("fast-scout", target="google/gemini-3.8-flash"), "recommended_use": verbatim}, + {**_session("primary-builder", pin="koshak", effort="xhigh"), "recommended_use": "Builds."}, + )) + assert set(catalog) == {"rows"}, "no host-authored guidance, source or fingerprint reaches the model" + api, session = catalog["rows"] + assert api == { + "subagent_id": "google/gemini-3.8-flash", "route_class": "API model", + "requested_effort": "(not explicitly set)", "recommended_use": verbatim, + } + assert list(session) == [ + "subagent_id", "route_class", "requested_effort", "requested_target", + "mutating_access", "credential_profile_id", "recommended_use", + ], "facts lead, the owner's words ride last" + assert session["subagent_id"] == "codex=gpt-6-astra/xhigh/@koshak" + assert session["requested_target"] == "codex=gpt-6-astra" + text = json.dumps(catalog) + for stored_or_dropped in ("fast-scout", "primary-builder", "account_policy", "config_fingerprint"): + assert stored_or_dropped not in text + + +def _post_settings(monkeypatch, body): + import asyncio + + from starlette.requests import Request + + import ouroboros.gateway.settings as gws + + saved = {} + + def _fake_load(): + from ouroboros.config import SETTINGS_DEFAULTS + return {**SETTINGS_DEFAULTS, **saved} + + def _fake_write(payload, *, allow_elevation=False, allow_context_lowering=False, + authored_keys=(), boundary=None): + saved.clear() + saved.update(payload) + if boundary is not None: + boundary.commit() + return payload + + monkeypatch.setattr(gws, "load_settings", _fake_load) + monkeypatch.setattr(gws, "_owner_write_settings", _fake_write) + monkeypatch.setattr(gws, "_unrecognised_review_models", lambda models: []) + monkeypatch.setattr(gws, "_apply_settings_to_env", lambda *a, **k: None) + + async def _receive(): + return {"type": "http.request", "body": json.dumps(body).encode()} + + request = Request({"type": "http", "method": "POST", "path": "/api/settings", + "headers": [("content-type", "application/json")], + "query_string": b"", "app": None}, receive=_receive) + return asyncio.run(gws.api_settings_post(request)), saved + + +def test_every_save_path_refuses_identical_engines_and_accepts_a_near_duplicate(monkeypatch): + from ouroboros.gateway.onboarding import _configured_owner_draft + + twins = {"enabled": True, "items": [_api("one", effort="low"), _api("two", effort="low")]} + near = {"enabled": True, "items": [_api("one", effort="low"), _api("two", effort="high")]} + + refused, saved = _post_settings(monkeypatch, {"OUROBOROS_SUBAGENTS": twins}) + assert refused.status_code == 400 and b"same engine" in refused.body + assert "OUROBOROS_SUBAGENTS" not in saved + accepted, saved = _post_settings(monkeypatch, {"OUROBOROS_SUBAGENTS": near}) + assert accepted.status_code == 200, accepted.body[:300] + assert json.loads(saved["OUROBOROS_SUBAGENTS"])["items"][1]["effort"] == "high" + + # Onboarding preview and completion share this one owner-draft seam. + config, error = _configured_owner_draft({"OUROBOROS_SUBAGENTS": twins}) + assert config is None and "same engine" in error + config, error = _configured_owner_draft({"OUROBOROS_SUBAGENTS": near}) + assert error == "" and [row.subagent_id for row in config.items] == ["one", "two"] + + +def test_an_actor_first_start_accepts_its_own_handle_and_stored_id_and_refuses_another_row( + tmp_path, monkeypatch, +): + """The bound start used to compare the raw argument with the stored id, so a + handle accepted by ``schedule_subagent`` was rejected at the physical start.""" + import ouroboros.subagent_runtime as runtime + + rows = (_session("primary-builder", effort="xhigh"), _session("other", target="cursor=kimi-k3-high")) + settings = _settings(*rows) + monkeypatch.setattr("ouroboros.config.runtime_settings", lambda: settings) + monkeypatch.setattr(runtime, "effective_runtime_subagent_settings", dict) + snapshot, _ = runtime.select_subagent_snapshot(settings, subagent_id="primary-builder") + + def _start(selector, *, retry=False): + ctx = SimpleNamespace( + task_id="bound-child", drive_root=tmp_path, budget_drive_root=str(tmp_path), task_metadata={}, + # No canonical work order: a selector that NAMES the bound actor + # passes the binding check and stops at the next typed refusal. + _configured_actor_bootstrap={"snapshot": snapshot, "selected_subagent_id": "primary-builder"}, + ) + extra = {"retry_of": "inv-1"} if retry else {} + return json.loads(runtime.delegate_start_entry(ctx, "", subagent_id=selector, **extra).text)["reason"] + + for retry in (False, True): + for own in ("primary-builder", "codex=gpt-6-astra/xhigh"): + assert _start(own, retry=retry) == "configured_work_order_unavailable", (own, retry) + for foreign in ("other", "cursor=kimi-k3-high", "no-such-row"): + assert _start(foreign, retry=retry) == "configured_actor_route_mismatch", (foreign, retry) + + # The roster moved on; the episode is still bound to its frozen snapshot. + monkeypatch.setattr("ouroboros.config.runtime_settings", lambda: _settings(rows[1])) + assert _start("codex=gpt-6-astra/xhigh") == "configured_work_order_unavailable" + assert _start("cursor=kimi-k3-high") == "configured_actor_route_mismatch" + + +def test_history_is_named_from_each_records_own_facts_never_from_the_live_roster(tmp_path, monkeypatch): + """The live row stored as ``fast-scout`` runs another engine today; the dated + fact keeps the engine it ran, and an old record without a typed identity + shows its recorded route target.""" + from ouroboros.context_runtime_facts import _delegation_capability_fact + from ouroboros.subagent_history import recorded_handle + + monkeypatch.setattr("ouroboros.config.DATA_DIR", tmp_path) + monkeypatch.setattr("ouroboros.config.load_settings", + lambda: _settings(_api("fast-scout", target="moonshotai/kimi-k3"))) + typed = { + "ts": "2026-09-20T10:00:00+00:00", "route": "cursor", "requested_model": "grok-4.6", + "applied_model": "grok-4.6", "selected_subagent_id": "fast-scout", "run_id": "run-2", + "identity": {"kind": "agent_session", "target_id": "cursor=grok-4.6", "effort": "high", + "credential_profile_id": "", "processing_preference": "", "access": "full"}, + } + untyped = {"ts": "2026-08-18T02:00:00+00:00", "route": "api_model", + "requested_model": "google/gemini-3.7-flash", "applied_model": "", + "selected_subagent_id": "fast-scout-2", "run_id": "run-1"} + (tmp_path / "state").mkdir() + (tmp_path / "state" / "subagent_last_delegation.json").write_text(json.dumps({ + **typed, "latest_by_subagent": {"fast-scout": typed, "fast-scout-2": untyped}, + }), encoding="utf-8") + + delegation = _delegation_capability_fact() + assert delegation["subagent_last_delegation"]["selected_subagent_id"] == "cursor=grok-4.6/high" + assert [row["selected_subagent_id"] for row in delegation["subagents_last_executions"]] == [ + "cursor=grok-4.6/high", "google/gemini-3.7-flash", + ] + assert "kimi" not in json.dumps(delegation), "the past is never relabelled from the live roster" + assert recorded_handle({"route": "codex", "requested_model": ""}) == "codex" + assert recorded_handle({}) == "" + + +def test_the_startup_receipt_and_the_start_result_name_the_snapshots_own_handle(tmp_path, monkeypatch): + import ouroboros.subagent_bootstrap as bootstrap + import ouroboros.subagent_runtime as runtime + import ouroboros.tools.delegate as delegate + from ouroboros.delegate_shared import delegate_result + + snapshot, _ = runtime.select_subagent_snapshot( + _settings(_session("primary-builder", pin="koshak", effort="xhigh")), subagent_id="primary-builder") + monkeypatch.setattr(bootstrap, "_durable_zero_run_receipt", lambda *_a, **_k: {}) + ctx = SimpleNamespace(task_id="child", drive_root=tmp_path, budget_drive_root=str(tmp_path)) + receipt = json.loads(bootstrap._prepare_actor_first_bootstrap( + ctx, {"id": "child", "objective": "Build", "configured_subagent": snapshot}, + SimpleNamespace(blocked=False), + )) + assert receipt["startup"]["selected_subagent_id"] == "codex=gpt-6-astra/xhigh/@koshak" + assert ctx._configured_actor_bootstrap["selected_subagent_id"] == "primary-builder", "custody keeps the stored key" + + monkeypatch.setattr(delegate, "_delegate_start", + lambda *_a, **_k: delegate_result({"status": "started", "run_id": "run-1"})) + started = json.loads(runtime.exact_start( + SimpleNamespace(task_id="child", drive_root=tmp_path, budget_drive_root=str(tmp_path), task_metadata={}), + "brief", {"snapshot": snapshot}).text) + assert started["selected_subagent_id"] == "codex=gpt-6-astra/xhigh/@koshak" diff --git a/tests/test_subagent_history_attempt_facts.py b/tests/test_subagent_history_attempt_facts.py index bf13191ae..5623559d7 100644 --- a/tests/test_subagent_history_attempt_facts.py +++ b/tests/test_subagent_history_attempt_facts.py @@ -58,7 +58,11 @@ def test_actual_auto_and_pinned_attempt_reach_result_and_context(tmp_path, monke assert row["applied_model"] == "observed-model" assert row["observed_route"]["model"] == "observed-model" assert row["attempt_id"] == usage["llm_call_refs"][0]["llm_call_id"] - assert _delegation_capability_fact()["subagents_last_executions"][0] == row["latest_by_subagent"]["worker"] + # The context projection is the stored row with ONE change: the actor is named + # by the handle of the row's own recorded identity, never by the stored key. + handle = f"{TARGET}/high" + (f"/@{pin}" if pin else "") + "/standard" + assert _delegation_capability_fact()["subagents_last_executions"][0] == { + **row["latest_by_subagent"]["worker"], "selected_subagent_id": handle} @pytest.mark.parametrize("provider", ["claudexor", "openai"]) diff --git a/web/modules/reviewer_slots.js b/web/modules/reviewer_slots.js index 425a4e393..adcc89c55 100644 --- a/web/modules/reviewer_slots.js +++ b/web/modules/reviewer_slots.js @@ -280,19 +280,7 @@ export function reviewerChoiceGroups({ return groups; } -export function subagentOptionLabel(row) { - const route = row?.route || {}; - const parts = [`#${String(row?.subagent_id || '')}`]; - if (route.kind === ROUTE_KIND_SESSION) { - const split = splitSessionTarget(route.target_id); - parts.push(split.harness || 'agent session'); - if (split.model) parts.push(split.model); - } else { - const fields = routeEditor.routeModelFields(route); - parts.push(fields.subscription ? `${fields.source} model` : 'API'); - if (fields.model) parts.push(fields.model); - } - if (row?.effort) parts.push(row.effort); +export function subagentOptionLabel(row, handle = routeEditor.subagentHandle(row)) { // Free text is a caption, never identity: one line, bounded, with the // characters that could visually reorder or break the facts stripped // (bidi controls, newlines). @@ -303,7 +291,7 @@ export function subagentOptionLabel(row) { // Code POINTS, not UTF-16 units: a slice must never split a surrogate pair. const points = Array.from(use); const hint = points.length > 48 ? `${points.slice(0, 45).join('')}…` : use; - return parts.join(' · ') + (hint ? ` — ${hint}` : ''); + return handle + (hint ? ` — ${hint}` : ''); } export function subagentOptionsFor(roster, savedId, { rosterKnown = true } = {}) { @@ -312,13 +300,15 @@ export function subagentOptionsFor(roster, savedId, { rosterKnown = true } = {}) // roster entry and the next Save really rewires the reviewer. And the // absence claim follows provenance: only a roster that was actually READ // may say a saved reference is not in it. - // Label contract (owner decision 2=A): DERIVED FACTS lead — channel first, - // then target and effort — so the delivery is visible BEFORE selection and - // free-text intent can never disguise it; the description is a trimmed, - // sanitized single-line caption after the facts. + // Label contract: the row's HANDLE leads — its route target plus its own + // set facets, the same name Ouroboros sees — so the delivery is visible + // BEFORE selection and no stored label can disguise it; the description is + // a trimmed, sanitized single-line caption after it. The stored id is the + // option VALUE only (the reviewer reference follows the row through it). + const handles = routeEditor.rosterHandles(roster); const options = (roster || []).map((row) => ({ value: String(row.subagent_id || ''), - label: subagentOptionLabel(row), + label: subagentOptionLabel(row, handles.get(String(row.subagent_id || ''))), })); const saved = String(savedId || ''); if (saved && !options.some((option) => option.value === saved)) { @@ -418,7 +408,19 @@ export function lastRunRouteChanged(entry, row) { /** The earlier route, named the way the owner picked it. */ function lastRunRanAs(entry, { harnesses = {}, modelSources = [], providerProfiles = {} } = {}) { const requested = entry?.requested || {}; - if (requested.subagent_id) return `the configured subagent #${requested.subagent_id}`; + if (requested.subagent_id) { + // Named from the receipt's OWN recorded route, never from today's roster. + const session = String(requested.route_kind || '') === ROUTE_KIND_SESSION; + const handle = routeEditor.subagentHandle({ + route: { + kind: requested.route_kind, + target_id: session ? requested.session_target : requested.model, + credential_profile_id: requested.profile_id, + }, + effort: requested.effort, processing_preference: requested.processing_preference, + }); + return handle ? `the configured subagent ${handle}` : 'a configured subagent'; + } if (String(requested.route_kind || '') === ROUTE_KIND_SESSION) { const { harness } = splitSessionTarget(requested.session_target); const label = harnessPresentation(harness, { diff --git a/web/modules/route_editor_primitives.js b/web/modules/route_editor_primitives.js index 0005ba6d3..75919f78b 100644 --- a/web/modules/route_editor_primitives.js +++ b/web/modules/route_editor_primitives.js @@ -255,6 +255,45 @@ export function mintStableId(prefix, takenIds) { return `${prefix}_${Date.now().toString(36)}`; } +// A roster row is NAMED by a projection of its route, never by a stored label +// (which rots once the owner re-points the row). JS twin of +// ouroboros/configured_subagents.py — engine_identity / subagent_handle / +// roster_handles / validate_unique_engines — held together by one parity table, +// web/tests/fixtures/subagent_handle_parity.json. +function engineIdentity(row) { + const route = row?.route || {}; + return [ + String(route.kind || ''), String(route.target_id || '').trim(), + String(route.credential_profile_id || '').trim(), String(row?.effort || ''), + String(row?.processing_preference || ''), + route.kind === ROUTE_KIND_AGENT_SESSION ? String(row?.access || 'full') : '', + ]; +} + +/** Route target plus the facets set on THIS row; a neighbour never renames it. */ +export function subagentHandle(row) { + const [, target, pin, effort, processing, access] = engineIdentity(row); + return [target, effort, access === 'full' ? '' : access, pin ? `@${pin}` : '', processing] + .filter(Boolean).join('/'); +} + +/** Stored id -> the label the live roster shows; twins are told apart by their stored key. */ +export function rosterHandles(rows) { + const base = (rows || []).map(subagentHandle); + return new Map((rows || []).map((row, index) => [ + String(row?.subagent_id || ''), + base.indexOf(base[index]) === base.lastIndexOf(base[index]) + ? base[index] : `${base[index]}~${String(row?.subagent_id || '')}`, + ])); +} + +/** Index of the EARLIER row running an identical engine, else -1 — a save-time rule; reads stay tolerant. */ +export function sameEngineAs(rows, index) { + const key = JSON.stringify(engineIdentity(rows?.[index])); + return (rows || []).findIndex( + (row, other) => other < index && JSON.stringify(engineIdentity(row)) === key); +} + export function composeSessionTarget(harness, model) { const h = String(harness || '').trim(); const m = String(model || '').trim(); diff --git a/web/modules/subagents_settings.js b/web/modules/subagents_settings.js index 7e603c4f9..a9ee46ada 100644 --- a/web/modules/subagents_settings.js +++ b/web/modules/subagents_settings.js @@ -11,7 +11,7 @@ import { compoundSessionEffortConflict, configuredApiProviders, changeRouteChoice, routeModelFields, routeModelInputHtml, routeTargetFromModel, routeSupportsAccount, effortSelectHtml, encodeRouteChoice, indexProfilesByHarness, mintStableId, profileOptionsFor, - routeChoiceGroups, selectHtml, serializeRouteSpec, sessionModelOptions, updateRouteControlOptions, + routeChoiceGroups, sameEngineAs, selectHtml, serializeRouteSpec, sessionModelOptions, updateRouteControlOptions, PROCESSING_CHOICES, PROCESSING_PREFERENCE_KEY, processingDetailsHtml, processingIntentLabel, accountScopedModelCatalog, } from './route_editor_primitives.js'; import { modelChooserHtml, bindModelChoosers } from './model_chooser.js'; @@ -159,10 +159,11 @@ export function parseAvailableSubagentsSetting(value) { return { setting, error: '' }; } -// One row's owner-facing errors, named the way the card is ("Subagent N"). -// `ids` accumulates in list order so a repeated stable ID blames the later row; -// the list validator and the per-row display read this one source. -function rowErrors(row, index, ids) { +// One row's owner-facing errors, named the way the card is ("Subagent N"); the +// list validator and the per-row display read this one source. `ids` accumulates +// in list order so a repeated stable ID blames the later row. `rows` rides on SAVE +// paths only (two rows may not run one engine): a roster saved earlier still loads. +function rowErrors(row, index, ids, rows = null) { const errors = []; const id = String(row?.subagent_id || '').trim(); if (!SUBAGENT_ID_PATTERN.test(id)) { @@ -205,6 +206,8 @@ function rowErrors(row, index, ids) { if (encodedEffort) { errors.push(`effort “${row.effort}” conflicts with compound route effort “${encodedEffort}”.`); } + const twin = rows && String(route.target_id || '').trim() ? sameEngineAs(rows, index) : -1; + if (twin >= 0) errors.push(`runs the same engine as Subagent ${twin + 1} — change its model, effort, access, account or processing, or remove it.`); return errors.map((text) => `Subagent ${index + 1} ${text}`); } @@ -213,14 +216,13 @@ function listLevelErrors(setting) { ? [`Available subagents supports at most ${MAX_AVAILABLE_SUBAGENTS} rows.`] : []; } -export function validateAvailableSubagentsSetting(setting) { +export function validateAvailableSubagentsSetting(setting, { uniqueEngines = false } = {}) { if (!setting || typeof setting.enabled !== 'boolean' || !Array.isArray(setting.items)) { return ['Available subagents configuration is not loaded.']; } - const errors = listLevelErrors(setting); const ids = new Set(); - setting.items.forEach((row, index) => errors.push(...rowErrors(row, index, ids))); - return errors; + const rows = uniqueEngines ? setting.items : null; + return [...listLevelErrors(setting), ...setting.items.flatMap((row, index) => rowErrors(row, index, ids, rows))]; } export function buildAvailableSubagentsSetting(setting) { @@ -462,7 +464,7 @@ export function createAvailableSubagentsEditor({ || 'Available subagents draft is still loading. Retry the preview before finishing.']; } if (state.parseError) return [state.parseError]; - return validateAvailableSubagentsSetting(state.setting); + return validateAvailableSubagentsSetting(state.setting, { uniqueEngines: true }); } // Patch verdicts and inherited intent in place, preserving the caret. @@ -475,7 +477,7 @@ export function createAvailableSubagentsEditor({ : (state.saveAttempted ? listLevelErrors(state.setting) : []); const ids = new Set(); state.setting.items.forEach((row, index) => { - const rowErrs = state.loaded ? rowErrors(row, index, ids) : []; + const rowErrs = state.loaded ? rowErrors(row, index, ids, state.setting.items) : []; const judged = Boolean(row._uiAttempted) && rowErrs.length > 0; if (judged && !structural) shown.push(...rowErrs); const el = container.querySelector(`[data-subagent-row="${row._uiKey || row.subagent_id}"]`); @@ -526,6 +528,12 @@ export function createAvailableSubagentsEditor({ onChange(buildAvailableSubagentsSetting(state.setting)); } + // A new row's hidden keys are neutral: a label copied from its source would rot with the route. + const mintRowKeys = () => ({ + subagent_id: mintStableId('subagent', state.setting.items.map((item) => item.subagent_id)), + _uiKey: mintStableId('actor_row', state.setting.items.map((item) => item._uiKey)), + }); + function bindRows(container) { container.querySelectorAll?.('[data-subagent-row]').forEach((rowElement) => { const row = state.setting.items.find( @@ -570,11 +578,9 @@ export function createAvailableSubagentsEditor({ } rowElement.querySelector('[data-subagent-duplicate]')?.addEventListener('click', () => { if (state.setting.items.length >= MAX_AVAILABLE_SUBAGENTS) return; - const copy = canonicalRow(row); - copy.subagent_id = mintStableId(`${row.subagent_id || 'subagent'}_copy`, - state.setting.items.map((item) => item.subagent_id)); - copy._uiKey = mintStableId('actor_row', - state.setting.items.map((item) => item._uiKey)); + // A copy IS the same engine, so it is born a judged draft: its card + // names the twin until one engine field changes. + const copy = { ...canonicalRow(row), ...mintRowKeys(), _uiAttempted: true }; state.setting.items.splice(state.setting.items.indexOf(row) + 1, 0, copy); markDirty({ structural: true }); paint(); @@ -642,17 +648,11 @@ export function createAvailableSubagentsEditor({ }); container.querySelector('[data-subagent-add]')?.addEventListener('click', () => { if (state.setting.items.length >= MAX_AVAILABLE_SUBAGENTS) return; - const id = mintStableId('subagent', state.setting.items.map((row) => row.subagent_id)); - const uiKey = mintStableId('actor_row', state.setting.items.map((row) => row._uiKey)); - state.setting.items.push({ - subagent_id: id, - recommended_use: '', - route: { kind: ROUTE_KIND_API_MODEL, target_id: '' }, - _uiKey: uiKey, - }); + const row = { recommended_use: '', route: { kind: ROUTE_KIND_API_MODEL, target_id: '' }, ...mintRowKeys() }; + state.setting.items.push(row); markDirty({ structural: true }); paint(); - revealRow(uiKey); + revealRow(row._uiKey); }); bindRows(container); disposeChoosers = bindModelChoosers(container); diff --git a/web/tests/fixtures/subagent_handle_parity.json b/web/tests/fixtures/subagent_handle_parity.json new file mode 100644 index 000000000..745c5dada --- /dev/null +++ b/web/tests/fixtures/subagent_handle_parity.json @@ -0,0 +1,61 @@ +{ + "_comment": "One table, two implementations: ouroboros.configured_subagents (subagent_handle, roster_handles, validate_unique_engines) and web/modules/route_editor_primitives.js (subagentHandle, rosterHandles, duplicateEngineIndexes). 'handle' is a pure function of ONE row; 'roster' is the label the live roster shows (twins get ~); 'same_engine_as' is the index of the earlier row a save refuses this row against (null = accepted).", + "rosters": [ + { + "case": "one bare API row: the handle is the target as stored", + "items": [ + {"subagent_id": "fast-scout", "recommended_use": "", "route": {"kind": "api_model", "target_id": "google/gemini-3.8-flash"}} + ], + "expected": [ + {"handle": "google/gemini-3.8-flash", "roster": "google/gemini-3.8-flash", "same_engine_as": null} + ] + }, + { + "case": "each facet of THIS row, in fixed order: effort, non-default access, pin, processing", + "items": [ + {"subagent_id": "primary-builder", "recommended_use": "", "route": {"kind": "agent_session", "target_id": "codex=gpt-6-astra"}, "effort": "xhigh"}, + {"subagent_id": "a", "recommended_use": "", "route": {"kind": "agent_session", "target_id": "cursor=kimi-k3-high"}, "access": "full"}, + {"subagent_id": "b", "recommended_use": "", "route": {"kind": "agent_session", "target_id": "claude=claude-opus-5", "credential_profile_id": "mironov"}, "effort": "high", "access": "workspace_write", "processing_preference": "fast"}, + {"subagent_id": "c", "recommended_use": "", "route": {"kind": "api_model", "target_id": "x-ai/grok-4.6"}, "effort": "max", "processing_preference": "economy"} + ], + "expected": [ + {"handle": "codex=gpt-6-astra/xhigh", "roster": "codex=gpt-6-astra/xhigh", "same_engine_as": null}, + {"handle": "cursor=kimi-k3-high", "roster": "cursor=kimi-k3-high", "same_engine_as": null}, + {"handle": "claude=claude-opus-5/high/workspace_write/@mironov/fast", "roster": "claude=claude-opus-5/high/workspace_write/@mironov/fast", "same_engine_as": null}, + {"handle": "x-ai/grok-4.6/max/economy", "roster": "x-ai/grok-4.6/max/economy", "same_engine_as": null} + ] + }, + { + "case": "a sibling that differs in ONE facet is accepted and never renames its neighbour", + "items": [ + {"subagent_id": "one", "recommended_use": "", "route": {"kind": "api_model", "target_id": "x-ai/grok-4.6"}}, + {"subagent_id": "two", "recommended_use": "", "route": {"kind": "api_model", "target_id": "x-ai/grok-4.6"}, "effort": "low"}, + {"subagent_id": "three", "recommended_use": "", "route": {"kind": "api_model", "target_id": "x-ai/grok-4.6"}, "processing_preference": "fast"}, + {"subagent_id": "s1", "recommended_use": "", "route": {"kind": "agent_session", "target_id": "codex=gpt-6-astra"}}, + {"subagent_id": "s2", "recommended_use": "", "route": {"kind": "agent_session", "target_id": "codex=gpt-6-astra"}, "access": "workspace_write"}, + {"subagent_id": "s3", "recommended_use": "", "route": {"kind": "agent_session", "target_id": "codex=gpt-6-astra", "credential_profile_id": "koshak"}} + ], + "expected": [ + {"handle": "x-ai/grok-4.6", "roster": "x-ai/grok-4.6", "same_engine_as": null}, + {"handle": "x-ai/grok-4.6/low", "roster": "x-ai/grok-4.6/low", "same_engine_as": null}, + {"handle": "x-ai/grok-4.6/fast", "roster": "x-ai/grok-4.6/fast", "same_engine_as": null}, + {"handle": "codex=gpt-6-astra", "roster": "codex=gpt-6-astra", "same_engine_as": null}, + {"handle": "codex=gpt-6-astra/workspace_write", "roster": "codex=gpt-6-astra/workspace_write", "same_engine_as": null}, + {"handle": "codex=gpt-6-astra/@koshak", "roster": "codex=gpt-6-astra/@koshak", "same_engine_as": null} + ] + }, + { + "case": "identical engines (effective defaults: omitted session access IS full; the description is not part of the engine) are told apart by the stored key and refused on save", + "items": [ + {"subagent_id": "fast-scout", "recommended_use": "first", "route": {"kind": "agent_session", "target_id": "cursor=grok-4.6"}, "effort": "high"}, + {"subagent_id": "fast-scout_copy_pls35j", "recommended_use": "second, other words", "route": {"kind": "agent_session", "target_id": "cursor=grok-4.6"}, "effort": "high", "access": "full"}, + {"subagent_id": "other", "recommended_use": "", "route": {"kind": "api_model", "target_id": "moonshotai/kimi-k3"}} + ], + "expected": [ + {"handle": "cursor=grok-4.6/high", "roster": "cursor=grok-4.6/high~fast-scout", "same_engine_as": null}, + {"handle": "cursor=grok-4.6/high", "roster": "cursor=grok-4.6/high~fast-scout_copy_pls35j", "same_engine_as": 0}, + {"handle": "moonshotai/kimi-k3", "roster": "moonshotai/kimi-k3", "same_engine_as": null} + ] + } + ] +} diff --git a/web/tests/reviewer_slots.test.js b/web/tests/reviewer_slots.test.js index 5288a7b1e..8527a5140 100644 --- a/web/tests/reviewer_slots.test.js +++ b/web/tests/reviewer_slots.test.js @@ -687,9 +687,10 @@ test('the roster select survives a saved reference the roster no longer lists', ]; const listed = subagentOptionsFor(roster, 'deep'); assert.deepEqual(listed.map((o) => o.value), ['deep', 'fast']); - // 2=A label contract: FACTS lead (channel first), description is a caption. - assert.equal(listed[0].label, '#deep · API · openai/gpt-5.6-sol · high — Long reasoning over big diffs'); - assert.equal(listed[1].label, '#fast · cursor · grok-4.6'); + // Label contract: the row's HANDLE leads (route target plus its own set + // facets), the description is a caption; the stored id is the VALUE only. + assert.equal(listed[0].label, 'openai/gpt-5.6-sol/high — Long reasoning over big diffs'); + assert.equal(listed[1].label, 'cursor=grok-4.6'); const missing = subagentOptionsFor(roster, 'gone'); assert.deepEqual(missing.map((o) => o.value), ['deep', 'fast', 'gone']); @@ -713,7 +714,7 @@ test('the one flat reviewer picker leads with roster references, then the inline const refGroups = reviewerChoiceGroups({ roster, row: refRow, harnesses }); assert.equal(refGroups[0].label, 'Available subagents'); assert.deepEqual(refGroups[0].options.map((o) => o.value), [`${SUBAGENT_CHOICE_PREFIX}deep`]); - assert.match(refGroups[0].options[0].label, /^#deep · API/); + assert.match(refGroups[0].options[0].label, /^openai\/gpt-5\.6-sol\/high — /); assert.deepEqual(refGroups.slice(1).map((g) => g.label), ['Subscriptions · models', 'API keys', 'Agents · sessions']); assert.ok(!refGroups.slice(1).flatMap((g) => g.options).some((o) => o.value === 'session:gone')); @@ -771,7 +772,7 @@ test('picker captions strip directional marks and never split a surrogate pair', recommended_use: '\u200Efast\u200F \u061Ccheap\u202Eevil', route: { kind: 'api_model', target_id: 'openai/gpt-5.6-luna' }, }], '')[0].label; - assert.equal(marked, '#row · API · openai/gpt-5.6-luna — fast cheap' + 'evil'); + assert.equal(marked, 'openai/gpt-5.6-luna — fast cheap' + 'evil'); const emoji = '\u{1F9EA}'.repeat(60); // 60 code points, 120 UTF-16 units const long = subagentOptionsFor([{ @@ -1144,9 +1145,19 @@ test('a last-run receipt is read against the route that produced it', () => { lastRunMetaPrefix(receipt({ route_kind: 'api_chat', model: 'claudexor::codex-models=gpt' }), sessionRow, { modelSources: [{ id: 'codex-models', label: 'Codex' }] }), 'Last run, before this row changed (it ran as a model on Codex)'); + // A reference is named from the receipt's OWN recorded route — never by the + // stored id, and never from today's roster (that would relabel the past). + assert.equal( + lastRunMetaPrefix(receipt({ route_kind: 'api_chat', subagent_id: 'deep', model: 'openai/gpt-5.6-sol', + effort: 'high', processing_preference: 'fast' }), apiRow), + 'Last run, before this row changed (it ran as the configured subagent openai/gpt-5.6-sol/high/fast)'); + assert.equal( + lastRunMetaPrefix(receipt({ route_kind: ROUTE_KIND_SESSION, subagent_id: 'deep', model: 'ignored', + session_target: 'codex=gpt-6-astra', profile_id: 'koshak' }), apiRow), + 'Last run, before this row changed (it ran as the configured subagent codex=gpt-6-astra/@koshak)'); assert.equal( lastRunMetaPrefix(receipt({ route_kind: 'api_chat', subagent_id: 'deep' }), apiRow), - 'Last run, before this row changed (it ran as the configured subagent #deep)'); + 'Last run, before this row changed (it ran as a configured subagent)'); }); test('no reviewer control teaches the stored prefix, and the advisory says what it delivers', () => { diff --git a/web/tests/subagent_handles.test.js b/web/tests/subagent_handles.test.js new file mode 100644 index 000000000..8c6fdf087 --- /dev/null +++ b/web/tests/subagent_handles.test.js @@ -0,0 +1,74 @@ +// A roster row is NAMED by a projection of its route. The Python owner is +// ouroboros/configured_subagents.py; this module pins the JS twin against the +// SAME table (tests/test_subagent_handles.py reads it too) and the one place +// the owner meets the name: the Review-lanes reviewer picker. +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; + +import { rosterHandles, sameEngineAs, selectHtml, subagentHandle } from '../modules/route_editor_primitives.js'; +import { + SUBAGENT_CHOICE_PREFIX, encodeReviewerChoice, reviewerChoiceGroups, +} from '../modules/reviewer_slots.js'; + +const PARITY = JSON.parse(readFileSync( + new URL('./fixtures/subagent_handle_parity.json', import.meta.url), 'utf8')).rosters; + +for (const roster of PARITY) { + test(`handle parity with Python: ${roster.case}`, () => { + const labels = rosterHandles(roster.items); + roster.expected.forEach((want, index) => { + const row = roster.items[index]; + assert.equal(subagentHandle(row), want.handle); + assert.equal(labels.get(row.subagent_id), want.roster); + assert.equal(sameEngineAs(roster.items, index), want.same_engine_as ?? -1); + }); + }); +} + +// The exact path reviewerPickerHtml takes: groups -> the shared select markup. +function pickerOptions(roster, row) { + const html = selectHtml('data-slot-route', reviewerChoiceGroups({ roster, row }), encodeReviewerChoice(row)); + const group = html.match(/([\s\S]*?)<\/optgroup>/)?.[1] || ''; + return [...group.matchAll(/