From 37456acef9a9831f67af0ed02317342af7c7773e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 07:25:43 +0900 Subject: [PATCH 01/35] feat(routing): add stateless candidate controls --- CHANGELOG.md | 8 + README.md | 9 + contextual_orchestrator/api_contract.py | 29 +++ contextual_orchestrator/cost_router.py | 20 +- contextual_orchestrator/orchestrator.py | 139 ++++++++++- contextual_orchestrator/server.py | 170 ++++++++++--- .../0032-model-group-cost-aware-discovery.md | 19 ++ tests/test_api_contract.py | 8 + tests/test_candidate_routing_controls.py | 223 ++++++++++++++++++ 9 files changed, 582 insertions(+), 43 deletions(-) create mode 100644 tests/test_candidate_routing_controls.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b38e0770..6625a7875 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. + ### Fixed - OpenRouter discovery no longer marks the entire credential account diff --git a/README.md b/README.md index 60ffff831..38cb790c1 100644 --- a/README.md +++ b/README.md @@ -262,6 +262,15 @@ 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` (at most 32 unique IDs) to omit known-bad + 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 preserves the existing request + and response contract. Concrete provider model names cannot be combined with + these controls; candidate IDs remain absent from `/v1/models`. - **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 2da5c8fea..7b4e21c99 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -20,6 +20,29 @@ }, }, "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, + "description": "Exact private agent ID to use for this request.", + }, + "exclude_candidate_ids": { + "type": "array", + "maxItems": 32, + "uniqueItems": True, + "items": {"type": "string", "minLength": 1}, + }, + }, + "additionalProperties": False, + }, "ModelGroupWrite": { "type": "object", "required": ["group_name", "member_agent_ids"], @@ -157,6 +180,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", @@ -383,6 +409,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 438a26fde..6c8600ce6 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -261,9 +261,15 @@ def complete( 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 {} + has_candidate_controls = any( + key in routing_controls for key in ("candidate_id", "exclude_candidate_ids") + ) routing_hints = hints if isinstance(hints, RoutingHints) else RoutingHints.from_mapping(hints) prompt_tokens_estimate = self.token_counter.count_messages(messages, model_name) decision = self.policy.decide(routing_hints, prompt_tokens_estimate) + 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( @@ -299,7 +305,9 @@ 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 + ): provider_response = self.orchestrator.proxy_completion( provider_request, endpoint=provider_endpoint, @@ -320,6 +328,9 @@ 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) + if 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 @@ -426,8 +437,13 @@ 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 + ): result = self.orchestrator.run(messages, **run_kwargs) + routing_evidence = self.orchestrator._candidate_routing_evidence(result) + if routing_evidence is not None: + 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 1f99f5fac..2bdc931f3 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -246,6 +246,12 @@ 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() +) SECRET_PATTERNS = ( re.compile(r"(?i)(api[_-]?key|token|secret|password)(['\"]?\s*[:=]\s*['\"]?)[A-Za-z0-9._~+/=-]{12,}"), @@ -3440,6 +3446,102 @@ 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, + ): + """Apply trusted request-local candidate pin and exclusion controls.""" + routing = routing or {} + candidate_id = routing.get("candidate_id") + excluded = routing.get("exclude_candidate_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() + ): + raise ValueError("candidate_id must be a non-empty agent ID") + if not isinstance(excluded, (list, tuple)) or len(excluded) > 32: + raise ValueError("exclude_candidate_ids must contain at most 32 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 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") + configured = {agent.id: agent for agent in self.candidates} + 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) + ): + 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") + candidate_token = _REQUEST_CANDIDATE_ID.set(candidate_id) + excluded_token = _REQUEST_EXCLUDED_CANDIDATE_IDS.set(frozenset(normalized)) + try: + yield + finally: + _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 _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 [] + attempted = [ + value + for row in rows + if isinstance(row, Mapping) + for value in [row.get("agent_id")] + if isinstance(value, str) and value + ] + served = next( + ( + value + for row in reversed(rows) + if isinstance(row, Mapping) + for value in [row.get("served_agent_id") or row.get("agent_id")] + if isinstance(value, str) and value + ), + None, + ) + 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.""" @@ -3590,11 +3692,17 @@ def proxy_completion( for key in _PASSTHROUGH_TRIGGER_KEYS ) ): - 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 messages = body.get("messages") if isinstance(messages, list): text = self._latest_user_text(messages) @@ -3700,6 +3808,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 ( @@ -3811,6 +3924,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( @@ -4534,6 +4659,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 return build_response_cache_key( messages, mode, @@ -5440,7 +5570,9 @@ 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): + if not self._request_candidate_allowed(agent) or any( + tag not in agent.tags for tag in required_tags + ): try: capable = self._ranked_agents( step.subtask, @@ -5757,6 +5889,7 @@ def _ranked_agents( agent for agent in source if not agent.disabled + and self._request_candidate_allowed(agent) and self._zdr_agent_allowed(agent) if ( not free_only @@ -14221,6 +14354,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"]) @@ -14313,6 +14447,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 4ea2f1d8b..fc9e5328a 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -3145,7 +3145,16 @@ def _validate_routing(routing: Any) -> dict[str, Any] | None: return None if not isinstance(routing, dict): raise RequestError(400, "invalid_routing", "routing must be an object") - unknown = sorted(set(routing) - {"channel", "latency_tolerant", "priority"}) + unknown = sorted( + set(routing) + - { + "channel", + "latency_tolerant", + "priority", + "candidate_id", + "exclude_candidate_ids", + } + ) if unknown: raise RequestError(400, "invalid_routing", "routing contains unsupported keys", {"fields": unknown}) channel = routing.get("channel") @@ -3189,9 +3198,57 @@ def _validate_routing(routing: Any) -> dict[str, Any] | None: 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(): + 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) or len(excluded) > 32: + raise RequestError( + 400, + "invalid_routing", + "routing.exclude_candidate_ids must be an array of at most 32 agent IDs", + ) + if any(not isinstance(value, str) or not 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", + ) return cleaned if cleaned else {} +def _validate_candidate_routing( + orchestrator: TaskOrchestrator, + routing: dict[str, Any] | None, + model_name: str, +) -> None: + """Fail closed on candidate controls before any response bytes are sent.""" + try: + with orchestrator.candidate_routing_policy(routing, model_name=model_name): + 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]: @@ -5342,6 +5399,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 @@ -6635,6 +6694,8 @@ 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) + request_routing = _validate_routing(body.get("routing")) + _validate_candidate_routing(orchestrator, request_routing, model_name) # 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) @@ -6727,16 +6788,24 @@ 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) + if evidence is not None: + result.setdefault("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")) + structured_routing = request_routing if structured_routing and ( structured_routing.get("channel") == "batch" or structured_routing.get("latency_tolerant") is True @@ -6844,7 +6913,7 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di self._authorize_trace_access() # stream + stream_options already coerced/validated before passthrough. attribution = _validate_attribution(body.get("attribution")) - routing = _validate_routing(body.get("routing")) + 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 @@ -6878,6 +6947,7 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di security, messages, model_name, + routing=routing, include_usage=include_usage, ) orchestrator.record_analytics_event( @@ -7190,6 +7260,10 @@ 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) + responses_routing_control = _validate_routing(body.get("routing")) + _validate_candidate_routing( + orchestrator, responses_routing_control, model_name + ) _validate_responses_conversation_controls(body) if "store" in body: _validate_responses_store(body) @@ -7303,7 +7377,7 @@ 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")) + routing = responses_routing_control # Responses passthrough has no batch channel plane yet. if routing and routing.get("channel") == "batch": raise RequestError( @@ -7432,6 +7506,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", @@ -7470,9 +7545,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")) 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. @@ -7567,7 +7640,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")), + hints=responses_routing_control, model_name=body["model"], provider_request=body, provider_endpoint="responses", @@ -7992,6 +8065,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}" @@ -8076,27 +8150,33 @@ 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, + with orchestrator.candidate_routing_policy( + routing, model_name=model_name + ): + 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) + 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["candidate_routing"] = evidence except ConnectionAbortedError: raise except ProviderUpstreamError as exc: @@ -8218,6 +8298,7 @@ def _stream_route_completion( messages: Any, model_name: str, *, + routing: dict[str, Any] | None = None, include_usage: bool = False, ) -> None: """Pipe live provider deltas as OpenAI chat-completion SSE frames.""" @@ -8270,10 +8351,21 @@ 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")): + with orchestrator.candidate_routing_policy( + routing, model_name=model_name + ): + for delta in orchestrator.stream_route(messages, **stream_kwargs): + if not self._write_sse(frame({"content": delta})): + return + 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..4a6f226ec 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,25 @@ 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 at most 32 +unique IDs with `routing.exclude_candidate_ids`. 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. + 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 diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py index e67f5902d..7624cf815 100644 --- a/tests/test_api_contract.py +++ b/tests/test_api_contract.py @@ -58,6 +58,14 @@ 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 + assert routing_schema["properties"]["exclude_candidate_ids"] == { + "type": "array", + "maxItems": 32, + "uniqueItems": True, + "items": {"type": "string", "minLength": 1}, + } assert OPENAPI_SPEC["paths"]["/api/v1/access_reports/{workflow_run_id}"]["get"][ "security" ] == [{"admin_bearer_auth": [], "trace_bearer_auth": []}] diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py new file mode 100644 index 000000000..cb5f68eba --- /dev/null +++ b/tests/test_candidate_routing_controls.py @@ -0,0 +1,223 @@ +"""Stateless candidate pin and exclusion controls across OpenAI chat paths.""" + +from __future__ import annotations + +import json +import threading +import urllib.error +import urllib.request + +from contextual_orchestrator import ModelAgent, TaskOrchestrator +from contextual_orchestrator.orchestrator import ModelClient +from contextual_orchestrator.server import SecurityConfig, 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") + 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": "candidate b"}, + "finish_reason": "stop", + } + ], + } + + proxy_send = proxy_send_once + + +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 _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_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"} From 473bb0dae3d3f58eb082cce4926ebd9d48ba380f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 07:29:29 +0900 Subject: [PATCH 02/35] docs(routing): show candidate control request --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index 38cb790c1..b00ecfb79 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,17 @@ is read from a **KV config store**, never `os.getenv`. `orchestration.routing`. Omitting both keys preserves the existing request and response contract. 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 From be3eebf25e11a1e5c91780c73ca40e15977fdb99 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 07:31:43 +0900 Subject: [PATCH 03/35] test(routing): cover Responses candidate controls --- tests/test_candidate_routing_controls.py | 58 ++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index cb5f68eba..229694241 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -88,6 +88,30 @@ def _post_sse(port: int, token: str, body: dict) -> tuple[int, list[dict]]: 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", + ) + 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) + + def _serve(): client = _CandidateClient() orchestrator = TaskOrchestrator( @@ -221,3 +245,37 @@ def test_candidate_pin_is_honored_by_structured_and_streaming_chat_paths() -> No 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_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"} From 9942b620bed03ca4f414338bf82a08cff4f267ed Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 07:33:49 +0900 Subject: [PATCH 04/35] fix(stream): preserve omitted routing compatibility --- contextual_orchestrator/server.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index fc9e5328a..d8d4e1df2 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 @@ -8150,9 +8151,14 @@ def progress(role: str, status: str) -> None: } emit("response.output_item.added", output_index=0, item=reasoning_item) try: - with orchestrator.candidate_routing_policy( - routing, model_name=model_name - ): + candidate_scope = ( + orchestrator.candidate_routing_policy( + routing, model_name=model_name + ) + 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}" @@ -8351,14 +8357,21 @@ def usage_frame(usage: dict[str, Any]) -> str: stream_kwargs.update( {"include_usage": True, "usage_callback": capture_usage} ) - with orchestrator.candidate_routing_policy( - routing, model_name=model_name - ): + candidate_scope = ( + orchestrator.candidate_routing_policy( + routing, model_name=model_name + ) + if routing + else nullcontext() + ) + with candidate_scope: for delta in orchestrator.stream_route(messages, **stream_kwargs): if not self._write_sse(frame({"content": delta})): return - record = orchestrator.get_workflow_run(run_id) - evidence = orchestrator._candidate_routing_evidence(record) + 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: From a426755ab0122bb9ea0714433b174f34fbf43230 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 07:52:30 +0900 Subject: [PATCH 05/35] fix(routing): enforce candidate controls end to end --- contextual_orchestrator/cost_router.py | 5 +- contextual_orchestrator/orchestrator.py | 79 +++++++++++++--- contextual_orchestrator/server.py | 50 ++++++---- .../0032-model-group-cost-aware-discovery.md | 5 + fuzz/targets.py | 14 +++ tests/test_candidate_routing_controls.py | 92 ++++++++++++++++++- 6 files changed, 213 insertions(+), 32 deletions(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 6c8600ce6..16498dc3d 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -262,8 +262,9 @@ def complete( if type(zdr_only) is not bool: raise TypeError("zdr_only must be a boolean") routing_controls = hints if isinstance(hints, dict) else {} - has_candidate_controls = any( - key in routing_controls for key in ("candidate_id", "exclude_candidate_ids") + has_candidate_controls = bool( + routing_controls.get("candidate_id") + or routing_controls.get("exclude_candidate_ids") ) routing_hints = hints if isinstance(hints, RoutingHints) else RoutingHints.from_mapping(hints) prompt_tokens_estimate = self.token_counter.count_messages(messages, model_name) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 2bdc931f3..5683facf4 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -252,6 +252,9 @@ def _cost_usd_decimal(output_tokens: int, price_per_million: float) -> Decimal: _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 +) SECRET_PATTERNS = ( re.compile(r"(?i)(api[_-]?key|token|secret|password)(['\"]?\s*[:=]\s*['\"]?)[A-Za-z0-9._~+/=-]{12,}"), @@ -3493,9 +3496,11 @@ def candidate_routing_policy( raise ValueError("candidate_id is not eligible for orchestrator/free") 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) @@ -3507,6 +3512,12 @@ def _request_candidate_allowed(agent: ModelAgent) -> bool: and (pinned is None or agent.id == pinned) ) + @staticmethod + def _record_candidate_attempt(agent_id: str) -> None: + attempted = _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() + if attempted is not None and agent_id not in attempted: + attempted.append(agent_id) + @staticmethod def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | None: pinned = _REQUEST_CANDIDATE_ID.get() @@ -3515,13 +3526,15 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N return None trace = result.get("trace") rows = trace if isinstance(trace, list) else [] - attempted = [ - value - for row in rows - if isinstance(row, Mapping) - for value in [row.get("agent_id")] - if isinstance(value, str) and value - ] + attempted = list(_REQUEST_ATTEMPTED_CANDIDATE_IDS.get() or ()) + if not attempted: + attempted = [ + value + for row in rows + if isinstance(row, Mapping) + for value in [row.get("agent_id")] + if isinstance(value, str) and value + ] served = next( ( value @@ -3734,7 +3747,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: @@ -3797,6 +3814,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: @@ -3880,6 +3898,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): @@ -4005,7 +4024,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: @@ -4213,6 +4236,11 @@ def send_synthesis( if candidate.id != preferred.id ), ] + ordered_candidates = [ + candidate + for candidate in ordered_candidates + if self._request_candidate_allowed(candidate) + ] for candidate in ordered_candidates: provider_key = ( f"provider:{candidate.provider_name.casefold()}" @@ -4237,6 +4265,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: @@ -4573,6 +4602,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: @@ -5584,6 +5614,12 @@ def conduct( capable = [] if capable: agent = capable[0] + if not self._request_candidate_allowed(agent) or any( + tag not in agent.tags for tag in required_tags + ): + 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) @@ -6214,9 +6250,15 @@ 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( - (text + ("\x00zdr_only" if _REQUEST_ZDR_ONLY.get() else "")).encode("utf-8") - ).hexdigest() + control_key = json.dumps( + { + "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: @@ -6232,7 +6274,14 @@ def _compute_triage_verdict(self, text: str) -> bool: except RuntimeError: candidates = [] if not candidates and not _REQUEST_ZDR_ONLY.get(): - candidates = list(self.agents) + candidates = [ + 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) + ] if not candidates: return False triage_agent = candidates[0] @@ -6241,6 +6290,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 @@ -6653,6 +6703,7 @@ def _invoke( request_settings = self.client.request_settings_snapshot() def call(agent: ModelAgent) -> tuple[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) @@ -6711,6 +6762,7 @@ def call(agent: ModelAgent) -> tuple[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 = ( @@ -6890,6 +6942,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) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index d8d4e1df2..6d1abbc8e 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -3135,7 +3135,9 @@ def _validate_attribution(attribution: Any) -> dict[str, Any] | None: return cleaned or None -def _validate_routing(routing: Any) -> dict[str, Any] | None: +def _validate_routing( + routing: Any, *, allow_candidate_controls: bool = False +) -> dict[str, Any] | None: """OpenAI-adjacent routing hints for sync vs batch channel selection. Fail closed on shape so callers cannot smuggle non-boolean latency flags or @@ -3146,16 +3148,10 @@ def _validate_routing(routing: Any) -> dict[str, Any] | None: return None if not isinstance(routing, dict): raise RequestError(400, "invalid_routing", "routing must be an object") - unknown = sorted( - set(routing) - - { - "channel", - "latency_tolerant", - "priority", - "candidate_id", - "exclude_candidate_ids", - } - ) + allowed = {"channel", "latency_tolerant", "priority"} + if allow_candidate_controls: + allowed.update({"candidate_id", "exclude_candidate_ids"}) + unknown = sorted(set(routing) - allowed) if unknown: raise RequestError(400, "invalid_routing", "routing contains unsupported keys", {"fields": unknown}) channel = routing.get("channel") @@ -6695,7 +6691,9 @@ 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) - request_routing = _validate_routing(body.get("routing")) + request_routing = _validate_routing( + body.get("routing"), allow_candidate_controls=True + ) _validate_candidate_routing(orchestrator, request_routing, model_name) # Coerce stream early so stream_options fail-closed matches route path # and tools/response_format passthrough cannot skip type checks. @@ -6899,9 +6897,12 @@ def proxy_tool_request() -> dict[str, Any]: 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) - ) + 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: if explicit_trace: raise RequestError( @@ -7261,7 +7262,9 @@ def proxy_tool_request() -> dict[str, Any]: # 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) - responses_routing_control = _validate_routing(body.get("routing")) + responses_routing_control = _validate_routing( + body.get("routing"), allow_candidate_controls=True + ) _validate_candidate_routing( orchestrator, responses_routing_control, model_name ) @@ -7432,6 +7435,21 @@ def proxy_tool_request() -> dict[str, Any]: "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. 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 4a6f226ec..1690263ad 100644 --- a/docs/planning/adrs/0032-model-group-cost-aware-discovery.md +++ b/docs/planning/adrs/0032-model-group-cost-aware-discovery.md @@ -172,6 +172,11 @@ 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. +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 diff --git a/fuzz/targets.py b/fuzz/targets.py index 3cc3641e3..289e87900 100644 --- a/fuzz/targets.py +++ b/fuzz/targets.py @@ -37,6 +37,8 @@ ``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. ``server._validate_routing`` -- request-local channel and candidate + controls. Successful candidate arrays are bounded, unique, and non-empty. No network, no secrets, no filesystem: every target runs fully offline. """ @@ -178,6 +180,18 @@ 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", []) + assert len(excluded) <= 32 + 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/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 229694241..68c6e46fb 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -7,9 +7,16 @@ import urllib.error import urllib.request +import pytest + from contextual_orchestrator import ModelAgent, TaskOrchestrator from contextual_orchestrator.orchestrator import ModelClient -from contextual_orchestrator.server import SecurityConfig, build_server +from contextual_orchestrator.server import ( + RequestError, + SecurityConfig, + _validate_routing, + build_server, +) class _CandidateClient(ModelClient): @@ -21,6 +28,8 @@ 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): @@ -279,3 +288,84 @@ def test_candidate_pin_is_honored_by_responses_json_and_stream_paths() -> None: 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_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_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": []} + + +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" From e564871d792adfbc04d8ad7657eb3ef3ca25a5dc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:16:28 +0900 Subject: [PATCH 06/35] fix(routing): validate request-local candidates --- contextual_orchestrator/api_contract.py | 7 +- contextual_orchestrator/orchestrator.py | 51 ++++++--- .../0032-model-group-cost-aware-discovery.md | 5 + tests/test_api_contract.py | 3 +- tests/test_candidate_routing_controls.py | 108 ++++++++++++++++++ 5 files changed, 157 insertions(+), 17 deletions(-) diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index 7b4e21c99..fdda2ee53 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -32,13 +32,18 @@ "candidate_id": { "type": "string", "minLength": 1, + "pattern": r".*\S.*", "description": "Exact private agent ID to use for this request.", }, "exclude_candidate_ids": { "type": "array", "maxItems": 32, "uniqueItems": True, - "items": {"type": "string", "minLength": 1}, + "items": { + "type": "string", + "minLength": 1, + "pattern": r".*\S.*", + }, }, }, "additionalProperties": False, diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 5683facf4..7db16a5a0 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -3490,10 +3490,24 @@ def candidate_routing_policy( or candidate.disabled or not self._zdr_agent_allowed(candidate) or not _is_general_chat_agent(candidate) + or "worker" in candidate.provider_exclusions ): 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 not any( + agent.id not in normalized + and not agent.disabled + and self._zdr_agent_allowed(agent) + and _is_general_chat_agent(agent) + and "worker" not in agent.provider_exclusions + and ( + model_name != self.FREE_MODEL + or self._is_general_free_agent(agent) + ) + for agent in self.candidates + ): + 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([]) @@ -3526,8 +3540,9 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N return None trace = result.get("trace") rows = trace if isinstance(trace, list) else [] - attempted = list(_REQUEST_ATTEMPTED_CANDIDATE_IDS.get() or ()) - if not attempted: + tracked_attempts = _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() + attempted = list(tracked_attempts or ()) + if tracked_attempts is None: attempted = [ value for row in rows @@ -3535,15 +3550,19 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N for value in [row.get("agent_id")] if isinstance(value, str) and value ] - served = next( - ( - value - for row in reversed(rows) - if isinstance(row, Mapping) - for value in [row.get("served_agent_id") or row.get("agent_id")] - if isinstance(value, str) and value - ), - None, + served = ( + None + if tracked_attempts == [] + else next( + ( + value + for row in reversed(rows) + if isinstance(row, Mapping) + for value in [row.get("served_agent_id") or row.get("agent_id")] + if isinstance(value, str) and value + ), + None, + ) ) evidence: dict[str, Any] = { "exclude_candidate_ids": excluded, @@ -5614,9 +5633,7 @@ def conduct( capable = [] if capable: agent = capable[0] - if not self._request_candidate_allowed(agent) or any( - tag not in agent.tags for tag in required_tags - ): + if not self._request_candidate_allowed(agent): raise ValueError( "no eligible candidate satisfies the active routing controls" ) @@ -5779,7 +5796,9 @@ def _plan_generated(self, task: str) -> list[WorkflowStep]: pool = "\n".join( f"- {agent.id}: model={agent.model}, tags={', '.join(agent.tags) or 'none'}" for agent in self.agents - if _is_general_chat_agent(agent) and self._zdr_agent_allowed(agent) + if _is_general_chat_agent(agent) + and self._zdr_agent_allowed(agent) + and self._request_candidate_allowed(agent) ) system = ( "You are the workflow conductor. Decompose the user's task into a short workflow.\n" @@ -5831,6 +5850,8 @@ 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 role in assigned.provider_exclusions ): # Unknown or stale ineligible assignments are reselected honestly. agent_id = self._select_agent(subtask, role).id 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 1690263ad..095c7f6a6 100644 --- a/docs/planning/adrs/0032-model-group-cost-aware-discovery.md +++ b/docs/planning/adrs/0032-model-group-cost-aware-discovery.md @@ -172,6 +172,11 @@ 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 diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py index 7624cf815..925bec209 100644 --- a/tests/test_api_contract.py +++ b/tests/test_api_contract.py @@ -60,11 +60,12 @@ def test_openapi_documents_compatibility_front_door() -> None: assert chat_schema["properties"]["include_orchestration_trace"]["type"] == "boolean" routing_schema = OPENAPI_SPEC["components"]["schemas"]["CandidateRoutingControls"] assert routing_schema["properties"]["candidate_id"]["minLength"] == 1 + assert routing_schema["properties"]["candidate_id"]["pattern"] == r".*\S.*" assert routing_schema["properties"]["exclude_candidate_ids"] == { "type": "array", "maxItems": 32, "uniqueItems": True, - "items": {"type": "string", "minLength": 1}, + "items": {"type": "string", "minLength": 1, "pattern": r".*\S.*"}, } assert OPENAPI_SPEC["paths"]["/api/v1/access_reports/{workflow_run_id}"]["get"][ "security" diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 68c6e46fb..0b8f694c3 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -219,6 +219,87 @@ def test_candidate_controls_fail_closed_and_omission_preserves_response_shape() 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_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"]} @@ -369,3 +450,30 @@ def test_attempt_evidence_keeps_failed_candidate_before_success() -> None: "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": [], + } From 97fd106a820e373691157b5d51fcefff609cf88e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:32:46 +0900 Subject: [PATCH 07/35] fix(routing): preflight conduct candidate roles Signed-off-by: Seongho Bae --- contextual_orchestrator/orchestrator.py | 38 ++++++++----- contextual_orchestrator/server.py | 34 +++++++++++- tests/test_candidate_routing_controls.py | 69 ++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 13 deletions(-) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 7db16a5a0..f7ef7d7a0 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -3455,6 +3455,8 @@ def candidate_routing_policy( 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 {} @@ -3490,22 +3492,27 @@ def candidate_routing_policy( or candidate.disabled or not self._zdr_agent_allowed(candidate) or not _is_general_chat_agent(candidate) - or "worker" in candidate.provider_exclusions + 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 not any( - agent.id not in normalized - and not agent.disabled - and self._zdr_agent_allowed(agent) - and _is_general_chat_agent(agent) - and "worker" not in agent.provider_exclusions - and ( - model_name != self.FREE_MODEL - or self._is_general_free_agent(agent) + 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 self.candidates ) - for agent in self.candidates + for role in required_roles ): raise ValueError("exclude_candidate_ids leaves no eligible agent") candidate_token = _REQUEST_CANDIDATE_ID.set(candidate_id) @@ -5619,8 +5626,11 @@ def conduct( additional_cost_usd=in_flight_cost, ) agent = self._agent(step.agent_id) + 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( @@ -5633,7 +5643,11 @@ def conduct( capable = [] if capable: agent = capable[0] - if not self._request_candidate_allowed(agent): + 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" ) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 6d1abbc8e..1f2e35c9a 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -3237,10 +3237,18 @@ 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): + 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 @@ -6897,6 +6905,30 @@ def proxy_tool_request() -> dict[str, Any]: return messages = _validate_messages(body.get("messages")) mode = _validate_mode(body.get("orchestration") or body.get("orchestration_mode") or body.get("mode") or "auto") + try: + with orchestrator.candidate_routing_policy( + request_routing, model_name=model_name + ): + route_selected = orchestrator.would_route( + messages, mode, model_name + ) + except ValueError as exc: + raise RequestError(400, "invalid_routing", str(exc)) from exc + _validate_candidate_routing( + orchestrator, + request_routing, + model_name, + required_roles=( + ("worker",) + if route_selected + else ("thinker", "worker", "verifier", "synthesizer") + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(messages) + else () + ), + ) with orchestrator.candidate_routing_policy( request_routing, model_name=model_name ): diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 0b8f694c3..93e8603c6 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -257,6 +257,75 @@ def test_candidate_controls_reject_an_unserviceable_worker_pool() -> None: 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: + status, body = _post( + server.server_address[1], + token, + { + "model": "orchestrator/auto", + "messages": [{"role": "user", "content": "conduct this"}], + "mode": "conduct", + "routing": {"candidate_id": "worker_only"}, + }, + ) + finally: + server.shutdown() + thread.join(timeout=5) + + assert status == 400 + assert body["error"]["code"] == "invalid_routing" + assert client.calls == [] + + def test_generated_planner_uses_only_request_eligible_candidates(monkeypatch) -> None: orchestrator = TaskOrchestrator( [ From 8a5c0d16b818b5d1d748d5532f8be85e42fe6ac4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:36:57 +0900 Subject: [PATCH 08/35] fix(routing): detach response-only evidence Signed-off-by: Seongho Bae --- contextual_orchestrator/cost_router.py | 1 + contextual_orchestrator/server.py | 1 + tests/test_candidate_routing_controls.py | 36 ++++++++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 16498dc3d..8777b88f4 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -444,6 +444,7 @@ def complete( 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"] diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 1f2e35c9a..bf5aad71e 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -8232,6 +8232,7 @@ def progress(role: str, status: str) -> None: 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 diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 93e8603c6..86ccce21f 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -326,6 +326,42 @@ def test_http_conduct_preflight_rejects_role_ineligible_pin() -> None: assert client.calls == [] +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( [ From 922ca7e8945cd8220a6b029dd02028f2df98e46a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:40:01 +0900 Subject: [PATCH 09/35] fix(routing): require exact candidate IDs Signed-off-by: Seongho Bae --- contextual_orchestrator/api_contract.py | 4 ++-- contextual_orchestrator/orchestrator.py | 6 +++++- contextual_orchestrator/server.py | 13 +++++++++++-- tests/test_api_contract.py | 8 ++++++-- tests/test_candidate_routing_controls.py | 7 +++++++ 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index fdda2ee53..58883865a 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -32,7 +32,7 @@ "candidate_id": { "type": "string", "minLength": 1, - "pattern": r".*\S.*", + "pattern": r"^\S(?:.*\S)?$", "description": "Exact private agent ID to use for this request.", }, "exclude_candidate_ids": { @@ -42,7 +42,7 @@ "items": { "type": "string", "minLength": 1, - "pattern": r".*\S.*", + "pattern": r"^\S(?:.*\S)?$", }, }, }, diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index f7ef7d7a0..760314c21 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -3468,7 +3468,9 @@ def candidate_routing_policy( 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() + 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)) or len(excluded) > 32: @@ -3476,6 +3478,8 @@ def candidate_routing_policy( 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 diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index bf5aad71e..0cc4e32df 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -3197,7 +3197,11 @@ def _validate_routing( 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(): + 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" ) @@ -3210,7 +3214,12 @@ def _validate_routing( "invalid_routing", "routing.exclude_candidate_ids must be an array of at most 32 agent IDs", ) - if any(not isinstance(value, str) or not value.strip() for value in excluded): + if any( + not isinstance(value, str) + or not value.strip() + or value != value.strip() + for value in excluded + ): raise RequestError( 400, "invalid_routing", diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py index 925bec209..e058f0426 100644 --- a/tests/test_api_contract.py +++ b/tests/test_api_contract.py @@ -60,12 +60,16 @@ def test_openapi_documents_compatibility_front_door() -> None: assert chat_schema["properties"]["include_orchestration_trace"]["type"] == "boolean" routing_schema = OPENAPI_SPEC["components"]["schemas"]["CandidateRoutingControls"] assert routing_schema["properties"]["candidate_id"]["minLength"] == 1 - assert routing_schema["properties"]["candidate_id"]["pattern"] == r".*\S.*" + assert routing_schema["properties"]["candidate_id"]["pattern"] == r"^\S(?:.*\S)?$" assert routing_schema["properties"]["exclude_candidate_ids"] == { "type": "array", "maxItems": 32, "uniqueItems": True, - "items": {"type": "string", "minLength": 1, "pattern": r".*\S.*"}, + "items": { + "type": "string", + "minLength": 1, + "pattern": r"^\S(?:.*\S)?$", + }, } assert OPENAPI_SPEC["paths"]["/api/v1/access_reports/{workflow_run_id}"]["get"][ "security" diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 86ccce21f..157d59093 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -534,6 +534,13 @@ def test_candidate_keys_are_rejected_on_unsupported_routing_surfaces() -> None: 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: From ccd9a9b05d203de7f7773f842f45d5a78003216d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:43:12 +0900 Subject: [PATCH 10/35] fix(routing): keep rejected preflight provider-free Signed-off-by: Seongho Bae --- contextual_orchestrator/server.py | 11 +-------- tests/test_candidate_routing_controls.py | 29 ++++++++++++++---------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 0cc4e32df..dbf255aa1 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -6914,22 +6914,13 @@ def proxy_tool_request() -> dict[str, Any]: return messages = _validate_messages(body.get("messages")) mode = _validate_mode(body.get("orchestration") or body.get("orchestration_mode") or body.get("mode") or "auto") - try: - with orchestrator.candidate_routing_policy( - request_routing, model_name=model_name - ): - route_selected = orchestrator.would_route( - messages, mode, model_name - ) - except ValueError as exc: - raise RequestError(400, "invalid_routing", str(exc)) from exc _validate_candidate_routing( orchestrator, request_routing, model_name, required_roles=( ("worker",) - if route_selected + if mode == "route" else ("thinker", "worker", "verifier", "synthesizer") ), required_tags=( diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 157d59093..3ae576d0b 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -307,22 +307,27 @@ def test_http_conduct_preflight_rejects_role_ineligible_pin() -> None: 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": "worker_only"}, - }, - ) + 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 status == 400 - assert body["error"]["code"] == "invalid_routing" + assert all(status == 400 for status, _body in responses) + assert all( + body["error"]["code"] == "invalid_routing" for _status, body in responses + ) assert client.calls == [] From 40dbfbdc97b65aec3be32b0e465212675d7c6f47 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:45:48 +0900 Subject: [PATCH 11/35] fix(api): reject terminal candidate whitespace Signed-off-by: Seongho Bae --- contextual_orchestrator/api_contract.py | 4 ++-- tests/test_api_contract.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index 58883865a..795c24c87 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -32,7 +32,7 @@ "candidate_id": { "type": "string", "minLength": 1, - "pattern": r"^\S(?:.*\S)?$", + "pattern": r"^\S(?:[^\r\n]*\S)?(?![\s\S])", "description": "Exact private agent ID to use for this request.", }, "exclude_candidate_ids": { @@ -42,7 +42,7 @@ "items": { "type": "string", "minLength": 1, - "pattern": r"^\S(?:.*\S)?$", + "pattern": r"^\S(?:[^\r\n]*\S)?(?![\s\S])", }, }, }, diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py index e058f0426..50898e94f 100644 --- a/tests/test_api_contract.py +++ b/tests/test_api_contract.py @@ -60,7 +60,8 @@ def test_openapi_documents_compatibility_front_door() -> None: assert chat_schema["properties"]["include_orchestration_trace"]["type"] == "boolean" routing_schema = OPENAPI_SPEC["components"]["schemas"]["CandidateRoutingControls"] assert routing_schema["properties"]["candidate_id"]["minLength"] == 1 - assert routing_schema["properties"]["candidate_id"]["pattern"] == r"^\S(?:.*\S)?$" + 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", "maxItems": 32, @@ -68,9 +69,15 @@ def test_openapi_documents_compatibility_front_door() -> None: "items": { "type": "string", "minLength": 1, - "pattern": r"^\S(?:.*\S)?$", + "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) assert OPENAPI_SPEC["paths"]["/api/v1/access_reports/{workflow_run_id}"]["get"][ "security" ] == [{"admin_bearer_auth": [], "trace_bearer_auth": []}] From ab7a813a69dae19541dc2888acd50c4ce37b29b7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 08:55:08 +0900 Subject: [PATCH 12/35] fix(routing): keep candidate controls request-local --- contextual_orchestrator/cost_router.py | 6 ++- tests/test_candidate_routing_controls.py | 63 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 8777b88f4..fff5f7403 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -439,7 +439,11 @@ def complete( race_token = self._race_usage_context.set(race_context) try: with self.orchestrator.request_policy(zdr_only), self.orchestrator.candidate_routing_policy( - routing_controls, model_name=model_name + routing_controls, + model_name=model_name, + required_roles=("thinker", "worker", "verifier", "synthesizer") + if mode == "conduct" + else (), ): result = self.orchestrator.run(messages, **run_kwargs) routing_evidence = self.orchestrator._candidate_routing_evidence(result) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 3ae576d0b..5fc2ac7de 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -10,6 +10,7 @@ import pytest from contextual_orchestrator import ModelAgent, TaskOrchestrator +from contextual_orchestrator.cost_router import CostRoutingCoordinator from contextual_orchestrator.orchestrator import ModelClient from contextual_orchestrator.server import ( RequestError, @@ -594,3 +595,65 @@ def test_cache_hit_reports_no_current_candidate_attempt() -> None: "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" + ) From 6c8fca886e6b2fe57abd5a84a96b606463641b33 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 10:25:43 +0900 Subject: [PATCH 13/35] fix(responses): preflight conduct candidate pins --- contextual_orchestrator/server.py | 34 ++++++++++- tests/test_candidate_routing_controls.py | 74 +++++++++++++++++++++--- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index dbf255aa1..96f6bb166 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -7522,6 +7522,23 @@ def proxy_tool_request() -> dict[str, Any]: 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=( + "thinker", + "worker", + "verifier", + "synthesizer", + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(messages) + else () + ), + ) if _responses_virtual_requires_provider_path(input_value, body): raise RequestError( 400, @@ -7548,7 +7565,6 @@ def proxy_tool_request() -> dict[str, Any]: "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, @@ -7588,6 +7604,22 @@ def proxy_tool_request() -> dict[str, Any]: 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=( + "thinker", + "worker", + "verifier", + "synthesizer", + ), + required_tags=( + ("vision",) + if orchestrator._source_image_parts(messages) + else () + ), + ) responses_attribution = dict( _validate_attribution(body.get("attribution")) or {} ) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 5fc2ac7de..4b9d66bcf 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -60,6 +60,14 @@ def proxy_send_once(self, agent, endpoint, payload): 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" + + 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", @@ -111,15 +119,18 @@ def _post_responses( }, method="POST", ) - 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) + 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(): @@ -482,6 +493,51 @@ def test_candidate_pin_is_honored_by_responses_json_and_stream_paths() -> None: 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: From c33cd6bf7d0d55f58cac3ba07500cbbf27b1e54e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 10:51:28 +0900 Subject: [PATCH 14/35] fix(routing): preflight auto candidate roles --- contextual_orchestrator/cost_router.py | 12 ++++++---- tests/test_candidate_routing_controls.py | 28 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index fff5f7403..9e6cdbcee 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -307,7 +307,11 @@ def complete( race_token = self._race_usage_context.set(race_context) try: with self.orchestrator.request_policy(zdr_only), self.orchestrator.candidate_routing_policy( - routing_controls, model_name=model_name + routing_controls, + model_name=model_name, + required_roles=("worker",) + if mode == "route" + else ("thinker", "worker", "verifier", "synthesizer"), ): provider_response = self.orchestrator.proxy_completion( provider_request, @@ -441,9 +445,9 @@ def complete( 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 mode == "conduct" - else (), + required_roles=("worker",) + if mode == "route" + else ("thinker", "worker", "verifier", "synthesizer"), ): result = self.orchestrator.run(messages, **run_kwargs) routing_evidence = self.orchestrator._candidate_routing_evidence(result) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 4b9d66bcf..cfcb06618 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -713,3 +713,31 @@ def test_response_routing_evidence_is_not_persisted_in_workflow_history() -> Non 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 == [] From 19e7752dc899bb7149541cd66bca1f80d85a7ed6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 11:28:36 +0900 Subject: [PATCH 15/35] fix: align candidate preflight with proxy path --- contextual_orchestrator/cost_router.py | 10 ++++-- contextual_orchestrator/orchestrator.py | 29 +++++++++++---- tests/test_candidate_routing_controls.py | 45 ++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 9e6cdbcee..e5dab1836 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -309,9 +309,13 @@ def complete( with self.orchestrator.request_policy(zdr_only), self.orchestrator.candidate_routing_policy( routing_controls, model_name=model_name, - required_roles=("worker",) - if mode == "route" - else ("thinker", "worker", "verifier", "synthesizer"), + 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, diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 760314c21..6b73230f2 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -3707,6 +3707,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], @@ -3727,13 +3745,10 @@ 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, ): result = self._orchestrated_provider_completion( body, diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index cfcb06618..2afe3dc96 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -741,3 +741,48 @@ def test_coordinator_auto_preflights_conduct_roles_before_triage() -> None: ) 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" From 828a1b86c96aa713911f097a4fe2dc6f83eb1d5c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 23:08:28 +0000 Subject: [PATCH 16/35] fix(routing): close endpoint/candidate-pin and worker-fallback evidence gaps Three Devin findings against 38c8aaf5, verified against the actual code before fixing: 1. candidate_routing_policy()'s preflight validated a pin/exclusion against the *full* configured pool, ignoring the active routing_endpoint_scope. A request combining routing.endpoint=A with candidate_id (or exclude_candidate_ids) for an agent only reachable on endpoint B passed preflight and could only fail later, as a selection RuntimeError mapped to 500. The pin, unknown-exclusion, and remaining-role-eligibility checks now validate against the endpoint-filtered candidate set, so the conflict is rejected as 400 invalid_routing before any provider call. 2. _plan_generated() invoked the selected planner without recording the attempt, unlike every other selection path (triage, invocation). Routing evidence could omit a candidate that actually received the request when the generated plan's own steps never reassigned that agent to a later role. The attempt is now recorded immediately before the provider call, matching the established pattern. 3. _candidate_routing_evidence() inferred served_candidate_id from the last trace row, but conduct() can serve an earlier step's output as the answer (the verifier-required fallback to the worker's output, or the verifier's own output when a synthesizer is not required) while every step's provider call still lands in the trace. The helper now prefers the trace row(s) whose recorded output actually matches the served answer, falling back to the prior last-row heuristic for callers that never populate answer/output (plain passthrough, cache hits). Also clarifies README wording for a fourth (analysis-only) finding: an explicit empty exclude_candidate_ids with no candidate_id is a genuine no-op for selection, so it intentionally does not produce routing evidence -- same as omitting the routing key entirely. No code change needed there; the docs just didn't spell out the empty-array case. New regression coverage in tests/test_candidate_routing_controls.py: endpoint+pin and endpoint+exclusion conflicts over both chat/completions and Responses, a generated-plan test asserting the planner's attempt survives even when unused by later steps, and template/generated conduct tests asserting served_candidate_id tracks the actual worker fallback rather than the synthesizer's trace row. Full suite: 3324 passed, 5 failed (all pre-existing and unrelated -- 3x orchestrator/free streaming regression in test_orchestrated_responses_stream.py, the fast_mlsirm import gap in test_psychometric_routing.py, and the usage_source mismatch in test_spend_analytics.py::test_exact_output_without_prompt_usage_is_explicitly_unavailable). interrogate: 100% project-wide. Two other findings (api_contract.py's untyped orchestration.routing response shape, and cost_router.py/server.py calling the leading-underscore _candidate_routing_evidence directly) are left as-is -- see the PR comment for the reasoning; neither gets a code change here. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- README.md | 9 +- contextual_orchestrator/orchestrator.py | 39 +++- tests/test_candidate_routing_controls.py | 225 ++++++++++++++++++++++- 3 files changed, 266 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 44424a50d..bc3b186e3 100644 --- a/README.md +++ b/README.md @@ -270,9 +270,12 @@ is read from a **KV config store**, never `os.getenv`. 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 preserves the existing request - and response contract. Concrete provider model names cannot be combined with - these controls; candidate IDs remain absent from `/v1/models`. + `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 { diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 4857135df..b160823a0 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -4034,7 +4034,16 @@ def candidate_routing_policy( 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") - configured = {agent.id: agent for agent in self.candidates} + # 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") @@ -4063,7 +4072,7 @@ def candidate_routing_policy( model_name != self.FREE_MODEL or self._is_general_free_agent(agent) ) - for agent in self.candidates + for agent in configured.values() ) for role in required_roles ): @@ -4110,13 +4119,32 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N for value in [row.get("agent_id")] if isinstance(value, str) and value ] + # conduct() can serve a non-final step's output as the answer (the + # verifier-required fallback to the worker's output, or the + # verifier's own output when a synthesizer step is not required) -- + # the trace still records every step's provider call, so the *last* + # row is not reliably the one that produced ``answer``. Prefer the + # row(s) whose recorded output actually match the served answer; + # fall back to the last-row heuristic for callers (plain passthrough, + # cache hits) that never populate "answer"/"output" at all. + answer = result.get("answer") + answering_rows = ( + [ + row + for row in rows + if isinstance(row, Mapping) and row.get("output") == answer + ] + if isinstance(answer, str) + else [] + ) + served_rows = answering_rows or rows served = ( None if tracked_attempts == [] else next( ( value - for row in reversed(rows) + for row in reversed(served_rows) if isinstance(row, Mapping) for value in [row.get("served_agent_id") or row.get("agent_id")] if isinstance(value, str) and value @@ -6953,6 +6981,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 diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 052805555..e1d2522e6 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import json import threading import urllib.error @@ -11,7 +12,7 @@ from contextual_orchestrator import ModelAgent, TaskOrchestrator from contextual_orchestrator.cost_router import CostRoutingCoordinator -from contextual_orchestrator.orchestrator import ModelClient +from contextual_orchestrator.orchestrator import ModelClient, WorkflowStep from contextual_orchestrator.server import ( RequestError, SecurityConfig, @@ -793,3 +794,225 @@ def proxy_completion(*_args, **_kwargs): assert observed == [True] assert result["orchestration"]["workflow_run_id"] == "run_plain_candidate" + + +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" From 226244d80ae41dae217b65fda6fb3d6d17d9790d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 23:37:03 +0000 Subject: [PATCH 17/35] fix(routing): narrow auto-mode candidate-pin preflight to the roles the resolved route actually needs Two new Devin findings on top of the previous round's fixes (endpoint- filtered preflight, generated-planner attempt recording, worker-fallback routing evidence): 1. candidate_routing_policy()'s auto-mode preflight unconditionally required all four conduct roles (thinker/worker/verifier/synthesizer) before triage, even for FREE_MODEL requests where would_route already decides the direct worker-only route from model_name alone with zero provider calls. A worker-only pin for FREE_MODEL was rejected with a 400 even though the resolved route would never need the other three roles. Extracted the role-selection logic into TaskOrchestrator.candidate_pin_required_roles(mode, model_name) and applied it consistently at all three preflight call sites (server.py's chat and both Responses endpoints, cost_router.py's direct CostRoutingCoordinator.complete callers). GATEWAY_DEFAULT_MODEL/ AUTO_MODEL auto requests still conservatively require the full role set, since that decision genuinely needs a pin-scoped live triage call preflight can't safely make ahead of time -- this is a deliberate, documented limit, not something this fix widens. 2. The reported "concurrent attempts disappear from evidence" ContextVar race finding turned out to be a false positive on investigation: race_first_valid's worker threads run inside copy_context().run(...), which only isolates ContextVar.set()/reset() calls made inside the copy -- it's a shallow copy, so an already-bound mutable list object (the attempted-candidate-ids list _record_candidate_attempt appends to in place, set once up front by candidate_routing_policy before the race starts) is the *same object* inside every raced worker's copied context. In-place mutation of that shared list is therefore visible back in the parent request context once the race resolves. Added test_race_attempts_all_reach_candidate_routing_evidence, which forces a genuine timing overlap between two racing candidates (an Event-gated slow loser and a fast winner) and asserts both actually land in attempted_candidate_ids -- proving the existing behavior already correct rather than "fixing" something that wasn't broken. Verified: tests/test_candidate_routing_controls.py + test_endpoint_race.py (41 passed), interrogate 100% on touched files, git diff --check clean. Full suite validation in progress. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/cost_router.py | 6 +- contextual_orchestrator/orchestrator.py | 31 +++++++ contextual_orchestrator/server.py | 20 ++-- tests/test_candidate_routing_controls.py | 112 +++++++++++++++++++++++ tests/test_endpoint_race.py | 71 ++++++++++++++ 5 files changed, 223 insertions(+), 17 deletions(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index b55c59fa8..3e75aa7ea 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -757,9 +757,9 @@ def complete( with self.orchestrator.request_policy(zdr_only), self.orchestrator.candidate_routing_policy( routing_controls, model_name=model_name, - required_roles=("worker",) - if mode == "route" - else ("thinker", "worker", "verifier", "synthesizer"), + required_roles=self.orchestrator.candidate_pin_required_roles( + mode, model_name + ), ): result = self.orchestrator.run(messages, **run_kwargs) routing_evidence = self.orchestrator._candidate_routing_evidence(result) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index b160823a0..92ebcc320 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -5410,6 +5410,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], diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index fe294f556..72775e9bc 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -7182,10 +7182,8 @@ def proxy_tool_request() -> dict[str, Any]: orchestrator, request_routing, model_name, - required_roles=( - ("worker",) - if mode == "route" - else ("thinker", "worker", "verifier", "synthesizer") + required_roles=orchestrator.candidate_pin_required_roles( + mode, model_name ), required_tags=( ("vision",) @@ -7828,11 +7826,8 @@ def proxy_tool_request() -> dict[str, Any]: orchestrator, responses_routing_control, model_name, - required_roles=( - "thinker", - "worker", - "verifier", - "synthesizer", + required_roles=orchestrator.candidate_pin_required_roles( + "auto", model_name ), required_tags=( ("vision",) @@ -7909,11 +7904,8 @@ def proxy_tool_request() -> dict[str, Any]: orchestrator, responses_routing_control, model_name, - required_roles=( - "thinker", - "worker", - "verifier", - "synthesizer", + required_roles=orchestrator.candidate_pin_required_roles( + "auto", model_name ), required_tags=( ("vision",) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index e1d2522e6..8c077674a 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -351,6 +351,118 @@ def test_http_conduct_preflight_rejects_role_ineligible_pin() -> None: 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_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( 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 = [ From 1cc5c0312f03b1a3edbb55e74dc1ad5c577c9061 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 00:01:18 +0000 Subject: [PATCH 18/35] fix(routing): normalize batch hints under candidate control, share triage/stream evidence scope, document race invariant Fourth round of Devin findings on this PR: 1. Structured chat (response_format present) rejected routing.channel= "batch" / routing.latency_tolerant=true outright, even when an active candidate_id pin or non-empty exclude_candidate_ids should force synchronous execution first -- CostRoutingCoordinator.complete already has this precedence for ordinary chat. Added the same has-candidate-controls check to the structured-chat branch: batch/ latency-tolerant hints are only rejected when no candidate control is active. 2. Auto-mode streaming chat opened a candidate_routing_policy scope for the would_route triage decision, closed it, then _stream_route_completion opened a fresh one for the actual streamed execution -- discarding the triage call's attempted-candidate entry from the terminal orchestration.routing evidence whenever the free-only triage pool and the full worker pool resolve to different agents. Restructured so one scope now spans both the triage decision and (for the streaming branch) the streamed execution; _stream_route_completion takes a new candidate_scope_open flag so it doesn't open a redundant nested scope that would shadow the caller's. Also reused the same candidate_routing_policy() the caller triage happens in. 3. Documented the race_first_valid/_record_candidate_attempt in-place- mutation invariant directly at the ContextVar definition and at _record_candidate_attempt: a future .set() or read-then-reassign inside a raced worker's copy_context() would silently escape the parent request's evidence. No behavior change -- the existing design was already proven correct last round (test_race_attempts_all_reach_candidate_routing_evidence); this is purely a guardrail comment for future edits. 4. Both /v1/chat/completions and /v1/responses parsed routing.* twice -- once for endpoint scoping, again inside the handler with identical arguments. _validate_routing is pure so this was never a live bug, but CodeRabbit flagged the duplication as a drift risk; both call sites now reuse the already-validated endpoint_routing value. Added test_structured_chat_with_active_candidate_pin_normalizes_batch_channel_to_sync, ..._normalizes_latency_tolerant_to_sync, and test_auto_stream_shares_candidate_scope_with_triage_when_triage_and_worker_differ (a genuinely divergent free-triage-pool vs full-worker-pool scenario, not reachable by the existing pin-forces-one-agent test). Verified: tests/test_candidate_routing_controls.py + test_endpoint_race.py (44 passed), tests/test_orchestrated_responses_stream.py + test_streaming.py + test_tool_loop_role_effort_catalog_http.py (34 passed, 3 known pre-existing failures unrelated to this change), interrogate 100% on touched files, git diff --check clean. Full suite validation in progress. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/orchestrator.py | 19 ++++ contextual_orchestrator/server.py | 133 ++++++++++++++-------- tests/test_candidate_routing_controls.py | 136 +++++++++++++++++++++++ 3 files changed, 243 insertions(+), 45 deletions(-) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 92ebcc320..59ae0ad0c 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -334,6 +334,18 @@ def _cost_usd_decimal(output_tokens: int, price_per_million: float) -> Decimal: _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,}"), @@ -4097,6 +4109,13 @@ def _request_candidate_allowed(agent: ModelAgent) -> bool: @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) diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 72775e9bc..10b3198cb 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -6983,9 +6983,12 @@ 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) - request_routing = _validate_routing( - body.get("routing"), allow_candidate_controls=True, allow_endpoint=True - ) + # 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 _validate_candidate_routing(orchestrator, request_routing, model_name) # Coerce stream early so stream_options fail-closed matches route path # and tools/response_format passthrough cannot skip type checks. @@ -7093,9 +7096,25 @@ def proxy_tool_request() -> dict[str, Any]: else: structured_messages = _validate_messages(body.get("messages")) structured_routing = request_routing - if structured_routing and ( - structured_routing.get("channel") == "batch" - or structured_routing.get("latency_tolerant") is True + # 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 ( + 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, @@ -7191,21 +7210,56 @@ def proxy_tool_request() -> dict[str, Any]: else () ), ) + started_at = time.perf_counter() + model_client = orchestrator.client + # 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: - if explicit_trace: - raise RequestError( - 400, - "unsupported_trace_disclosure", - "remove include_orchestration_trace or use Responses streaming", - ) - include_trace = False - elif include_trace: + if route_stream: + 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 + if include_trace: self._authorize_trace_access() # stream + stream_options already coerced/validated before passthrough. attribution = _validate_attribution(body.get("attribution")) @@ -7228,8 +7282,6 @@ def proxy_tool_request() -> dict[str, Any]: # 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, @@ -7237,27 +7289,6 @@ def proxy_tool_request() -> dict[str, Any]: presence_penalty=presence_penalty, frequency_penalty=frequency_penalty, ): - if route_stream: - self._stream_route_completion( - orchestrator, - security, - messages, - model_name, - routing=routing, - 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, - }, - ) - return result = self._run(lambda: coordinator.complete( messages, mode=mode, @@ -7593,9 +7624,12 @@ def proxy_tool_request() -> dict[str, Any]: # 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) - responses_routing_control = _validate_routing( - body.get("routing"), allow_candidate_controls=True, allow_endpoint=True - ) + # 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 ) @@ -8756,8 +8790,17 @@ def _stream_route_completion( *, 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()) @@ -8808,11 +8851,11 @@ def usage_frame(usage: dict[str, Any]) -> str: {"include_usage": True, "usage_callback": capture_usage} ) candidate_scope = ( - orchestrator.candidate_routing_policy( + nullcontext() + if candidate_scope_open or not routing + else orchestrator.candidate_routing_policy( routing, model_name=model_name ) - if routing - else nullcontext() ) with candidate_scope: for delta in orchestrator.stream_route(messages, **stream_kwargs): diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 8c077674a..3ed55ffc8 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -76,6 +76,26 @@ def chat(self, agent, messages, effort_profile=None): 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", @@ -579,6 +599,65 @@ def test_candidate_pin_is_honored_by_structured_and_streaming_chat_paths() -> No 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"]} @@ -684,6 +763,63 @@ def test_auto_stream_triage_and_completion_both_honor_the_pin() -> None: 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", + } + + def test_core_rejects_file_affinity_that_conflicts_with_pin() -> None: orchestrator = TaskOrchestrator( [ From 31655f68719974fb4495258517a904824da9ec32 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 00:34:42 +0000 Subject: [PATCH 19/35] fix(routing): extend candidate-control batch precedence to Responses, close direct-API malformed-control gap, fix duplicate-text served-candidate misattribution Three more Devin findings against 1cc5c031, verified against the actual code before fixing (fifth triage round on PR #983): 1. /v1/responses' routing validation rejected routing.channel=batch and routing.latency_tolerant=true outright, without the has-candidate-controls precedence already applied to structured chat in the prior round (and already present in CostRoutingCoordinator.complete()). An active candidate_id or exclude_candidate_ids control now forces sync the same way, for both the JSON and streaming Responses paths. New tests: test_responses_with_active_candidate_pin_normalizes_batch_channel_to_sync and its latency_tolerant sibling in tests/test_candidate_routing_controls.py. 2. candidate_routing_policy()'s no-controls early return checked truthiness, not type: a direct Python-API caller passing a present-but-falsy malformed exclude_candidate_ids (empty string, False, an explicit None, an empty mapping) was silently treated as "no control" instead of failing loudly. The HTTP layer's own _validate_routing already rejects these shapes with a 400 before candidate_routing_policy ever sees them; this closes the same gap for direct callers (TaskOrchestrator.candidate_routing_policy itself, and cost_router.CostRoutingCoordinator.complete()'s hints passthrough). Present-but-non-list/tuple exclude_candidate_ids and present-but-non-string (and non-None) candidate_id now raise before the early return; absent fields, an empty list/tuple, and an explicit None candidate_id remain no-ops per the existing contract. New test: test_candidate_routing_policy_rejects_present_but_falsy_malformed_controls. 3. _candidate_routing_evidence() matched trace rows by output text equality (added in 828a1b86 to handle the verifier-rejection worker fallback) but preferred the *latest* matching row via reversed(). When a later step (e.g. a synthesizer) happens to emit byte-identical text to an earlier fallback answer, evidence named the later, unserved candidate. Implemented the minimal fix (not the invasive conduct()-return-shape redesign): among text matches, prefer the *earliest* row -- conduct()'s only fallback path serves an earlier step's already-produced output, so the served step is never later in the trace than a row that merely duplicates its text. The prior last-row heuristic is preserved unchanged for the no-text-match fallback (plain passthrough / cache-hit callers). New test: test_served_candidate_id_prefers_earliest_row_on_duplicate_fallback_text, confirmed to fail against the pre-fix code (misidentifies synth_agent) and pass against the fix (correctly identifies worker_agent). Full suite: 3334 passed, 5 failed (all pre-existing and unrelated -- 3x orchestrator/free streaming regression in test_orchestrated_responses_stream.py, the fast_mlsirm import gap in test_psychometric_routing.py, and the usage_source mismatch in test_spend_analytics.py::test_exact_output_without_prompt_usage_is_explicitly_unavailable), 2 skipped, 748.42s. interrogate: 100% on touched files and project-wide. Two findings from this same round (race evidence tracking, cache key isolation) were purely informational confirmations of already-documented behavior from earlier rounds -- no code change, not re-touched. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/orchestrator.py | 28 +++- contextual_orchestrator/server.py | 25 ++- tests/test_candidate_routing_controls.py | 196 +++++++++++++++++++++++ 3 files changed, 246 insertions(+), 3 deletions(-) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 59ae0ad0c..f2aec5204 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -4023,6 +4023,22 @@ def candidate_routing_policy( 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 candidate_id is not None + 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 at most 32 agent IDs") if candidate_id is None and not excluded: yield return @@ -4157,13 +4173,23 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N else [] ) served_rows = answering_rows or rows + # Among *text* matches, prefer the earliest row: conduct()'s only + # fallback path serves an earlier step's already-produced output + # (the worker's, once the verifier rejects a later step), so the + # served step is never later in the trace than a row that merely + # happens to duplicate its text (e.g. a synthesizer that + # independently produces byte-identical output). When nothing + # matched by text (the plain-passthrough/cache-hit callers that + # never populate answer/output), fall back to the prior last-row + # heuristic over the full, unfiltered trace (#983 finding 3). + ordered_rows = served_rows if answering_rows else reversed(served_rows) served = ( None if tracked_attempts == [] else next( ( value - for row in reversed(served_rows) + for row in ordered_rows if isinstance(row, Mapping) for value in [row.get("served_agent_id") or row.get("agent_id")] if isinstance(value, str) and value diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 10b3198cb..0b7784371 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -7748,13 +7748,34 @@ def proxy_tool_request() -> dict[str, Any]: if "routing" in body: 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", diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 3ed55ffc8..dc6372ed3 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -297,6 +297,48 @@ def test_candidate_controls_reject_an_unserviceable_worker_pool() -> None: 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 + + for value in (False, 0, 1.5, [], {}): + with pytest.raises(ValueError, match="candidate_id"): + with orchestrator.candidate_routing_policy({"candidate_id": value}): + pass + + # Absent fields, an explicit None routing mapping, an explicit None + # candidate_id, and an explicit empty list exclude_candidate_ids all + # remain no-ops -- the existing contract for "no control requested". + with orchestrator.candidate_routing_policy(None): + pass + with orchestrator.candidate_routing_policy({}): + pass + with orchestrator.candidate_routing_policy({"candidate_id": None}): + 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( [ @@ -692,6 +734,87 @@ def test_candidate_pin_is_honored_by_responses_json_and_stream_paths() -> None: 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( @@ -1264,3 +1387,76 @@ def chat(self, agent, messages, effort_profile=None): ] 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" From 6183433197d6a48d20750c8650bed436980802fc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 01:14:55 +0000 Subject: [PATCH 20/35] fix(routing): resolve served candidate by step identity, not text match; close two candidate-preflight gaps _candidate_routing_evidence's "prefer earliest text match" heuristic (from the prior commit) is a mirror-image trade-off, not a fix: when the genuinely served answer is itself a later step whose output coincidentally duplicates an earlier step's text, "prefer earliest" misattributes credit to the wrong (earlier) candidate. No earliest/latest ordering over text-matched rows can be correct in both directions simultaneously. conduct() already knows, at the point `answer` is assigned, exactly which step produced it -- in every branch (template/generated plan, verifier accepted/rejected-with-fallback). Thread that identity through explicitly as `answering_step_id` on the result dict, and have _candidate_routing_evidence resolve the served row by that id when present, falling back to the prior text-matching/last-row heuristics only for callers that never populate it (plain passthrough/failover, and workflow records persisted before this field existed). Also fixes two related preflight gaps a fresh Devin review found on this PR's own diff: - api_contract.py's CandidateRoutingControls OpenAPI schema forbade the supported `routing.endpoint` field via additionalProperties: false, so OpenAPI validators would reject valid endpoint-scoped requests that server.py's _validate_routing(..., allow_endpoint=True) already accepts. - Two candidate-routing preflight call sites (structured /v1/chat/completions with tools/response_format, and the raw single-agent /v1/responses provider passthrough) validated only roles, not the request's vision requirement -- unlike the ordinary chat and orchestrated Responses branches, which already derive required_tags from the request's own messages. An image-bearing request pinned to a non-vision candidate on either path fell through to a later, less specific execution error instead of failing closed with invalid_routing at preflight. New regression tests cover: a later, non-fallback step's output coincidentally duplicating an earlier step's text (the mirror-image scenario the prior fix missed); the text-matching fallback path still working when answering_step_id is absent; and HTTP-level 400 invalid_routing on both newly-covered vision-preflight paths. Full suite: 3338 passed, 5 known pre-existing failures (unrelated: reasoning-summary streaming order, responses message-array preservation, stream-failure event, fast_mlsirm import, spend-analytics usage_source), 2 skipped. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/api_contract.py | 10 ++ contextual_orchestrator/orchestrator.py | 73 +++++--- contextual_orchestrator/server.py | 39 ++++- tests/test_candidate_routing_controls.py | 202 +++++++++++++++++++++++ 4 files changed, 303 insertions(+), 21 deletions(-) diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index c300c7cde..7446572a9 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -47,6 +47,16 @@ "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, }, diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index f2aec5204..1f76dfe6e 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -4154,34 +4154,58 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N for value in [row.get("agent_id")] if isinstance(value, str) and value ] - # conduct() can serve a non-final step's output as the answer (the - # verifier-required fallback to the worker's output, or the - # verifier's own output when a synthesizer step is not required) -- - # the trace still records every step's provider call, so the *last* - # row is not reliably the one that produced ``answer``. Prefer the - # row(s) whose recorded output actually match the served answer; - # fall back to the last-row heuristic for callers (plain passthrough, - # cache hits) that never populate "answer"/"output" at all. - answer = result.get("answer") + # conduct() records the id of the step whose output actually became + # ``answer`` as ``answering_step_id`` (a non-final step when the + # verifier rejects and falls back to the worker's output). Resolving + # the served row by that identity is unambiguous even when a later, + # genuinely-served step's output happens to duplicate an earlier + # step's text byte-for-byte -- text equality alone cannot tell a + # fallback-to-earlier-step apart from a later step that coincidentally + # repeats an earlier step's text (#983 finding 6). Callers that never + # run a multi-step workflow, or a workflow record persisted before + # this field existed, fall back to the text-matched/last-row + # heuristics below. + answering_step_id = result.get("answering_step_id") answering_rows = ( [ row for row in rows - if isinstance(row, Mapping) and row.get("output") == answer + if isinstance(row, Mapping) and row.get("id") == answering_step_id ] - if isinstance(answer, str) + if isinstance(answering_step_id, int) else [] ) + if not answering_rows: + # conduct() can serve a non-final step's output as the answer (the + # verifier-required fallback to the worker's output, or the + # verifier's own output when a synthesizer step is not required) -- + # the trace still records every step's provider call, so the *last* + # row is not reliably the one that produced ``answer``. Prefer the + # row(s) whose recorded output actually match the served answer; + # fall back to the last-row heuristic for callers (plain passthrough, + # cache hits) that never populate "answer"/"output" at all. + answer = result.get("answer") + answering_rows = ( + [ + row + for row in rows + if isinstance(row, Mapping) and row.get("output") == answer + ] + if isinstance(answer, str) + else [] + ) served_rows = answering_rows or rows - # Among *text* matches, prefer the earliest row: conduct()'s only - # fallback path serves an earlier step's already-produced output - # (the worker's, once the verifier rejects a later step), so the - # served step is never later in the trace than a row that merely - # happens to duplicate its text (e.g. a synthesizer that - # independently produces byte-identical output). When nothing - # matched by text (the plain-passthrough/cache-hit callers that - # never populate answer/output), fall back to the prior last-row - # heuristic over the full, unfiltered trace (#983 finding 3). + # Among *text* matches (no ``answering_step_id`` evidence available), + # prefer the earliest row: this ordering is a best-effort fallback + # only, kept for workflow records persisted before + # ``answering_step_id`` existed. It cannot by itself distinguish a + # fallback-to-earlier-step from a later step that coincidentally + # duplicates an earlier one's text -- exactly why the + # ``answering_step_id`` lookup above takes priority whenever it is + # available (#983 finding 6). When nothing matched by text (the + # plain-passthrough/cache-hit callers that never populate + # answer/output), fall back to the prior last-row heuristic over the + # full, unfiltered trace (#983 finding 3). ordered_rows = served_rows if answering_rows else reversed(served_rows) served = ( None @@ -6889,6 +6913,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")) @@ -6901,8 +6929,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 @@ -6914,12 +6944,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, diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 0b7784371..961c3fb27 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -6989,7 +6989,23 @@ def register_video_job(agent: ModelAgent, provider_result: dict[str, Any]) -> di # _validate_routing is pure, so this is behavior-preserving # and keeps the two call sites from drifting apart. request_routing = endpoint_routing - _validate_candidate_routing(orchestrator, request_routing, model_name) + # 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) @@ -8047,6 +8063,27 @@ def proxy_tool_request() -> dict[str, Any]: 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) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index dc6372ed3..ca164fe9c 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -1460,3 +1460,205 @@ def chat(self, agent, messages, effort_profile=None): 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_candidate_routing_evidence_falls_back_to_text_match_without_answering_step_id() -> None: + """A workflow record persisted before ``answering_step_id`` existed (or + any other caller that omits it) must still resolve routing evidence via + the prior text-matching/last-row heuristics rather than crashing or + silently returning no evidence.""" + + 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 evidence["served_candidate_id"] == "worker_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 == [] From 3536c2bdd8decd3818efd66673423093df124f4a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 01:45:02 +0000 Subject: [PATCH 21/35] fix(routing): share the candidate-routing scope across triage and conduct on the streamed auto path; document routing evidence in the OpenAPI schema For a streamed /v1/chat/completions request with mode="auto" against GATEWAY_DEFAULT_MODEL/AUTO_MODEL, server.py's would_route() triage call (which can attempt a real provider call via _needs_workflow -> _triage_workflow_required -> _compute_triage_verdict) ran inside a candidate_routing_policy scope that closed immediately after, whenever the decision was "conduct" rather than "route" (or the request was not streamed at all). CostRoutingCoordinator.complete() then opened its own, independent scope for the conduct execution, discarding the just-recorded triage attempt: the triage agent never appeared in attempted_candidate_ids even though a real provider call was made against it. Fix: extend the existing candidate_routing_policy scope (already shared with _stream_route_completion via candidate_scope_open for the route branch, from an earlier round of this PR) across the conduct fallthrough too, and give CostRoutingCoordinator.complete() the same candidate_scope_open flag so it skips opening a second, independent scope on this path -- reusing the caller's already-open one instead of resetting the attempted-candidate ContextVar out from under it. New regression test constructs an agent pool where a free-only, lower-priority triage_agent can only win the triage ranking while higher-priority conduct-role agents always win the general ranking (so triage_agent's presence in evidence isn't confounded with it merely winning a conduct role too), confirmed to fail without the fix (attempted_candidate_ids == ['synth_agent'], triage_agent silently lost) and pass with it (['triage_agent', 'synth_agent']). Also documents the previously-untyped orchestration.routing response field as a proper CandidateRoutingEvidence OpenAPI schema (Devin: "Routing evidence remains undocumented" -- OpenAPI declared the candidate-control request inputs but left the response evidence shape opaque to generated clients). Full suite: 3339 passed, 5 known pre-existing failures (unrelated to this PR: reasoning-summary streaming order, responses message-array preservation, stream-failure event, fast_mlsirm import, spend-analytics usage_source), 2 skipped. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/api_contract.py | 43 ++++++++++- contextual_orchestrator/cost_router.py | 36 ++++++++-- contextual_orchestrator/server.py | 90 +++++++++++++----------- tests/test_candidate_routing_controls.py | 76 ++++++++++++++++++++ 4 files changed, 195 insertions(+), 50 deletions(-) diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index 7446572a9..5d794199f 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -60,6 +60,42 @@ }, "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"], @@ -112,7 +148,12 @@ "type": "string", "enum": ["measured", "unavailable"], }, - "orchestration": {"type": "object"}, + "orchestration": { + "type": "object", + "properties": { + "routing": {"$ref": "#/components/schemas/CandidateRoutingEvidence"}, + }, + }, }, }, "ModelGroupWrite": { diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 3e75aa7ea..13fd59b2c 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,6 +556,16 @@ 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") @@ -754,13 +766,23 @@ def complete( } race_token = self._race_usage_context.set(race_context) try: - with self.orchestrator.request_policy(zdr_only), self.orchestrator.candidate_routing_policy( - routing_controls, - model_name=model_name, - required_roles=self.orchestrator.candidate_pin_required_roles( - mode, model_name - ), - ): + # 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: diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 961c3fb27..7e30cdaa9 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -7275,48 +7275,54 @@ def proxy_tool_request() -> dict[str, Any]: }, ) return - 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, - ): - 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( diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index ca164fe9c..c63bab40d 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -943,6 +943,82 @@ def test_auto_stream_shares_candidate_scope_with_triage_when_triage_and_worker_d } +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"] + + def test_core_rejects_file_affinity_that_conflicts_with_pin() -> None: orchestrator = TaskOrchestrator( [ From f234a9ed024e4a10b2e7b7bb9bb7f4e123d2df5d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:17:58 +0000 Subject: [PATCH 22/35] fix(routing): thread served-step identity through structured synthesis, record embed/judge attempts Two follow-up fixes to the candidate routing evidence work (#983), both from a fresh Devin review round after round 7 landed: 1. "Repeated output misidentifies served candidate" -- the structured/ tool-loop workflow record built by _orchestrated_provider_completion (used by the /v1/chat/completions tool-loop and /v1/responses provider passthrough) never recorded which trace row (the initial synthesis row, or the repair row when a repair ran and succeeded) actually produced `answer`. When that output happened to duplicate an earlier internal conduct() workflow step's text byte-for-byte, _candidate_routing_evidence fell back to the fragile text-match heuristic and could misattribute the served candidate to the internal step instead of the real synthesizer (or repairer). Fixed by recording `answering_step_id` on the persisted workflow record, mirroring conduct()'s own answering_step_id fix from round 6. Two regression tests cover both the synthesis-duplicates- earlier-step and repair-duplicates-earlier-step cases, each empirically verified (via a stash A/B) to fail without the fix and pass with it. 2. "Attempt evidence omits provider calls" -- three call sites bypassed _record_candidate_attempt because they call the provider client directly instead of going through _invoke() (which self-instruments): _FastMLSIJudgeAdapter.complete_structured (calls client.proxy_send directly), and _embed_cached/_descriptor_vector_cached (call client.embed directly, but only on a cache miss -- a cache hit must NOT record a phantom attempt). Fixed by adding the missing _record_candidate_attempt calls at the correct point in each -- after the cache-miss check but before the provider call. Three regression tests cover all three sites, each empirically verified to fail without the fix. Full suite: 3344 passed, 5 known pre-existing failures (unchanged from round 7), 2 skipped. interrogate: 100% docstring coverage (unchanged, these are private methods). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/orchestrator.py | 29 +++++ tests/test_candidate_routing_controls.py | 132 +++++++++++++++++++++++ tests/test_measured_routing_evidence.py | 56 ++++++++++ tests/test_model_judge.py | 36 +++++++ 4 files changed, 253 insertions(+) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 1f76dfe6e..af4fb4578 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -445,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 ) @@ -5258,6 +5264,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(), @@ -7531,6 +7550,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 @@ -7558,6 +7582,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)] diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index c63bab40d..c970501a3 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -1643,6 +1643,138 @@ def test_candidate_routing_evidence_falls_back_to_text_match_without_answering_s assert evidence["served_candidate_id"] == "worker_agent" +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 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..5f11b19ca 100644 --- a/tests/test_model_judge.py +++ b/tests/test_model_judge.py @@ -676,6 +676,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") From e8ed2eab4a170366e5be15e3b8f4f09cd9da6f18 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:55:37 +0000 Subject: [PATCH 23/35] fix(routing): carry answering_step_id through run(), fix candidate_id=None gaps Three more fixes to the candidate routing evidence work (#983), from a fresh Devin + CodeRabbit review round on round 8's push: 1. "Duplicate outputs misidentify serving candidate" (Devin) -- TaskOrchestrator.run() reconstructs its own persisted workflow-run record from complete()'s result, but dropped the answering_step_id field that conduct() now returns (round 6). Every run()-based caller that resolves routing evidence from the persisted record -- CostRoutingCoordinator.complete() in cost_router.py chiefly, i.e. the coordinator/HTTP ordinary chat and non-streamed Responses paths -- therefore lost that identity and fell back to the fragile text-match heuristic, which can misattribute a duplicate-text answer to an earlier step. Fixed by carrying result.get("answering_step_id") through into the record run() persists. New regression test exercises the exact scenario through run() (not conduct() directly), duplicating round 6/8's proof pattern. 2. "candidate_id=None passes the malformed check" (CodeRabbit, Major) -- candidate_routing_policy's present-but-malformed guard required `candidate_id is not None` before checking `not isinstance(candidate_id, str)`, so an explicit `candidate_id: None` slipped through to the no-op branch instead of raising -- directly contradicting the guard's own documented intent ("a present-but-malformed control must fail validation even when its value is falsy ... explicit None"), and inconsistent with exclude_candidate_ids=None, which already raised correctly. Removed the `is not None` guard so candidate_id is treated exactly like exclude_candidate_ids: presence decides whether the type check applies. The existing test asserting the old (buggy) "None is a no-op" behavior is corrected to assert the documented-and-now-actual behavior. 3. "direct Python API callers can lose or bypass routing validation" (CodeRabbit, Minor) -- CostRoutingCoordinator.complete()'s has_candidate_controls used truthiness (bool(hints.get("candidate_id") or hints.get("exclude_candidate_ids"))), so candidate_id=None was indistinguishable from an absent key and let a batch-channel request take the early BatchRequest return path before candidate_routing_policy's validation (fix 2, above) ever ran -- silently dropping the malformed control instead of surfacing it. Switched to presence-based detection for candidate_id, keeping only an explicit empty exclude_candidate_ids list/tuple as the genuine no-op exemption (preserving the earlier, separate #983 fix for that case). Two new regression tests cover both the malformed-None-not-dropped case and the still-a-no-op explicit-empty-list case. All three empirically verified via a git-stash A/B: every new/corrected assertion fails against the pre-fix code and passes with the fix. Full suite: 3347 passed (3344 + 3 new tests), 5 known pre-existing failures (unchanged), 2 skipped. interrogate: 100% docstring coverage. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/cost_router.py | 21 +++- contextual_orchestrator/orchestrator.py | 20 +++- tests/test_candidate_routing_controls.py | 132 +++++++++++++++++++++-- 3 files changed, 159 insertions(+), 14 deletions(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 13fd59b2c..f9dc0e11c 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -572,9 +572,24 @@ def complete( if type(zdr_only) is not bool: raise TypeError("zdr_only must be a boolean") routing_controls = hints if isinstance(hints, dict) else {} - has_candidate_controls = bool( - routing_controls.get("candidate_id") - or routing_controls.get("exclude_candidate_ids") + # Detect an active candidate control by key *presence*, not + # truthiness: an explicitly malformed value (candidate_id=None, + # exclude_candidate_ids=None or a non-list/tuple) must still force + # the sync path below so TaskOrchestrator.candidate_routing_policy's + # real validation gets a chance to reject it, rather than 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 -- it is not a request for any + # candidate behavior -- so it alone stays excluded from this check + # (#983 Devin/CodeRabbit finding: direct Python API callers can lose + # or bypass routing validation). + excluded_control = routing_controls.get("exclude_candidate_ids") + excluded_is_explicit_empty = ( + isinstance(excluded_control, (list, tuple)) and not excluded_control + ) + has_candidate_controls = "candidate_id" in routing_controls or ( + "exclude_candidate_ids" in routing_controls + and not excluded_is_explicit_empty ) routing_hints = hints if isinstance(hints, RoutingHints) else RoutingHints.from_mapping(hints) try: diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index af4fb4578..c4224e534 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -4037,11 +4037,7 @@ def candidate_routing_policy( # 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 candidate_id is not None - and not isinstance(candidate_id, str) - ): + 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 at most 32 agent IDs") @@ -5696,6 +5692,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(), diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index c970501a3..8af7685e1 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -319,20 +319,25 @@ def test_candidate_routing_policy_rejects_present_but_falsy_malformed_controls() ): pass - for value in (False, 0, 1.5, [], {}): + # 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, an explicit None - # candidate_id, and an explicit empty list exclude_candidate_ids all - # remain no-ops -- the existing contract for "no control requested". + # 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({"candidate_id": None}): - pass with orchestrator.candidate_routing_policy({"exclude_candidate_ids": []}): pass with orchestrator.candidate_routing_policy({"exclude_candidate_ids": ()}): @@ -492,6 +497,49 @@ def test_http_auto_preflight_accepts_worker_only_pin_when_free_model_always_rout 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 @@ -1613,6 +1661,78 @@ def chat(self, agent, messages, effort_profile=None): 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_falls_back_to_text_match_without_answering_step_id() -> None: """A workflow record persisted before ``answering_step_id`` existed (or any other caller that omits it) must still resolve routing evidence via From ef5ae0143a2b19eb32ad3e826a6c21755ba7451d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 12:05:59 +0900 Subject: [PATCH 24/35] test(routing): reject heuristic candidate-control limits --- ...t_candidate_routing_no_heuristic_limits.py | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 tests/test_candidate_routing_no_heuristic_limits.py 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" From 3d045c6ee416821e23c6f27963921e5d591e136d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 12:06:33 +0900 Subject: [PATCH 25/35] build(repair): add exact PR983 no-heuristics source fix --- ...fix_983_no_heuristic_candidate_controls.py | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 scripts/source_fix_983_no_heuristic_candidate_controls.py diff --git a/scripts/source_fix_983_no_heuristic_candidate_controls.py b/scripts/source_fix_983_no_heuristic_candidate_controls.py new file mode 100644 index 000000000..3318581af --- /dev/null +++ b/scripts/source_fix_983_no_heuristic_candidate_controls.py @@ -0,0 +1,88 @@ +"""Apply the exact no-heuristics repair for PR #983 candidate controls.""" + +from pathlib import Path + + +def replace_once(path: str, old: str, new: str) -> None: + target = Path(path) + text = target.read_text() + count = text.count(old) + if count != 1: + raise SystemExit(f"{path}: expected one exact match, found {count}") + target.write_text(text.replace(old, new, 1)) + + +def splice(path: str, start: str, end: str, replacement: str) -> None: + target = Path(path) + text = target.read_text() + start_index = text.find(start) + if start_index < 0: + raise SystemExit(f"{path}: start marker not found") + end_index = text.find(end, start_index) + if end_index < 0: + raise SystemExit(f"{path}: end marker not found") + target.write_text(text[:start_index] + replacement + text[end_index:]) + + +replace_once( + "contextual_orchestrator/server.py", + ' if not isinstance(excluded, list) or len(excluded) > 32:\n raise RequestError(\n 400,\n "invalid_routing",\n "routing.exclude_candidate_ids must be an array of at most 32 agent IDs",\n )', + ' if not isinstance(excluded, list):\n raise RequestError(\n 400,\n "invalid_routing",\n "routing.exclude_candidate_ids must be an array of agent IDs",\n )', +) + +replace_once( + "contextual_orchestrator/orchestrator.py", + ' raise ValueError("exclude_candidate_ids must contain at most 32 agent IDs")', + ' raise ValueError("exclude_candidate_ids must contain agent IDs")', +) +replace_once( + "contextual_orchestrator/orchestrator.py", + ' if not isinstance(excluded, (list, tuple)) or len(excluded) > 32:\n raise ValueError("exclude_candidate_ids must contain at most 32 agent IDs")', + ' if not isinstance(excluded, (list, tuple)):\n raise ValueError("exclude_candidate_ids must contain agent IDs")', +) + +start = ''' # conduct() records the id of the step whose output actually became\n''' +end = ''' evidence: dict[str, Any] = {\n''' +replacement = ''' # Serving identity is evidence, not an inference target. Multi-step\n # workflows record the exact answering_step_id; provider-shaped paths\n # may record served_agent_id explicitly. Historical records lacking\n # either identity remain auditable for attempts but fail closed for\n # served_candidate_id. Output equality and trace position are not\n # admissible serving-identity evidence.\n answering_step_id = result.get("answering_step_id")\n answering_rows = (\n [\n row\n for row in rows\n if isinstance(row, Mapping) and row.get("id") == answering_step_id\n ]\n if isinstance(answering_step_id, int)\n else []\n )\n served: str | None = None\n if len(answering_rows) == 1:\n row = answering_rows[0]\n value = row.get("served_agent_id") or row.get("agent_id")\n if isinstance(value, str) and value:\n served = value\n elif tracked_attempts != []:\n explicit_served = [\n value\n for row in rows\n if isinstance(row, Mapping)\n for value in [row.get("served_agent_id")]\n if isinstance(value, str) and value\n ]\n distinct_served = tuple(dict.fromkeys(explicit_served))\n if len(distinct_served) == 1:\n served = distinct_served[0]\n''' +splice("contextual_orchestrator/orchestrator.py", start, end, replacement) + +replace_once( + "README.md", + '`routing.exclude_candidate_ids` (at most 32 unique IDs) to omit known-bad', + '`routing.exclude_candidate_ids` (unique exact IDs) to omit evidence-ineligible', +) +replace_once( + "docs/planning/adrs/0032-model-group-cost-aware-discovery.md", + 'pin an exact private agent ID with `routing.candidate_id` and exclude at most 32\nunique IDs with `routing.exclude_candidate_ids`.', + 'pin an exact private agent ID with `routing.candidate_id` and exclude unique exact\nIDs with `routing.exclude_candidate_ids`. Candidate membership has no repository-authored\ncardinality cutoff; normal authenticated request-size controls remain the resource boundary.', +) + +replace_once( + "tests/test_candidate_routing_controls.py", + 'def test_candidate_routing_evidence_falls_back_to_text_match_without_answering_step_id() -> None:\n """A workflow record persisted before ``answering_step_id`` existed (or\n any other caller that omits it) must still resolve routing evidence via\n the prior text-matching/last-row heuristics rather than crashing or\n silently returning no evidence."""', + 'def test_candidate_routing_evidence_fails_closed_without_answering_step_id() -> None:\n """A historical workflow without explicit serving identity must not infer one."""', +) +replace_once( + "tests/test_candidate_routing_controls.py", + ' assert evidence is not None\n assert evidence["served_candidate_id"] == "worker_agent"\n\n\ndef test_orchestrated_provider_completion_answering_step_id_identifies_synthesis_over_duplicate_internal_step()', + ' assert evidence is not None\n assert "served_candidate_id" not in evidence\n\n\ndef test_orchestrated_provider_completion_answering_step_id_identifies_synthesis_over_duplicate_internal_step()', +) + +for path, section in ( + ( + "docs/planning/adrs/0032-model-group-cost-aware-discovery.md", + """\n\n### No-heuristics amendment — 2026-09-02\n\nThe original request-local control used a fixed 32-ID exclusion ceiling and legacy\nserving-identity recovery from output equality/trace position. Neither decision rule\nwas identified by RouteLLM, FrugalGPT, an API standard, or measured deployment\nevidence. The cardinality ceiling is removed; authenticated request-size enforcement\nis the resource boundary. Serving identity is now reported only from exact\n`answering_step_id` or an explicit `served_agent_id`; historical rows without either\nremain attempt provenance and omit `served_candidate_id`. This is a fail-closed\nidentity rule rather than an inferred ranking/tie-break.\n""", + ), + ( + "docs/product-technical-gap-baseline.md", + """\n\n## 2026-09-02 — PR #983 candidate-control no-heuristics repair\n\nLive RCA found two decision-affecting rules in the request-local candidate-control\nowner: a repository-authored 32-ID exclusion ceiling and serving-candidate inference\nfrom output equality/trace position when exact identity was absent. Neither had an\nidentified mathematical, standards, experimental, or research basis. The canonical\nrepair removes the cardinality rule, retaining normal authenticated request-size\ncontrols, and makes serving identity fail closed unless exact `answering_step_id` or\nexplicit `served_agent_id` evidence exists. Regression coverage exercises more than\n32 exclusions, missing identity, and explicit identity provenance. Exact-head hosted\nchecks remain authoritative before merge.\n""", + ), + ( + "CHANGELOG.md", + """\n- 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.\n""", + ), +): + target = Path(path) + text = target.read_text() + if section.strip() not in text: + target.write_text(text.rstrip() + section + "\n") From 6d93c16369b967d559e556f1170a1acd561ce1e5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 12:06:44 +0900 Subject: [PATCH 26/35] ci(repair): add PR983 no-heuristics source fix --- ...ix-983-no-heuristic-candidate-controls.yml | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 .github/workflows/source-fix-983-no-heuristic-candidate-controls.yml diff --git a/.github/workflows/source-fix-983-no-heuristic-candidate-controls.yml b/.github/workflows/source-fix-983-no-heuristic-candidate-controls.yml new file mode 100644 index 000000000..d5a2aac8c --- /dev/null +++ b/.github/workflows/source-fix-983-no-heuristic-candidate-controls.yml @@ -0,0 +1,88 @@ +name: Source fix PR983 no-heuristic candidate controls + +on: + push: + branches: + - feat/stateless-candidate-controls + paths: + - .github/source-fix-983-no-heuristic-candidate-controls.trigger + +permissions: + contents: write + +jobs: + repair: + if: github.repository == 'ContextualWisdomLab/contextual-orchestrator' + runs-on: ubuntu-latest + steps: + - name: Checkout exact repair head + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # actions/checkout@v7 + with: + ref: ${{ github.sha }} + fetch-depth: 0 + persist-credentials: true + + - name: Set up Python + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # actions/setup-python@v6 + with: + python-version: "3.12" + + - name: Install hash-locked dependencies + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check --require-hashes -r requirements.lock + python -m pip install --disable-pip-version-check --no-deps -e . + + - name: Prove candidate-control regressions are RED + run: | + set -euo pipefail + set +e + python -m pytest -q tests/test_candidate_routing_no_heuristic_limits.py + status=$? + set -e + if [ "$status" -eq 0 ]; then + echo "::error::No-heuristics candidate-control regressions were unexpectedly GREEN before repair." + exit 1 + fi + + - name: Apply exact guarded owner repair + run: python scripts/source_fix_983_no_heuristic_candidate_controls.py + + - name: Verify candidate-control contracts + run: | + set -euo pipefail + python -m pytest -q \ + tests/test_candidate_routing_no_heuristic_limits.py \ + tests/test_candidate_routing_controls.py \ + tests/test_api_contract.py + git diff --check + + - name: Remove completed one-shot machinery + run: | + set -euo pipefail + rm -f \ + .github/workflows/source-fix-983-no-heuristic-candidate-controls.yml \ + .github/source-fix-983-no-heuristic-candidate-controls.trigger \ + scripts/source_fix_983_no_heuristic_candidate_controls.py + git diff --check + + - name: Commit verified owner repair + env: + EXPECTED_TRIGGER_HEAD: ${{ github.sha }} + run: | + set -euo pipefail + git fetch origin feat/stateless-candidate-controls + remote_head="$(git rev-parse origin/feat/stateless-candidate-controls)" + if [ "$remote_head" != "$EXPECTED_TRIGGER_HEAD" ]; then + echo "::error::Canonical branch moved; refusing to overwrite or guess a merge." + exit 1 + fi + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add -A + if git diff --cached --quiet; then + echo "::error::Source fix produced no publishable delta." + exit 1 + fi + git commit -m "fix(routing): remove heuristic candidate-control limits" + git push origin HEAD:feat/stateless-candidate-controls From 5166349dea1be54d53edc3e33ce185a90b159163 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 12:06:48 +0900 Subject: [PATCH 27/35] chore(repair): trigger PR983 no-heuristics source fix --- .github/source-fix-983-no-heuristic-candidate-controls.trigger | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/source-fix-983-no-heuristic-candidate-controls.trigger diff --git a/.github/source-fix-983-no-heuristic-candidate-controls.trigger b/.github/source-fix-983-no-heuristic-candidate-controls.trigger new file mode 100644 index 000000000..5fb902d52 --- /dev/null +++ b/.github/source-fix-983-no-heuristic-candidate-controls.trigger @@ -0,0 +1 @@ +source-fix-983-no-heuristic-candidate-controls From b05bf9003d2d35f19c653ea1923f09997f321e13 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:20:27 +0000 Subject: [PATCH 28/35] fix(routing): never trust provider-supplied _candidate_routing/orchestration fields Two more fixes to the candidate routing evidence work (#983), both from a fresh Devin review round on round 9's push: 1. "Provider fields forge routing evidence" -- CostRoutingCoordinator. complete()'s provider_request passthrough branch unconditionally republished a `_candidate_routing` field popped off the raw provider response as gateway-computed `orchestration.routing` evidence. TaskOrchestrator.proxy_completion() only ever sets that key itself when an active candidate control made _candidate_routing_evidence return non-None -- so with no active control, observing that key can only mean it arrived already-present on the provider's own (untrusted) response body. A coincidentally- or adversarially-named provider field could therefore forge fake served_candidate_id/attempted_candidate_ids into an ordinary response, violating the documented "present only when the request supplied routing controls" contract. 2. "Provider metadata crashes tool responses" -- server.py's single-agent tool-loop passthrough had the identical trust gap, plus a second bug: `result.setdefault("orchestration", {})["routing"] = evidence` assumes a pre-existing "orchestration" field (if the provider happened to return one) is a mapping. A provider response with a non-dict "orchestration" field (a string, list, ...) crashed this line with `TypeError: 'str' object does not support item assignment` *after* a successful inference call, turning a working response into a 500. Both call sites shared the same underlying question -- "was a candidate control genuinely active for this request" -- so this adds TaskOrchestrator._has_active_candidate_controls(routing) as the single source of truth (matching candidate_routing_policy's own no-op condition: key presence for candidate_id, presence-and-non-empty for exclude_candidate_ids) and uses it in three places: - cost_router.py's has_candidate_controls (refactored to call it, replacing the equivalent inline logic added in round 9) - cost_router.py's provider_request branch, gating the `_candidate_routing` republish - server.py's proxy_tool_request, gating the republish AND checking isinstance(orchestration, dict) before merging into it (falling back to a fresh dict, matching the OpenAPI schema's own "orchestration is an object" contract, when the provider's field isn't one) Two new regression tests (one coordinator-level, one full HTTP tool-loop request) inject a forged `_candidate_routing` field and a non-mapping `orchestration` field via a custom test client and assert: no crash, and the forged evidence never reaches the response. Verified via git stash A/B: both fail against the pre-fix code (the forged evidence leaks through; the HTTP request 500s with the exact TypeError Devin predicted) and pass with the fix. Pulled in four concurrent commits from a separate, already in-flight automated repair (`source-fix-983-no-heuristic-candidate-controls`, addressing distinct Devin findings about a hardcoded 32-ID exclusion cap and served_candidate_id's text-match fallback) via a clean fast-forward merge -- entirely disjoint files from this change, no conflicts. That repair's own code-changing commit had not landed yet as of this push, so its regression file (tests/test_candidate_routing_no_heuristic_limits.py) still has 2 known, pre-existing, not-yet-fixed failures unrelated to this PR; not this change's concern to fix (a separate automated workflow owns that repair). Full suite: 3349 passed, 5 known pre-existing failures (unchanged from round 9), 2 skipped -- run before the fast-forward merge landed, so it does not yet include the 3 not-yet-fixed no-heuristics regression tests (2 fail, 1 passes) tracked separately above. interrogate: 100% docstring coverage. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- contextual_orchestrator/cost_router.py | 42 ++++++---- contextual_orchestrator/orchestrator.py | 40 +++++++++ contextual_orchestrator/server.py | 23 +++++- tests/test_candidate_routing_controls.py | 101 +++++++++++++++++++++++ 4 files changed, 186 insertions(+), 20 deletions(-) diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index f9dc0e11c..2d3d023de 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -572,24 +572,22 @@ def complete( if type(zdr_only) is not bool: raise TypeError("zdr_only must be a boolean") routing_controls = hints if isinstance(hints, dict) else {} - # Detect an active candidate control by key *presence*, not - # truthiness: an explicitly malformed value (candidate_id=None, - # exclude_candidate_ids=None or a non-list/tuple) must still force - # the sync path below so TaskOrchestrator.candidate_routing_policy's - # real validation gets a chance to reject it, rather than silently - # falling through the batch branch's early return and dropping the + # 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 -- it is not a request for any - # candidate behavior -- so it alone stays excluded from this check - # (#983 Devin/CodeRabbit finding: direct Python API callers can lose - # or bypass routing validation). - excluded_control = routing_controls.get("exclude_candidate_ids") - excluded_is_explicit_empty = ( - isinstance(excluded_control, (list, tuple)) and not excluded_control - ) - has_candidate_controls = "candidate_id" in routing_controls or ( - "exclude_candidate_ids" in routing_controls - and not excluded_is_explicit_empty + # 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: @@ -666,7 +664,15 @@ 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) - if routing_evidence is not 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) diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index c4224e534..58b004aa5 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -4138,6 +4138,46 @@ def _record_candidate_attempt(agent_id: str) -> None: 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() diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 7e30cdaa9..4520e46be 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -7104,8 +7104,27 @@ def proxy_tool_request() -> dict[str, Any]: single_agent=True, ) evidence = result.pop("_candidate_routing", None) - if evidence is not None: - result.setdefault("orchestration", {})["routing"] = evidence + # 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) diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 8af7685e1..06b9f1545 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -1291,6 +1291,107 @@ def proxy_completion(*_args, **_kwargs): 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 From f1674b4c634859995f192eb5d5fb7893435a71c8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 12:24:54 +0900 Subject: [PATCH 29/35] chore(repair): retrigger PR983 no-heuristics on current head --- .github/source-fix-983-no-heuristic-candidate-controls.trigger | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/source-fix-983-no-heuristic-candidate-controls.trigger b/.github/source-fix-983-no-heuristic-candidate-controls.trigger index 5fb902d52..cc14de347 100644 --- a/.github/source-fix-983-no-heuristic-candidate-controls.trigger +++ b/.github/source-fix-983-no-heuristic-candidate-controls.trigger @@ -1 +1,2 @@ source-fix-983-no-heuristic-candidate-controls +retrigger-after-b05bf9003d2d35f19c653ea1923f09997f321e13 From 34f47a14bfaea137bba1a8594bd60a97466cbef6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 08:21:56 +0000 Subject: [PATCH 30/35] fix(routing): remove heuristic candidate-control limits Applies scripts/source_fix_983_no_heuristic_candidate_controls.py's documented repair directly -- its GitHub Actions workflow had been queued 4.5+ hours with zero progress due to severe org-wide Actions capacity congestion. Removes the unsupported 32-ID exclude_candidate_ids cardinality ceiling (normal authenticated request-size controls remain the resource boundary) and the output-equality/trace-position fallback for resolving served_candidate_id (neither had RouteLLM, FrugalGPT, API-standard, or measured-deployment support). Serving identity is now reported only from an exact answering_step_id row or explicit served_agent_id provenance; historical records with neither remain attempt provenance but omit served_candidate_id rather than guessing. The pre-written script's two orchestrator.py replace_once calls had gone ambiguous (matched 2 occurrences instead of 1) because an earlier round on this same branch had independently added a second, differently-conditioned raise using identical wording -- applied both by hand instead, anchoring on full surrounding context. Extending the fix to route_once()/stream_route()'s own trace-step construction was necessary to avoid a regression: those single-worker paths never set served_agent_id (only route_once's cross-endpoint failover case did), so the stricter evidence resolution would have silently stopped reporting served_candidate_id for the overwhelmingly common single-candidate route request. Both now record served_agent_id explicitly (equal to agent_id when there was no internal failover), which is a real fact being recorded, not an inference -- so the evidence function never needs to fall back to bare agent_id/trace position. Verified against tests/test_candidate_routing_no_heuristic_limits.py, tests/test_candidate_routing_controls.py (54 total), and tests/test_api_contract.py. Removes the now-completed one-shot repair machinery per convention. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- ...83-no-heuristic-candidate-controls.trigger | 2 - ...ix-983-no-heuristic-candidate-controls.yml | 88 ------------------ CHANGELOG.md | 1 + README.md | 2 +- contextual_orchestrator/orchestrator.py | 89 ++++++------------- contextual_orchestrator/server.py | 4 +- .../0032-model-group-cost-aware-discovery.md | 16 +++- docs/product-technical-gap-baseline.md | 12 +++ ...fix_983_no_heuristic_candidate_controls.py | 88 ------------------ tests/test_candidate_routing_controls.py | 9 +- 10 files changed, 61 insertions(+), 250 deletions(-) delete mode 100644 .github/source-fix-983-no-heuristic-candidate-controls.trigger delete mode 100644 .github/workflows/source-fix-983-no-heuristic-candidate-controls.yml delete mode 100644 scripts/source_fix_983_no_heuristic_candidate_controls.py diff --git a/.github/source-fix-983-no-heuristic-candidate-controls.trigger b/.github/source-fix-983-no-heuristic-candidate-controls.trigger deleted file mode 100644 index cc14de347..000000000 --- a/.github/source-fix-983-no-heuristic-candidate-controls.trigger +++ /dev/null @@ -1,2 +0,0 @@ -source-fix-983-no-heuristic-candidate-controls -retrigger-after-b05bf9003d2d35f19c653ea1923f09997f321e13 diff --git a/.github/workflows/source-fix-983-no-heuristic-candidate-controls.yml b/.github/workflows/source-fix-983-no-heuristic-candidate-controls.yml deleted file mode 100644 index d5a2aac8c..000000000 --- a/.github/workflows/source-fix-983-no-heuristic-candidate-controls.yml +++ /dev/null @@ -1,88 +0,0 @@ -name: Source fix PR983 no-heuristic candidate controls - -on: - push: - branches: - - feat/stateless-candidate-controls - paths: - - .github/source-fix-983-no-heuristic-candidate-controls.trigger - -permissions: - contents: write - -jobs: - repair: - if: github.repository == 'ContextualWisdomLab/contextual-orchestrator' - runs-on: ubuntu-latest - steps: - - name: Checkout exact repair head - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # actions/checkout@v7 - with: - ref: ${{ github.sha }} - fetch-depth: 0 - persist-credentials: true - - - name: Set up Python - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # actions/setup-python@v6 - with: - python-version: "3.12" - - - name: Install hash-locked dependencies - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check --require-hashes -r requirements.lock - python -m pip install --disable-pip-version-check --no-deps -e . - - - name: Prove candidate-control regressions are RED - run: | - set -euo pipefail - set +e - python -m pytest -q tests/test_candidate_routing_no_heuristic_limits.py - status=$? - set -e - if [ "$status" -eq 0 ]; then - echo "::error::No-heuristics candidate-control regressions were unexpectedly GREEN before repair." - exit 1 - fi - - - name: Apply exact guarded owner repair - run: python scripts/source_fix_983_no_heuristic_candidate_controls.py - - - name: Verify candidate-control contracts - run: | - set -euo pipefail - python -m pytest -q \ - tests/test_candidate_routing_no_heuristic_limits.py \ - tests/test_candidate_routing_controls.py \ - tests/test_api_contract.py - git diff --check - - - name: Remove completed one-shot machinery - run: | - set -euo pipefail - rm -f \ - .github/workflows/source-fix-983-no-heuristic-candidate-controls.yml \ - .github/source-fix-983-no-heuristic-candidate-controls.trigger \ - scripts/source_fix_983_no_heuristic_candidate_controls.py - git diff --check - - - name: Commit verified owner repair - env: - EXPECTED_TRIGGER_HEAD: ${{ github.sha }} - run: | - set -euo pipefail - git fetch origin feat/stateless-candidate-controls - remote_head="$(git rev-parse origin/feat/stateless-candidate-controls)" - if [ "$remote_head" != "$EXPECTED_TRIGGER_HEAD" ]; then - echo "::error::Canonical branch moved; refusing to overwrite or guess a merge." - exit 1 - fi - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add -A - if git diff --cached --quiet; then - echo "::error::Source fix produced no publishable delta." - exit 1 - fi - git commit -m "fix(routing): remove heuristic candidate-control limits" - git push origin HEAD:feat/stateless-candidate-controls diff --git a/CHANGELOG.md b/CHANGELOG.md index c4d981c8d..12c470f2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1267,3 +1267,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 bc3b186e3..91e5078f3 100644 --- a/README.md +++ b/README.md @@ -266,7 +266,7 @@ is read from a **KV config store**, never `os.getenv`. 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` (at most 32 unique IDs) to omit known-bad + `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 diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 58b004aa5..3a862a30b 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -4040,7 +4040,7 @@ def candidate_routing_policy( 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 at most 32 agent IDs") + raise ValueError("exclude_candidate_ids must contain agent IDs") if candidate_id is None and not excluded: yield return @@ -4052,8 +4052,8 @@ def candidate_routing_policy( or candidate_id != candidate_id.strip() ): raise ValueError("candidate_id must be a non-empty agent ID") - if not isinstance(excluded, (list, tuple)) or len(excluded) > 32: - raise ValueError("exclude_candidate_ids must contain at most 32 agent IDs") + 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") @@ -4196,17 +4196,12 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N for value in [row.get("agent_id")] if isinstance(value, str) and value ] - # conduct() records the id of the step whose output actually became - # ``answer`` as ``answering_step_id`` (a non-final step when the - # verifier rejects and falls back to the worker's output). Resolving - # the served row by that identity is unambiguous even when a later, - # genuinely-served step's output happens to duplicate an earlier - # step's text byte-for-byte -- text equality alone cannot tell a - # fallback-to-earlier-step apart from a later step that coincidentally - # repeats an earlier step's text (#983 finding 6). Callers that never - # run a multi-step workflow, or a workflow record persisted before - # this field existed, fall back to the text-matched/last-row - # heuristics below. + # 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 = ( [ @@ -4217,52 +4212,23 @@ def _candidate_routing_evidence(result: Mapping[str, Any]) -> dict[str, Any] | N if isinstance(answering_step_id, int) else [] ) - if not answering_rows: - # conduct() can serve a non-final step's output as the answer (the - # verifier-required fallback to the worker's output, or the - # verifier's own output when a synthesizer step is not required) -- - # the trace still records every step's provider call, so the *last* - # row is not reliably the one that produced ``answer``. Prefer the - # row(s) whose recorded output actually match the served answer; - # fall back to the last-row heuristic for callers (plain passthrough, - # cache hits) that never populate "answer"/"output" at all. - answer = result.get("answer") - answering_rows = ( - [ - row - for row in rows - if isinstance(row, Mapping) and row.get("output") == answer - ] - if isinstance(answer, str) - else [] - ) - served_rows = answering_rows or rows - # Among *text* matches (no ``answering_step_id`` evidence available), - # prefer the earliest row: this ordering is a best-effort fallback - # only, kept for workflow records persisted before - # ``answering_step_id`` existed. It cannot by itself distinguish a - # fallback-to-earlier-step from a later step that coincidentally - # duplicates an earlier one's text -- exactly why the - # ``answering_step_id`` lookup above takes priority whenever it is - # available (#983 finding 6). When nothing matched by text (the - # plain-passthrough/cache-hit callers that never populate - # answer/output), fall back to the prior last-row heuristic over the - # full, unfiltered trace (#983 finding 3). - ordered_rows = served_rows if answering_rows else reversed(served_rows) - served = ( - None - if tracked_attempts == [] - else next( - ( - value - for row in ordered_rows - if isinstance(row, Mapping) - for value in [row.get("served_agent_id") or row.get("agent_id")] - if isinstance(value, str) and value - ), - None, - ) - ) + 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)), @@ -5631,6 +5597,7 @@ def stream_route( "access": [], "latency_ms": round(latency_seconds * 1000, 2), "output": answer, + "served_agent_id": agent.id, } if isinstance(usage, dict): trace_step["usage"] = usage @@ -6710,8 +6677,8 @@ def route_once( } if attempt_usage is not None: row["usage"] = attempt_usage + row["served_agent_id"] = attempt_served_id if attempt_served_id != candidate.id: - row["served_agent_id"] = attempt_served_id row["failover_from"] = candidate.id answer, served_id = attempt_answer, attempt_served_id verification = self._realtime_route_judge( diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 4520e46be..a848d64a5 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -3247,11 +3247,11 @@ def _validate_routing( cleaned["candidate_id"] = candidate_id.strip() excluded = routing.get("exclude_candidate_ids") if excluded is not None: - if not isinstance(excluded, list) or len(excluded) > 32: + if not isinstance(excluded, list): raise RequestError( 400, "invalid_routing", - "routing.exclude_candidate_ids must be an array of at most 32 agent IDs", + "routing.exclude_candidate_ids must be an array of agent IDs", ) if any( not isinstance(value, str) 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 095c7f6a6..843e3fbb1 100644 --- a/docs/planning/adrs/0032-model-group-cost-aware-discovery.md +++ b/docs/planning/adrs/0032-model-group-cost-aware-discovery.md @@ -157,8 +157,9 @@ OpenRouter. (2026). *Zero data retention enforcement*. https://openrouter.ai/doc 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 at most 32 -unique IDs with `routing.exclude_candidate_ids`. The controls are validated +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 @@ -193,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..ece937848 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2680,3 +2680,15 @@ 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. diff --git a/scripts/source_fix_983_no_heuristic_candidate_controls.py b/scripts/source_fix_983_no_heuristic_candidate_controls.py deleted file mode 100644 index 3318581af..000000000 --- a/scripts/source_fix_983_no_heuristic_candidate_controls.py +++ /dev/null @@ -1,88 +0,0 @@ -"""Apply the exact no-heuristics repair for PR #983 candidate controls.""" - -from pathlib import Path - - -def replace_once(path: str, old: str, new: str) -> None: - target = Path(path) - text = target.read_text() - count = text.count(old) - if count != 1: - raise SystemExit(f"{path}: expected one exact match, found {count}") - target.write_text(text.replace(old, new, 1)) - - -def splice(path: str, start: str, end: str, replacement: str) -> None: - target = Path(path) - text = target.read_text() - start_index = text.find(start) - if start_index < 0: - raise SystemExit(f"{path}: start marker not found") - end_index = text.find(end, start_index) - if end_index < 0: - raise SystemExit(f"{path}: end marker not found") - target.write_text(text[:start_index] + replacement + text[end_index:]) - - -replace_once( - "contextual_orchestrator/server.py", - ' if not isinstance(excluded, list) or len(excluded) > 32:\n raise RequestError(\n 400,\n "invalid_routing",\n "routing.exclude_candidate_ids must be an array of at most 32 agent IDs",\n )', - ' if not isinstance(excluded, list):\n raise RequestError(\n 400,\n "invalid_routing",\n "routing.exclude_candidate_ids must be an array of agent IDs",\n )', -) - -replace_once( - "contextual_orchestrator/orchestrator.py", - ' raise ValueError("exclude_candidate_ids must contain at most 32 agent IDs")', - ' raise ValueError("exclude_candidate_ids must contain agent IDs")', -) -replace_once( - "contextual_orchestrator/orchestrator.py", - ' if not isinstance(excluded, (list, tuple)) or len(excluded) > 32:\n raise ValueError("exclude_candidate_ids must contain at most 32 agent IDs")', - ' if not isinstance(excluded, (list, tuple)):\n raise ValueError("exclude_candidate_ids must contain agent IDs")', -) - -start = ''' # conduct() records the id of the step whose output actually became\n''' -end = ''' evidence: dict[str, Any] = {\n''' -replacement = ''' # Serving identity is evidence, not an inference target. Multi-step\n # workflows record the exact answering_step_id; provider-shaped paths\n # may record served_agent_id explicitly. Historical records lacking\n # either identity remain auditable for attempts but fail closed for\n # served_candidate_id. Output equality and trace position are not\n # admissible serving-identity evidence.\n answering_step_id = result.get("answering_step_id")\n answering_rows = (\n [\n row\n for row in rows\n if isinstance(row, Mapping) and row.get("id") == answering_step_id\n ]\n if isinstance(answering_step_id, int)\n else []\n )\n served: str | None = None\n if len(answering_rows) == 1:\n row = answering_rows[0]\n value = row.get("served_agent_id") or row.get("agent_id")\n if isinstance(value, str) and value:\n served = value\n elif tracked_attempts != []:\n explicit_served = [\n value\n for row in rows\n if isinstance(row, Mapping)\n for value in [row.get("served_agent_id")]\n if isinstance(value, str) and value\n ]\n distinct_served = tuple(dict.fromkeys(explicit_served))\n if len(distinct_served) == 1:\n served = distinct_served[0]\n''' -splice("contextual_orchestrator/orchestrator.py", start, end, replacement) - -replace_once( - "README.md", - '`routing.exclude_candidate_ids` (at most 32 unique IDs) to omit known-bad', - '`routing.exclude_candidate_ids` (unique exact IDs) to omit evidence-ineligible', -) -replace_once( - "docs/planning/adrs/0032-model-group-cost-aware-discovery.md", - 'pin an exact private agent ID with `routing.candidate_id` and exclude at most 32\nunique IDs with `routing.exclude_candidate_ids`.', - 'pin an exact private agent ID with `routing.candidate_id` and exclude unique exact\nIDs with `routing.exclude_candidate_ids`. Candidate membership has no repository-authored\ncardinality cutoff; normal authenticated request-size controls remain the resource boundary.', -) - -replace_once( - "tests/test_candidate_routing_controls.py", - 'def test_candidate_routing_evidence_falls_back_to_text_match_without_answering_step_id() -> None:\n """A workflow record persisted before ``answering_step_id`` existed (or\n any other caller that omits it) must still resolve routing evidence via\n the prior text-matching/last-row heuristics rather than crashing or\n silently returning no evidence."""', - 'def test_candidate_routing_evidence_fails_closed_without_answering_step_id() -> None:\n """A historical workflow without explicit serving identity must not infer one."""', -) -replace_once( - "tests/test_candidate_routing_controls.py", - ' assert evidence is not None\n assert evidence["served_candidate_id"] == "worker_agent"\n\n\ndef test_orchestrated_provider_completion_answering_step_id_identifies_synthesis_over_duplicate_internal_step()', - ' assert evidence is not None\n assert "served_candidate_id" not in evidence\n\n\ndef test_orchestrated_provider_completion_answering_step_id_identifies_synthesis_over_duplicate_internal_step()', -) - -for path, section in ( - ( - "docs/planning/adrs/0032-model-group-cost-aware-discovery.md", - """\n\n### No-heuristics amendment — 2026-09-02\n\nThe original request-local control used a fixed 32-ID exclusion ceiling and legacy\nserving-identity recovery from output equality/trace position. Neither decision rule\nwas identified by RouteLLM, FrugalGPT, an API standard, or measured deployment\nevidence. The cardinality ceiling is removed; authenticated request-size enforcement\nis the resource boundary. Serving identity is now reported only from exact\n`answering_step_id` or an explicit `served_agent_id`; historical rows without either\nremain attempt provenance and omit `served_candidate_id`. This is a fail-closed\nidentity rule rather than an inferred ranking/tie-break.\n""", - ), - ( - "docs/product-technical-gap-baseline.md", - """\n\n## 2026-09-02 — PR #983 candidate-control no-heuristics repair\n\nLive RCA found two decision-affecting rules in the request-local candidate-control\nowner: a repository-authored 32-ID exclusion ceiling and serving-candidate inference\nfrom output equality/trace position when exact identity was absent. Neither had an\nidentified mathematical, standards, experimental, or research basis. The canonical\nrepair removes the cardinality rule, retaining normal authenticated request-size\ncontrols, and makes serving identity fail closed unless exact `answering_step_id` or\nexplicit `served_agent_id` evidence exists. Regression coverage exercises more than\n32 exclusions, missing identity, and explicit identity provenance. Exact-head hosted\nchecks remain authoritative before merge.\n""", - ), - ( - "CHANGELOG.md", - """\n- 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.\n""", - ), -): - target = Path(path) - text = target.read_text() - if section.strip() not in text: - target.write_text(text.rstrip() + section + "\n") diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 06b9f1545..5d6bfd23e 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -1834,11 +1834,8 @@ def chat(self, agent, messages, effort_profile=None): assert evidence["served_candidate_id"] == "synth_agent" -def test_candidate_routing_evidence_falls_back_to_text_match_without_answering_step_id() -> None: - """A workflow record persisted before ``answering_step_id`` existed (or - any other caller that omits it) must still resolve routing evidence via - the prior text-matching/last-row heuristics rather than crashing or - silently returning no evidence.""" +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( [ @@ -1861,7 +1858,7 @@ def test_candidate_routing_evidence_falls_back_to_text_match_without_answering_s ) assert evidence is not None - assert evidence["served_candidate_id"] == "worker_agent" + assert "served_candidate_id" not in evidence def test_orchestrated_provider_completion_answering_step_id_identifies_synthesis_over_duplicate_internal_step() -> None: From 765270700e681d18afcf0b39c95e9310ea2477da Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:03:52 +0000 Subject: [PATCH 31/35] fix(routing): scope unconditional served_agent_id to active candidate tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prior no-heuristics repair for candidate-routing evidence made route_once and stream_route stamp served_agent_id on every trace row unconditionally, so _candidate_routing_evidence would have an explicit serving fact even when the serving agent was unchanged. That broke a separate, pre-existing regression guard in test_provider_reliability.py and test_tool_execution_fallback.py: the ordinary, no-candidate-policy path must never carry served_agent_id/failover metadata for an unchanged serving agent ("the default mock path must behave exactly as before"). Root-caused by diffing a clean origin/main worktree (passing) against this branch's head (failing) to rule out full-suite test pollution before concluding it was a real regression from the served_agent_id change. Fix: only stamp served_agent_id unconditionally while request-local candidate-attempt tracking is actually active (inside a candidate_routing_policy scope, via _REQUEST_ATTEMPTED_CANDIDATE_IDS.get() is not None). The ordinary path's trace-row shape is unchanged; failover still always stamps it regardless of policy state. Full local suite: 3355 passed, 2 pre-existing sandbox-only failures (fast_mlsirm unavailable in this sandbox's proxy policy; a known local tokenizer-usage-source artifact in test_spend_analytics that passes on real CI) — both unrelated and pre-existing. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- CHANGELOG.md | 10 ++++++++++ contextual_orchestrator/orchestrator.py | 14 ++++++++++++-- docs/product-technical-gap-baseline.md | 22 ++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12c470f2c..d53d30b6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,16 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ### Fixed +- `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. diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 3a862a30b..7b28a990e 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -5597,8 +5597,12 @@ def stream_route( "access": [], "latency_ms": round(latency_seconds * 1000, 2), "output": answer, - "served_agent_id": agent.id, } + 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( @@ -6677,9 +6681,15 @@ def route_once( } if attempt_usage is not None: row["usage"] = attempt_usage - row["served_agent_id"] = attempt_served_id 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, diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index ece937848..9cf2000f8 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2692,3 +2692,25 @@ controls, and makes serving identity fail closed unless exact `answering_step_id 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. From 05eb95f2d35f14b111050639731be8dde8bb9374 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:27:00 +0000 Subject: [PATCH 32/35] fix(api): remove stale maxItems:32 from exclude_candidate_ids schema The runtime's own repository-authored 32-ID exclusion-count cutoff was already removed as unsupported (no identified mathematical, standards, experimental, or research basis for that specific number), but the published OpenAPI schema for CandidateRoutingControls.exclude_candidate_ids still declared maxItems: 32. Generated/OpenAPI clients therefore still rejected exclusion lists the runtime intentionally accepts, making the schema false at source -- the PR body, CHANGELOG, ADR direction, and a prior Devin review resolution all claimed the cutoff was gone, but the schema, a second independent publication surface for the same invariant, still enforced it. Removes maxItems: 32 with no replacement cardinality heuristic; uniqueItems, lexical ID constraints, and normal authenticated request-size bounds are unchanged. Adds a RED-before/GREEN-after regression validating a 64-ID exclusion list against the schema (confirmed it fails against the old schema, passes against the corrected one). Verified the runtime validator (server.py's _validate_routing) has no other hidden count-based cutoff on this field. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- CHANGELOG.md | 7 +++++++ contextual_orchestrator/api_contract.py | 1 - docs/product-technical-gap-baseline.md | 20 ++++++++++++++++++++ tests/test_api_contract.py | 10 +++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d53d30b6c..dd5cc3b39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ### Fixed +- 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. - `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 diff --git a/contextual_orchestrator/api_contract.py b/contextual_orchestrator/api_contract.py index 5d794199f..1fccba44c 100644 --- a/contextual_orchestrator/api_contract.py +++ b/contextual_orchestrator/api_contract.py @@ -39,7 +39,6 @@ }, "exclude_candidate_ids": { "type": "array", - "maxItems": 32, "uniqueItems": True, "items": { "type": "string", diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 9cf2000f8..11ae7cf85 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2714,3 +2714,23 @@ files plus the full local suite (3355 passed, 2 pre-existing sandbox-only failur 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. diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py index ab9c22350..0673bd611 100644 --- a/tests/test_api_contract.py +++ b/tests/test_api_contract.py @@ -64,7 +64,6 @@ def test_openapi_documents_compatibility_front_door() -> None: assert routing_schema["properties"]["candidate_id"]["pattern"] == exact_id_pattern assert routing_schema["properties"]["exclude_candidate_ids"] == { "type": "array", - "maxItems": 32, "uniqueItems": True, "items": { "type": "string", @@ -78,6 +77,15 @@ def test_openapi_documents_compatibility_front_door() -> None: ): 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"] == [ From 1889e1aec022c0f8580cb3cc6248c8ae87f0f73d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 22:56:39 +0000 Subject: [PATCH 33/35] fix(fuzz): remove stale 32-item exclude_candidate_ids cap from the fuzz target Devin's review found that fuzz/targets.py's exercise_request_body still asserts `len(excluded) <= 32` on the real server._validate_routing() output, even though this PR's earlier "no-heuristics correction" removed that exact cardinality ceiling from both the OpenAPI schema and the runtime Python validator (there is now no repository-authored candidate-count cutoff, only the normal authenticated request-size boundary). The fuzz target was left asserting an invariant the validator it drives no longer enforces, so a legitimately valid >32-item exclusion list would report as a fuzzing false positive. Also updated the module docstring's target #11 summary, which still said "bounded", to match. RED confirmed: reverted the fuzz/targets.py change and reran the new regression -- AssertionError at the removed `assert len(excluded) <= 32` line, exactly as Devin described. GREEN: new deterministic regression (40 unique exclude_candidate_ids) passes; full tests/fuzz/test_fuzz_properties.py suite -- 20 passed. interrogate on fuzz/targets.py: 100%. git diff --check: clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- fuzz/targets.py | 10 ++++++++-- tests/fuzz/test_fuzz_properties.py | 12 ++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/fuzz/targets.py b/fuzz/targets.py index 909a424ae..2cec7e4e2 100644 --- a/fuzz/targets.py +++ b/fuzz/targets.py @@ -38,7 +38,8 @@ (ADR 0041). Must never raise and must never return ``True`` unless every present monetary value is a valid non-negative finite zero. 11. ``server._validate_routing`` -- request-local channel and candidate - controls. Successful candidate arrays are bounded, unique, and non-empty. + 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. @@ -211,7 +212,12 @@ def exercise_request_body(raw: bytes) -> None: pass else: excluded = (routing or {}).get("exclude_candidate_ids", []) - assert len(excluded) <= 32 + # 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) diff --git a/tests/fuzz/test_fuzz_properties.py b/tests/fuzz/test_fuzz_properties.py index 44198fc17..41b35a149 100644 --- a/tests/fuzz/test_fuzz_properties.py +++ b/tests/fuzz/test_fuzz_properties.py @@ -87,6 +87,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: From 47011b818cbfb752b5ae7bcfb811d6c1e3d7ae65 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 12:34:20 +0000 Subject: [PATCH 34/35] fix(routing): model judge must not select a verifier-excluded sole candidate Merged current main to pick up main's test_admin_contract.py `import json` fix (PR #1035) that this PR's stale base predated -- clean, no conflicts. Hosted CI's "Full unit and contract suite" (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 (tests/test_candidate_routing_controls.py). Neither failed in any earlier local round because this sandbox's blocked fast-mlsirm GitHub-archive download always short-circuits _model_judge_verification to its fail-closed return before a judge is ever selected -- masking a real, pre-existing (present unchanged at merge-base 212ff437, predates #983) selection bug that only a hosted run with fast-mlsirm actually importable can exercise. Root cause: _ranked_agents deliberately still returns role-ineligible members (appended 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/_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" (#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 enforces this same exclusion for a *backup* judge (test_fast_mlsirm_judge_failover_honors_verifier_exclusions); this closes the identical gap for the *primary* selection. Fix: add `if "verifier" not in agent.provider_exclusions` to the judge-selection generator in _model_judge_verification. Verification: - RED-before/GREEN-after: new regression test_model_judge_never_selects_a_verifier_excluded_sole_candidate (tests/test_model_judge.py) fails on the pre-fix code (records the excluded worker_only agent as judge) and passes after (next(...) raises StopIteration, caught by the existing fail-closed handler, judge never constructed). - 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 in docs/product-technical-gap-baseline.md's 2026-09-02 entry (fast_mlsirm unavailable; test_spend_analytics's local-tokenizer artifact -- same missing-fast-mlsirm mechanism). - interrogate on orchestrator.py: 100%. - git diff --check: clean. CHANGELOG.md and docs/product-technical-gap-baseline.md updated with a dated entry per this PR's established practice. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- CHANGELOG.md | 16 +++++ contextual_orchestrator/orchestrator.py | 17 ++++++ docs/product-technical-gap-baseline.md | 50 ++++++++++++++++ tests/test_model_judge.py | 79 +++++++++++++++++++++++++ 4 files changed, 162 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 946b77037..e6bf243b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,22 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) 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 diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 7b28a990e..c6b787481 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -8551,9 +8551,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 ) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 11ae7cf85..8d03c445d 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2734,3 +2734,53 @@ 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/tests/test_model_judge.py b/tests/test_model_judge.py index 5f11b19ca..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( From 53aa9c5c7fcbb6a0dc573e22c67e3ef9e8f9474d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 13:09:51 +0000 Subject: [PATCH 35/35] fix(routing): let ZDR-only triage fall back to the pinned candidate _compute_triage_verdict's empty free-only ranking fallback was gated behind `not _REQUEST_ZDR_ONLY.get()`, so a zdr_only request that pins a paid ZDR-eligible candidate via routing.candidate_id always hit an empty free-only pool (the pin restricts every candidate list to that one agent, and a paid agent never satisfies free_only) and then skipped the fallback entirely just because ZDR was active -- silently returning False (route, not conduct) with zero live triage call and zero routing evidence. The fallback's own per-agent filter already enforces ZDR eligibility (_zdr_agent_allowed) and the active pin/exclusion (_request_candidate_allowed), so gating the whole fallback build on "not zdr_only" was redundant, not protective. Removing that gate lets the fallback run whenever the free-only pool is empty; it narrows itself to ZDR-eligible agents (the ZDR-eligible pinned one, in this shape) with zero risk of contacting a non-ZDR provider. New regression test in tests/test_candidate_routing_controls.py posts an auto-mode request with zdr_only=true and a paid ZDR candidate pin, confirming triage genuinely calls the pinned candidate and the resulting conduct verdict is honored (a full multi-step workflow runs instead of the one-call route path). Verified RED against the pre-fix code (git stash A/B) and GREEN with the fix. Devin Review, PR #983: "ZDR pins skip workflow triage". Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- CHANGELOG.md | 12 +++++ contextual_orchestrator/orchestrator.py | 19 ++++++- tests/test_candidate_routing_controls.py | 68 ++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6bf243b3..0a36c2b0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,18 @@ 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 diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index c6b787481..127b3acc0 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -7693,7 +7693,24 @@ 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 diff --git a/tests/test_candidate_routing_controls.py b/tests/test_candidate_routing_controls.py index 5d6bfd23e..ead6b6faf 100644 --- a/tests/test_candidate_routing_controls.py +++ b/tests/test_candidate_routing_controls.py @@ -1067,6 +1067,74 @@ def test_auto_stream_shares_candidate_scope_with_triage_when_conducting() -> Non 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( [