diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f0b268c9..0a36c2b0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ## [0.2.0] - Unreleased +### Added + +- Add request-local `routing.candidate_id` and `exclude_candidate_ids` controls + for trusted virtual-model chat and Responses calls. Pins and exclusions are + strictly validated, never persisted, honored by route, structured, and + streaming paths, and disclosed only as per-response routing evidence; the + public model catalog remains unchanged. + ### Deprecated - Internal callers now use @@ -20,6 +28,51 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ### Fixed +- Auto-mode triage now runs a live decision call even when `zdr_only` is + active and `routing.candidate_id` pins a paid ZDR-eligible candidate. + `_compute_triage_verdict`'s free-only ranking is always empty in that + shape (the pin restricts every candidate list to that one agent, and a + paid agent never satisfies `free_only`), and the empty-pool fallback was + gated behind `not zdr_only` even though the fallback's own per-agent + filter (`_zdr_agent_allowed` plus the active pin/exclusion) already makes + it safe to consult under an active ZDR policy. The gate discarded the one + legitimate evidence source instead of protecting against contacting a + non-ZDR provider, so the request silently took the direct route with zero + triage call and zero routing evidence regardless of task complexity + (Devin Review, PR #983: "ZDR pins skip workflow triage"). +- The published OpenAPI schema for `CandidateRoutingControls.exclude_candidate_ids` + no longer declares `maxItems: 32`. The runtime validator's own repository-authored + 32-ID cardinality cutoff was already removed as unsupported; the schema still + publishing that limit meant generated/OpenAPI clients rejected exclusion lists the + runtime intentionally accepts, making the schema false at source. `uniqueItems`, + lexical ID constraints, and normal authenticated request-size bounds are + unchanged; no replacement cardinality heuristic was introduced. +- The real-time model judge no longer selects a verifier-excluded agent as + the judge when it is the sole candidate. `_ranked_agents` deliberately + still returns role-ineligible members (appended after every eligible one), + so a caller that wants only role-eligible candidates must re-apply + `role not in agent.provider_exclusions` itself — `_plan_generated` and + `_parse_workflow_plan` already do; `_model_judge_verification`'s judge + selection did not. With a single-candidate pool excluded from `verifier` + (e.g. a worker-only pinned agent under PR #983's `orchestrator/free` + provable-route carve-out), that judge selection picked the ineligible + agent anyway, producing an extra, unrequested live call once fast-mlsirm + is actually importable (observed as a duplicate served-candidate call in + hosted CI, which every earlier sandboxed verification round of this PR + could not reproduce locally because the sandbox's blocked fast-mlsirm + archive download always short-circuits the judge to its fail-closed path + first). `_invoke`'s own failover already enforced this exclusion for a + *backup* judge; this closes the same gap for the *primary* selection. +- `route_once` and `stream_route` no longer stamp `served_agent_id` on every + trace row unconditionally. An earlier no-heuristics repair for candidate- + routing evidence made that stamp unconditional to give + `_candidate_routing_evidence` an explicit serving fact even when unchanged, + but this broke the pre-existing contract (regression-guarded by + `test_provider_reliability.py` and `test_tool_execution_fallback.py`) that + the ordinary, no-candidate-policy path never carries failover metadata for + an unchanged serving agent. The stamp is now unconditional only while + request-local candidate-attempt tracking is actually active (i.e. inside a + `candidate_routing_policy` scope); the ordinary path is unaffected. - Workflow workers now preserve the caller message array exactly once, while the added envelope carries only the subtask and Conductor-style prior-step access list instead of duplicating the task or source attachments. @@ -1256,3 +1309,4 @@ This is the current development baseline, not a published release. It provides the OpenAI-compatible gateway, route/conduct orchestration, workflow and access evidence, provider credential boundaries, cost and readiness reporting, and security-focused contract tests. +- Candidate routing controls no longer impose an unsupported 32-ID exclusion cutoff or infer serving identity from output text/trace order; serving identity now requires explicit provenance and otherwise fails closed. diff --git a/README.md b/README.md index 6eedf5755..91e5078f3 100644 --- a/README.md +++ b/README.md @@ -264,6 +264,29 @@ is read from a **KV config store**, never `os.getenv`. (`{"routing": {"latency_tolerant": true}}` on `/v1/chat/completions`) plus KV thresholds. Interactive requests stay on the fast sync path; latency-tolerant or bulk requests are dispatched to a batch backend. +- **Stateless candidate control.** Trusted callers may add + `routing.candidate_id` to pin one private agent ID and + `routing.exclude_candidate_ids` (unique exact IDs) to omit evidence-ineligible + candidates for one virtual-model request. The gateway validates the full set + before any provider call, forces synchronous execution, and returns + requested, excluded, attempted, and served IDs under + `orchestration.routing`. Omitting both keys -- or supplying only an empty + `exclude_candidate_ids` array with no `candidate_id` -- preserves the + existing request and response contract: neither constrains selection, so + there is no routing decision to attest to. Concrete provider model names + cannot be combined with these controls; candidate IDs remain absent from + `/v1/models`. + + ```json + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "Review this change"}], + "routing": { + "candidate_id": "candidate_b", + "exclude_candidate_ids": ["candidate_a"] + } + } + ``` - **Batch routing to pg-llm-batch.** The production batch backend is an injected [`pg-llm-batch`](https://github.com/ContextualWisdomLab/pg-llm-batch) OpenAI-compatible Batch API client (submit JSONL -> poll -> retrieve). A local diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index 03f89ba3a..1fccba44c 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -22,6 +22,79 @@ }, }, "schemas": { + "CandidateRoutingControls": { + "type": "object", + "properties": { + "channel": {"type": "string", "enum": ["sync", "batch"]}, + "latency_tolerant": {"type": "boolean"}, + "priority": { + "type": "string", + "enum": ["interactive", "normal", "bulk"], + }, + "candidate_id": { + "type": "string", + "minLength": 1, + "pattern": r"^\S(?:[^\r\n]*\S)?(?![\s\S])", + "description": "Exact private agent ID to use for this request.", + }, + "exclude_candidate_ids": { + "type": "array", + "uniqueItems": True, + "items": { + "type": "string", + "minLength": 1, + "pattern": r"^\S(?:[^\r\n]*\S)?(?![\s\S])", + }, + }, + "endpoint": { + "type": "string", + "minLength": 1, + "pattern": r"^\S(?:[^\r\n]*\S)?(?![\s\S])", + "description": ( + "Pin the request to one configured endpoint selector. " + "Forces synchronous routing (channel=sync); cannot be " + "combined with channel=batch or latency_tolerant=true." + ), + }, + }, + "additionalProperties": False, + }, + "CandidateRoutingEvidence": { + "type": "object", + "description": ( + "Per-response disclosure of how active candidate controls " + "were applied. Present only when the request supplied " + "routing.candidate_id and/or routing.exclude_candidate_ids." + ), + "properties": { + "candidate_id": { + "type": "string", + "description": "Echoes the request's routing.candidate_id, when pinned.", + }, + "exclude_candidate_ids": { + "type": "array", + "items": {"type": "string"}, + "description": "The request's routing.exclude_candidate_ids, sorted.", + }, + "attempted_candidate_ids": { + "type": "array", + "items": {"type": "string"}, + "description": ( + "Every private agent ID a provider call was attempted " + "against while serving this request, in first-attempt order." + ), + }, + "served_candidate_id": { + "type": "string", + "description": ( + "The private agent ID whose output was actually returned " + "to the caller, when determinable." + ), + }, + }, + "required": ["exclude_candidate_ids", "attempted_candidate_ids"], + "additionalProperties": False, + }, "AuthoritativeUsage": { "type": ["object", "null"], "required": ["prompt_tokens", "completion_tokens"], @@ -74,7 +147,12 @@ "type": "string", "enum": ["measured", "unavailable"], }, - "orchestration": {"type": "object"}, + "orchestration": { + "type": "object", + "properties": { + "routing": {"$ref": "#/components/schemas/CandidateRoutingEvidence"}, + }, + }, }, }, "ModelGroupWrite": { @@ -214,6 +292,9 @@ "description": "When true, select only model-group members with ZDR evidence.", }, "response_format": {"type": "object"}, + "routing": { + "$ref": "#/components/schemas/CandidateRoutingControls" + }, "include_orchestration_trace": { "type": "boolean", "description": "Requires the same caller to have the trace purpose", @@ -448,6 +529,9 @@ "model": {"type": "string"}, "input": {"oneOf": [{"type": "string"}, {"type": "array"}]}, "stream": {"type": "boolean"}, + "routing": { + "$ref": "#/components/schemas/CandidateRoutingControls" + }, "zdr_only": { "type": "boolean", "description": "When true, select only model-group members with ZDR evidence.", diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 751c9e366..2d3d023de 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -18,6 +18,7 @@ from __future__ import annotations +import contextlib import hashlib import re from contextvars import ContextVar @@ -543,6 +544,7 @@ def complete( provider_request: Optional[Dict[str, Any]] = None, provider_endpoint: str = "chat/completions", zdr_only: bool = False, + candidate_scope_open: bool = False, ) -> Dict[str, Any]: """Route a request (sync or batch) and record its usage + cost. @@ -554,17 +556,47 @@ def complete( Each trace step backed by valid provider token counts is ``measured``. A missing count is recorded with an ``unavailable`` status and numeric storage sentinels; API usage and cost remain null. + + ``candidate_scope_open=True`` tells the ``messages``-based sync path + (``provider_request is None``) that the caller already has a + ``candidate_routing_policy`` scope open -- e.g. because it ran + :meth:`TaskOrchestrator.would_route`'s triage call under that scope + before deciding to conduct rather than route -- so this call must not + open a second, independent scope that would discard the triage + attempt from ``attempted_candidate_ids`` (#983). Ignored on the + ``provider_request`` path, which always scopes itself around its own + ``proxy_completion`` call. """ if not isinstance(cache_bypass, bool): raise TypeError("cache_bypass must be a boolean") if type(zdr_only) is not bool: raise TypeError("zdr_only must be a boolean") + routing_controls = hints if isinstance(hints, dict) else {} + # TaskOrchestrator._has_active_candidate_controls is the single + # source of truth: it detects an active control by key *presence*, + # not truthiness, so an explicitly malformed value (candidate_id= + # None, exclude_candidate_ids=None or a non-list/tuple) still + # forces the sync path below, giving candidate_routing_policy's + # real validation a chance to reject it instead of silently falling + # through the batch branch's early return and dropping the + # malformed control entirely. An explicit empty exclude_candidate_ids + # list/tuple is the one genuine no-op (#983 Devin/CodeRabbit + # finding: direct Python API callers can lose or bypass routing + # validation). The same predicate also gates whether this method's + # own provider-response evidence handling below may trust a + # `_candidate_routing` field (#983 Devin finding: "Provider fields + # forge routing evidence"). + has_candidate_controls = self.orchestrator._has_active_candidate_controls( + routing_controls + ) routing_hints = hints if isinstance(hints, RoutingHints) else RoutingHints.from_mapping(hints) try: prompt_tokens = self.token_counter.count_messages(messages, model_name) except TokenCountUnavailable: prompt_tokens = None decision = self.policy.decide(routing_hints, prompt_tokens) + if has_candidate_controls and decision.channel == "batch": + decision = replace(decision, channel="sync", reason="candidate controls require sync routing") if decision.channel == "batch" and provider_request is None: request = BatchRequest( @@ -600,7 +632,17 @@ def complete( } race_token = self._race_usage_context.set(race_context) try: - with self.orchestrator.request_policy(zdr_only): + with self.orchestrator.request_policy(zdr_only), self.orchestrator.candidate_routing_policy( + routing_controls, + model_name=model_name, + required_roles=("thinker", "worker", "verifier", "synthesizer") + if self.orchestrator.proxy_completion_requires_conduct( + provider_request, + endpoint=provider_endpoint, + single_agent=False, + ) + else ("worker",), + ): provider_response = self.orchestrator.proxy_completion( provider_request, endpoint=provider_endpoint, @@ -621,6 +663,17 @@ def complete( ): raise RuntimeError("provider completion omitted orchestration lineage") result = dict(self.orchestrator.get_workflow_run(lineage["workflow_run_id"])) + routing_evidence = provider_response.pop("_candidate_routing", None) + # Only republish gateway-computed evidence: the raw provider + # response is untrusted (#983 Devin finding: "Provider fields + # forge routing evidence"). Without an active candidate control + # on this request, proxy_completion() never sets + # `_candidate_routing` itself, so a `_candidate_routing` field + # observed here with no active control can only have arrived + # already-present on the provider's own response body -- never + # trust it as gateway evidence in that case. + if has_candidate_controls and routing_evidence is not None: + lineage["routing"] = routing_evidence race_records = list(race_context["records"]) records = list(race_records) # The caller's request prompt is attributed at most once per @@ -734,8 +787,28 @@ def complete( } race_token = self._race_usage_context.set(race_context) try: - with self.orchestrator.request_policy(zdr_only): + # candidate_scope_open=True means the caller already has a + # candidate_routing_policy scope open (see the docstring above); + # entering a second, independent one here would reset the + # attempted-candidate ContextVar and discard whatever the caller + # already recorded under it, so reuse a no-op context instead. + candidate_scope = ( + contextlib.nullcontext() + if candidate_scope_open + else self.orchestrator.candidate_routing_policy( + routing_controls, + model_name=model_name, + required_roles=self.orchestrator.candidate_pin_required_roles( + mode, model_name + ), + ) + ) + with self.orchestrator.request_policy(zdr_only), candidate_scope: result = self.orchestrator.run(messages, **run_kwargs) + routing_evidence = self.orchestrator._candidate_routing_evidence(result) + if routing_evidence is not None: + result = dict(result) + result["candidate_routing"] = routing_evidence if isinstance(result.get("workflow_run_id"), str): race_context["workflow_run_id"] = result["workflow_run_id"] race_context["workflow_ready"] = True diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index a09c4e8f0..127b3acc0 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -325,6 +325,27 @@ def _cost_usd_decimal(output_tokens: int, price_per_million: float) -> Decimal: default=None, ) _REQUEST_ZDR_ONLY: ContextVar[bool] = ContextVar("request_zdr_only", default=False) +_REQUEST_CANDIDATE_ID: ContextVar[str | None] = ContextVar( + "request_candidate_id", default=None +) +_REQUEST_EXCLUDED_CANDIDATE_IDS: ContextVar[frozenset[str]] = ContextVar( + "request_excluded_candidate_ids", default=frozenset() +) +_REQUEST_ATTEMPTED_CANDIDATE_IDS: ContextVar[list[str] | None] = ContextVar( + "request_attempted_candidate_ids", default=None +) +# Invariant: once ``candidate_routing_policy`` binds this ContextVar to a list +# (``.set([])``), every later write into it for that request MUST mutate that +# same list object in place (see ``_record_candidate_attempt``) -- never +# ``.set()`` a new list and never a compound read-then-reassign. A raced +# ``race_first_valid`` attempt runs inside a ``copy_context().run(...)`` +# worker thread; ``copy_context()`` only isolates ``ContextVar.set()``/ +# ``reset()`` calls made inside the copy, so an in-place mutation of the +# already-bound list is visible back in the parent request context, but a +# rebind inside the worker thread would be invisible outside it and would +# silently drop that candidate from ``orchestration.routing`` evidence. See +# PR #983 and tests/test_endpoint_race.py:: +# test_race_attempts_all_reach_candidate_routing_evidence. SECRET_PATTERNS = ( re.compile(r"(?i)(api[_-]?key|token|secret|password)(['\"]?\s*[:=]\s*['\"]?)[A-Za-z0-9._~+/=-]{12,}"), @@ -424,6 +445,12 @@ def complete_structured( agent, request, effort_profile ) request["stream"] = False + # complete() reaches _invoke(), which records the attempt itself; + # this structured path calls proxy_send directly, so the attempt + # must be recorded here or it never appears in + # attempted_candidate_ids evidence (#983 Devin finding: "Attempt + # evidence omits provider calls"). + self.orchestrator._record_candidate_attempt(agent.id) response = self.orchestrator.client.proxy_send( agent, "chat/completions", request ) @@ -3989,6 +4016,229 @@ def request_policy(self, zdr_only: bool = False): finally: _REQUEST_ZDR_ONLY.reset(token) + @contextmanager + def candidate_routing_policy( + self, + routing: Mapping[str, Any] | None, + *, + model_name: str = GATEWAY_DEFAULT_MODEL, + required_roles: tuple[str, ...] = ("worker",), + required_tags: tuple[str, ...] = (), + ): + """Apply trusted request-local candidate pin and exclusion controls.""" + routing = routing or {} + candidate_id = routing.get("candidate_id") + excluded = routing.get("exclude_candidate_ids", ()) + # A present-but-malformed control must fail validation even when its + # value is falsy (empty string, False, None, an empty mapping, ...) + # -- only a field that is entirely *absent*, or an explicit empty + # list/tuple, is a no-op. The HTTP layer's own _validate_routing + # already rejects these shapes with a 400 before they reach here; + # this closes the same gap for direct Python-API callers, who would + # otherwise have a malformed control silently treated as "no + # control" instead of surfacing the caller bug (#983 finding 2). + if "candidate_id" in routing and not isinstance(candidate_id, str): + raise ValueError("candidate_id must be a non-empty agent ID") + if "exclude_candidate_ids" in routing and not isinstance(excluded, (list, tuple)): + raise ValueError("exclude_candidate_ids must contain agent IDs") + if candidate_id is None and not excluded: + yield + return + if model_name not in {self.GATEWAY_DEFAULT_MODEL, self.AUTO_MODEL, self.FREE_MODEL}: + raise ValueError("candidate controls require a virtual gateway model") + if candidate_id is not None and ( + not isinstance(candidate_id, str) + or not candidate_id.strip() + or candidate_id != candidate_id.strip() + ): + raise ValueError("candidate_id must be a non-empty agent ID") + if not isinstance(excluded, (list, tuple)): + raise ValueError("exclude_candidate_ids must contain agent IDs") + normalized = tuple(value.strip() for value in excluded if isinstance(value, str)) + if len(normalized) != len(excluded) or any(not value for value in normalized): + raise ValueError("exclude_candidate_ids must contain non-empty agent IDs") + if any(value != value.strip() for value in excluded): + raise ValueError("exclude_candidate_ids must contain exact agent IDs") + if len(normalized) != len(set(normalized)): + raise ValueError("exclude_candidate_ids must contain unique agent IDs") + candidate_id = candidate_id.strip() if isinstance(candidate_id, str) else None + if candidate_id in normalized: + raise ValueError("candidate_id cannot also be excluded") + # Endpoint-filtered: a pin or exclusion is validated against exactly + # the candidates the active routing.endpoint scope (if any) would + # actually let selection reach, so an endpoint/candidate conflict + # fails this preflight instead of surfacing as a later selection + # RuntimeError. + configured = { + agent.id: agent + for agent in self.candidates + if _agent_matches_request_endpoint(agent) + } + unknown = sorted(set(normalized) - configured.keys()) + if unknown: + raise ValueError("exclude_candidate_ids contains an unknown agent ID") + if candidate_id is not None: + candidate = configured.get(candidate_id) + if ( + candidate is None + or candidate.disabled + or not self._zdr_agent_allowed(candidate) + or not _is_general_chat_agent(candidate) + or any(role in candidate.provider_exclusions for role in required_roles) + or any(tag not in candidate.tags for tag in required_tags) + ): + raise ValueError("candidate_id is not an eligible agent") + if model_name == self.FREE_MODEL and not self._is_general_free_agent(candidate): + raise ValueError("candidate_id is not eligible for orchestrator/free") + elif any( + not any( + agent.id not in normalized + and not agent.disabled + and self._zdr_agent_allowed(agent) + and _is_general_chat_agent(agent) + and role not in agent.provider_exclusions + and all(tag in agent.tags for tag in required_tags) + and ( + model_name != self.FREE_MODEL + or self._is_general_free_agent(agent) + ) + for agent in configured.values() + ) + for role in required_roles + ): + raise ValueError("exclude_candidate_ids leaves no eligible agent") + candidate_token = _REQUEST_CANDIDATE_ID.set(candidate_id) + excluded_token = _REQUEST_EXCLUDED_CANDIDATE_IDS.set(frozenset(normalized)) + attempted_token = _REQUEST_ATTEMPTED_CANDIDATE_IDS.set([]) + try: + yield + finally: + _REQUEST_ATTEMPTED_CANDIDATE_IDS.reset(attempted_token) + _REQUEST_EXCLUDED_CANDIDATE_IDS.reset(excluded_token) + _REQUEST_CANDIDATE_ID.reset(candidate_token) + + @staticmethod + def _request_candidate_allowed(agent: ModelAgent) -> bool: + pinned = _REQUEST_CANDIDATE_ID.get() + return ( + agent.id not in _REQUEST_EXCLUDED_CANDIDATE_IDS.get() + and (pinned is None or agent.id == pinned) + ) + + @staticmethod + def _record_candidate_attempt(agent_id: str) -> None: + # Must always mutate the bound list in place (``.append``) and never + # rebind the ContextVar with ``.set(...)`` here: a raced attempt runs + # inside a copy_context().run(...) worker thread, and a ``.set()`` + # there would be invisible outside that thread once the race + # finishes, silently dropping this attempt from the parent request's + # ``orchestration.routing`` evidence. See the invariant comment on + # _REQUEST_ATTEMPTED_CANDIDATE_IDS above and PR #983. + attempted = _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() + if attempted is not None and agent_id not in attempted: + attempted.append(agent_id) + + @staticmethod + def _has_active_candidate_controls(routing: Any) -> bool: + """Return whether a raw routing mapping requests an active candidate control. + + Uses key *presence* for ``candidate_id`` (so an explicitly malformed + ``candidate_id: None`` is not indistinguishable from an absent key) + and *presence plus non-empty* for ``exclude_candidate_ids`` (an + explicit empty list/tuple is the one genuine no-op; any other + present value -- including a malformed non-list/tuple -- counts as + active so a caller earlier in the same request, still validating, + does not silently drop it). This mirrors + ``candidate_routing_policy``'s own no-op condition and the + equivalent check in ``CostRoutingCoordinator.complete`` (#983 + CodeRabbit finding: direct Python API callers can lose or bypass + routing validation) -- a malformed shape only ever reaches a caller + of *this* method after ``candidate_routing_policy`` has already + validated it without raising, so the "malformed still counts" + branch is unreachable there in practice, but sharing one predicate + keeps every caller's notion of "active" identical by construction + instead of by convention. + + This is the single source of truth every caller must use to decide, + from a request's raw routing mapping alone and without opening a + ``candidate_routing_policy`` scope, whether gateway-computed + candidate routing evidence may legitimately be present on this + response. A caller must never trust a ``_candidate_routing`` or + ``orchestration`` field that arrived already present on a raw + provider response when this predicate is ``False`` -- the gateway + only ever populates those fields itself when it is ``True`` (#983 + Devin finding: "Provider fields forge routing evidence"). + """ + if not isinstance(routing, Mapping): + return False + if "candidate_id" in routing: + return True + if "exclude_candidate_ids" not in routing: + return False + excluded = routing.get("exclude_candidate_ids") + return not (isinstance(excluded, (list, tuple)) and not excluded) + + @staticmethod + def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | None: + pinned = _REQUEST_CANDIDATE_ID.get() + excluded = sorted(_REQUEST_EXCLUDED_CANDIDATE_IDS.get()) + if pinned is None and not excluded: + return None + trace = result.get("trace") + rows = trace if isinstance(trace, list) else [] + tracked_attempts = _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() + attempted = list(tracked_attempts or ()) + if tracked_attempts is None: + attempted = [ + value + for row in rows + if isinstance(row, Mapping) + for value in [row.get("agent_id")] + if isinstance(value, str) and value + ] + # Serving identity is evidence, not an inference target. Multi-step + # workflows record the exact answering_step_id; provider-shaped paths + # may record served_agent_id explicitly. Historical records lacking + # either identity remain auditable for attempts but fail closed for + # served_candidate_id. Output equality and trace position are not + # admissible serving-identity evidence. + answering_step_id = result.get("answering_step_id") + answering_rows = ( + [ + row + for row in rows + if isinstance(row, Mapping) and row.get("id") == answering_step_id + ] + if isinstance(answering_step_id, int) + else [] + ) + served: str | None = None + if len(answering_rows) == 1: + row = answering_rows[0] + value = row.get("served_agent_id") or row.get("agent_id") + if isinstance(value, str) and value: + served = value + elif tracked_attempts != []: + explicit_served = [ + value + for row in rows + if isinstance(row, Mapping) + for value in [row.get("served_agent_id")] + if isinstance(value, str) and value + ] + distinct_served = tuple(dict.fromkeys(explicit_served)) + if len(distinct_served) == 1: + served = distinct_served[0] + evidence: dict[str, Any] = { + "exclude_candidate_ids": excluded, + "attempted_candidate_ids": list(dict.fromkeys(attempted)), + } + if pinned is not None: + evidence["candidate_id"] = pinned + if served is not None: + evidence["served_candidate_id"] = served + return evidence + @staticmethod def _zdr_agent_allowed(agent: ModelAgent) -> bool: """Return whether one agent is eligible under the active privacy policy.""" @@ -4131,6 +4381,24 @@ def _reload_state(self) -> None: } ) + @staticmethod + def proxy_completion_requires_conduct( + body: Mapping[str, Any], + *, + endpoint: str = "chat/completions", + single_agent: bool = True, + ) -> bool: + """Return whether a provider-shaped request takes the conduct path.""" + normalized_endpoint = endpoint.strip("/") + return not single_agent and ( + normalized_endpoint == "responses" + or any( + key in body + and not _is_omit_equivalent_control(key, body.get(key)) + for key in _PASSTHROUGH_TRIGGER_KEYS + ) + ) + def proxy_completion( self, body: dict[str, Any], @@ -4163,19 +4431,22 @@ def proxy_completion( """ normalized_endpoint = endpoint.strip("/") api_surface = "responses" if normalized_endpoint == "responses" else "chat.completions" - if not single_agent and ( - normalized_endpoint == "responses" - or any( - key in body - and not _is_omit_equivalent_control(key, body.get(key)) - for key in _PASSTHROUGH_TRIGGER_KEYS - ) + if self.proxy_completion_requires_conduct( + body, + endpoint=normalized_endpoint, + single_agent=single_agent, ): - return self._orchestrated_provider_completion( + result = self._orchestrated_provider_completion( body, endpoint=normalized_endpoint, effort_profile=effort_profile, ) + workflow_id = (result.get("orchestration") or {}).get("workflow_run_id") + workflow = self.get_workflow_run(workflow_id) if isinstance(workflow_id, str) else {} + evidence = self._candidate_routing_evidence(workflow) + if evidence is not None: + result["_candidate_routing"] = evidence + return result # Every path below this point resolves one "worker"-role agent (see # _select_agent(..., "worker", ...) and _failover_candidates(..., # "worker", ...) further down), so an unset caller profile defaults @@ -4219,7 +4490,11 @@ def proxy_completion( ) if ( isinstance(required_agent_id, str) - and (agent is None or not self._zdr_agent_allowed(agent)) + and ( + agent is None + or not self._zdr_agent_allowed(agent) + or not self._request_candidate_allowed(agent) + ) ): raise RuntimeError("required file provider is unavailable") if agent is not None and agent.disabled: @@ -4284,6 +4559,7 @@ def proxy_completion( ) measured = bool(agent.group_name or requested_model == self.FREE_MODEL) started_at = time.perf_counter() + self._record_candidate_attempt(agent.id) try: result = self.client.proxy_send(agent, endpoint, upstream) except Exception as exc: @@ -4295,6 +4571,11 @@ def proxy_completion( self._group_router.observe_success( agent.id, time.perf_counter() - started_at ) + evidence = self._candidate_routing_evidence( + {"trace": [{"agent_id": agent.id, "served_agent_id": agent.id}]} + ) + if evidence is not None: + result["_candidate_routing"] = evidence return result allowed_agent_ids = ({agent.id} if isinstance(required_agent_id, str) else ( @@ -4349,6 +4630,7 @@ def proxy_completion( every_failure_was_request_too_large = True for candidate in candidates: started_at = time.perf_counter() + self._record_candidate_attempt(candidate.id) candidate_payload = dict(upstream) candidate_payload["model"] = candidate.model if isinstance(file_replicas, dict): @@ -4394,6 +4676,18 @@ def proxy_completion( self._group_router.observe_success( candidate.id, time.perf_counter() - started_at ) + evidence = self._candidate_routing_evidence( + { + "trace": [ + { + "agent_id": candidate.id, + "served_agent_id": candidate.id, + } + ] + } + ) + if evidence is not None: + result["_candidate_routing"] = evidence return result if last_failure is not None and every_failure_was_request_too_large: raise ProviderRequestTooLargeError( @@ -4475,7 +4769,11 @@ def _orchestrated_provider_completion( ) if ( isinstance(required_agent_id, str) - and (final_agent is None or not self._zdr_agent_allowed(final_agent)) + and ( + final_agent is None + or not self._zdr_agent_allowed(final_agent) + or not self._request_candidate_allowed(final_agent) + ) ): raise RuntimeError("required file provider is unavailable") if final_agent is None: @@ -4730,6 +5028,11 @@ def send_synthesis( and candidate.id not in request_exclusions ), ] + ordered_candidates = [ + candidate + for candidate in ordered_candidates + if self._request_candidate_allowed(candidate) + ] for candidate in ordered_candidates: candidate_endpoint = candidate.base_url.rstrip("/").casefold() if last_model_not_found is not None and candidate_endpoint != preferred_endpoint: @@ -4757,6 +5060,7 @@ def send_synthesis( active_profile, api_surface=api_surface, ) + self._record_candidate_attempt(candidate.id) try: send = self.client.proxy_send if virtual_model: @@ -4962,6 +5266,19 @@ def send_synthesis( "policy_mode": "conduct", "prompt_text": task, "answer": synthesis_output, + # Identify the exact trace row (the repair row when a repair + # ran and succeeded, else the initial synthesis row) whose + # output actually became ``answer`` -- mirrors conduct()'s own + # ``answering_step_id`` (#983 finding 6) so + # _candidate_routing_evidence resolves the serving candidate + # by identity instead of falling back to a text match that + # cannot tell this row apart from an earlier internal + # workflow step whose output it coincidentally duplicates + # (#983 Devin finding: "Repeated output misidentifies served + # candidate"). + "answering_step_id": ( + repair_step["id"] if repair_step is not None else synthesis_step["id"] + ), "cache_status": "bypass", "trace": trace, "policy_snapshot": self.policy.as_dict(), @@ -5183,6 +5500,37 @@ def would_route( ) ) + def candidate_pin_required_roles( + self, mode: str, model_name: str = GATEWAY_DEFAULT_MODEL + ) -> tuple[str, ...]: + """Roles a candidate pin/exclusion must satisfy for ``mode``, provider-free. + + ``mode="route"`` only ever serves the worker role. ``mode="auto"`` + resolves to that same direct route whenever :meth:`would_route`'s own + ``model_name`` branch already decides it -- i.e. whenever + ``model_name`` is not one of the two virtual models real + route-vs-conduct triage chooses between. The only such value + reachable here is ``FREE_MODEL``: :meth:`candidate_routing_policy` + itself rejects a pin/exclusion against any other non-virtual + ``model_name`` before roles are ever checked. That FREE_MODEL case + is therefore decided with zero provider calls, so a pin only needs + the worker role there too. + + For ``mode="auto"`` against ``GATEWAY_DEFAULT_MODEL``/``AUTO_MODEL`` + -- and for ``mode="conduct"`` -- the real decision needs a live + triage call that, with a pin active, would run against the pinned + candidate itself (triage agent selection is pin-scoped) before this + preflight has cleared it as eligible. Validation stays conservative + there and requires the full conduct role set instead of risking + that call; this is a known architectural limit of provider-free + preflight, not an oversight (see PR #983 discussion). + """ + if mode == "route": + return ("worker",) + if mode == "auto" and model_name not in {self.GATEWAY_DEFAULT_MODEL, self.AUTO_MODEL}: + return ("worker",) + return ("thinker", "worker", "verifier", "synthesizer") + def stream_route( self, messages: list[ChatMessage], @@ -5210,6 +5558,7 @@ def stream_route( if include_usage: stream_kwargs["include_usage"] = True stream = self.client.stream_chat(agent, messages, **stream_kwargs) + self._record_candidate_attempt(agent.id) started_at = time.perf_counter() try: for delta in stream: @@ -5249,6 +5598,11 @@ def stream_route( "latency_ms": round(latency_seconds * 1000, 2), "output": answer, } + if _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() is not None: + # Streaming never fails over to a different agent, so this is + # always an explicit "served == agent" fact for candidate-routing + # evidence, never a failover signal. + trace_step["served_agent_id"] = agent.id if isinstance(usage, dict): trace_step["usage"] = usage record = self._with_effort_snapshot( @@ -5296,6 +5650,11 @@ def _cache_key( "max_output_tokens": getattr(self.client, "max_output_tokens", None), } parameters = {**parameters, "zdr_only": _REQUEST_ZDR_ONLY.get()} + pinned_candidate = _REQUEST_CANDIDATE_ID.get() + excluded_candidates = sorted(_REQUEST_EXCLUDED_CANDIDATE_IDS.get()) + if pinned_candidate is not None or excluded_candidates: + parameters["candidate_id"] = pinned_candidate + parameters["exclude_candidate_ids"] = excluded_candidates endpoint_partition = _request_endpoint_partition() cache_partition = ( endpoint_partition @@ -5344,6 +5703,20 @@ def run( "policy_mode": mode, "prompt_text": prompt, "answer": result["answer"], + # conduct() (round 6, #983) records which trace row's output + # actually became "answer" as answering_step_id, so + # _candidate_routing_evidence can resolve the served + # candidate by identity instead of a fragile text match. + # run() must carry it through here or every conduct() caller + # that reaches routing evidence via a persisted workflow + # run (CostRoutingCoordinator.complete() in cost_router.py) + # loses that identity and can misattribute a duplicate-text + # answer to an earlier step (#983 Devin finding: "Duplicate + # outputs misidentify serving candidate"). route_once() + # results have no answering_step_id; None here is the + # correct no-signal case _candidate_routing_evidence already + # falls back on. + "answering_step_id": result.get("answering_step_id"), "cache_status": result.get("cache_status", "disabled"), "trace": result["trace"], "policy_snapshot": self.policy.as_dict(), @@ -6311,6 +6684,12 @@ def route_once( if attempt_served_id != candidate.id: row["served_agent_id"] = attempt_served_id row["failover_from"] = candidate.id + elif _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() is not None: + # Candidate-routing evidence needs an explicit serving fact on + # every attempt, even an unchanged one -- the ordinary no-policy + # path below must keep omitting the key entirely (regression + # guard: default mock path stays "no failover metadata"). + row["served_agent_id"] = attempt_served_id answer, served_id = attempt_answer, attempt_served_id verification = self._realtime_route_judge( text=text, @@ -6501,7 +6880,12 @@ def conduct( additional_cost_usd=in_flight_cost, ) agent = self._agent(step.agent_id) - if any(tag not in agent.tags for tag in required_tags): + candidate_controls_active = _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() is not None + if not self._request_candidate_allowed(agent) or any( + tag not in agent.tags for tag in required_tags + ) or ( + candidate_controls_active and step.role in agent.provider_exclusions + ): try: capable = self._ranked_agents( step.subtask, @@ -6513,6 +6897,14 @@ def conduct( capable = [] if capable: agent = capable[0] + if candidate_controls_active and ( + not self._request_candidate_allowed(agent) + or any(tag not in agent.tags for tag in required_tags) + or step.role in agent.provider_exclusions + ): + raise ValueError( + "no eligible candidate satisfies the active routing controls" + ) if progress is not None: progress(step.role, "started") prior = "\n\n".join(f"Step {i}: {outputs[i]}" for i in step.access) @@ -6567,6 +6959,10 @@ def last_output(role: str) -> str: ids = [step.id for step in steps if step.role == role] return outputs.get(ids[-1], "") if ids else "" + def last_step_id(role: str) -> int | None: + ids = [step.id for step in steps if step.role == role] + return ids[-1] if ids else None + # Generated plans may omit a thinker; the first step's output is the upstream evidence. upstream = last_output("thinker") or outputs.get(steps[0].id, "") verification = self._judge_verifier_output(last_output("verifier"), upstream, last_output("worker")) @@ -6579,8 +6975,10 @@ def last_output(role: str) -> str: excluded_agent_ids=_excluded_agent_ids, ) answer = outputs[steps[-1].id] + answering_step_id = steps[-1].id if not verification["accepted"] and self.policy.verifier_required and last_output("worker"): answer = last_output("worker") + answering_step_id = last_step_id("worker") else: verification = self._judge_verifier_output(outputs.get(2, ""), outputs.get(0, ""), outputs.get(1, "")) if self.policy.verifier_judge == "model": # pragma: no branch - OrchestrationPolicy validates this to be constant @@ -6592,12 +6990,15 @@ def last_output(role: str) -> str: excluded_agent_ids=_excluded_agent_ids, ) answer = outputs[steps[2].id] if not self.policy.verifier_required else outputs[steps[-1].id] + answering_step_id = steps[2].id if not self.policy.verifier_required else steps[-1].id if not verification["accepted"] and self.policy.verifier_required: answer = outputs[steps[1].id] + answering_step_id = steps[1].id result = { "mode": "conduct", "answer": answer, + "answering_step_id": answering_step_id, "trace": trace, "verification": verification, "plan_source": plan_source, @@ -6716,6 +7117,7 @@ def _plan_generated(self, task: str) -> list[WorkflowStep]: for agent in self.agents if _is_general_chat_agent(agent) and self._zdr_agent_allowed(agent) + and self._request_candidate_allowed(agent) and _agent_matches_request_endpoint(agent) and any(agent.id in eligible_by_role[role] for role in roles) ) @@ -6734,6 +7136,11 @@ def _plan_generated(self, task: str) -> list[WorkflowStep]: {"role": "user", "content": task}, ] effort_profile = self._role_effort_profile("planner") + # Recorded immediately before the provider call, as the triage and + # invocation paths do -- otherwise routing evidence can omit a + # candidate that actually received the request (the planner is not + # guaranteed to appear again among the generated plan's own steps). + self._record_candidate_attempt(planner.id) raw = ( self.client.chat(planner, planner_messages, effort_profile=effort_profile) if effort_profile is not None @@ -6779,6 +7186,7 @@ def _parse_workflow_plan(self, raw: str) -> list[WorkflowStep]: assigned is None or not _is_general_chat_agent(assigned) or not self._zdr_agent_allowed(assigned) + or not self._request_candidate_allowed(assigned) or assigned.id not in eligible_ids ): # Unknown or stale ineligible assignments are reselected honestly. @@ -6897,6 +7305,7 @@ def _ranked_agents( agent for agent in source if not agent.disabled + and self._request_candidate_allowed(agent) and _agent_matches_request_endpoint(agent) and self._zdr_agent_allowed(agent) if ( @@ -7168,6 +7577,11 @@ def _embed_cached(self, text: str) -> list[float] | None: embedding_member = self._embedding_agent_id() if embedding_member is None: return None + # Only a cache miss reaches the provider -- record the attempt here, + # not above the cache lookup, so a cache hit (never calls embed()) + # does not falsely appear in attempted_candidate_ids evidence (#983 + # Devin finding: "Attempt evidence omits provider calls"). + self._record_candidate_attempt(embedding_member) try: vectors = self.client.embed(self._agent(embedding_member), [text]) except Exception: # noqa: BLE001 - similarity is best-effort evidence @@ -7195,6 +7609,11 @@ def _descriptor_vector_cached(self, agent: ModelAgent) -> list[float] | None: embedding_member = self._embedding_agent_id() if embedding_member is None: return None + # Only a cache miss reaches the provider -- record the attempt here, + # not above the cache lookup, so a cache hit (never calls embed()) + # does not falsely appear in attempted_candidate_ids evidence (#983 + # Devin finding: "Attempt evidence omits provider calls"). + self._record_candidate_attempt(embedding_member) try: vectors = self.client.embed( self._agent(embedding_member), [self._agent_descriptor_text(agent)] @@ -7250,14 +7669,16 @@ def _triage_workflow_required(self, text: str) -> bool: assurance; an absent triage agent degrades to the direct path because no evidence source exists at all. Verdicts are cached by content hash. """ - digest = hashlib.sha256( - ( - _request_endpoint_partition() - + "\x1f" - + text - + ("\x00zdr_only" if _REQUEST_ZDR_ONLY.get() else "") - ).encode("utf-8") - ).hexdigest() + control_key = json.dumps( + { + "endpoint_partition": _request_endpoint_partition(), + "candidate_id": _REQUEST_CANDIDATE_ID.get(), + "exclude_candidate_ids": sorted(_REQUEST_EXCLUDED_CANDIDATE_IDS.get()), + "zdr_only": _REQUEST_ZDR_ONLY.get(), + }, + sort_keys=True, + ) + digest = hashlib.sha256((text + "\x00" + control_key).encode("utf-8")).hexdigest() with self._evidence_lock: cached = self._triage_cache.get(digest) if cached is not None: @@ -7272,9 +7693,32 @@ def _compute_triage_verdict(self, text: str) -> bool: candidates = self._ranked_agents(text, "worker", free_only=True) except RuntimeError: candidates = [] - if not candidates and not _REQUEST_ZDR_ONLY.get(): + if not candidates: + # The free-only ranking is empty for two different reasons that + # must not be conflated: genuinely no evidence source exists at + # all, or (#983 Devin finding "ZDR pins skip workflow triage") a + # zdr_only request pinned/is scoped to a paid candidate, which + # can never appear in a free_only ranking regardless of ZDR + # eligibility. This fallback pool already re-derives every + # safety predicate itself per agent -- disabled, ZDR eligibility + # (_zdr_agent_allowed), the active pin/exclusion + # (_request_candidate_allowed), chat capability, and endpoint + # scope -- so it is correct and safe to build regardless of + # whether zdr_only is active: an active ZDR policy already + # narrows it to ZDR-eligible agents (or the ZDR-eligible pinned + # one) on its own, with zero risk of contacting a non-ZDR + # provider. Gating the whole fallback build behind "not + # zdr_only" therefore only ever discarded a genuine evidence + # source (the pinned candidate itself), silently defaulting the + # route-vs-conduct decision to "route" with no live triage call. candidates = [ - agent for agent in self.agents if _agent_matches_request_endpoint(agent) + agent + for agent in self.agents + if not agent.disabled + and self._zdr_agent_allowed(agent) + and self._request_candidate_allowed(agent) + and _is_general_chat_agent(agent) + and _agent_matches_request_endpoint(agent) ] if not candidates: return False @@ -7284,6 +7728,7 @@ def _compute_triage_verdict(self, text: str) -> bool: {"role": "user", "content": text}, ] try: + self._record_candidate_attempt(triage_agent.id) reply = self.client.chat(triage_agent, messages, temperature=0.0) return _parse_triage_reply(reply) except Exception: # noqa: BLE001 - fail closed toward verified orchestration @@ -7714,6 +8159,7 @@ def _invoke( request_settings = self.client.request_settings_snapshot() def call(agent: ModelAgent) -> tuple[str, str, str, dict[str, Any] | None]: + self._record_candidate_attempt(agent.id) with self.client.request_settings(**request_settings): output = ( self.client.chat(agent, messages, effort_profile=effort_profile) @@ -7772,6 +8218,7 @@ def call(agent: ModelAgent) -> tuple[str, str, str, dict[str, Any] | None]: retry_attempt = 0 while True: try: + self._record_candidate_attempt(agent.id) attempt_start = time.perf_counter() effort_profile = self._role_effort_profile(role) output = ( @@ -7960,6 +8407,7 @@ def _failover_candidates( agent for agent in ordered if not agent.disabled + and self._request_candidate_allowed(agent) and self._zdr_agent_allowed(agent) and _is_general_chat_agent(agent) and all(tag in agent.tags for tag in required_tags) @@ -8120,9 +8568,26 @@ def _model_judge_verification( } judge_adapter: _FastMLSIJudgeAdapter | None = None try: + # _ranked_agents deliberately still returns role-ineligible + # members (it appends them after every eligible one -- see its + # own docstring), so every other caller in this module that + # wants only role-eligible candidates re-applies + # `role not in agent.provider_exclusions` itself + # (_plan_generated, _parse_workflow_plan). This selection was + # missing that filter: with a single-candidate pool excluded + # from "verifier" (e.g. a worker-only pinned agent), `next(...)` + # picked that ineligible agent as judge anyway, so a live judge + # call could still land on an agent the pool operator explicitly + # declared unfit to verify -- an extra, unrequested provider + # call the no-heuristics candidate-controls contract on #983 + # does not allow. `_invoke`'s own failover path already enforces + # this exclusion for a *backup* judge (see + # test_fast_mlsirm_judge_failover_honors_verifier_exclusions); + # this closes the same gap for the *primary* selection here. judge = next( agent for agent in self._ranked_agents(task, "verifier", free_only=free_only) + if "verifier" not in agent.provider_exclusions if allowed_agent_ids is None or agent.id in allowed_agent_ids if excluded_agent_ids is None or agent.id not in excluded_agent_ids ) @@ -15698,6 +16163,7 @@ def chat_completion_response( "routing_reason": result.get("routing_reason"), "usage_record_id": result.get("usage_record_id"), "cost": result.get("cost"), + "routing": result.get("candidate_routing"), } if include_trace: orchestration["trace"] = redact_value(result["trace"]) @@ -15792,6 +16258,7 @@ def chat_completion_chunks( "workflow_run_id": result.get("workflow_run_id"), "mode": result.get("mode"), "verification": result.get("verification"), + "routing": result.get("candidate_routing"), } if include_trace and "trace" in result: orchestration["trace"] = redact_value(result["trace"]) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index eb6a77519..a848d64a5 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -2,6 +2,7 @@ from __future__ import annotations +from contextlib import nullcontext from dataclasses import dataclass, field from email.message import Message from http.cookies import CookieError, SimpleCookie @@ -3172,7 +3173,7 @@ def _validate_attribution(attribution: Any) -> dict[str, Any] | None: def _validate_routing( - routing: Any, *, allow_endpoint: bool = False + routing: Any, *, allow_candidate_controls: bool = False, allow_endpoint: bool = False ) -> dict[str, Any] | None: """OpenAI-adjacent routing hints for sync vs batch channel selection. @@ -3185,6 +3186,8 @@ def _validate_routing( if not isinstance(routing, dict): raise RequestError(400, "invalid_routing", "routing must be an object") allowed = {"channel", "latency_tolerant", "priority"} + if allow_candidate_controls: + allowed.update({"candidate_id", "exclude_candidate_ids"}) if allow_endpoint: allowed.add("endpoint") unknown = sorted(set(routing) - allowed) @@ -3231,6 +3234,50 @@ def _validate_routing( priority = routing.get("priority") if isinstance(priority, str) and priority.strip(): cleaned["priority"] = priority.strip().lower() + candidate_id = routing.get("candidate_id") + if candidate_id is not None: + if ( + not isinstance(candidate_id, str) + or not candidate_id.strip() + or candidate_id != candidate_id.strip() + ): + raise RequestError( + 400, "invalid_routing", "routing.candidate_id must be a non-empty string" + ) + cleaned["candidate_id"] = candidate_id.strip() + excluded = routing.get("exclude_candidate_ids") + if excluded is not None: + if not isinstance(excluded, list): + raise RequestError( + 400, + "invalid_routing", + "routing.exclude_candidate_ids must be an array of agent IDs", + ) + if any( + not isinstance(value, str) + or not value.strip() + or value != value.strip() + for value in excluded + ): + raise RequestError( + 400, + "invalid_routing", + "routing.exclude_candidate_ids must contain non-empty strings", + ) + normalized = [value.strip() for value in excluded] + if len(normalized) != len(set(normalized)): + raise RequestError( + 400, + "invalid_routing", + "routing.exclude_candidate_ids must contain unique agent IDs", + ) + cleaned["exclude_candidate_ids"] = normalized + if cleaned.get("candidate_id") in set(cleaned.get("exclude_candidate_ids", ())): + raise RequestError( + 400, + "invalid_routing", + "routing.candidate_id cannot also be excluded", + ) if "endpoint" in routing: endpoint = routing.get("endpoint") if not isinstance(endpoint, str) or not endpoint.strip(): @@ -3258,6 +3305,27 @@ def _validate_routing( return cleaned if cleaned else {} +def _validate_candidate_routing( + orchestrator: TaskOrchestrator, + routing: dict[str, Any] | None, + model_name: str, + *, + required_roles: tuple[str, ...] = ("worker",), + required_tags: tuple[str, ...] = (), +) -> None: + """Fail closed on candidate controls before any response bytes are sent.""" + try: + with orchestrator.candidate_routing_policy( + routing, + model_name=model_name, + required_roles=required_roles, + required_tags=required_tags, + ): + pass + except ValueError as exc: + raise RequestError(400, "invalid_routing", str(exc)) from exc + + def _validate_batch_requests( body: dict[str, Any], expose_trace: bool, *, zdr_only: bool ) -> list[BatchRequest]: @@ -5463,6 +5531,8 @@ def _orchestrated_response( response["usage_record_ids"] = result["usage_record_ids"] if result.get("cost") is not None: response["cost"] = result["cost"] + if result.get("candidate_routing") is not None: + response["orchestration"] = {"routing": result["candidate_routing"]} return response @@ -6571,7 +6641,7 @@ def do_POST(self) -> None: # noqa: N802 request_policy.__enter__() if path in {"/v1/chat/completions", "/v1/responses"}: endpoint_routing = _validate_routing( - body.get("routing"), allow_endpoint=True + body.get("routing"), allow_candidate_controls=True, allow_endpoint=True ) endpoint_policy = orchestrator.routing_endpoint_scope( endpoint_routing.get("endpoint") if endpoint_routing else None, @@ -6913,6 +6983,29 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di # proxy_completion pool match sees the same id as form/JS padded names. model_name = _validate_chat_model(body) _require_pool_model(orchestrator, model_name) + # Reuse the routing already validated once for endpoint + # scoping above instead of re-validating the same + # body.get("routing") with identical args (#983) -- + # _validate_routing is pure, so this is behavior-preserving + # and keeps the two call sites from drifting apart. + request_routing = endpoint_routing + # Preflight against the request's own required tags (e.g. an + # image-bearing message needs "vision") the same way the + # ordinary chat and orchestrated Responses branches do, so an + # incompatible pin fails closed here with invalid_routing + # instead of surfacing later as a generic execution error + # (#983). + structured_routing_messages = _validate_messages(body.get("messages")) + _validate_candidate_routing( + orchestrator, + request_routing, + model_name, + required_tags=( + ("vision",) + if orchestrator._source_image_parts(structured_routing_messages) + else () + ), + ) # Coerce stream early so stream_options fail-closed matches route path # and tools/response_format passthrough cannot skip type checks. stream = body.get("stream", False) @@ -7001,21 +7094,62 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di self._authorize_trace_access() started_at = time.perf_counter() if tool_loop: - proxied = self._run( - lambda: orchestrator.proxy_completion( - body, - endpoint="chat/completions", - single_agent=True, - ) - ) + def proxy_tool_request() -> dict[str, Any]: + with orchestrator.candidate_routing_policy( + request_routing, model_name=model_name + ): + result = orchestrator.proxy_completion( + body, + endpoint="chat/completions", + single_agent=True, + ) + evidence = result.pop("_candidate_routing", None) + # Only republish gateway-computed evidence, + # and never assume the provider's own + # "orchestration" field (if any) is a + # mapping -- both are untrusted provider + # response content (#983 Devin findings: + # "Provider fields forge routing evidence" + # and "Provider metadata crashes tool + # responses"). Without an active candidate + # control, proxy_completion() never sets + # `_candidate_routing` itself, so observing + # one here can only mean it arrived + # already-present on the provider's raw + # response body. + if evidence is not None and orchestrator._has_active_candidate_controls( + request_routing + ): + orchestration = result.get("orchestration") + if not isinstance(orchestration, dict): + orchestration = {} + result["orchestration"] = orchestration + orchestration["routing"] = evidence + return result + + proxied = self._run(proxy_tool_request) else: structured_messages = _validate_messages(body.get("messages")) - structured_routing = _validate_routing( - body.get("routing"), allow_endpoint=True + structured_routing = request_routing + # Candidate controls force synchronous execution the + # same way CostRoutingCoordinator.complete() does for + # ordinary chat (cost_router.py has_candidate_controls); + # only reject deferred batch hints when no candidate + # control is active to pin/exclude a candidate. + structured_has_candidate_controls = bool( + structured_routing + and ( + structured_routing.get("candidate_id") + or structured_routing.get("exclude_candidate_ids") + ) ) - if structured_routing and ( - structured_routing.get("channel") == "batch" - or structured_routing.get("latency_tolerant") is True + if ( + not structured_has_candidate_controls + and structured_routing + and ( + structured_routing.get("channel") == "batch" + or structured_routing.get("latency_tolerant") is True + ) ): raise RequestError( 400, @@ -7098,83 +7232,116 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di return messages = _validate_messages(body.get("messages")) mode = _validate_mode(body.get("orchestration") or body.get("orchestration_mode") or body.get("mode") or "auto") - route_stream = bool( - stream and orchestrator.would_route(messages, mode, model_name) - ) - if route_stream: - if explicit_trace: - raise RequestError( - 400, - "unsupported_trace_disclosure", - "remove include_orchestration_trace or use Responses streaming", - ) - include_trace = False - elif include_trace: - self._authorize_trace_access() - # stream + stream_options already coerced/validated before passthrough. - attribution = _validate_attribution(body.get("attribution")) - routing = _validate_routing( - body.get("routing"), allow_endpoint=True + _validate_candidate_routing( + orchestrator, + request_routing, + model_name, + required_roles=orchestrator.candidate_pin_required_roles( + mode, model_name + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(messages) + else () + ), ) - # Require model — silent default to contextual-orchestrator hid - # which deployment the caller selected on the chat Completions path. - # The pool was validated before the structured/passthrough - # branch so every chat shape shares the same client-error contract. - attribution = dict(attribution or {}) - # OpenAI chat ``user`` → account when unset. - # Same fail-closed rules as Completions: present key must be a - # non-empty string ≤64 chars (null omit; scalars coerce; empty reject). - end_user_id = _validate_completions_user(body) - if end_user_id is not None and not attribution.get("account"): - attribution["account"] = end_user_id - if model_name and not attribution.get("model_name"): - attribution["model_name"] = model_name - if not attribution.get("service"): - attribution["service"] = "chat_completions_api" - # sampling/controls already validated before passthrough branch. - if "metadata" in body: - _validate_openai_metadata(body) started_at = time.perf_counter() model_client = orchestrator.client - with model_client.request_settings( - max_output_tokens=max_tokens, - temperature=temperature, - top_p=top_p, - presence_penalty=presence_penalty, - frequency_penalty=frequency_penalty, + # This scope stays open across both the would_route triage + # decision and (for the streaming branch) the streamed + # provider execution, so both share one attempted-candidate + # list instead of the triage attempt being recorded in a + # scope that closes before the stream starts (see #983). + with orchestrator.candidate_routing_policy( + request_routing, model_name=model_name ): + route_stream = bool( + stream and orchestrator.would_route(messages, mode, model_name) + ) if route_stream: - self._stream_route_completion( - orchestrator, - security, - messages, - model_name, - include_usage=include_usage, - ) - orchestrator.record_analytics_event( - "chat_completion_requested", - { - "endpoint_path": "/v1/chat/completions", - "actor_scope": "inference", - "status_code": 200, - "run_mode": "route", - "duration_ms": round((time.perf_counter() - started_at) * 1000, 2), - "response_streamed": True, - }, - ) + if explicit_trace: + raise RequestError( + 400, + "unsupported_trace_disclosure", + "remove include_orchestration_trace or use Responses streaming", + ) + include_trace = False + with model_client.request_settings( + max_output_tokens=max_tokens, + temperature=temperature, + top_p=top_p, + presence_penalty=presence_penalty, + frequency_penalty=frequency_penalty, + ): + self._stream_route_completion( + orchestrator, + security, + messages, + model_name, + routing=request_routing, + include_usage=include_usage, + candidate_scope_open=True, + ) + orchestrator.record_analytics_event( + "chat_completion_requested", + { + "endpoint_path": "/v1/chat/completions", + "actor_scope": "inference", + "status_code": 200, + "run_mode": "route", + "duration_ms": round((time.perf_counter() - started_at) * 1000, 2), + "response_streamed": True, + }, + ) return - result = self._run(lambda: coordinator.complete( - messages, - mode=mode, - attribution=attribution, - hints=routing, - model_name=model_name, - workflow_run_id=f"run_{uuid.uuid4().hex}", - cache_bypass=cache_bypass, - cache_partition=cache_partition, - owner_id=security.principal_id(self.headers), - zdr_only=zdr_only, - )) + if include_trace: + self._authorize_trace_access() + # stream + stream_options already coerced/validated before passthrough. + attribution = _validate_attribution(body.get("attribution")) + routing = request_routing + # Require model — silent default to contextual-orchestrator hid + # which deployment the caller selected on the chat Completions path. + # The pool was validated before the structured/passthrough + # branch so every chat shape shares the same client-error contract. + attribution = dict(attribution or {}) + # OpenAI chat ``user`` → account when unset. + # Same fail-closed rules as Completions: present key must be a + # non-empty string ≤64 chars (null omit; scalars coerce; empty reject). + end_user_id = _validate_completions_user(body) + if end_user_id is not None and not attribution.get("account"): + attribution["account"] = end_user_id + if model_name and not attribution.get("model_name"): + attribution["model_name"] = model_name + if not attribution.get("service"): + attribution["service"] = "chat_completions_api" + # sampling/controls already validated before passthrough branch. + if "metadata" in body: + _validate_openai_metadata(body) + with model_client.request_settings( + max_output_tokens=max_tokens, + temperature=temperature, + top_p=top_p, + presence_penalty=presence_penalty, + frequency_penalty=frequency_penalty, + ): + # candidate_scope_open=True: this whole branch (including + # the would_route triage call above) runs inside the one + # candidate_routing_policy scope opened at the top of this + # block, so the triage attempt is not lost when it decides + # to conduct instead of route (#983). + result = self._run(lambda: coordinator.complete( + messages, + mode=mode, + attribution=attribution, + hints=routing, + model_name=model_name, + workflow_run_id=f"run_{uuid.uuid4().hex}", + cache_bypass=cache_bypass, + cache_partition=cache_partition, + owner_id=security.principal_id(self.headers), + zdr_only=zdr_only, + candidate_scope_open=True, + )) # Latency-tolerant requests get dispatched to the batch backend. if result.get("channel") == "batch": orchestrator.record_analytics_event( @@ -7498,6 +7665,15 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di # Fail-closed shape checks before passthrough so buyers never # get a 200 after shipping invalid OpenAI-shaped metadata/input. model_name = _validate_responses_model(body) + # Reuse the routing already validated once for endpoint + # scoping above instead of re-validating the same + # body.get("routing") with identical args (#983) -- + # _validate_routing is pure, so this is behavior-preserving + # and keeps the two call sites from drifting apart. + responses_routing_control = endpoint_routing + _validate_candidate_routing( + orchestrator, responses_routing_control, model_name + ) _validate_responses_conversation_controls(body) if "store" in body: _validate_responses_store(body) @@ -7611,17 +7787,36 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di if "metadata" in body: _validate_openai_metadata(body) if "routing" in body: - routing = _validate_routing( - body.get("routing"), allow_endpoint=True - ) + routing = responses_routing_control # Responses passthrough has no batch channel plane yet. - if routing and routing.get("channel") == "batch": + # Candidate controls force synchronous execution the + # same way CostRoutingCoordinator.complete() and + # structured chat do (cost_router.py + # has_candidate_controls); only reject deferred batch + # hints when no candidate control is active to pin or + # exclude a candidate (#983 finding 1). + responses_has_candidate_controls = bool( + routing + and ( + routing.get("candidate_id") + or routing.get("exclude_candidate_ids") + ) + ) + if ( + not responses_has_candidate_controls + and routing + and routing.get("channel") == "batch" + ): raise RequestError( 400, "invalid_routing", "routing.channel=batch is not supported on /v1/responses", ) - if routing and routing.get("latency_tolerant") is True: + if ( + not responses_has_candidate_controls + and routing + and routing.get("latency_tolerant") is True + ): raise RequestError( 400, "invalid_routing", @@ -7667,6 +7862,21 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di "file_provider_unavailable", "no common referenced file provider is available", ) + pinned = (responses_routing_control or {}).get("candidate_id") + excluded = set( + (responses_routing_control or {}).get( + "exclude_candidate_ids", () + ) + ) + if ( + (pinned is not None and pinned not in valid_agents) + or not (valid_agents - excluded) + ): + raise RequestError( + 400, + "invalid_routing", + "candidate controls are incompatible with referenced file providers", + ) body["_file_replicas"] = provider_ids # stream=false / omit → non-SSE JSON response (honest no-stream path). # stream=true is not implemented for Responses passthrough. @@ -7707,6 +7917,20 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di TaskOrchestrator.AUTO_MODEL, TaskOrchestrator.FREE_MODEL, } and stream: + messages = _responses_to_chat_payload(body)["messages"] + _validate_candidate_routing( + orchestrator, + responses_routing_control, + model_name, + required_roles=orchestrator.candidate_pin_required_roles( + "auto", model_name + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(messages) + else () + ), + ) if _responses_virtual_requires_provider_path(input_value, body): raise RequestError( 400, @@ -7733,7 +7957,6 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di "invalid_response_format", "structured output is not supported for streamed orchestrated Responses requests", ) - messages = _responses_to_chat_payload(body)["messages"] started_at = time.perf_counter() stream_succeeded = self._stream_orchestrated_response( orchestrator, @@ -7742,6 +7965,7 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di model_name, coordinator=coordinator, attribution=responses_attribution, + routing=responses_routing_control, ) orchestrator.record_analytics_event( "responses_orchestrated", @@ -7772,6 +7996,19 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di and not _responses_virtual_requires_provider_path(input_value, body) ): messages = _responses_to_chat_payload(body)["messages"] + _validate_candidate_routing( + orchestrator, + responses_routing_control, + model_name, + required_roles=orchestrator.candidate_pin_required_roles( + "auto", model_name + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(messages) + else () + ), + ) responses_attribution = dict( _validate_attribution(body.get("attribution")) or {} ) @@ -7780,11 +8017,7 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di responses_attribution["account"] = responses_user_id responses_attribution.setdefault("model_name", body["model"]) responses_attribution.setdefault("service", "responses_api") - responses_routing = dict( - _validate_routing( - body.get("routing"), allow_endpoint=True - ) or {} - ) + responses_routing = dict(responses_routing_control or {}) # Responses has no batch job envelope on this path; # force the coordinator's synchronous contract even # when a priority or token threshold would select batch. @@ -7855,6 +8088,27 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di started_at = time.perf_counter() tool_loop = bool(body.get("tools")) responses_messages = _responses_to_chat_payload(body)["messages"] + # The early role-only preflight above (before "input" was + # even parsed) cannot know whether this request carries an + # image; re-check now that the request's true required + # tags are known, the same way the orchestrated Responses + # branches above already do, so an incompatible pin fails + # closed here with invalid_routing instead of surfacing + # later as a generic execution error on this provider + # passthrough path (#983). + _validate_candidate_routing( + orchestrator, + responses_routing_control, + model_name, + required_roles=orchestrator.candidate_pin_required_roles( + "conduct", model_name + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(responses_messages) + else () + ), + ) response_max_tokens = next( ( body.get(key) @@ -7879,9 +8133,7 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di responses_messages, mode="conduct", attribution=responses_attribution, - hints=_validate_routing( - body.get("routing"), allow_endpoint=True - ), + hints=responses_routing_control, model_name=body["model"], provider_request=body, provider_endpoint="responses", @@ -8380,6 +8632,7 @@ def _stream_orchestrated_response( *, coordinator: Any = None, attribution: dict[str, Any] | None = None, + routing: dict[str, Any] | None = None, ) -> bool: """Stream orchestration as native Responses reasoning-summary events.""" response_id = f"resp_{uuid.uuid4().hex}" @@ -8464,27 +8717,39 @@ def progress(role: str, status: str) -> None: } emit("response.output_item.added", output_index=0, item=reasoning_item) try: - if orchestrator.would_route(messages, "auto", model_name): - progress("worker", "started") - workflow_run_id = f"run_{uuid.uuid4().hex}" - parts = list( - orchestrator.stream_route( - messages, - workflow_run_id=workflow_run_id, - model_name=model_name, - ) + candidate_scope = ( + orchestrator.candidate_routing_policy( + routing, model_name=model_name ) - progress("worker", "completed") - result = ( - orchestrator.get_workflow_run(workflow_run_id) - if coordinator is not None - else {"answer": "".join(parts)} - ) - else: - conduct_kwargs = {"model_name": model_name, "progress": progress} - if getattr(orchestrator.conduct, "__func__", None) is TaskOrchestrator.conduct: - conduct_kwargs["workflow_run_id"] = f"run_{uuid.uuid4().hex}" - result = orchestrator.conduct(messages, **conduct_kwargs) + if routing + else nullcontext() + ) + with candidate_scope: + if orchestrator.would_route(messages, "auto", model_name): + progress("worker", "started") + workflow_run_id = f"run_{uuid.uuid4().hex}" + parts = list( + orchestrator.stream_route( + messages, + workflow_run_id=workflow_run_id, + model_name=model_name, + ) + ) + progress("worker", "completed") + result = ( + orchestrator.get_workflow_run(workflow_run_id) + if coordinator is not None + else {"answer": "".join(parts)} + ) + else: + conduct_kwargs = {"model_name": model_name, "progress": progress} + if getattr(orchestrator.conduct, "__func__", None) is TaskOrchestrator.conduct: + conduct_kwargs["workflow_run_id"] = f"run_{uuid.uuid4().hex}" + result = orchestrator.conduct(messages, **conduct_kwargs) + evidence = orchestrator._candidate_routing_evidence(result) + if evidence is not None: + result = dict(result) + result["candidate_routing"] = evidence except ConnectionAbortedError: raise except ProviderUpstreamError as exc: @@ -8606,9 +8871,19 @@ def _stream_route_completion( messages: Any, model_name: str, *, + routing: dict[str, Any] | None = None, include_usage: bool = False, + candidate_scope_open: bool = False, ) -> None: - """Pipe live provider deltas as OpenAI chat-completion SSE frames.""" + """Pipe live provider deltas as OpenAI chat-completion SSE frames. + + ``candidate_scope_open=True`` tells this method the caller already + has a ``candidate_routing_policy`` scope active (e.g. spanning the + auto-mode ``would_route`` triage decision) and it must not open a + second, independent scope here — doing so would silently discard + the triage attempt recorded in the caller's scope before the + streamed worker attempt is recorded in a fresh one (see #983). + """ run_id = f"run_{uuid.uuid4().hex}" completion_id = _new_chat_completion_id() created = int(time.time()) @@ -8658,10 +8933,28 @@ def usage_frame(usage: dict[str, Any]) -> str: stream_kwargs.update( {"include_usage": True, "usage_callback": capture_usage} ) - for delta in orchestrator.stream_route(messages, **stream_kwargs): - if not self._write_sse(frame({"content": delta})): - return - if not self._write_sse(frame({}, finish="stop")): + candidate_scope = ( + nullcontext() + if candidate_scope_open or not routing + else orchestrator.candidate_routing_policy( + routing, model_name=model_name + ) + ) + with candidate_scope: + for delta in orchestrator.stream_route(messages, **stream_kwargs): + if not self._write_sse(frame({"content": delta})): + return + evidence = None + if routing: + record = orchestrator.get_workflow_run(run_id) + evidence = orchestrator._candidate_routing_evidence(record) + terminal_delta: dict[str, Any] = {} + terminal = frame(terminal_delta, finish="stop") + if evidence is not None: + terminal_payload = json.loads(terminal.removeprefix("data: ")) + terminal_payload["orchestration"] = {"routing": evidence} + terminal = f"data: {json.dumps(terminal_payload, ensure_ascii=False)}\n\n" + if not self._write_sse(terminal): return if ( include_usage diff --git a/docs/planning/adrs/0032-model-group-cost-aware-discovery.md b/docs/planning/adrs/0032-model-group-cost-aware-discovery.md index 201d141f4..843e3fbb1 100644 --- a/docs/planning/adrs/0032-model-group-cost-aware-discovery.md +++ b/docs/planning/adrs/0032-model-group-cost-aware-discovery.md @@ -153,6 +153,36 @@ OpenRouter. (2026). *Provider logging and data policies*. https://openrouter.ai/ OpenRouter. (2026). *Zero data retention enforcement*. https://openrouter.ai/docs/features/provider-routing#zero-data-retention-enforcement +## Request-local candidate control amendment (2026-09-01) + +Trusted sidecars sometimes possess stronger current failure evidence than the +gateway's process-local measurements. For one virtual-model request they may +pin an exact private agent ID with `routing.candidate_id` and exclude unique exact +IDs with `routing.exclude_candidate_ids`. Candidate membership has no repository-authored +cardinality cutoff; normal authenticated request-size controls remain the resource boundary. The controls are validated +before execution, never persisted, never expand `/v1/models`, and apply to the +same selection boundary for plain, structured, and streamed requests. A pin +that is unknown, disabled, non-chat, excluded, or incompatible with active ZDR +policy fails closed. Concrete model names cannot be combined with candidate +controls because two simultaneous routing authorities would be ambiguous. + +The response records requested, excluded, attempted, and served candidate IDs +under `orchestration.routing` only when a caller supplied the controls. This is +an operational override, not a learned-routing claim: RouteLLM and FrugalGPT +motivate evidence-based model routing, while this amendment only makes one +caller-held observation explicit and auditable. Ordinary omitted-control +behavior and response shape remain unchanged. + +A response-cache hit records an empty attempted-candidate list and omits the +served-candidate field because no provider served that request. Historical +trace rows remain available as cache provenance but are never reported as a +current request attempt. + +Redistributable research artifacts are already committed at +`docs/papers/routellm-routing-2406.18665.pdf` and +`docs/papers/frugalgpt-cost-2305.05176.pdf`; `docs/papers/README.md` records +their citations and provenance. + OpenRouter. (2026). *Create speech*. https://openrouter.ai/docs/api/api-reference/speech/create-audio-speech OpenRouter. (2026). *Image generation*. https://openrouter.ai/docs/guides/overview/multimodal/image-generation @@ -164,3 +194,14 @@ OpenRouter. (2026). *Submit a rerank request*. https://openrouter.ai/docs/api/ap OpenRouter. (2026). *Submit a video generation request*. https://openrouter.ai/docs/api/api-reference/video-generation/create-videos Ma, H., Lai, G., & Ye, H.-J. (2026). *MMR-Bench: A comprehensive benchmark for multimodal LLM routing* [Preprint]. arXiv. https://arxiv.org/abs/2601.17814 + +### No-heuristics amendment — 2026-09-02 + +The original request-local control used a fixed 32-ID exclusion ceiling and legacy +serving-identity recovery from output equality/trace position. Neither decision rule +was identified by RouteLLM, FrugalGPT, an API standard, or measured deployment +evidence. The cardinality ceiling is removed; authenticated request-size enforcement +is the resource boundary. Serving identity is now reported only from exact +`answering_step_id` or an explicit `served_agent_id`; historical rows without either +remain attempt provenance and omit `served_candidate_id`. This is a fail-closed +identity rule rather than an inferred ranking/tie-break. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index d145a0b1d..8d03c445d 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2680,3 +2680,107 @@ shows this is now occasional, not the dominant failure mode (most is an overall deadline on `_invoke`'s candidate/retry loop, not another timeout increase on the sidecar's client side — deferred rather than rushed into this heavily-tested core file without dedicated validation. + +## 2026-09-02 — PR #983 candidate-control no-heuristics repair + +Live RCA found two decision-affecting rules in the request-local candidate-control +owner: a repository-authored 32-ID exclusion ceiling and serving-candidate inference +from output equality/trace position when exact identity was absent. Neither had an +identified mathematical, standards, experimental, or research basis. The canonical +repair removes the cardinality rule, retaining normal authenticated request-size +controls, and makes serving identity fail closed unless exact `answering_step_id` or +explicit `served_agent_id` evidence exists. Regression coverage exercises more than +32 exclusions, missing identity, and explicit identity provenance. Exact-head hosted +checks remain authoritative before merge. + +## 2026-09-02 — PR #983 follow-up: unconditional `served_agent_id` regression + +A full local suite run of the above repair surfaced a real regression it introduced: +making `route_once`/`stream_route` stamp `served_agent_id` on every trace row +unconditionally (to give the no-heuristics evidence reader an explicit fact even for +an unchanged serving agent) broke a separate, pre-existing regression guard — +`test_provider_reliability.py`'s and `test_tool_execution_fallback.py`'s "the default +mock path must behave exactly as before: single attempt, no failover metadata" — by +adding `served_agent_id` to trace rows that must never carry it outside a failover. +Root-caused by diffing the failure against a clean `origin/main` worktree (no +failure) versus the PR head (regressed), confirming the stamp change was the exact +cause rather than a full-suite pollution artifact. Fixed by scoping the unconditional +stamp to only fire while request-local candidate-attempt tracking is active (inside a +`candidate_routing_policy` scope, detectable via +`_REQUEST_ATTEMPTED_CANDIDATE_IDS.get() is not None`), leaving the ordinary +no-candidate-policy path's trace shape unchanged. All 200 tests across the affected +files plus the full local suite (3355 passed, 2 pre-existing sandbox-only failures: +`fast_mlsirm` unavailable, `test_spend_analytics` local-tokenizer artifact) pass +clean. Lesson: a production fact-recording change made for one evidence consumer's +sake must be checked against every other consumer of the same trace shape, not just +the consumer it was written for. + +## 2026-09-02 — PR #983 follow-up: stale OpenAPI schema cardinality cutoff + +Owner-verified, exact-head finding: `contextual_orchestrator/api_contract.py`'s +published `CandidateRoutingControls.exclude_candidate_ids` schema still declared +`maxItems: 32` after the runtime's own repository-authored 32-ID cutoff had already +been removed as unsupported (see the first 2026-09-02 entry above). The PR body, +CHANGELOG, ADR direction, and Devin resolution all claimed the cutoff was gone, but +the schema — the actual source generated/OpenAPI clients build against — still +enforced it, so the documentation claim was false at the live source. Fixed by +removing `maxItems: 32` from the schema (no replacement cardinality heuristic; +`uniqueItems`, lexical ID constraints, and normal authenticated request-size bounds +are untouched) and adding a RED-before/GREEN-after regression +(`test_openapi_documents_compatibility_front_door` in `tests/test_api_contract.py`) +validating a 64-ID exclusion list against the schema, which fails against the old +`maxItems: 32` schema and passes against the corrected one. Verified the runtime +validator (`server.py`'s `_validate_routing`) has no other hidden count-based +cutoff on this field. Lesson: a schema/contract file is a second, independent +publication surface for the same invariant as runtime code — removing a rule from +one without checking the other leaves the claim false in whichever one still has it. + +## 2026-09-03 — PR #983 follow-up: base merge and a pre-existing judge-selection double-call + +Merged current `main` into the branch (clean, no conflicts) to pick up main's +`test_admin_contract.py` `import json` fix (main PR #1035) that this PR's stale base +predated. Hosted CI's "Full unit and contract suite" job (run `33692781067`) then +showed two of this PR's own new tests failing with an extra `worker_only` call in +`client.calls`: `test_http_auto_preflight_accepts_worker_only_pin_when_free_model_ +always_routes` and `test_coordinator_auto_route_only_pin_succeeds_for_free_model` +(both in `tests/test_candidate_routing_controls.py`). Neither test failed locally in +this or any earlier round, in isolation or full-suite, because this sandbox's +blocked `fast-mlsirm` GitHub-archive download (documented since the first +2026-09-02 entry above) always short-circuits `_model_judge_verification` to its +"fast-mlsirm judge is unavailable" fail-closed return before any judge is selected +or called — hiding a real, pre-existing (predates PR #983 entirely; present +unchanged at merge-base `212ff437`) selection bug that only a hosted run with +fast-mlsirm actually importable can exercise. Root cause: `_ranked_agents` +deliberately still returns role-ineligible members — it appends them after every +eligible one, per its own docstring — so a caller wanting only role-eligible +candidates must re-apply `role not in agent.provider_exclusions` itself, exactly as +`_plan_generated` and `_parse_workflow_plan` already do. `_model_judge_verification`'s +judge-selection `next(...)` was missing that filter, so with a single-candidate pool +excluded from `verifier` (PR #983's own new `orchestrator/free` worker-only +provable-route fixture), it picked that ineligible agent as judge anyway instead of +failing closed — an extra, unrequested live call. `_invoke`'s own failover path +already enforced this same exclusion for a *backup* judge +(`test_fast_mlsirm_judge_failover_honors_verifier_exclusions`); this closes the +identical gap for the *primary* selection. Fixed by adding +`if "verifier" not in agent.provider_exclusions` to the judge-selection generator in +`_model_judge_verification` (`contextual_orchestrator/orchestrator.py`). RED-before +confirmed by a stub `_resolve_fast_mlsirm_components` whose judge constructor +records the selected agent id: before the fix it recorded the sole, role-excluded +`worker_only` agent; after the fix `next(...)` raises `StopIteration` (caught by the +existing broad fail-closed handler) and the judge is never constructed +(`test_model_judge_never_selects_a_verifier_excluded_sole_candidate`, +`tests/test_model_judge.py`). GREEN: that regression plus +`test_fast_mlsirm_judge_failover_honors_verifier_exclusions` (2 passed); +`test_model_judge.py` + `test_candidate_routing_controls.py` + +`test_candidate_routing_no_heuristic_limits.py` + `test_api_contract.py` + +`test_admin_contract.py` (98 passed); full local suite (Python 3.12, matching CI's +`uv run` toolchain) 3441 passed, 2 pre-existing sandbox-only failures unrelated to +this change and already documented above (`fast_mlsirm` unavailable; +`test_spend_analytics`'s local-tokenizer artifact — the same missing-fast-mlsirm +mechanism, now also confirmed to flip an unrelated `spend_analytics` conduct-mode +trace's `usage_source` from the expected `mixed` to `tokenizer` when the judge step +never runs). `interrogate` on `orchestrator.py`: 100%. Lesson: a fail-closed branch +that this sandbox can only reach one way (dependency unavailable) can mask a real +selection bug in the branch that never runs locally; a caller of a "still includes +ineligible members, ranked last" helper must re-apply the eligibility filter at +every call site, not assume it inherited from one. diff --git a/fuzz/targets.py b/fuzz/targets.py index 415dad9f9..321b2bb54 100644 --- a/fuzz/targets.py +++ b/fuzz/targets.py @@ -37,7 +37,10 @@ ``opencode_zen``/``nvidia_nim``/``nvidia_nim_sub``/``openai`` rows (ADR 0041). Must never raise and must never return ``True`` unless every present monetary value is a valid non-negative finite zero. -11. ``rater_observation.RaterInvocation.from_mapping`` -- the governed rater +11. ``server._validate_routing`` -- request-local channel and candidate + controls. Successful candidate arrays are unique and non-empty, with no + repository-authored cardinality cutoff. +12. ``rater_observation.RaterInvocation.from_mapping`` -- the governed rater observation boundary. Arbitrary JSON must fail closed or round-trip to the same bounded published-language payload. 12. ``web_search._parse_results`` -- an untrusted SearXNG (or SearXNG-API- @@ -205,6 +208,23 @@ def exercise_request_body(raw: bytes) -> None: else: assert body.get("metadata") == metadata assert all(isinstance(value, str) for value in metadata.values()) + if "routing" in body: + try: + routing = server._validate_routing( + body["routing"], allow_candidate_controls=True + ) + except RequestError: + pass + else: + excluded = (routing or {}).get("exclude_candidate_ids", []) + # No repository-authored cardinality cutoff: the fixed 32-ID + # exclusion ceiling was removed from both the OpenAPI schema and + # this runtime validator (PR #983's no-heuristics correction). + # The normal authenticated request-size boundary is the only + # remaining limit, so this fuzz invariant must not reassert a + # stale cap the validator no longer enforces. + assert len(excluded) == len(set(excluded)) + assert all(isinstance(value, str) and value for value in excluded) # response_format.json_schema.name must match [a-zA-Z0-9_-]{1,64} ASCII. if "response_format" in body: diff --git a/tests/fuzz/test_fuzz_properties.py b/tests/fuzz/test_fuzz_properties.py index 416e647a1..be3425a25 100644 --- a/tests/fuzz/test_fuzz_properties.py +++ b/tests/fuzz/test_fuzz_properties.py @@ -89,6 +89,18 @@ def test_request_body_rejects_unhashable_message_role() -> None: exercise_request_body(b'{"messages":[{"role":[],"":[],"modnt":""}]}') +def test_request_body_accepts_more_than_32_valid_exclude_candidate_ids() -> None: + """The fixed 32-ID exclusion ceiling was removed from the runtime + validator and OpenAPI schema (PR #983's no-heuristics correction); the + normal authenticated request-size boundary is the only remaining limit. + This fuzz invariant must not reassert a cardinality cap the validator no + longer enforces, or it reports a false positive on legitimately valid + input.""" + excluded = [f"agent_{i}" for i in range(40)] + body = json.dumps({"routing": {"exclude_candidate_ids": excluded}}).encode("utf-8") + exercise_request_body(body) + + @_SETTINGS @given(_json_values) def test_agent_config_parser(value: object) -> None: diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py index 7beb3d698..0673bd611 100644 --- a/tests/test_api_contract.py +++ b/tests/test_api_contract.py @@ -58,6 +58,34 @@ def test_openapi_documents_compatibility_front_door() -> None: "content" ]["application/json"]["schema"] assert chat_schema["properties"]["include_orchestration_trace"]["type"] == "boolean" + routing_schema = OPENAPI_SPEC["components"]["schemas"]["CandidateRoutingControls"] + assert routing_schema["properties"]["candidate_id"]["minLength"] == 1 + exact_id_pattern = r"^\S(?:[^\r\n]*\S)?(?![\s\S])" + assert routing_schema["properties"]["candidate_id"]["pattern"] == exact_id_pattern + assert routing_schema["properties"]["exclude_candidate_ids"] == { + "type": "array", + "uniqueItems": True, + "items": { + "type": "string", + "minLength": 1, + "pattern": exact_id_pattern, + }, + } + for invalid in ( + {"candidate_id": "candidate_a\n"}, + {"exclude_candidate_ids": ["candidate_a\n"]}, + ): + with pytest.raises(ValidationError): + validate(instance=invalid, schema=routing_schema) + # exclude_candidate_ids has no repository-authored cardinality cutoff: the + # runtime accepts any number of unique exclusions bounded only by the + # normal authenticated request-body size limit, so the published schema + # must not reject a longer, otherwise-valid list either (#983). + long_exclusion_list = [f"candidate_{index}" for index in range(64)] + validate( + instance={"exclude_candidate_ids": long_exclusion_list}, + schema=routing_schema, + ) chat_response = OPENAPI_SPEC["components"]["schemas"]["ChatCompletionResponse"] assert chat_response["properties"]["usage"]["$ref"].endswith("AuthoritativeUsage") assert chat_response["properties"]["usage_measurement_status"]["enum"] == [ diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py new file mode 100644 index 000000000..ead6b6faf --- /dev/null +++ b/tests/test_candidate_routing_controls.py @@ -0,0 +1,2158 @@ +"""Stateless candidate pin and exclusion controls across OpenAI chat paths.""" + +from __future__ import annotations + +import dataclasses +import json +import threading +import urllib.error +import urllib.request + +import pytest + +from contextual_orchestrator import ModelAgent, TaskOrchestrator +from contextual_orchestrator.cost_router import CostRoutingCoordinator +from contextual_orchestrator.orchestrator import ModelClient, WorkflowStep +from contextual_orchestrator.server import ( + RequestError, + SecurityConfig, + _validate_routing, + build_server, +) + + +class _CandidateClient(ModelClient): + def __init__(self) -> None: + super().__init__() + self.calls: list[str] = [] + + def chat(self, agent, messages, effort_profile=None): + self.calls.append(agent.id) + if agent.id == "candidate_a": + raise RuntimeError("candidate a failed") + if messages and "workflow_required" in str(messages[0].get("content")): + return '{"workflow_required": false}' + return "candidate b" + + def stream_chat(self, agent, messages, **kwargs): + self.calls.append(agent.id) + yield "candidate b" + + def proxy_send_once(self, agent, endpoint, payload): + self.calls.append(agent.id) + if agent.id == "candidate_a": + raise urllib.error.HTTPError( + "https://provider.example/v1", 503, "unavailable", None, None + ) + return { + "id": "chatcmpl-candidate-b", + "object": "chat.completion", + "created": 1, + "model": agent.model, + "choices": [ + { + "index": 0, + "message": { + "role": "assistant", + "content": ( + '{"answer": "candidate b"}' + if isinstance(payload.get("response_format"), dict) + else "candidate b" + ), + }, + "finish_reason": "stop", + } + ], + } + + proxy_send = proxy_send_once + + +class _ConductTriageClient(_CandidateClient): + def chat(self, agent, messages, effort_profile=None): + self.calls.append(agent.id) + if messages and "workflow_required" in str(messages[0].get("content")): + return '{"workflow_required": true}' + return "candidate b" + + +class _DivergentTriageClient(ModelClient): + """Route-forcing triage reply from whichever agent is asked -- used to make + the free-only triage pool and the full worker pool resolve to two + genuinely different agents (see #983 finding 2).""" + + def __init__(self) -> None: + super().__init__() + self.calls: list[str] = [] + + def chat(self, agent, messages, temperature=None, top_p=None, effort_profile=None): + self.calls.append(agent.id) + if messages and "workflow_required" in str(messages[0].get("content")): + return '{"workflow_required": false}' + return "unexpected chat() call" # pragma: no cover - triage-only client + + def stream_chat(self, agent, messages, **kwargs): + self.calls.append(agent.id) + yield "streamed worker output" + + +def _post(port: int, token: str, body: dict) -> tuple[int, dict]: + request = urllib.request.Request( + f"http://127.0.0.1:{port}/v1/chat/completions", + data=json.dumps(body).encode(), + headers={ + "authorization": f"Bearer {token}", + "content-type": "application/json", + "connection": "close", + }, + method="POST", + ) + try: + with urllib.request.urlopen(request, timeout=5) as response: + return response.status, json.loads(response.read()) + except urllib.error.HTTPError as exc: + return exc.code, json.loads(exc.read()) + + +def _post_sse(port: int, token: str, body: dict) -> tuple[int, list[dict]]: + request = urllib.request.Request( + f"http://127.0.0.1:{port}/v1/chat/completions", + data=json.dumps(body).encode(), + headers={ + "authorization": f"Bearer {token}", + "content-type": "application/json", + "connection": "close", + }, + method="POST", + ) + with urllib.request.urlopen(request, timeout=5) as response: + events = [ + json.loads(line.removeprefix("data: ")) + for line in response.read().decode().splitlines() + if line.startswith("data: {") + ] + return response.status, events + + +def _post_responses( + port: int, token: str, body: dict, *, stream: bool = False +) -> tuple[int, dict | list[dict]]: + request = urllib.request.Request( + f"http://127.0.0.1:{port}/v1/responses", + data=json.dumps(body).encode(), + headers={ + "authorization": f"Bearer {token}", + "content-type": "application/json", + "connection": "close", + }, + method="POST", + ) + try: + with urllib.request.urlopen(request, timeout=5) as response: + raw = response.read().decode() + if stream: + return response.status, [ + json.loads(line.removeprefix("data: ")) + for line in raw.splitlines() + if line.startswith("data: {") + ] + return response.status, json.loads(raw) + except urllib.error.HTTPError as exc: + return exc.code, json.loads(exc.read()) + + +def _serve(): + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent("candidate_a", "model-a", provider_name="provider-a"), + ModelAgent("candidate_b", "model-b", provider_name="provider-b"), + ModelAgent("disabled_candidate", "model-disabled", disabled=True), + ], + client=client, + ) + token = "candidate-routing-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + return server, thread, token, client + + +def _tool_body(routing: dict | None = None, *, model: str = "orchestrator/auto") -> dict: + body = { + "model": model, + "messages": [{"role": "user", "content": "route this request"}], + "tools": [ + { + "type": "function", + "function": { + "name": "lookup", + "parameters": {"type": "object", "properties": {}}, + }, + } + ], + } + if routing is not None: + body["routing"] = routing + return body + + +def test_failed_pin_then_excluded_candidate_can_be_retried_with_a_new_pin() -> None: + server, thread, token, client = _serve() + try: + failed_status, _ = _post( + server.server_address[1], + token, + _tool_body({"candidate_id": "candidate_a"}), + ) + succeeded_status, succeeded = _post( + server.server_address[1], + token, + _tool_body( + { + "candidate_id": "candidate_b", + "exclude_candidate_ids": ["candidate_a"], + } + ), + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert failed_status == 503 + assert succeeded_status == 200 + assert client.calls == ["candidate_a", "candidate_b"] + assert succeeded["model"] == "model-b" + assert succeeded["orchestration"]["routing"] == { + "candidate_id": "candidate_b", + "exclude_candidate_ids": ["candidate_a"], + "attempted_candidate_ids": ["candidate_b"], + "served_candidate_id": "candidate_b", + } + + +def test_candidate_controls_fail_closed_and_omission_preserves_response_shape() -> None: + server, thread, token, _client = _serve() + try: + cases = ( + ({"candidate_id": "missing"}, "orchestrator/auto"), + ({"candidate_id": "disabled_candidate"}, "orchestrator/auto"), + ({"candidate_id": "candidate_b", "exclude_candidate_ids": ["candidate_b"]}, "orchestrator/auto"), + ({"exclude_candidate_ids": ["candidate_a", "candidate_a"]}, "orchestrator/auto"), + ({"candidate_id": "candidate_b"}, "model-b"), + ) + for routing, model in cases: + status, body = _post( + server.server_address[1], token, _tool_body(routing, model=model) + ) + assert status == 400, body + assert body["error"]["code"] == "invalid_routing" + + status, body = _post(server.server_address[1], token, _tool_body()) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200 + assert "orchestration" not in body + + +def test_candidate_controls_reject_an_unserviceable_worker_pool() -> None: + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_excluded", + "excluded-model", + provider_exclusions=("worker",), + ), + ModelAgent("worker_agent", "worker-model", tags=("cost:free",)), + ] + ) + + with pytest.raises(ValueError, match="eligible agent"): + with orchestrator.candidate_routing_policy( + {"candidate_id": "worker_excluded"} + ): + pass + with pytest.raises(ValueError, match="leaves no eligible agent"): + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["worker_agent"]}, + model_name="orchestrator/free", + ): + pass + + zdr_orchestrator = TaskOrchestrator( + [ + ModelAgent("plain_agent", "plain-model"), + ModelAgent("zdr_agent", "zdr-model", tags=("privacy:zdr",)), + ] + ) + with zdr_orchestrator.request_policy(True): + with pytest.raises(ValueError, match="leaves no eligible agent"): + with zdr_orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["zdr_agent"]} + ): + pass + + +def test_candidate_routing_policy_rejects_present_but_falsy_malformed_controls() -> None: + """A direct Python-API caller who passes a present-but-falsy malformed + control (empty string, False, explicit None, an empty mapping) must get + a loud ValueError, not a silent "no control" no-op -- the HTTP layer's + own ``_validate_routing`` already rejects these shapes with a 400 + before candidate_routing_policy ever sees them, but a caller who talks + to the orchestrator directly has no such gate (#983 finding 2).""" + orchestrator = TaskOrchestrator( + [ + ModelAgent("candidate_a", "model-a"), + ModelAgent("candidate_b", "model-b"), + ] + ) + + malformed_exclude_candidate_ids = ("", False, None, {}) + for value in malformed_exclude_candidate_ids: + with pytest.raises(ValueError, match="exclude_candidate_ids"): + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": value} + ): + pass + + # candidate_id=None must raise exactly like every other malformed-falsy + # shape (CodeRabbit finding on #983: the prior `candidate_id is not + # None` guard let an explicit None silently reach the no-op branch + # below instead of surfacing the caller bug, unlike + # exclude_candidate_ids=None above which already raised correctly). + for value in (False, 0, 1.5, [], {}, None): + with pytest.raises(ValueError, match="candidate_id"): + with orchestrator.candidate_routing_policy({"candidate_id": value}): + pass + + # Absent fields, an explicit None routing mapping, and an explicit + # empty list/tuple exclude_candidate_ids all remain no-ops -- the + # existing contract for "no control requested". An *absent* + # candidate_id key is the only candidate_id shape that is a no-op; an + # explicitly present candidate_id=None is not (see the loop above). + with orchestrator.candidate_routing_policy(None): + pass + with orchestrator.candidate_routing_policy({}): + pass + with orchestrator.candidate_routing_policy({"exclude_candidate_ids": []}): + pass + with orchestrator.candidate_routing_policy({"exclude_candidate_ids": ()}): + pass + + +def test_candidate_pin_preflight_checks_conduct_roles_and_required_tags() -> None: + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + provider_exclusions=("verifier",), + ), + ModelAgent("vision_agent", "vision-model", tags=("vision",)), + ] + ) + + with pytest.raises(ValueError, match="eligible agent"): + with orchestrator.candidate_routing_policy( + {"candidate_id": "worker_only"}, + required_roles=("thinker", "worker", "verifier", "synthesizer"), + ): + pass + with pytest.raises(ValueError, match="eligible agent"): + with orchestrator.candidate_routing_policy( + {"candidate_id": "worker_only"}, required_tags=("vision",) + ): + pass + with pytest.raises(ValueError, match="leaves no eligible agent"): + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["vision_agent"]}, + required_roles=("verifier",), + ): + pass + + +def test_http_conduct_preflight_rejects_role_ineligible_pin() -> None: + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + provider_exclusions=("verifier",), + ) + ], + client=client, + ) + token = "candidate-routing-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + responses = [ + _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "conduct this"}], + "mode": mode, + "routing": {"candidate_id": "worker_only"}, + }, + ) + for mode in ("conduct", "auto") + ] + finally: + server.shutdown() + thread.join(timeout=5) + + assert all(status == 400 for status, _body in responses) + assert all( + body["error"]["code"] == "invalid_routing" for _status, body in responses + ) + assert client.calls == [] + + +def test_http_auto_preflight_accepts_worker_only_pin_when_free_model_always_routes() -> None: + """``orchestrator/free`` auto mode never needs a live triage call to prove + the direct route: ``TaskOrchestrator.would_route`` short-circuits to + True from ``model_name`` alone for FREE_MODEL, so preflight can safely + require only the worker role -- unlike GATEWAY_DEFAULT_MODEL/AUTO_MODEL + auto requests (see ``test_http_conduct_preflight_rejects_role_ineligible_pin``), + where the route-vs-conduct decision genuinely requires a pin-scoped + triage provider call and preflight stays conservative instead of risking + it. Covers both /v1/chat/completions and non-streamed/streamed + /v1/responses so every auto-mode preflight site shares the contract. + """ + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + tags=("cost:free",), + provider_exclusions=("thinker", "verifier", "synthesizer"), + ) + ], + client=client, + ) + token = "candidate-routing-free-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + chat_status, chat_body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/free", + "messages": [{"role": "user", "content": "hello"}], + "mode": "auto", + "routing": {"candidate_id": "worker_only"}, + }, + ) + responses_status, responses_body = _post_responses( + server.server_address[1], + token, + { + "model": "orchestrator/free", + "input": "hello", + "routing": {"candidate_id": "worker_only"}, + }, + ) + stream_status, stream_events = _post_responses( + server.server_address[1], + token, + { + "model": "orchestrator/free", + "input": "hello", + "stream": True, + "routing": {"candidate_id": "worker_only"}, + }, + stream=True, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert chat_status == 200 + assert chat_body["choices"][0]["message"]["content"] == "candidate b" + assert chat_body["orchestration"]["mode"] == "route" + assert chat_body["orchestration"]["routing"]["served_candidate_id"] == "worker_only" + + assert responses_status == 200 + assert isinstance(responses_body, dict) + assert responses_body["output"][-1]["content"][0]["text"] == "candidate b" + + assert stream_status == 200 + assert isinstance(stream_events, list) and stream_events + + assert client.calls == ["worker_only", "worker_only", "worker_only"] + + +def test_coordinator_malformed_candidate_id_none_is_not_dropped_by_batch_routing() -> None: + """A direct ``CostRoutingCoordinator.complete`` caller (the Python API, + not HTTP -- ``server.py``'s own ``_validate_routing`` already normalizes + an explicit ``candidate_id: None`` to "missing" before this layer ever + sees it) who passes a malformed ``candidate_id: None`` alongside a batch + channel request must still get the loud ``ValueError`` from + ``candidate_routing_policy``, not a silently-accepted batch job envelope + that drops the malformed control entirely. ``has_candidate_controls`` + previously used truthiness (``bool(hints.get("candidate_id") or ...)``), + so ``candidate_id: None`` was indistinguishable from an absent key and + let the request take the early batch-channel return path before + validation ever ran (CodeRabbit finding on #983: "direct Python API + callers can lose or bypass routing validation").""" + orchestrator = TaskOrchestrator([ModelAgent("candidate_a", "model-a")]) + + with pytest.raises(ValueError, match="candidate_id"): + CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "hello"}], + mode="auto", + model_name=TaskOrchestrator.AUTO_MODEL, + hints={"candidate_id": None, "channel": "batch"}, + ) + + +def test_coordinator_explicit_empty_exclusion_still_takes_the_batch_path() -> None: + """Mirror/regression guard for the fix above: an explicit empty + ``exclude_candidate_ids: []`` remains a genuine no-op (not a malformed + value), so it must still be free to take the batch channel exactly as + it did before -- this is the earlier #983 fix ("빈 제외 목록을 후보 + 제어로 처리하지 마십시오") that the presence-based ``candidate_id`` + check above must not regress.""" + orchestrator = TaskOrchestrator([ModelAgent("candidate_a", "model-a")]) + + result = CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "hello"}], + mode="auto", + model_name=TaskOrchestrator.AUTO_MODEL, + hints={"exclude_candidate_ids": [], "channel": "batch"}, + ) + + assert result["channel"] == "batch" + + +def test_coordinator_auto_route_only_pin_succeeds_for_free_model() -> None: + """Direct ``CostRoutingCoordinator.complete`` callers get the same + provable-route carve-out as HTTP callers (see + ``test_http_auto_preflight_accepts_worker_only_pin_when_free_model_always_routes``): + FREE_MODEL's auto-mode decision is provider-free, so a worker-only pin + must not be rejected the way the GATEWAY_DEFAULT_MODEL/AUTO_MODEL case + still is in ``test_coordinator_auto_preflights_conduct_roles_before_triage``. + """ + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + tags=("cost:free",), + provider_exclusions=("thinker", "verifier", "synthesizer"), + ) + ], + client=client, + ) + + result = CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "hello"}], + mode="auto", + model_name="orchestrator/free", + hints={"candidate_id": "worker_only"}, + ) + + assert result["mode"] == "route" + assert result["candidate_routing"]["served_candidate_id"] == "worker_only" + assert client.calls == ["worker_only"] + + +def test_response_candidate_evidence_does_not_mutate_workflow_history() -> None: + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ModelAgent("candidate_b", "model-b", provider_name="provider-b")], + client=client, + ) + token = "candidate-routing-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "conduct this"}], + "mode": "conduct", + "routing": {"candidate_id": "candidate_b"}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200 + assert body["orchestration"]["routing"]["candidate_id"] == "candidate_b" + assert orchestrator._workflow_runs + assert all( + "candidate_routing" not in record + for record in orchestrator._workflow_runs.values() + ) + + +def test_generated_planner_uses_only_request_eligible_candidates(monkeypatch) -> None: + orchestrator = TaskOrchestrator( + [ + ModelAgent("candidate_a", "model-a"), + ModelAgent("candidate_b", "model-b"), + ] + ) + prompts: list[str] = [] + + def plan(_agent, messages, **_kwargs): + prompts.append(messages[0]["content"]) + return json.dumps( + { + "steps": [ + { + "id": 0, + "role": "worker", + "agent_id": "candidate_a", + "subtask": "work", + "access": [], + }, + { + "id": 1, + "role": "synthesizer", + "agent_id": "candidate_a", + "subtask": "answer", + "access": [0], + }, + ] + } + ) + + monkeypatch.setattr(orchestrator.client, "chat", plan) + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["candidate_a"]} + ): + steps = orchestrator._plan_generated("plan this") + + assert "candidate_a" not in prompts[0] + assert "candidate_b" in prompts[0] + assert {step.agent_id for step in steps} == {"candidate_b"} + + +def test_candidate_pin_is_honored_by_structured_and_streaming_chat_paths() -> None: + server, thread, token, client = _serve() + routing = {"candidate_id": "candidate_b", "exclude_candidate_ids": ["candidate_a"]} + try: + structured_status, structured = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "structured"}], + "response_format": {"type": "json_object"}, + "routing": routing, + }, + ) + stream_status, events = _post_sse( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "short"}], + "mode": "route", + "stream": True, + "routing": routing, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert structured_status == 200 + assert structured["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert stream_status == 200 + terminal = next(event for event in events if event.get("choices", [{}])[0].get("finish_reason") == "stop") + assert terminal["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert set(client.calls) == {"candidate_b"} + + +def test_structured_chat_with_active_candidate_pin_normalizes_batch_channel_to_sync() -> None: + """An active candidate_id pin forces sync execution instead of the flat + "batch routing is not supported" rejection, matching + CostRoutingCoordinator.complete's own has_candidate_controls precedence + (#983 finding 1).""" + server, thread, token, client = _serve() + try: + status, body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "structured"}], + "response_format": {"type": "json_object"}, + "routing": { + "candidate_id": "candidate_b", + "exclude_candidate_ids": ["candidate_a"], + "channel": "batch", + }, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200, body + assert "job_id" not in body + assert body["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert set(client.calls) == {"candidate_b"} + + +def test_structured_chat_with_active_candidate_exclusion_normalizes_latency_tolerant_to_sync() -> None: + """An active exclude_candidate_ids control forces sync execution instead + of rejecting routing.latency_tolerant=true outright (#983 finding 1).""" + server, thread, token, client = _serve() + try: + status, body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "structured"}], + "response_format": {"type": "json_object"}, + "routing": { + "exclude_candidate_ids": ["candidate_a"], + "latency_tolerant": True, + }, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200, body + assert "job_id" not in body + assert body["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert set(client.calls) == {"candidate_b"} + + +def test_candidate_pin_is_honored_by_responses_json_and_stream_paths() -> None: + server, thread, token, client = _serve() + routing = {"candidate_id": "candidate_b", "exclude_candidate_ids": ["candidate_a"]} + try: + json_status, json_body = _post_responses( + server.server_address[1], + token, + {"model": "orchestrator/auto", "input": "short", "routing": routing}, + ) + stream_status, stream_events = _post_responses( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "input": "short", + "stream": True, + "routing": routing, + }, + stream=True, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert json_status == 200 + assert isinstance(json_body, dict) + assert json_body["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert stream_status == 200 + assert isinstance(stream_events, list) + completed = next(event for event in stream_events if event["type"] == "response.completed") + assert completed["response"]["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert set(client.calls) == {"candidate_b"} + + +def test_responses_with_active_candidate_pin_normalizes_batch_channel_to_sync() -> None: + """An active candidate_id pin forces sync execution instead of the flat + "routing.channel=batch is not supported on /v1/responses" rejection, for + both the JSON and streaming Responses paths -- matching the same + precedence already applied to structured chat (#983 finding 1).""" + server, thread, token, client = _serve() + routing = { + "candidate_id": "candidate_b", + "exclude_candidate_ids": ["candidate_a"], + "channel": "batch", + } + try: + json_status, json_body = _post_responses( + server.server_address[1], + token, + {"model": "orchestrator/auto", "input": "short", "routing": routing}, + ) + stream_status, stream_events = _post_responses( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "input": "short", + "stream": True, + "routing": routing, + }, + stream=True, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert json_status == 200, json_body + assert isinstance(json_body, dict) + assert "job_id" not in json_body + assert json_body["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert stream_status == 200 + assert isinstance(stream_events, list) + completed = next(event for event in stream_events if event["type"] == "response.completed") + assert completed["response"]["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert set(client.calls) == {"candidate_b"} + + +def test_responses_with_active_candidate_exclusion_normalizes_latency_tolerant_to_sync() -> None: + """An active exclude_candidate_ids control forces sync execution instead + of rejecting routing.latency_tolerant=true outright, for both the JSON + and streaming Responses paths (#983 finding 1).""" + server, thread, token, client = _serve() + routing = {"exclude_candidate_ids": ["candidate_a"], "latency_tolerant": True} + try: + json_status, json_body = _post_responses( + server.server_address[1], + token, + {"model": "orchestrator/auto", "input": "short", "routing": routing}, + ) + stream_status, stream_events = _post_responses( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "input": "short", + "stream": True, + "routing": routing, + }, + stream=True, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert json_status == 200, json_body + assert isinstance(json_body, dict) + assert "job_id" not in json_body + assert json_body["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert stream_status == 200 + assert isinstance(stream_events, list) + completed = next(event for event in stream_events if event["type"] == "response.completed") + assert completed["response"]["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + assert set(client.calls) == {"candidate_b"} + + +def test_responses_preflight_rejects_conduct_ineligible_pin_before_provider_calls() -> None: + client = _ConductTriageClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + provider_exclusions=("verifier",), + ) + ], + client=client, + ) + token = "candidate-routing-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + responses = [ + _post_responses( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "input": "conduct this", + "stream": stream, + "routing": {"candidate_id": "worker_only"}, + }, + stream=stream, + ) + for stream in (False, True) + ] + finally: + server.shutdown() + thread.join(timeout=5) + + assert all(status == 400 for status, _body in responses) + assert all( + isinstance(body, dict) and body["error"]["code"] == "invalid_routing" + for _status, body in responses + ) + assert client.calls == [] + + +def test_auto_stream_triage_and_completion_both_honor_the_pin() -> None: + server, thread, token, client = _serve() + try: + status, events = _post_sse( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "auto route"}], + "stream": True, + "routing": { + "candidate_id": "candidate_b", + "exclude_candidate_ids": ["candidate_a"], + }, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200 + assert events + assert len(client.calls) >= 2 + assert set(client.calls) == {"candidate_b"} + + +def test_auto_stream_shares_candidate_scope_with_triage_when_triage_and_worker_differ() -> None: + """The would_route triage decision and the streamed worker call must + share one candidate-routing scope so the free triage agent's attempt is + not discarded from terminal streamed evidence (#983 finding 2). + + The free-only triage pool contains only ``triage_agent`` while the full + worker pool ranks ``worker_agent`` first (higher operator priority), so + the agent the triage call attempts and the agent that actually streams + the answer are genuinely different -- a shape + test_auto_stream_triage_and_completion_both_honor_the_pin cannot + exercise, since a candidate_id pin forces both calls onto one agent. + """ + client = _DivergentTriageClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent("triage_agent", "model-triage", tags=("cost:free",), priority=10), + ModelAgent("worker_agent", "model-worker", priority=20), + ModelAgent("excluded_agent", "model-excluded"), + ], + client=client, + ) + token = "divergent-triage-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, events = _post_sse( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "short auto request"}], + "stream": True, + "routing": {"exclude_candidate_ids": ["excluded_agent"]}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200 + assert set(client.calls) == {"triage_agent", "worker_agent"} + terminal = next( + event + for event in events + if event.get("choices", [{}])[0].get("finish_reason") == "stop" + ) + routing_evidence = terminal["orchestration"]["routing"] + assert routing_evidence["served_candidate_id"] == "worker_agent" + assert set(routing_evidence["attempted_candidate_ids"]) == { + "triage_agent", + "worker_agent", + } + + +class _StreamedConductTriageClient(ModelClient): + """Triage reports "needs conduct"; every subsequent conduct-step call + returns text distinguishable by agent id.""" + + def __init__(self) -> None: + super().__init__() + self.calls: list[str] = [] + + def chat(self, agent, messages, temperature=None, top_p=None, effort_profile=None): + self.calls.append(agent.id) + if messages and "workflow_required" in str(messages[0].get("content")): + return '{"workflow_required": true}' + return f"{agent.id} output" + + +def test_auto_stream_shares_candidate_scope_with_triage_when_conducting() -> None: + """When a streamed auto-mode request's triage decides it needs the + conduct workflow (not the direct route), server.py falls through past + the ``with orchestrator.candidate_routing_policy(...)`` block that ran + the triage call and into ``CostRoutingCoordinator.complete()`` -- which, + absent ``candidate_scope_open=True``, would open its own independent + scope and silently discard the triage agent's already-recorded attempt. + This mirrors test_auto_stream_shares_candidate_scope_with_triage_when_triage_and_worker_differ + above, but for the conduct branch instead of the route branch (#983, + "Conducted streams omit triage attempts").""" + client = _StreamedConductTriageClient() + orchestrator = TaskOrchestrator( + [ + # Lower priority than every conduct-role agent below, so the + # free-only triage ranking picks it (it's the only free_only + # candidate) while the general (non-free) ranking used for + # every conduct role always prefers the higher-priority agents + # instead -- otherwise triage_agent could win a conduct role + # too and appear in attempted_candidate_ids regardless of + # whether the scope-sharing fix under test is applied. + ModelAgent("triage_agent", "model-triage", tags=("cost:free",), priority=10), + ModelAgent("thinker_agent", "model-thinker", priority=20), + ModelAgent("worker_agent", "model-worker", priority=20), + ModelAgent("verifier_agent", "model-verifier", priority=20), + ModelAgent("synth_agent", "model-synth", priority=20), + ModelAgent("excluded_agent", "model-excluded", priority=20), + ], + client=client, + ) + token = "conduct-triage-scope-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, events = _post_sse( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "please conduct this"}], + "stream": True, + "routing": {"exclude_candidate_ids": ["excluded_agent"]}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200 + assert "triage_agent" in client.calls + terminal = next( + event + for event in events + if event.get("choices", [{}])[0].get("finish_reason") == "stop" + ) + routing_evidence = terminal["orchestration"]["routing"] + assert "triage_agent" in routing_evidence["attempted_candidate_ids"] + + +class _ZdrConductTriageClient(ModelClient): + """Triage always reports "needs conduct"; every subsequent conduct-step + call returns text distinguishable by agent id.""" + + def __init__(self) -> None: + super().__init__() + self.calls: list[str] = [] + + def chat(self, agent, messages, temperature=None, top_p=None, effort_profile=None): + self.calls.append(agent.id) + if messages and "workflow_required" in str(messages[0].get("content")): + return '{"workflow_required": true}' + return f"{agent.id} output" + + +def test_auto_zdr_only_paid_pin_still_gets_a_live_triage_decision() -> None: + """#983 Devin finding "ZDR pins skip workflow triage": + ``_compute_triage_verdict``'s free-only ranking is always empty when + ``zdr_only`` pins a paid candidate -- the pin restricts every candidate + list to that one agent, and a paid agent never satisfies ``free_only``. + The old ``if not candidates and not _REQUEST_ZDR_ONLY.get()`` guard then + skipped the eligible-general-chat-pool fallback entirely just because + ZDR was active, even though that fallback's own per-agent filter + (``_zdr_agent_allowed`` + ``_request_candidate_allowed``) already makes + it safe to consult: it silently returned ``False`` (route, not conduct) + with zero provider calls and zero routing evidence instead of ever + asking the one legitimate ZDR-eligible, pin-matching candidate. This + pins mode="auto" actually reaching a live triage decision against the + pinned candidate, and the resulting conduct verdict being honored (a + full multi-step workflow runs, not the single-call route path).""" + client = _ZdrConductTriageClient() + orchestrator = TaskOrchestrator( + [ModelAgent("paid_zdr_agent", "paid-zdr-model", tags=("privacy:zdr",))], + client=client, + ) + token = "zdr-pin-triage-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "please conduct this"}], + "zdr_only": True, + "routing": {"candidate_id": "paid_zdr_agent"}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200, body + # Triage genuinely called the pinned candidate instead of being skipped. + assert "paid_zdr_agent" in client.calls + # The conduct verdict was honored: more than the one call route_once + # would have made, and the pinned candidate is the only one attempted + # (a single-candidate pool proves every conduct role also had to use it). + assert len(client.calls) > 1 + routing_evidence = body["orchestration"]["routing"] + assert routing_evidence["candidate_id"] == "paid_zdr_agent" + assert routing_evidence["attempted_candidate_ids"] == ["paid_zdr_agent"] + + +def test_core_rejects_file_affinity_that_conflicts_with_pin() -> None: + orchestrator = TaskOrchestrator( + [ + ModelAgent("candidate_a", "model-a"), + ModelAgent("candidate_b", "model-b"), + ] + ) + with orchestrator.candidate_routing_policy( + {"candidate_id": "candidate_b"}, model_name="orchestrator/auto" + ): + try: + orchestrator.proxy_completion( + { + "model": "orchestrator/auto", + "input": "file request", + "_required_agent_id": "candidate_a", + }, + endpoint="responses", + ) + except RuntimeError as exc: + assert "required file provider" in str(exc) + else: # pragma: no cover - security regression + raise AssertionError("conflicting file affinity was accepted") + + +def test_candidate_keys_are_rejected_on_unsupported_routing_surfaces() -> None: + with pytest.raises(RequestError) as error: + _validate_routing({"candidate_id": "candidate_b"}) + assert error.value.code == "invalid_routing" + assert _validate_routing( + {"exclude_candidate_ids": []}, allow_candidate_controls=True + ) == {"exclude_candidate_ids": []} + for routing in ( + {"candidate_id": " candidate_b"}, + {"exclude_candidate_ids": ["candidate_a", " candidate_a"]}, + ): + with pytest.raises(RequestError) as exact_id_error: + _validate_routing(routing, allow_candidate_controls=True) + assert exact_id_error.value.code == "invalid_routing" + + +def test_attempt_evidence_keeps_failed_candidate_before_success() -> None: + server, thread, token, client = _serve() + try: + status, body = _post( + server.server_address[1], + token, + _tool_body({"exclude_candidate_ids": ["disabled_candidate"]}), + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200 + assert client.calls == ["candidate_a", "candidate_b"] + assert body["orchestration"]["routing"]["attempted_candidate_ids"] == [ + "candidate_a", + "candidate_b", + ] + assert body["orchestration"]["routing"]["served_candidate_id"] == "candidate_b" + + +def test_cache_hit_reports_no_current_candidate_attempt() -> None: + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ModelAgent("candidate_b", "model-b", provider_name="provider-b")], + client=client, + cache_ttl=60, + ) + messages = [{"role": "user", "content": "cached candidate request"}] + + with orchestrator.candidate_routing_policy({"candidate_id": "candidate_b"}): + first = orchestrator.complete(messages, mode="route") + first_evidence = orchestrator._candidate_routing_evidence(first) + with orchestrator.candidate_routing_policy({"candidate_id": "candidate_b"}): + second = orchestrator.complete(messages, mode="route") + second_evidence = orchestrator._candidate_routing_evidence(second) + + assert first["cache_status"] == "miss" + assert first_evidence["attempted_candidate_ids"] == ["candidate_b"] + assert first_evidence["served_candidate_id"] == "candidate_b" + assert second["cache_status"] == "hit" + assert second_evidence == { + "candidate_id": "candidate_b", + "exclude_candidate_ids": [], + "attempted_candidate_ids": [], + } + + +def test_conduct_pin_rejects_candidate_excluded_from_required_role() -> None: + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent("worker_only", "model-worker", provider_exclusions=("verifier",)), + ModelAgent("all_roles", "model-all"), + ], + client=client, + ) + token = "candidate-conduct-token" + server = build_server(orchestrator, port=0, security=SecurityConfig(auth_token=token)) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "orchestration": "conduct", + "messages": [{"role": "user", "content": "conduct this"}], + "routing": {"candidate_id": "worker_only"}, + }, + ) + route_status, _ = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "orchestration": "route", + "messages": [{"role": "user", "content": "route this"}], + "routing": {"candidate_id": "worker_only"}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 400 + assert body["error"]["code"] == "invalid_routing" + assert route_status == 200 + assert client.calls and set(client.calls) == {"worker_only"} + + +def test_response_routing_evidence_is_not_persisted_in_workflow_history() -> None: + orchestrator = TaskOrchestrator( + [ModelAgent("candidate_b", "model-b")], client=_CandidateClient() + ) + response = CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "route this"}], + mode="route", + model_name="orchestrator/auto", + workflow_run_id="run_candidate_evidence", + hints={"candidate_id": "candidate_b"}, + ) + + assert response["candidate_routing"]["candidate_id"] == "candidate_b" + assert "candidate_routing" not in orchestrator.get_workflow_run( + "run_candidate_evidence" + ) + + +def test_coordinator_auto_preflights_conduct_roles_before_triage() -> None: + """Direct coordinator callers get the same provider-free auto preflight + as HTTP callers: auto may select conduct, so a pin must satisfy every + conduct role before model-backed triage is allowed to run. + """ + client = _ConductTriageClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + provider_exclusions=("verifier",), + ) + ], + client=client, + ) + + with pytest.raises(ValueError, match="eligible agent"): + CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "conduct this"}], + mode="auto", + model_name="orchestrator/auto", + hints={"candidate_id": "worker_only"}, + ) + + assert client.calls == [] + + +def test_coordinator_plain_proxy_preflights_only_worker_role() -> None: + """Plain provider requests must not require conduct-only role eligibility.""" + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + provider_exclusions=("verifier",), + ) + ], + client=_CandidateClient(), + ) + observed: list[bool] = [] + + def proxy_completion(*_args, **_kwargs): + observed.append(orchestrator._request_candidate_allowed(orchestrator.agents[0])) + return { + "model": "worker-model", + "usage": {"prompt_tokens": 1, "completion_tokens": 1}, + "orchestration": {"workflow_run_id": "run_plain_candidate"}, + } + + orchestrator.proxy_completion = proxy_completion # type: ignore[method-assign] + orchestrator.get_workflow_run = lambda _run_id: { # type: ignore[method-assign] + "workflow_run_id": "run_plain_candidate", + "mode": "route", + "answer": "worker answer", + "trace": [], + } + + result = CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "plain proxy"}], + mode="auto", + model_name="orchestrator/auto", + hints={"candidate_id": "worker_only"}, + provider_request={ + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "plain proxy"}], + }, + ) + + assert observed == [True] + assert result["orchestration"]["workflow_run_id"] == "run_plain_candidate" + + +def test_coordinator_never_republishes_provider_supplied_candidate_routing_without_active_controls() -> None: + """A raw provider response that happens to already contain a + ``_candidate_routing`` key (an unrelated/coincidental or adversarial + provider extension sharing the gateway's own internal sentinel field + name) must never be republished as gateway-computed + ``orchestration.routing`` evidence when this request had no active + candidate control. ``TaskOrchestrator.proxy_completion`` only ever sets + that key itself when ``_candidate_routing_evidence`` actually ran and + returned non-None (which requires an active control), so observing the + key here with no active control can only mean it arrived on the + provider's own untrusted response body (#983 Devin finding: "Provider + fields forge routing evidence").""" + orchestrator = TaskOrchestrator( + [ModelAgent("worker_only", "worker-model")], + client=_CandidateClient(), + ) + + def proxy_completion(*_args, **_kwargs): + return { + "model": "worker-model", + "usage": {"prompt_tokens": 1, "completion_tokens": 1}, + "orchestration": {"workflow_run_id": "run_forged_evidence"}, + # An adversarial/coincidental provider-supplied field sharing + # the gateway's own internal sentinel name. + "_candidate_routing": { + "served_candidate_id": "attacker_controlled_agent", + "attempted_candidate_ids": ["attacker_controlled_agent"], + "exclude_candidate_ids": [], + }, + } + + orchestrator.proxy_completion = proxy_completion # type: ignore[method-assign] + orchestrator.get_workflow_run = lambda _run_id: { # type: ignore[method-assign] + "workflow_run_id": "run_forged_evidence", + "mode": "route", + "answer": "worker answer", + "trace": [], + } + + result = CostRoutingCoordinator(orchestrator).complete( + [{"role": "user", "content": "plain proxy"}], + mode="auto", + model_name="orchestrator/auto", + provider_request={ + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "plain proxy"}], + }, + ) + + assert "routing" not in result["orchestration"] + + +def test_http_tool_loop_never_republishes_provider_supplied_candidate_routing_or_crashes() -> None: + """Mirror of the coordinator-level forging test above, but for the HTTP + single-agent tool-loop passthrough (server.py's ``proxy_tool_request``), + which additionally must not crash when the provider's own + "orchestration" field (if any) is not a mapping -- both are untrusted + provider response content the gateway must never trust or blindly merge + into (#983 Devin findings: "Provider fields forge routing evidence" and + "Provider metadata crashes tool responses").""" + + class _ForgedProviderEvidenceClient(_CandidateClient): + def proxy_send_once(self, agent, endpoint, payload): + response = super().proxy_send_once(agent, endpoint, payload) + # Adversarial/coincidental provider fields sharing the + # gateway's own internal sentinel names. + response["_candidate_routing"] = { + "served_candidate_id": "attacker_controlled_agent", + "attempted_candidate_ids": ["attacker_controlled_agent"], + "exclude_candidate_ids": [], + } + response["orchestration"] = "not-a-mapping" + return response + + proxy_send = proxy_send_once + + client = _ForgedProviderEvidenceClient() + orchestrator = TaskOrchestrator( + [ModelAgent("worker_only", "worker-model")], + client=client, + ) + token = "forged-evidence-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, body = _post(server.server_address[1], token, _tool_body()) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 200, body + # No active candidate control was requested, so the gateway must never + # have touched the provider's own (malformed) "orchestration" field -- + # not crashed on it, and not overwritten it with forged evidence. + assert body.get("orchestration") == "not-a-mapping" + assert "attacker_controlled_agent" not in json.dumps(body) + + +def test_endpoint_scoped_preflight_rejects_pin_and_exclusion_conflicts() -> None: + """routing.endpoint plus a candidate_id/exclude_candidate_ids naming an + agent configured on a *different* endpoint must fail the same 400 + invalid_routing preflight as any other ineligible pin -- never pass + preflight and only then blow up as a selection RuntimeError/500.""" + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ + ModelAgent("candidate_a", "model-a", base_url="https://a.example/v1"), + ModelAgent("candidate_b", "model-b", base_url="https://b.example/v1"), + ], + client=client, + ) + token = "candidate-endpoint-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + for routing in ( + # candidate_b is not reachable on endpoint A: a pin conflict. + {"endpoint": "https://a.example", "candidate_id": "candidate_b"}, + # candidate_a is the only agent reachable on endpoint A: excluding + # it leaves nothing selection could actually serve from A. + {"endpoint": "https://a.example", "exclude_candidate_ids": ["candidate_a"]}, + ): + chat_status, chat_body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "hi"}], + "routing": routing, + }, + ) + assert chat_status == 400, chat_body + assert chat_body["error"]["code"] == "invalid_routing", chat_body + + responses_status, responses_body = _post_responses( + server.server_address[1], + token, + {"model": "orchestrator/auto", "input": "hi", "routing": routing}, + ) + assert responses_status == 400, responses_body + assert ( + responses_body["error"]["code"] == "invalid_routing" + ), responses_body + finally: + server.shutdown() + thread.join(timeout=5) + + assert client.calls == [] + + +def test_generated_planner_attempt_is_recorded_even_when_unused_by_steps() -> None: + """The generated planner call is itself a real provider call; routing + evidence must include that candidate even when the plan it returns never + reassigns it to a later role.""" + orchestrator = TaskOrchestrator( + [ + ModelAgent("planner_agent", "model-planner"), + ModelAgent( + "worker_agent", "model-worker", provider_exclusions=("thinker",) + ), + ModelAgent("excluded_agent", "model-excluded"), + ] + ) + + def plan(_agent, _messages, **_kwargs): + return json.dumps( + { + "steps": [ + { + "id": 0, + "role": "worker", + "agent_id": "worker_agent", + "subtask": "work", + "access": [], + }, + { + "id": 1, + "role": "synthesizer", + "agent_id": "worker_agent", + "subtask": "answer", + "access": [0], + }, + ] + } + ) + + orchestrator.client.chat = plan # type: ignore[method-assign] + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + steps = orchestrator._plan_generated("plan this") + evidence = orchestrator._candidate_routing_evidence({"trace": []}) + + # worker_agent is the only agent eligible for "thinker" once + # worker_agent's own provider_exclusions rule it out, so planner_agent + # is deterministically the planner -- and never appears in a step. + assert {step.agent_id for step in steps} == {"worker_agent"} + assert evidence["attempted_candidate_ids"] == ["planner_agent"] + + +def test_served_candidate_id_matches_verifier_rejected_worker_fallback( + monkeypatch, +) -> None: + """conduct() (template plan) can serve the worker's output -- not the + synthesizer's -- when a required verifier rejects the synthesized + result. Every step's provider call still lands in the trace, so routing + evidence must report whichever candidate actually produced ``answer``, + not whichever step happened to execute last.""" + + class _AgentIdClient(ModelClient): + def chat(self, agent, messages, effort_profile=None): + return f"{agent.id} output" + + orchestrator = TaskOrchestrator( + [ + ModelAgent("thinker_agent", "model-thinker"), + ModelAgent("worker_agent", "model-worker"), + ModelAgent("verifier_agent", "model-verifier"), + ModelAgent("synth_agent", "model-synth"), + ModelAgent("excluded_agent", "model-excluded"), + ], + client=_AgentIdClient(), + ) + monkeypatch.setattr( + orchestrator, + "_plan", + lambda task, *, model_name=TaskOrchestrator.GATEWAY_DEFAULT_MODEL: [ + WorkflowStep(0, "thinker", "thinker_agent", "decompose"), + WorkflowStep(1, "worker", "worker_agent", "work", (0,)), + WorkflowStep(2, "verifier", "verifier_agent", "verify", (0, 1)), + WorkflowStep(3, "synthesizer", "synth_agent", "synthesize", (0, 1, 2)), + ], + ) + monkeypatch.setattr( + orchestrator, + "_model_judge_verification", + lambda *args, **kwargs: { + "accepted": False, + "reason": "test forces rejection", + "verifier_output": "verifier_agent output", + "judge": "model", + }, + ) + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + result = orchestrator.conduct([{"role": "user", "content": "task"}]) + evidence = orchestrator._candidate_routing_evidence(result) + + assert [row["agent_id"] for row in result["trace"]] == [ + "thinker_agent", + "worker_agent", + "verifier_agent", + "synth_agent", + ] + assert result["answer"] == "worker_agent output" + assert evidence["served_candidate_id"] == "worker_agent" + + +def test_served_candidate_id_matches_generated_worker_fallback(monkeypatch) -> None: + """Same fallback identity guarantee as the template-plan case above, but + for a generated plan whose final step is a synthesizer distinct from the + worker whose output actually gets served on rejection.""" + + class _AgentIdClient(ModelClient): + def chat(self, agent, messages, effort_profile=None): + return f"{agent.id} output" + + orchestrator = TaskOrchestrator( + [ + ModelAgent("worker_agent", "model-worker"), + ModelAgent("verifier_agent", "model-verifier"), + ModelAgent("synth_agent", "model-synth"), + ModelAgent("excluded_agent", "model-excluded"), + ], + client=_AgentIdClient(), + ) + orchestrator.policy = dataclasses.replace( + orchestrator.policy, workflow_planning="generated" + ) + monkeypatch.setattr( + orchestrator, + "_plan_generated", + lambda task: [ + WorkflowStep(0, "worker", "worker_agent", "work"), + WorkflowStep(1, "verifier", "verifier_agent", "verify", (0,)), + WorkflowStep(2, "synthesizer", "synth_agent", "synthesize", (0, 1)), + ], + ) + monkeypatch.setattr( + orchestrator, + "_model_judge_verification", + lambda *args, **kwargs: { + "accepted": False, + "reason": "test forces rejection", + "verifier_output": "verifier_agent output", + "judge": "model", + }, + ) + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + result = orchestrator.conduct([{"role": "user", "content": "task"}]) + evidence = orchestrator._candidate_routing_evidence(result) + + assert result["plan_source"] == "generated" + assert [row["agent_id"] for row in result["trace"]] == [ + "worker_agent", + "verifier_agent", + "synth_agent", + ] + assert result["answer"] == "worker_agent output" + assert evidence["served_candidate_id"] == "worker_agent" + + +def test_served_candidate_id_prefers_earliest_row_on_duplicate_fallback_text( + monkeypatch, +) -> None: + """When a *later* synthesizer step happens to emit byte-identical text to + the worker's already-served fallback answer, routing evidence must still + name the worker (the candidate actually served) and not the synthesizer, + whose trace row merely duplicates the text after the fact. The fallback + worker step always runs -- and is recorded in the trace -- strictly + before any step whose output could coincidentally match it, so preferring + the earliest text match is safe (#983 finding 3, the minimal fix: the + prior ``reversed()`` walk preferred the *latest* text match and could + misidentify a coincidental duplicate as the served candidate).""" + + class _CollidingClient(ModelClient): + def chat(self, agent, messages, effort_profile=None): + if agent.id == "synth_agent": + # Deliberately duplicate the worker's fallback text so two + # trace rows share the exact same "output" as `answer`. + return "worker_agent output" + return f"{agent.id} output" + + orchestrator = TaskOrchestrator( + [ + ModelAgent("thinker_agent", "model-thinker"), + ModelAgent("worker_agent", "model-worker"), + ModelAgent("verifier_agent", "model-verifier"), + ModelAgent("synth_agent", "model-synth"), + ModelAgent("excluded_agent", "model-excluded"), + ], + client=_CollidingClient(), + ) + monkeypatch.setattr( + orchestrator, + "_plan", + lambda task, *, model_name=TaskOrchestrator.GATEWAY_DEFAULT_MODEL: [ + WorkflowStep(0, "thinker", "thinker_agent", "decompose"), + WorkflowStep(1, "worker", "worker_agent", "work", (0,)), + WorkflowStep(2, "verifier", "verifier_agent", "verify", (0, 1)), + WorkflowStep(3, "synthesizer", "synth_agent", "synthesize", (0, 1, 2)), + ], + ) + monkeypatch.setattr( + orchestrator, + "_model_judge_verification", + lambda *args, **kwargs: { + "accepted": False, + "reason": "test forces rejection", + "verifier_output": "verifier_agent output", + "judge": "model", + }, + ) + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + result = orchestrator.conduct([{"role": "user", "content": "task"}]) + evidence = orchestrator._candidate_routing_evidence(result) + + assert [row["agent_id"] for row in result["trace"]] == [ + "thinker_agent", + "worker_agent", + "verifier_agent", + "synth_agent", + ] + # Both the worker row (index 1) and the synthesizer row (index 3) now + # carry output == "worker_agent output" -- the fallback answer -- but + # only the worker was actually served. + assert result["trace"][1]["output"] == "worker_agent output" + assert result["trace"][3]["output"] == "worker_agent output" + assert result["answer"] == "worker_agent output" + assert evidence["served_candidate_id"] == "worker_agent" + + +def test_served_candidate_id_matches_later_step_on_duplicate_accepted_text( + monkeypatch, +) -> None: + """Mirror image of the fallback-duplicate case above: here the verifier + *accepts* the synthesizer's output, so the synthesizer -- not the worker + -- is the genuinely served candidate, even though the synthesizer's + output happens to duplicate the worker's earlier text byte-for-byte. No + earliest/latest ordering over text matches can be correct for both this + case and the fallback case simultaneously: routing evidence must resolve + the served row from ``answering_step_id`` (the step conduct() actually + served), not from which row's text happens to match first (#983, + "Duplicate outputs misidentify served candidate" -- the exact + mirror-image of the fallback-duplicate fix above).""" + + class _CollidingClient(ModelClient): + def chat(self, agent, messages, effort_profile=None): + if agent.id in ("worker_agent", "synth_agent"): + # Deliberately duplicate text across an earlier and a later + # step so two trace rows share the exact same "output". + return "duplicate output" + return f"{agent.id} output" + + orchestrator = TaskOrchestrator( + [ + ModelAgent("thinker_agent", "model-thinker"), + ModelAgent("worker_agent", "model-worker"), + ModelAgent("verifier_agent", "model-verifier"), + ModelAgent("synth_agent", "model-synth"), + ModelAgent("excluded_agent", "model-excluded"), + ], + client=_CollidingClient(), + ) + monkeypatch.setattr( + orchestrator, + "_plan", + lambda task, *, model_name=TaskOrchestrator.GATEWAY_DEFAULT_MODEL: [ + WorkflowStep(0, "thinker", "thinker_agent", "decompose"), + WorkflowStep(1, "worker", "worker_agent", "work", (0,)), + WorkflowStep(2, "verifier", "verifier_agent", "verify", (0, 1)), + WorkflowStep(3, "synthesizer", "synth_agent", "synthesize", (0, 1, 2)), + ], + ) + monkeypatch.setattr( + orchestrator, + "_model_judge_verification", + lambda *args, **kwargs: { + "accepted": True, + "reason": "test forces acceptance", + "verifier_output": "verifier_agent output", + "judge": "model", + }, + ) + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + result = orchestrator.conduct([{"role": "user", "content": "task"}]) + evidence = orchestrator._candidate_routing_evidence(result) + + assert [row["agent_id"] for row in result["trace"]] == [ + "thinker_agent", + "worker_agent", + "verifier_agent", + "synth_agent", + ] + # Both the worker row (index 1) and the synthesizer row (index 3) carry + # output == "duplicate output" -- the accepted answer -- but only the + # synthesizer (the later, genuinely served step) actually produced it. + assert result["trace"][1]["output"] == "duplicate output" + assert result["trace"][3]["output"] == "duplicate output" + assert result["answer"] == "duplicate output" + assert result["answering_step_id"] == 3 + assert evidence["served_candidate_id"] == "synth_agent" + + +def test_run_persists_answering_step_id_for_candidate_routing_evidence( + monkeypatch, +) -> None: + """``TaskOrchestrator.run()`` persists a workflow-run record built from + ``conduct()``'s result; it must carry ``answering_step_id`` through, or + every ``run()``-based caller that resolves routing evidence from the + persisted record -- chiefly ``CostRoutingCoordinator.complete()`` in + cost_router.py, the coordinator/HTTP ordinary chat and non-streamed + Responses path -- loses that identity and falls back to the fragile + text-match heuristic, which can misattribute a duplicate-text answer to + an earlier step (#983 Devin finding: "Duplicate outputs misidentify + serving candidate"). This is the same scenario as + ``test_served_candidate_id_matches_later_step_on_duplicate_accepted_text`` + above, but exercised through ``run()`` -- the exact record-construction + boundary the finding named -- instead of calling ``conduct()`` + directly.""" + + class _CollidingClient(ModelClient): + def chat(self, agent, messages, effort_profile=None): + if agent.id in ("worker_agent", "synth_agent"): + # Deliberately duplicate text across an earlier and a later + # step so two trace rows share the exact same "output". + return "duplicate output" + return f"{agent.id} output" + + orchestrator = TaskOrchestrator( + [ + ModelAgent("thinker_agent", "model-thinker"), + ModelAgent("worker_agent", "model-worker"), + ModelAgent("verifier_agent", "model-verifier"), + ModelAgent("synth_agent", "model-synth"), + ModelAgent("excluded_agent", "model-excluded"), + ], + client=_CollidingClient(), + ) + monkeypatch.setattr( + orchestrator, + "_plan", + lambda task, *, model_name=TaskOrchestrator.GATEWAY_DEFAULT_MODEL: [ + WorkflowStep(0, "thinker", "thinker_agent", "decompose"), + WorkflowStep(1, "worker", "worker_agent", "work", (0,)), + WorkflowStep(2, "verifier", "verifier_agent", "verify", (0, 1)), + WorkflowStep(3, "synthesizer", "synth_agent", "synthesize", (0, 1, 2)), + ], + ) + monkeypatch.setattr( + orchestrator, + "_model_judge_verification", + lambda *args, **kwargs: { + "accepted": True, + "reason": "test forces acceptance", + "verifier_output": "verifier_agent output", + "judge": "model", + }, + ) + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + record = orchestrator.run( + [{"role": "user", "content": "task"}], mode="conduct" + ) + evidence = orchestrator._candidate_routing_evidence(record) + + assert [row["output"] for row in record["trace"]][1] == "duplicate output" + assert [row["output"] for row in record["trace"]][3] == "duplicate output" + assert record["answer"] == "duplicate output" + assert record["answering_step_id"] == 3 + assert evidence is not None + assert evidence["served_candidate_id"] == "synth_agent" + + +def test_candidate_routing_evidence_fails_closed_without_answering_step_id() -> None: + """A historical workflow without explicit serving identity must not infer one.""" + + orchestrator = TaskOrchestrator( + [ + ModelAgent("worker_agent", "model-worker"), + ModelAgent("excluded_agent", "model-excluded"), + ] + ) + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + orchestrator._record_candidate_attempt("worker_agent") + evidence = orchestrator._candidate_routing_evidence( + { + "answer": "worker_agent output", + "trace": [ + {"id": 0, "agent_id": "worker_agent", "output": "worker_agent output"}, + ], + } + ) + + assert evidence is not None + assert "served_candidate_id" not in evidence + + +def test_orchestrated_provider_completion_answering_step_id_identifies_synthesis_over_duplicate_internal_step() -> None: + """The structured/tool-loop path built by ``_orchestrated_provider_completion`` + persists an internal ``conduct()`` workflow plus its own final synthesis + row. When the synthesizer's output happens to duplicate an earlier + internal workflow step's text byte-for-byte, routing evidence must still + resolve to the synthesizer that actually served the response -- not to + the internal step it duplicates -- by recording ``answering_step_id`` on + the persisted workflow record, mirroring ``conduct()``'s own fix (#983 + Devin finding: "Repeated output misidentifies served candidate").""" + agents = [ + ModelAgent("synth_agent", "synth-model", "mock://catalog"), + ModelAgent("excluded_agent", "excluded-model", "mock://catalog"), + ] + orchestrator = TaskOrchestrator(agents) + orchestrator._select_agent = lambda *_args, **_kwargs: agents[0] # type: ignore[method-assign] + orchestrator._failover_candidates = lambda *_args, **_kwargs: [agents[0]] # type: ignore[method-assign] + orchestrator.conduct = lambda *_args, **_kwargs: { # type: ignore[method-assign] + "trace": [ + { + "id": 0, + "role": "worker", + "agent_id": "internal_worker", + "output": "duplicate text", + }, + ], + "verification": None, + } + + def send(agent, _endpoint, _payload): + return {"choices": [{"message": {"content": "duplicate text"}}]} + + orchestrator.client.proxy_send_once = send + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + result = orchestrator._orchestrated_provider_completion( + { + "model": TaskOrchestrator.AUTO_MODEL, + "messages": [{"role": "user", "content": "task"}], + }, + endpoint="chat/completions", + effort_profile=None, + ) + workflow_id = result["orchestration"]["workflow_run_id"] + persisted = orchestrator.get_workflow_run(workflow_id) + evidence = orchestrator._candidate_routing_evidence(persisted) + + assert [row["output"] for row in persisted["trace"]] == [ + "duplicate text", + "duplicate text", + ] + assert persisted["answer"] == "duplicate text" + assert persisted["answering_step_id"] == persisted["trace"][1]["id"] + assert persisted["trace"][1]["agent_id"] == "synth_agent" + assert evidence is not None + assert evidence["served_candidate_id"] == "synth_agent" + + +def test_orchestrated_provider_completion_answering_step_id_identifies_repair_over_duplicate_internal_step() -> None: + """Mirror of the synthesis-duplicate case above for the repair branch: + when the caller's ``response_format`` rejects the first synthesis + attempt and the *repair* retry succeeds with output that duplicates an + earlier internal workflow step's text, routing evidence must resolve to + the repair row (the one that actually produced ``answer``), not the + internal step it duplicates. No earliest/latest text-match ordering can + be correct for both this case and the synthesis-duplicate case + simultaneously -- exactly why ``answering_step_id`` must point at the + repair row here (#983 Devin finding: "Repeated output misidentifies + served candidate", suggested-fix case: "synthesis and repair output + duplicate an earlier workflow step").""" + agents = [ + ModelAgent("synth_agent", "synth-model", "mock://catalog"), + ModelAgent("excluded_agent", "excluded-model", "mock://catalog"), + ] + orchestrator = TaskOrchestrator(agents) + orchestrator._select_agent = lambda *_args, **_kwargs: agents[0] # type: ignore[method-assign] + orchestrator._failover_candidates = lambda *_args, **_kwargs: [agents[0]] # type: ignore[method-assign] + orchestrator.conduct = lambda *_args, **_kwargs: { # type: ignore[method-assign] + "trace": [ + { + "id": 0, + "role": "worker", + "agent_id": "internal_worker", + "output": '{"final": "true"}', + }, + ], + "verification": None, + } + + calls: list[str] = [] + + def send(agent, _endpoint, _payload): + calls.append(agent.id) + if len(calls) == 1: + return {"choices": [{"message": {"content": "not valid json"}}]} + return {"choices": [{"message": {"content": '{"final": "true"}'}}]} + + orchestrator.client.proxy_send_once = send + + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + result = orchestrator._orchestrated_provider_completion( + { + "model": TaskOrchestrator.AUTO_MODEL, + "messages": [{"role": "user", "content": "task"}], + "response_format": {"type": "json_object"}, + }, + endpoint="chat/completions", + effort_profile=None, + ) + workflow_id = result["orchestration"]["workflow_run_id"] + persisted = orchestrator.get_workflow_run(workflow_id) + evidence = orchestrator._candidate_routing_evidence(persisted) + + assert calls == ["synth_agent", "synth_agent"] + assert [row["role"] for row in persisted["trace"]] == [ + "worker", + "synthesizer", + "repair", + ] + assert persisted["trace"][0]["output"] == '{"final": "true"}' + assert persisted["trace"][1]["output"] == "not valid json" + assert persisted["trace"][2]["output"] == '{"final": "true"}' + assert persisted["answer"] == '{"final": "true"}' + assert persisted["answering_step_id"] == persisted["trace"][2]["id"] + assert persisted["trace"][2]["agent_id"] == "synth_agent" + assert evidence is not None + assert evidence["served_candidate_id"] == "synth_agent" + + +def test_http_structured_chat_preflight_rejects_vision_incompatible_pin() -> None: + """A structured (response_format) chat request that carries an image must + fail closed with invalid_routing when pinned to a candidate that lacks + the "vision" tag, the same way the ordinary chat path already does -- + this branch previously validated only roles, so the incompatible pin + would have surfaced later as a generic execution error instead (#983).""" + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ModelAgent("worker_only", "worker-model")], + client=client, + ) + token = "structured-vision-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, body = _post( + server.server_address[1], + token, + { + "model": TaskOrchestrator.AUTO_MODEL, + "messages": [ + { + "role": "user", + "content": [ + {"type": "text", "text": "inspect"}, + { + "type": "image_url", + "image_url": {"url": "data:image/png;base64,AA=="}, + }, + ], + } + ], + "response_format": {"type": "json_object"}, + "routing": {"candidate_id": "worker_only"}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 400, body + assert body["error"]["code"] == "invalid_routing" + assert client.calls == [] + + +def test_http_responses_provider_path_preflight_rejects_vision_incompatible_pin() -> None: + """The raw single-agent /v1/responses provider passthrough (reached when + a request is not eligible for orchestrated Responses handling, e.g. it + carries response_format) must also fail closed with invalid_routing on a + vision-incompatible pin -- this path previously ran only the early + role-only preflight (#983).""" + client = _CandidateClient() + orchestrator = TaskOrchestrator( + [ModelAgent("worker_only", "worker-model")], + client=client, + ) + token = "responses-vision-token" + server = build_server( + orchestrator, port=0, security=SecurityConfig(auth_token=token) + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + status, body = _post_responses( + server.server_address[1], + token, + { + "model": TaskOrchestrator.AUTO_MODEL, + "input": [ + { + "type": "message", + "role": "user", + "content": [ + {"type": "input_text", "text": "inspect"}, + { + "type": "input_image", + "image_url": "data:image/png;base64,AA==", + }, + ], + } + ], + "response_format": {"type": "json_object"}, + "routing": {"candidate_id": "worker_only"}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 400, body + assert body["error"]["code"] == "invalid_routing" + assert client.calls == [] diff --git a/tests/test_candidate_routing_no_heuristic_limits.py b/tests/test_candidate_routing_no_heuristic_limits.py new file mode 100644 index 000000000..96a8dccdd --- /dev/null +++ b/tests/test_candidate_routing_no_heuristic_limits.py @@ -0,0 +1,67 @@ +"""No-heuristics contracts for request-local candidate controls.""" + +from contextual_orchestrator import ModelAgent, TaskOrchestrator +from contextual_orchestrator.server import _validate_routing + + +def test_exclusion_membership_has_no_repository_authored_cardinality_cutoff() -> None: + """Request membership is bounded by normal request parsing, not a magic route count.""" + excluded = [f"candidate_{index}" for index in range(40)] + routing = _validate_routing( + {"exclude_candidate_ids": excluded}, allow_candidate_controls=True + ) + assert routing == {"exclude_candidate_ids": excluded} + + orchestrator = TaskOrchestrator( + [ModelAgent(candidate, f"model-{index}") for index, candidate in enumerate(excluded)] + + [ModelAgent("remaining_candidate", "remaining-model")] + ) + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": excluded}, model_name=TaskOrchestrator.AUTO_MODEL + ): + assert orchestrator._request_candidate_allowed(orchestrator._agent("remaining_candidate")) + + +def test_served_candidate_evidence_fails_closed_without_explicit_identity() -> None: + """Never infer the serving candidate from output equality or trace position.""" + orchestrator = TaskOrchestrator( + [ModelAgent("worker_agent", "worker-model"), ModelAgent("excluded_agent", "excluded-model")] + ) + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + orchestrator._record_candidate_attempt("worker_agent") + evidence = orchestrator._candidate_routing_evidence( + { + "answer": "duplicate output", + "trace": [ + {"id": 0, "agent_id": "worker_agent", "output": "duplicate output"} + ], + } + ) + assert evidence is not None + assert evidence["attempted_candidate_ids"] == ["worker_agent"] + assert "served_candidate_id" not in evidence + + +def test_explicit_served_identity_remains_auditable_without_answering_step_id() -> None: + """Provider-shaped paths may report a serving identity when they record it explicitly.""" + orchestrator = TaskOrchestrator( + [ModelAgent("worker_agent", "worker-model"), ModelAgent("excluded_agent", "excluded-model")] + ) + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["excluded_agent"]} + ): + orchestrator._record_candidate_attempt("worker_agent") + evidence = orchestrator._candidate_routing_evidence( + { + "trace": [ + { + "agent_id": "worker_agent", + "served_agent_id": "worker_agent", + } + ] + } + ) + assert evidence is not None + assert evidence["served_candidate_id"] == "worker_agent" diff --git a/tests/test_endpoint_race.py b/tests/test_endpoint_race.py index b1f859ad2..d5f4576ec 100644 --- a/tests/test_endpoint_race.py +++ b/tests/test_endpoint_race.py @@ -212,6 +212,77 @@ def chat(agent: ModelAgent, _messages: list[dict], **_kwargs: object) -> str: assert result["answer"] == "completed by fast_endpoint" +def test_race_attempts_all_reach_candidate_routing_evidence() -> None: + """Every candidate a race actually calls survives into routing evidence. + + Each ``race_first_valid`` attempt runs ``call(agent)`` inside a + ``copy_context().run(...)`` worker-thread context, and + ``_record_candidate_attempt`` records the call by mutating the *list + object* the ``_REQUEST_ATTEMPTED_CANDIDATE_IDS`` ContextVar was bound to + before the race started (``candidate_routing_policy`` sets it once, + up front) -- it never rebinds the ContextVar itself with ``.set()``. + ``copy_context()`` only isolates ``ContextVar.set()``/``reset()`` calls + made inside the copy; it is a shallow copy that keeps every already-set + variable pointing at the exact same value object, so an in-place + mutation of that shared list from a raced worker thread is visible from + the parent request context once the race returns. This asserts that + guarantee holds for every candidate whose provider call actually ran, + not just the winner or a candidate that happened to run on the main + thread. + """ + raw_contract = dict(contract(capability_set=("text",)).__dict__) + agents = [ + ModelAgent( + endpoint_id, "provider/shared", tags=("reasoning",), + group_name="shared_text_group", endpoint_equivalence=raw_contract, + ) + for endpoint_id in ("slow_endpoint", "fast_endpoint") + ] + [ModelAgent("decoy_agent", "decoy-model", tags=("reasoning",))] + orchestrator = TaskOrchestrator(agents) + started: list[str] = [] + loser_started = threading.Event() + release_loser = threading.Event() + + def chat(agent: ModelAgent, _messages: list[dict], **_kwargs: object) -> str: + started.append(agent.id) + if agent.id == "slow_endpoint": + # Signal that this attempt's _record_candidate_attempt has + # already run (it runs before chat() in call()), then block so + # the winner cannot be declared before this call is provably + # in flight -- a genuine timing overlap, not a coincidence of + # which thread the OS scheduler happened to start first. + loser_started.set() + release_loser.wait(2) + else: + assert loser_started.wait(2), "slow_endpoint never started its call" + return f"completed by {agent.id}" + + orchestrator.client.chat = chat # type: ignore[method-assign] + try: + # An exclusion (rather than a candidate_id pin) activates the same + # request-scoped attempt-tracking ContextVar production traffic + # uses, without narrowing the race down to a single allowed + # candidate. + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["decoy_agent"]}, + model_name=orchestrator.GATEWAY_DEFAULT_MODEL, + ): + result = orchestrator.route_once( + [{"role": "user", "content": "use the declared replica group"}], + model_name="shared-text-group", + ) + evidence = orchestrator._candidate_routing_evidence(result) + finally: + release_loser.set() + + # Both race members actually ran (the fast one wins while the slow one + # is still mid-flight) -- both must be attempted. + assert set(started) == {"slow_endpoint", "fast_endpoint"} + assert evidence is not None + assert set(evidence["attempted_candidate_ids"]) == set(started) + assert evidence["served_candidate_id"] == "fast_endpoint" + + def test_text_race_preserves_request_scoped_sampling_and_token_limits() -> None: raw_contract = dict(contract(capability_set=("text",)).__dict__) agents = [ diff --git a/tests/test_measured_routing_evidence.py b/tests/test_measured_routing_evidence.py index 5a0c35a30..4a7814ee8 100644 --- a/tests/test_measured_routing_evidence.py +++ b/tests/test_measured_routing_evidence.py @@ -129,6 +129,62 @@ def test_mock_embedding_vectors_are_deterministic() -> None: assert len(first) == orchestrator.client.MOCK_EMBEDDING_DIMENSION +def test_embed_cached_records_candidate_attempt_only_on_provider_call() -> None: + """``_embed_cached`` calls ``client.embed`` directly (not through + ``_invoke``, which self-instruments), so it must record its own attempt -- + but only when a provider call actually happens: a cache hit must not + falsely appear in ``attempted_candidate_ids`` evidence (#983 Devin + finding: "Attempt evidence omits provider calls").""" + orchestrator = _orch( + ModelAgent("embedding_member", "mock-embed", tags=("embedding",)), + ModelAgent("worker_agent", "mock-worker", tags=("reasoning",)), + ModelAgent("other_worker_agent", "mock-worker-2", tags=("reasoning",)), + ) + + # An embedding-only agent is not a general chat agent, so it cannot be + # pinned via routing.candidate_id -- excluding an unrelated candidate + # instead is enough to activate evidence collection (a second general + # chat agent stays eligible so the exclusion preflight itself passes). + with orchestrator.candidate_routing_policy({"exclude_candidate_ids": ["worker_agent"]}): + first = orchestrator._embed_cached("some task text") + first_evidence = orchestrator._candidate_routing_evidence({"trace": []}) + with orchestrator.candidate_routing_policy({"exclude_candidate_ids": ["worker_agent"]}): + second = orchestrator._embed_cached("some task text") + second_evidence = orchestrator._candidate_routing_evidence({"trace": []}) + + assert first is not None and first == second + assert first_evidence["attempted_candidate_ids"] == ["embedding_member"] + assert second_evidence["attempted_candidate_ids"] == [] + + +def test_descriptor_vector_cached_records_candidate_attempt_only_on_provider_call() -> None: + """Mirror of the task-vector case above for the per-agent descriptor + embedding cache used by declaration-only affinity ranking (#983 Devin + finding: "Attempt evidence omits provider calls").""" + embedding_member = ModelAgent("embedding_member", "mock-embed", tags=("embedding",)) + worker = ModelAgent("worker_agent", "mock-worker", tags=("reasoning",)) + other_worker = ModelAgent("other_worker_agent", "mock-worker-2", tags=("reasoning",)) + orchestrator = _orch(embedding_member, worker, other_worker) + + # An embedding-only agent is not a general chat agent, so it cannot be + # pinned via routing.candidate_id -- excluding an unrelated candidate + # instead is enough to activate evidence collection. + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["other_worker_agent"]} + ): + first = orchestrator._descriptor_vector_cached(worker) + first_evidence = orchestrator._candidate_routing_evidence({"trace": []}) + with orchestrator.candidate_routing_policy( + {"exclude_candidate_ids": ["other_worker_agent"]} + ): + second = orchestrator._descriptor_vector_cached(worker) + second_evidence = orchestrator._candidate_routing_evidence({"trace": []}) + + assert first is not None and first == second + assert first_evidence["attempted_candidate_ids"] == ["embedding_member"] + assert second_evidence["attempted_candidate_ids"] == [] + + def test_no_embedding_member_disables_affinity_entirely() -> None: agent = ModelAgent("plain_agent", "mock", tags=("reasoning",)) orchestrator = _orch(agent) diff --git a/tests/test_model_judge.py b/tests/test_model_judge.py index cfa249cea..a2e9433b6 100644 --- a/tests/test_model_judge.py +++ b/tests/test_model_judge.py @@ -633,6 +633,85 @@ def chat(self, agent: ModelAgent, messages: list, **kwargs: object) -> str: # t ) +def test_model_judge_never_selects_a_verifier_excluded_sole_candidate() -> None: + """A verifier-excluded agent must never become the model judge even as + the *primary* selection, not only on failover (see the failover-only + guarantee just above). ``_ranked_agents`` deliberately still returns + role-ineligible members -- it appends them after every eligible one, + per its own docstring -- so a caller that wants only role-eligible + candidates must re-apply ``role not in agent.provider_exclusions`` + itself, exactly as ``_plan_generated``/``_parse_workflow_plan`` already + do. The judge-selection ``next(...)`` in ``_model_judge_verification`` + was missing that filter: with a single-candidate pool excluded from + "verifier" (e.g. a worker-only pinned candidate -- PR #983's + ``orchestrator/free`` provable-route carve-out), it picked that + ineligible agent as judge anyway, an extra unrequested live call the + no-heuristics candidate-controls contract does not allow (observed as + a spurious duplicate call in hosted CI, where fast-mlsirm is actually + importable, once #983 added the first single-candidate + verifier-excluded pool).""" + judge_constructed_with: list[str] = [] + + class _Judge: + def __init__(self, adapter, *, mode: str, accept_threshold: float) -> None: + del mode, accept_threshold + judge_constructed_with.append(adapter.judge) + + def judge(self, **_: object) -> object: + raise AssertionError( + "judge() must never be called for an ineligible sole candidate" + ) + + class _Criterion: + def __init__(self, criterion_id: str, description: str, weight: float) -> None: + self.criterion_id = criterion_id + self.description = description + self.weight = weight + + calls: list[str] = [] + + class _Client(ModelClient): + def chat(self, agent: ModelAgent, messages: list, **kwargs: object) -> str: # type: ignore[override] + del messages, kwargs + calls.append(agent.id) + return "unexpected" + + def proxy_send(self, agent: ModelAgent, endpoint: str, payload: dict) -> dict: # type: ignore[override] + del endpoint, payload + calls.append(agent.id) + return {"choices": [{"message": {"content": "unexpected"}}]} + + orchestrator = TaskOrchestrator( + [ + ModelAgent( + "worker_only", + "worker-model", + tags=("cost:free",), + provider_exclusions=("thinker", "verifier", "synthesizer"), + ) + ], + client=_Client(), + ) + + with patch.object( + orchestrator_module, + "_resolve_fast_mlsirm_components", + return_value=orchestrator_module.FastMLSIRMJudgeComponents( + judge_cls=_Judge, + criterion_cls=_Criterion, + format_error=ValueError, + ), + ): + result = orchestrator._model_judge_verification( + "task", {"verifier_output": "report"} + ) + + assert result["accepted"] is False + assert result["reason"] == "model judge unavailable; verification failed closed" + assert judge_constructed_with == [] + assert calls == [] + + def test_fast_mlsirm_adapter_routes_structured_completion_through_gateway() -> None: orchestrator, _ = _orch("unused") adapter = orchestrator_module._FastMLSIJudgeAdapter( @@ -676,6 +755,42 @@ def test_fast_mlsirm_adapter_routes_structured_completion_through_gateway() -> N assert completion["trace"][0]["usage"]["total_tokens"] == 5 +def test_fast_mlsirm_adapter_structured_completion_records_candidate_attempt() -> None: + """``complete_structured`` calls ``client.proxy_send`` directly rather than + through ``_invoke`` (which self-instruments via ``_record_candidate_attempt`` + in its race-member ``call()`` closure), so this path must record its own + attempt -- otherwise the billed provider call silently drops out of + ``attempted_candidate_ids`` evidence (#983 Devin finding: "Attempt + evidence omits provider calls").""" + orchestrator, _ = _orch("unused") + adapter = orchestrator_module._FastMLSIJudgeAdapter( + orchestrator, + "task", + "general_agent", + mode="route", + ) + response_format = { + "type": "json_schema", + "json_schema": {"name": "judge", "strict": True, "schema": {"type": "object"}}, + } + with patch.object( + orchestrator.client, + "proxy_send", + return_value={ + "choices": [{"message": {"content": '{"meets_threshold":true,"rationale":"ok"}'}}], + }, + ): + with orchestrator.candidate_routing_policy({"candidate_id": "general_agent"}): + adapter.complete_structured( + [{"role": "user", "content": "judge"}], + mode="conduct", + response_format=response_format, + ) + evidence = orchestrator._candidate_routing_evidence({"trace": []}) + + assert evidence["attempted_candidate_ids"] == ["general_agent"] + + def test_strict_schema_validation_and_repair_stay_in_the_conduct_trace() -> None: """An invalid synthesis is repaired once and both provider calls stay visible.""" orchestrator, _ = _orch("unused")