diff --git a/CHANGELOG.d/openrouter-not-evidence-only.md b/CHANGELOG.d/openrouter-not-evidence-only.md new file mode 100644 index 000000000..0c071d971 --- /dev/null +++ b/CHANGELOG.d/openrouter-not-evidence-only.md @@ -0,0 +1 @@ +Stopped treating OpenRouter as a whole-account, ZDR-motivated serving exclusion. `PROVIDER_MODEL_SOURCES`'s `openrouter` entry no longer sets `evidence_only=True`: ZDR eligibility is a route/model-level property, never grounds to block an entire provider account from serving, and OpenRouter was the one provider source with genuinely reliable native pricing/`is_free` evidence, so excluding it directly caused `orchestrator/free`'s previously-documented structural emptiness (ADR 0041). Also fixed a backwards side effect of the old flag: `_apply_discovered_model_evidence` could never mark OpenRouter's own rows `zdr_capable=True` even when they exactly matched OpenRouter's own declared ZDR feed. The provider-neutral evidence-application contract from PR #901 (OpenRouter's feed also crediting matching rows from every other provider) is unchanged. Since OpenRouter can multiplex one model id across several backing providers, `ModelClient` now pins every OpenRouter request made under an active `zdr_only` scope with OpenRouter's own documented `"provider": {"zdr": true}` request-time enforcement, applied at the shared `_send`/`_stream_send`/`_send_raw` transport chokepoints, and at the async Batch API path (`_batch_run`) via the same helper on each uploaded JSONL request body. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b38e0770..3284733d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,19 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) retried as if it were a network blip. Fixes the shared classifier itself (not just the discovery retry call site), so every current and future caller of `is_transient_error` benefits. +- (Devin review on #953) `_pin_openrouter_zdr` no longer raises a bare + `TypeError` from `dict()` when a caller-supplied `provider` field is + present but not an object (an int, bool, list, or string) under an active + `zdr_only` scope. It now validates the field and raises a named `ValueError` + ("provider must be an object with optional OpenRouter routing keys") + instead, matching this codebase's existing convention for malformed + caller-input fields. Every call site sharing this one choke point (chat, + streaming, tools/binary-media passthrough, and the batch JSONL path) + benefits; a valid `provider` object or an absent/`None` one keep their + existing behavior unchanged. +- OpenRouter embedding Batch JSONL now carries the same request-scoped + `provider.zdr=true` enforcement when `zdr_only` selects an attested + OpenRouter embedding agent. ### Added diff --git a/contextual_orchestrator/batch_routing.py b/contextual_orchestrator/batch_routing.py index 82ce36600..400e91c79 100644 --- a/contextual_orchestrator/batch_routing.py +++ b/contextual_orchestrator/batch_routing.py @@ -460,6 +460,7 @@ class EmbeddingBatchRequest: token_count: int = 0 zdr_only: bool = False agent_id: Optional[str] = None + provider_routing: Optional[Dict[str, Any]] = None def wire_custom_id(self) -> str: """Return a provider-safe id while retaining the internal request mapping. @@ -476,14 +477,16 @@ def wire_custom_id(self) -> str: def to_jsonl_line(self, endpoint: str = "/v1/embeddings") -> Dict[str, Any]: """Render this request as an OpenAI Batch API embeddings JSONL line.""" + body: Dict[str, Any] = {"model": self.model, "input": self.input_text} + if self.provider_routing is not None: + body["provider"] = dict(self.provider_routing) return { # The provider body stays OpenAI-compatible; the backend's tracked # request map carries the immutable route identity separately. "custom_id": self.wire_custom_id(), "method": "POST", "url": endpoint, - # ``zdr_only`` is enforced before this provider JSONL is built. - "body": {"model": self.model, "input": self.input_text}, + "body": body, } diff --git a/contextual_orchestrator/cost_router.py b/contextual_orchestrator/cost_router.py index 438a26fde..009206a8b 100644 --- a/contextual_orchestrator/cost_router.py +++ b/contextual_orchestrator/cost_router.py @@ -780,7 +780,12 @@ def submit_embeddings_batch( raise TypeError("zdr_only must be a boolean") if agent_id is not None and (not isinstance(agent_id, str) or not agent_id): raise TypeError("agent_id must be a non-empty string when provided") - resolved_model, resolved_agent_id = self._resolve_embedding_target(model, zdr_only, agent_id) + resolved_model, resolved_agent_id, resolved_provider = self._resolve_embedding_target( + model, zdr_only, agent_id + ) + provider_routing = ( + {"zdr": True} if zdr_only and resolved_provider == "openrouter" else None + ) shared_attribution = dict(attribution or {}) requests, part_counts, part_limits = self._build_embedding_requests( inputs, @@ -788,6 +793,7 @@ def submit_embeddings_batch( attribution=shared_attribution, zdr_only=zdr_only, agent_id=resolved_agent_id, + provider_routing=provider_routing, ) job = self.embedding_batch_backend.submit(requests, metadata=metadata) self._embedding_jobs[job.job_id] = job @@ -800,10 +806,10 @@ def submit_embeddings_batch( def _resolve_embedding_target( self, model: str, zdr_only: bool, agent_id: Optional[str] - ) -> tuple[str, Optional[str]]: + ) -> tuple[str, Optional[str], Optional[str]]: """Resolve one embedding member without losing a caller's member choice.""" if agent_id is None and not zdr_only: - return model, None + return model, None, None selection_model = ( None if model in {"contextual-orchestrator", getattr(self.orchestrator, "AUTO_MODEL", "")} @@ -812,10 +818,11 @@ def _resolve_embedding_target( with self.orchestrator.request_policy(zdr_only): candidates = self.orchestrator._capability_agents("embedding", selection_model) if agent_id is None: - return candidates[0].model, candidates[0].id + selected = candidates[0] + return selected.model, selected.id, _resolved_provider_name(selected) for candidate in candidates: if candidate.id == agent_id: - return candidate.model, candidate.id + return candidate.model, candidate.id, _resolved_provider_name(candidate) raise RuntimeError(f"embedding agent {agent_id!r} is not eligible for this request") def _build_embedding_requests( @@ -826,6 +833,7 @@ def _build_embedding_requests( attribution: Dict[str, Any], zdr_only: bool, agent_id: Optional[str], + provider_routing: Optional[Dict[str, Any]], ) -> tuple[List[EmbeddingBatchRequest], List[int], Dict[str, int]]: """Map original embedding inputs into token-budgeted provider parts.""" max_tokens, max_chars = self._embedding_request_limits() @@ -850,6 +858,7 @@ def _build_embedding_requests( token_count=token_count, zdr_only=zdr_only, agent_id=agent_id, + provider_routing=provider_routing, ) ) return requests, part_counts, { @@ -1156,6 +1165,31 @@ def _provider_from_base_url(base_url: str) -> str: return host +def _resolved_provider_name(agent: Any) -> str: + """Return a canonical provider name for one selected agent snapshot. + + ``base_url`` is what actually decides an outbound HTTP destination; + ``provider_name`` is a free-text label unvalidated at ``ModelAgent`` + construction, so it can be empty *or* nonempty-but-wrong (a typo, a + stale copy-paste). Trusting a nonempty ``provider_name`` unconditionally + — the previous ``agent.provider_name or ...`` short-circuit — let an + agent whose ``base_url`` is OpenRouter's own endpoint report a different + provider identity, which made ``submit_embeddings_batch``'s ZDR pin + (``provider_routing = {"zdr": True} if resolved_provider == "openrouter" + ...``) silently skip OpenRouter requests under an active ``zdr_only`` + scope. The exact destination hostname is checked first and is + authoritative whenever it is OpenRouter's, mirroring + ``orchestrator._resolved_openrouter_provider`` so both ZDR-pin choke + points (the embedding-batch path here and the chat/streaming/raw/batch + JSONL path there) share one normalization rule (CodeRabbit review on + #953, discussion_r3898471887 / discussion_r3898659143). + """ + host = _provider_from_base_url(agent.base_url) + if host == "openrouter.ai": + return "openrouter" + return agent.provider_name or host + + def _positive_int(value: Any, default: int) -> int: """Return ``value`` as a positive int, or ``default`` when invalid.""" try: diff --git a/contextual_orchestrator/model_discovery.py b/contextual_orchestrator/model_discovery.py index ca7399a03..0bac39ed9 100644 --- a/contextual_orchestrator/model_discovery.py +++ b/contextual_orchestrator/model_discovery.py @@ -1334,6 +1334,11 @@ def discover_all_models( # OpenRouter's authenticated catalog supplies routable account-model rows; # its public ZDR endpoint adds route-specific privacy evidence without # turning the whole provider account into either ZDR-only or non-serving. + # Request-time ZDR enforcement for OpenRouter specifically is a runtime + # concern (see ModelClient's `provider: {"zdr": true}` pin), not a + # discovery-time exclusion: OpenRouter can multiplex a model across several + # backing providers, so a stale discovery-time snapshot cannot by itself + # guarantee which provider serves a given request. routed = _apply_discovered_model_evidence( _deduplicate_discovered_models(discovered), _openrouter_zdr_model_ids(timeout=timeout), diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 3e00d4407..b59e2bd0a 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -304,6 +304,66 @@ def _cost_usd_decimal(output_tokens: int, price_per_million: float) -> Decimal: ) _REQUEST_ZDR_ONLY: ContextVar[bool] = ContextVar("request_zdr_only", default=False) + +def _resolved_openrouter_provider(agent: ModelAgent) -> str: + """Canonical provider identity for the ZDR-pin decision, base_url-first. + + ``ModelAgent.provider_name`` is free-text and unvalidated at construction + (hand-authored JSON, ``model_discovery.py`` auto-discovery, or KV-driven + config can all leave it empty or typo'd). Trusting it verbatim here would + let an agent whose ``base_url`` is OpenRouter's own endpoint silently skip + the ``provider.zdr=true`` enforcement pin under an explicit ``zdr_only`` + scope while still routing bytes to OpenRouter (base_url decides where the + request goes; this function only decides whether the pin is applied) — + a silent ZDR-policy bypass, not a crash (CodeRabbit review on #953, + discussion_r3898471887). Treating the exact OpenRouter hostname as + authoritative also covers a nonempty typo in that free-text field. Every + call site that funnels through this shared choke point (chat, streaming, + raw, binary media, and non-embedding batch JSONL) therefore gets the same + protection the embedding batch path already has. + """ + host = urlparse(agent.base_url).hostname or "" + if host == "openrouter.ai": + return "openrouter" + return agent.provider_name or host + + +def _pin_openrouter_zdr(agent: ModelAgent, payload: dict[str, Any]) -> dict[str, Any]: + """Force OpenRouter to enforce zero-data-retention at request time. + + OpenRouter can multiplex one model id across several backing providers; + a discovery-time ZDR feed snapshot proves a route was ZDR-attested when + it was fetched, not which provider actually serves a later request. Their + documented ``provider: {"zdr": true}`` request field is OpenRouter's own + server-side enforcement (https://openrouter.ai/docs/features/provider-routing) + and is authoritative for the request being sent right now, so it is + applied here rather than trusted to have been decided correctly upstream. + A caller-supplied ``provider`` object (e.g. explicit routing preferences) + is preserved and only gains the ``zdr`` key. + + ``provider`` is an optional caller passthrough field reaching this shared + choke point unvalidated from every call site (chat, streaming, tools and + binary-media passthrough, and the batch JSONL path). A malformed truthy + non-mapping value (an int, bool, list, or string) must fail with a named, + caller-actionable validation error here rather than an opaque ``TypeError`` + from ``dict()`` deep inside provider-transport code (Devin review on #953). + + The "is this agent OpenRouter" check itself goes through + ``_resolved_openrouter_provider`` rather than a bare ``agent.provider_name`` + comparison, so a misconfigured agent (empty/wrong ``provider_name`` but a + ``base_url`` that is actually OpenRouter's) still gets pinned instead of + silently bypassing ZDR enforcement (CodeRabbit review on #953). + """ + if not _REQUEST_ZDR_ONLY.get() or _resolved_openrouter_provider(agent) != "openrouter": + return payload + provider_routing = payload.get("provider") + if provider_routing is not None and not isinstance(provider_routing, dict): + raise ValueError("provider must be an object with optional OpenRouter routing keys") + provider_routing = dict(provider_routing or {}) + provider_routing["zdr"] = True + return {**payload, "provider": provider_routing} + + SECRET_PATTERNS = ( re.compile(r"(?i)(api[_-]?key|token|secret|password)(['\"]?\s*[:=]\s*['\"]?)[A-Za-z0-9._~+/=-]{12,}"), re.compile(r"(?i)(bearer\s+)[A-Za-z0-9._~+/=-]{12,}"), @@ -1732,6 +1792,7 @@ def _send( timeout: float | None = None, ) -> str: """Perform one provider HTTP request (isolated so retry/backoff stays testable).""" + payload = _pin_openrouter_zdr(agent, payload) api_key = _provider_credential(agent) headers = {"content-type": "application/json"} if api_key: @@ -2002,6 +2063,7 @@ def _stream_send( ): """Stream content deltas from a provider SSE response (real transport, testable).""" self._local.usage = None + payload = _pin_openrouter_zdr(agent, payload) api_key = _provider_credential(agent) headers = {"content-type": "application/json", "accept": "text/event-stream"} if api_key: @@ -2178,6 +2240,7 @@ def proxy_send_bytes( self, agent: ModelAgent, endpoint: str, payload: dict[str, Any] ) -> tuple[bytes, str]: """Passthrough a provider response whose body is binary media.""" + payload = _pin_openrouter_zdr(agent, payload) if agent.base_url.startswith("mock://"): return b"mock audio", "audio/mpeg" api_key = _provider_credential(agent) # pragma: no cover @@ -2330,6 +2393,7 @@ def _send_raw( destination: ProviderDestination | None = None, ) -> dict[str, Any]: # pragma: no cover """One provider HTTP request returning the FULL provider JSON (for passthrough).""" + payload = _pin_openrouter_zdr(agent, payload) api_key = _provider_credential(agent) headers = {"content-type": "application/json"} if api_key: @@ -2566,12 +2630,12 @@ def _batch_run( "custom_id": custom_id, "method": "POST", "url": "/v1/chat/completions", - "body": self.apply_effort_profile(agent, { + "body": _pin_openrouter_zdr(agent, self.apply_effort_profile(agent, { "model": agent.model, "messages": messages, "temperature": settings["temperature"] if temperature is None else temperature, "max_tokens": settings["max_output_tokens"], - }, effort_profile), + }, effort_profile)), }, ensure_ascii=False) for custom_id, messages in requests.items() ] diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 4ea2f1d8b..062292d6c 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -2293,6 +2293,8 @@ def _validate_mode(mode: Any) -> str: def _validate_capability_request(path: str, body: dict[str, Any]) -> None: """Validate the required trust-boundary fields for media/rerank passthrough.""" + if "provider" in body and not isinstance(body["provider"], dict): + raise RequestError(400, "invalid_provider", "provider must be an object") if "model" in body: model = body["model"] if not isinstance(model, 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 faef83e40..548bc07e7 100644 --- a/docs/planning/adrs/0032-model-group-cost-aware-discovery.md +++ b/docs/planning/adrs/0032-model-group-cost-aware-discovery.md @@ -43,6 +43,12 @@ does not exclude non-ZDR routes from ordinary requests. Missing or failed ZDR evidence therefore fails closed only for `zdr_only` selection, not for general inference. +Every OpenRouter wire transport applies the provider's documented +`provider.zdr=true` request-time enforcement inside that policy scope. This +includes JSON chat, streaming, structured passthrough, chat and embedding +batch JSONL, and the binary-response speech transport; the response media +type does not weaken the privacy contract of its JSON request body. + OpenRouter discovery retains the concrete free-model list returned by its model catalog, including exact `vendor/model:free` identifiers and any row whose complete structured monetary price is zero. The aggregate `openrouter/free` diff --git a/docs/planning/adrs/0041-generalize-models-dev-cost-classification.md b/docs/planning/adrs/0041-generalize-models-dev-cost-classification.md index f4f2eaa7b..8e6326331 100644 --- a/docs/planning/adrs/0041-generalize-models-dev-cost-classification.md +++ b/docs/planning/adrs/0041-generalize-models-dev-cost-classification.md @@ -177,6 +177,29 @@ nothing about the retry can turn a paid model free. Motivated by the `orchestrator/free` review-sidecar reliability gap in `ContextualWisdomLab/.github` PR #1433. +## Amendment (2026-08-31): OpenRouter is no longer `evidence_only` + +This ADR's Context section characterized OpenRouter's `evidence_only=True` +(commit `952996ec`) as settled, deliberate ZDR-privacy hardening "that stays +untouched." That characterization was false; it was reversed on direct +review this pass: ZDR eligibility is a route/model-level property +(`is_zdr_model`, exact feed matching), never grounds to block an entire +provider account from serving. OpenRouter's `ProviderModelSource` entry no +longer sets `evidence_only=True`. + +This directly closes the gap this ADR's Context section itself identified +("only OpenRouter's own API ever reports real pricing... and OpenRouter... +never serves inference... `orchestrator/free` was therefore structurally +empty in practice"): OpenRouter can now serve like every other discovered +provider, independent of and in addition to this ADR's Models.dev join for +the other five sources. The provider-neutral ZDR-evidence-application +contract (OpenRouter's feed also crediting matching rows from *other* +providers, insisted on during PR #901's review) is unchanged; what changed +is that OpenRouter's own rows are no longer the one arbitrary exception to +it. See `docs/product-technical-gap-baseline.md`'s 2026-08-31 entry for the +full mechanism, the request-time ZDR-pinning enforcement this required, and +its stated scope limits. + ## References Models.dev. (2026). *Models.dev API*. https://models.dev/api.json diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 66e8c7ae5..f9e80f108 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -346,9 +346,9 @@ completion finishes within any particular bound — the gateway preflight's separate curl timeout (originally 30s) was itself later found to be too tight for real reasoning-model latency and raised to 120s in `ContextualWisdomLab/.github#1440` (see that entry above); the two timeouts -are independent and this entry originally conflated them. Per owner review -on that PR, source correctness alone does not establish -operational acceptance: the fix also carries a RED→GREEN parity test +are independent and this entry originally conflated them. Source correctness +alone does not establish operational acceptance: the fix also carries a +RED→GREEN parity test (`test_gateway_preflight_max_tokens_is_synchronized_with_the_routing_probe`, confirmed to fail on the pre-fix `16` literal and pass once synchronized) and a negative control @@ -2398,6 +2398,80 @@ wall-clock limits from inference, discovery, OpenRouter ZDR lookup, and local readiness paths; only operator cancellation or a superseded PR head may terminate that work. +## 2026-08-31 OpenRouter is a normal, routable provider again + +Supersedes the 2026-08-30 entry above's characterization of `evidence_only=True` +on `openrouter` (commit `952996ec`) as settled ZDR hardening: that +characterization was false, as the entry above now records. On direct review +this pass, "ZDR eligibility is grounds to block a whole provider account" +turns out to be backwards -- ZDR is a route/model-level property, never a +provider-account-level one. `PROVIDER_MODEL_SOURCES`'s `openrouter` entry no +longer sets `evidence_only=True`. Concretely this fixes two bugs at once: + +1. **`orchestrator/free` structural emptiness (ADR 0041's own finding).** + OpenRouter is the one provider source with genuinely reliable native + pricing/`is_free` evidence; excluding it from serving regardless of that + evidence directly caused the "structurally empty in practice" state ADR + 0041 documented. OpenRouter can now serve like any other discovered + provider. +2. **A backwards ZDR-evidence exclusion.** `_apply_discovered_model_evidence` + computed `zdr_capable=not model.evidence_only and matches(...)`, which + meant OpenRouter's own rows could never be marked ZDR-capable even when + they exactly matched OpenRouter's own declared ZDR feed + (`https://openrouter.ai/api/v1/endpoints/zdr`). That exclusion is gone; + OpenRouter's own matching rows are now credited exactly like every other + provider's. + +**What is preserved, not removed**: the underlying reason a "provider-neutral, +not OpenRouter-only" evidence-application contract was insisted on during +PR #901's review (matching model ids from OpenRouter's feed onto *other* +providers' discovered rows, not just OpenRouter's own) is completely +untouched -- `_apply_discovered_model_evidence` still applies evidence to +every provider's rows identically; OpenRouter's own rows simply stop being +the one arbitrary exception to that rule. + +**The genuine technical risk this raises, and how it is closed**: OpenRouter +can multiplex one model id across several backing providers, so a +discovery-time ZDR feed snapshot proves a route *was* attested when fetched, +not which provider serves a *later* request. Client-side endpoint tracking +to predict this would only be as reliable as the last snapshot. Instead, +`ModelClient` now applies OpenRouter's own documented request-time +enforcement -- `"provider": {"zdr": true}` in the request body +(https://openrouter.ai/docs/features/provider-routing) -- via +`_pin_openrouter_zdr`, called from every wire-level transport an OpenRouter +agent can reach under an active `zdr_only` request scope: `_send` (the +`route`/`conduct` chat path), `_stream_send` (SSE streaming), and +`_send_raw` (the tools/structured-output passthrough path both +`proxy_send` and `proxy_send_once` funnel through), plus `proxy_send_bytes` +(binary speech responses whose request body is still JSON). This is OpenRouter's own +server-side enforcement for the request being sent right now, not a +client-side prediction — strictly stronger than what discovery-time +filtering could ever guarantee. + +**The asynchronous Batch API path is pinned too**: `_batch_run` (JSONL file +upload then a separate `/batches` job) does not go through the three +transport functions above, but it independently calls `_pin_openrouter_zdr` +on each request body it serializes into the uploaded JSONL, so a `zdr_only` +batch request against OpenRouter gets the same `"provider": {"zdr": true}` +enforcement as the synchronous paths. + +Embedding Batch JSONL follows the same contract. After `zdr_only` resolves an +attested OpenRouter embedding agent, `CostRoutingCoordinator` records the +provider-routing pin on each `EmbeddingBatchRequest`; its JSONL body emits +`"provider": {"zdr": true}` without exposing the internal `zdr_only` field. + +Verified: `tests/test_orchestrator_client_boundaries.py` adds direct unit +coverage of `_pin_openrouter_zdr` (no-op outside `zdr_only`, no-op for +non-OpenRouter agents, adds/merges the pin correctly) plus wiring-verification +tests on `_send`/`_stream_send`/`_send_raw`/`_batch_run` that capture the actual +outgoing JSON body. `tests/test_model_discovery.py`, +`tests/test_auto_discovery_server.py`, and `tests/test_review_gateway.py` +were updated where they asserted the old, now-reversed +`openrouter` + `evidence_only=True` behavior; the general `evidence_only` +mechanism itself (for any future provider that might legitimately need it) +is untouched and still tested, just no longer applied to OpenRouter by +default. Full suite green; `interrogate` 100% on the touched modules. + ### GAP RESOLVED ON PR HEAD — 2026-08-31: model groups, free discovery, and measured capacity [PR #971](https://github.com/ContextualWisdomLab/contextual-orchestrator/pull/971) diff --git a/tests/test_auto_discovery_server.py b/tests/test_auto_discovery_server.py index afe7e2928..f2d162787 100644 --- a/tests/test_auto_discovery_server.py +++ b/tests/test_auto_discovery_server.py @@ -175,13 +175,13 @@ def test_auto_discovery_activates_provider_catalog_rows(monkeypatch) -> None: assert agent.disabled is False -def test_auto_discovery_never_activates_openrouter_evidence_rows(monkeypatch) -> None: - """OpenRouter catalog rows provide evidence but never serving agents.""" +def test_auto_discovery_never_activates_evidence_only_rows(monkeypatch) -> None: + """A row explicitly marked evidence_only never becomes a serving agent.""" evidence = DiscoveredModel( - provider_name="openrouter", + provider_name="example_evidence_provider", model_id="provider/router-chat", - credential_name="OPENROUTER_API_KEY", - chat_base_url="https://openrouter.ai/api/v1", + credential_name="EXAMPLE_EVIDENCE_PROVIDER_API_KEY", + chat_base_url="https://example-evidence-provider.example/v1", auth_scheme="Bearer", capabilities=("chat", "response_format"), evidence_only=True, diff --git a/tests/test_batch_embeddings.py b/tests/test_batch_embeddings.py index a67881f2c..6938338df 100644 --- a/tests/test_batch_embeddings.py +++ b/tests/test_batch_embeddings.py @@ -291,6 +291,106 @@ def test_batch_embeddings_zdr_only_omitted_model_selects_zdr_capable_embedding_a server.shutdown() +def test_openrouter_zdr_embedding_batch_pins_provider_routing() -> None: + agent = ModelAgent( + "zdr_embedding", + "openai/text-embedding-3-small", + provider_name="openrouter", + tags=("embedding", "privacy:zdr"), + ) + backend = _RecordingEmbeddingBackend() + coordinator = CostRoutingCoordinator( + TaskOrchestrator([agent]), + InMemoryConfigStore(), + embedding_batch_backend=backend, + ) + + coordinator.submit_embeddings_batch( + ["private"], + model=agent.model, + zdr_only=True, + agent_id=agent.id, + ) + + assert backend.requests[0].to_jsonl_line()["body"]["provider"] == {"zdr": True} + + +def test_openrouter_zdr_embedding_batch_infers_legacy_provider_name() -> None: + agent = ModelAgent( + "legacy_zdr_embedding", + "openai/text-embedding-3-small", + base_url="https://openrouter.ai/api/v1", + tags=("embedding", "privacy:zdr"), + ) + backend = _RecordingEmbeddingBackend() + coordinator = CostRoutingCoordinator( + TaskOrchestrator([agent]), + InMemoryConfigStore(), + embedding_batch_backend=backend, + ) + + coordinator.submit_embeddings_batch( + ["private"], + model=agent.model, + zdr_only=True, + agent_id=agent.id, + ) + + assert backend.requests[0].provider_routing == {"zdr": True} + + +def test_openrouter_zdr_embedding_batch_overrides_mistyped_provider_name() -> None: + """The batch ZDR pin is applied even for a nonempty but wrong ``provider_name``. + + Mirrors ``orchestrator._resolved_openrouter_provider``'s fix for the same + pattern: an agent whose ``base_url`` is OpenRouter's own endpoint but + whose ``provider_name`` is a typo/mislabel ("openai") must still resolve + to "openrouter" for the ZDR-pin decision, since ``base_url`` — not the + free-text ``provider_name`` — determines the actual outbound destination + (CodeRabbit review on #953, discussion_r3898659143). + """ + agent = ModelAgent( + "mistyped_zdr_embedding", + "openai/text-embedding-3-small", + provider_name="openai", + base_url="https://openrouter.ai/api/v1", + tags=("embedding", "privacy:zdr"), + ) + backend = _RecordingEmbeddingBackend() + coordinator = CostRoutingCoordinator( + TaskOrchestrator([agent]), + InMemoryConfigStore(), + embedding_batch_backend=backend, + ) + + coordinator.submit_embeddings_batch( + ["private"], + model=agent.model, + zdr_only=True, + agent_id=agent.id, + ) + + assert backend.requests[0].provider_routing == {"zdr": True} + + +def test_openrouter_zdr_embedding_batch_uses_atomic_target_snapshot(monkeypatch) -> None: + backend = _RecordingEmbeddingBackend() + coordinator = CostRoutingCoordinator( + TaskOrchestrator([ModelAgent("removed_agent", "openai/text-embedding-3-small")]), + InMemoryConfigStore(), + embedding_batch_backend=backend, + ) + monkeypatch.setattr( + coordinator, + "_resolve_embedding_target", + lambda *_args: ("openai/text-embedding-3-small", "removed_agent", "openrouter"), + ) + + coordinator.submit_embeddings_batch(["private"], zdr_only=True) + + assert backend.requests[0].provider_routing == {"zdr": True} + + def test_pending_batch_preserves_resolved_model_identity() -> None: orchestrator = TaskOrchestrator([ModelAgent("embedding_worker", "resolved-embedding")]) coordinator = CostRoutingCoordinator( diff --git a/tests/test_batch_routing_boundaries.py b/tests/test_batch_routing_boundaries.py index bb04dbbdf..f9d016673 100644 --- a/tests/test_batch_routing_boundaries.py +++ b/tests/test_batch_routing_boundaries.py @@ -79,10 +79,15 @@ def test_embedding_request_jsonl_line_shape() -> None: def test_embedding_request_jsonl_preserves_zdr_policy() -> None: - request = EmbeddingBatchRequest(input_text="private", zdr_only=True) + request = EmbeddingBatchRequest( + input_text="private", + zdr_only=True, + provider_routing={"zdr": True}, + ) assert request.zdr_only is True assert "zdr_only" not in request.to_jsonl_line()["body"] + assert request.to_jsonl_line()["body"]["provider"] == {"zdr": True} def test_chat_request_jsonl_preserves_zdr_policy() -> None: diff --git a/tests/test_model_discovery.py b/tests/test_model_discovery.py index 097a68ced..21903c77c 100644 --- a/tests/test_model_discovery.py +++ b/tests/test_model_discovery.py @@ -651,7 +651,7 @@ def test_openrouter_discovery_preserves_every_declared_modality() -> None: embedding = next(model for model in discovered if "embedding" in model.capabilities) assert embedding.output_modalities == ("embeddings",) assert {"input:text", "output:embeddings"} <= set( - agent_from_discovered(replace(embedding, evidence_only=False)).tags + agent_from_discovered(embedding).tags ) @@ -763,7 +763,7 @@ def test_discovery_retains_full_catalog_and_marks_free_models() -> None: assert [model.model_id for model in discovered] == ["vendor/free-model", "paid/model", "request-fee/model"] assert [model.model_id for model in free_discovered_models(discovered)] == ["vendor/free-model"] - assert agent_from_discovered(replace(discovered[0], evidence_only=False)).group_name == "model_vendor_free_model_7959c29fc9" + assert agent_from_discovered(discovered[0]).group_name == "model_vendor_free_model_7959c29fc9" def _nim_vision_model() -> DiscoveredModel: @@ -2164,11 +2164,12 @@ def test_agent_from_discovered_builds_disabled_agent_with_correct_auth() -> None def test_agent_from_discovered_rejects_evidence_only_rows() -> None: + """Any row explicitly marked evidence_only stays unroutable, regardless of provider.""" discovered = DiscoveredModel( - provider_name="openrouter", + provider_name="example_evidence_provider", model_id="provider/evidence-model", - credential_name="OPENROUTER_API_KEY", - chat_base_url="https://openrouter.ai/api/v1", + credential_name="EXAMPLE_EVIDENCE_PROVIDER_API_KEY", + chat_base_url="https://example-evidence-provider.example/v1", auth_scheme="Bearer", evidence_only=True, ) diff --git a/tests/test_multimodal_model_group_http.py b/tests/test_multimodal_model_group_http.py index ac7ffb970..e133ebd69 100644 --- a/tests/test_multimodal_model_group_http.py +++ b/tests/test_multimodal_model_group_http.py @@ -129,6 +129,21 @@ def test_speech_endpoint_preserves_binary_media_response() -> None: server.shutdown() +def test_speech_endpoint_rejects_non_object_provider_routing() -> None: + agent = ModelAgent("speech_member", "provider/speech", tags=("speech",)) + server = build_server(TaskOrchestrator([agent]), port=0, security=SecurityConfig(auth_token=TOKEN)) + threading.Thread(target=server.serve_forever, daemon=True).start() + try: + status, body = _post_error( + server.server_address[1], + "/v1/audio/speech", + {"input": "hello", "voice": "alloy", "provider": 1}, + ) + assert status == 400 and body["error"]["code"] == "invalid_provider" + finally: + server.shutdown() + + def test_video_poll_and_content_use_the_submission_provider() -> None: """Async video follow-ups stay bound to the measured submission winner.""" first = ModelAgent( diff --git a/tests/test_orchestrator_client_boundaries.py b/tests/test_orchestrator_client_boundaries.py index e87bab84f..4fe0c6bb6 100644 --- a/tests/test_orchestrator_client_boundaries.py +++ b/tests/test_orchestrator_client_boundaries.py @@ -7,6 +7,7 @@ import threading import types import urllib.error +from typing import Any from unittest.mock import patch import pytest @@ -20,7 +21,10 @@ _coerce_message_content_text, _local_provider_slot, _local_provider_state, + _pin_openrouter_zdr, + _REQUEST_ZDR_ONLY, _resolve_fast_mlsirm_components, + _resolved_openrouter_provider, _validate_batch_results, ) from contextual_orchestrator.provider_errors import ProviderUpstreamError @@ -444,6 +448,273 @@ def __iter__(self): assert "connection reset" not in str(excinfo.value) +# -- OpenRouter request-time ZDR pin ------------------------------------------------ + + +def _openrouter_agent(**overrides) -> ModelAgent: + fields = { + "id": "openrouter_agent", + "model": "some-vendor/some-model", + "base_url": "https://openrouter.ai/api/v1", + "provider_name": "openrouter", + "credential_key": "OPENROUTER_API_KEY", + } + fields.update(overrides) + return ModelAgent(**fields) + + +def test_pin_openrouter_zdr_is_noop_outside_zdr_only_context() -> None: + """Only an active zdr_only request scope may add the provider.zdr pin.""" + agent = _openrouter_agent() + payload = {"model": agent.model, "messages": []} + assert _pin_openrouter_zdr(agent, payload) is payload + + +def test_pin_openrouter_zdr_is_noop_for_non_openrouter_agents() -> None: + """The pin is OpenRouter-specific; every other provider is untouched.""" + agent = _agent(provider_name="openai", base_url="https://api.openai.com/v1") + payload = {"model": agent.model, "messages": []} + token = _REQUEST_ZDR_ONLY.set(True) + try: + assert _pin_openrouter_zdr(agent, payload) is payload + finally: + _REQUEST_ZDR_ONLY.reset(token) + + +def test_pin_openrouter_zdr_infers_provider_from_base_url_when_name_is_empty() -> None: + """An OpenRouter agent with an unset ``provider_name`` still gets pinned. + + ``provider_name`` is free-text and unvalidated at construction; a + hand-authored or auto-discovered agent can carry ``base_url`` pointing at + OpenRouter's own endpoint while ``provider_name`` stays empty (its + default). Trusting ``provider_name`` verbatim here would silently skip + the ``provider.zdr=true`` enforcement pin under an active ``zdr_only`` + scope even though the request still routes to OpenRouter (CodeRabbit + review on #953, discussion_r3898471887). + """ + agent = _openrouter_agent(id="legacy_openrouter_agent", provider_name="") + assert _resolved_openrouter_provider(agent) == "openrouter" + payload = {"model": agent.model, "messages": []} + token = _REQUEST_ZDR_ONLY.set(True) + try: + pinned = _pin_openrouter_zdr(agent, payload) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert pinned["provider"] == {"zdr": True} + + +def test_pin_openrouter_zdr_overrides_mistyped_provider_name_from_exact_host() -> None: + """The actual OpenRouter destination wins over unvalidated provider text.""" + agent = _openrouter_agent(provider_name="open_router") + payload = {"model": agent.model, "messages": []} + token = _REQUEST_ZDR_ONLY.set(True) + try: + pinned = _pin_openrouter_zdr(agent, payload) + finally: + _REQUEST_ZDR_ONLY.reset(token) + + assert pinned["provider"] == {"zdr": True} + + +def test_pin_openrouter_zdr_adds_provider_zdr_flag() -> None: + """A zdr_only request to an OpenRouter agent gets OpenRouter's own enforcement pin.""" + agent = _openrouter_agent() + payload = {"model": agent.model, "messages": []} + token = _REQUEST_ZDR_ONLY.set(True) + try: + pinned = _pin_openrouter_zdr(agent, payload) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert pinned["provider"] == {"zdr": True} + assert "provider" not in payload # the original payload is never mutated in place + + +def test_pin_openrouter_zdr_preserves_caller_supplied_provider_routing() -> None: + """An explicit caller provider-routing preference keeps its other keys.""" + agent = _openrouter_agent() + payload = { + "model": agent.model, + "messages": [], + "provider": {"order": ["mistral"], "allow_fallbacks": False}, + } + token = _REQUEST_ZDR_ONLY.set(True) + try: + pinned = _pin_openrouter_zdr(agent, payload) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert pinned["provider"] == { + "order": ["mistral"], + "allow_fallbacks": False, + "zdr": True, + } + + +@pytest.mark.parametrize("malformed_provider", [5, True, ["order"], "openrouter", 0, ""]) +def test_pin_openrouter_zdr_rejects_non_mapping_provider(malformed_provider: Any) -> None: + """A non-object ``provider`` under zdr_only fails closed with a named + validation error, not the bare ``TypeError`` ``dict()`` would raise + (Devin review on #953: malformed speech routing returned server errors). + """ + agent = _openrouter_agent() + payload = {"model": agent.model, "messages": [], "provider": malformed_provider} + token = _REQUEST_ZDR_ONLY.set(True) + try: + with pytest.raises(ValueError, match="provider must be an object"): + _pin_openrouter_zdr(agent, payload) + finally: + _REQUEST_ZDR_ONLY.reset(token) + + +def _capture_request_body(sink: dict) -> Any: + """Return an ``_open_provider`` stand-in that records the outgoing JSON body.""" + + def _fake_open_provider(request, *_args, **_kwargs): + sink["body"] = json.loads(request.data.decode("utf-8")) + return _RegistryResponse({"choices": [{"message": {"content": "OK"}}]}) + + return _fake_open_provider + + +def _capture_binary_request_body(sink: dict) -> Any: + def _fake_open_provider(request, *_args, **_kwargs): + sink["body"] = json.loads(request.data.decode("utf-8")) + response = _RegistryResponse({}) + response.headers = types.SimpleNamespace(get_content_type=lambda: "audio/mpeg") + return response + + return _fake_open_provider + + +def test_send_pins_openrouter_zdr_on_the_wire() -> None: + """``_send`` (the normal chat transport) actually applies the pin, not just the helper.""" + agent = _openrouter_agent() + client = ModelClient() + captured: dict[str, Any] = {} + with patch.object(client, "_open_provider", side_effect=_capture_request_body(captured)): + token = _REQUEST_ZDR_ONLY.set(True) + try: + client._send(agent, {"model": agent.model, "messages": []}) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert captured["body"]["provider"] == {"zdr": True} + + +def test_send_pins_openrouter_zdr_on_the_wire_for_legacy_provider_name() -> None: + """``_send`` still applies the pin for an agent with a missing ``provider_name``. + + Proves the fix end-to-end on the real transport, not just against the + ``_pin_openrouter_zdr`` helper in isolation: an agent whose ``base_url`` + is OpenRouter's own endpoint but whose ``provider_name`` was left at its + empty default must not reach the wire without ``provider.zdr=true`` under + an active ``zdr_only`` scope (CodeRabbit review on #953, + discussion_r3898471887). + """ + agent = _openrouter_agent(id="legacy_openrouter_agent", provider_name="") + client = ModelClient() + captured: dict[str, Any] = {} + with patch.object(client, "_open_provider", side_effect=_capture_request_body(captured)): + token = _REQUEST_ZDR_ONLY.set(True) + try: + client._send(agent, {"model": agent.model, "messages": []}) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert captured["body"]["provider"] == {"zdr": True} + + +def test_send_pins_openrouter_zdr_on_the_wire_for_mistyped_provider_name() -> None: + """``_send`` still applies the pin when ``provider_name`` is nonempty but wrong. + + Closes the gap CodeRabbit and Devin Review both flagged against + ``df97709a``: ``_resolved_openrouter_provider`` used to return any + *nonempty* ``agent.provider_name`` before ever checking ``base_url``, so + an agent with ``provider_name="openai"`` and ``base_url`` actually + pointing at OpenRouter still reported ``"openai"`` and silently skipped + the ``provider.zdr=true`` enforcement pin — even though ``base_url``, not + the free-text ``provider_name`` label, decides where the request + actually goes. Proved end-to-end on the real ``_send`` transport (the + captured outgoing JSON body), not just against the + ``_resolved_openrouter_provider``/``_pin_openrouter_zdr`` helpers in + isolation, per CodeRabbit's explicit ask for on-wire coverage of this + exact case (CodeRabbit review on #953, discussion_r3898659143; Devin + review on #953, discussion_r3898661634). + """ + agent = _openrouter_agent(id="mistyped_openrouter_agent", provider_name="openai") + client = ModelClient() + captured: dict[str, Any] = {} + with patch.object(client, "_open_provider", side_effect=_capture_request_body(captured)): + token = _REQUEST_ZDR_ONLY.set(True) + try: + client._send(agent, {"model": agent.model, "messages": []}) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert captured["body"]["provider"] == {"zdr": True} + + +def test_stream_send_pins_openrouter_zdr_on_the_wire() -> None: + """``_stream_send`` applies the same pin as the non-streaming transport.""" + agent = _openrouter_agent() + client = ModelClient() + captured: dict[str, Any] = {} + + def _fake_open_provider(request, *_args, **_kwargs): + captured["body"] = json.loads(request.data.decode("utf-8")) + return _StreamResponse([b"data: [DONE]"]) + + with patch.object(client, "_open_provider", side_effect=_fake_open_provider): + token = _REQUEST_ZDR_ONLY.set(True) + try: + list(client._stream_send(agent, {"model": agent.model, "messages": [], "stream": True})) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert captured["body"]["provider"] == {"zdr": True} + + +def test_send_raw_pins_openrouter_zdr_on_the_wire() -> None: + """``_send_raw`` (the passthrough transport) applies the same pin.""" + agent = _openrouter_agent() + client = ModelClient() + captured: dict[str, Any] = {} + with patch.object(client, "_open_provider", side_effect=_capture_request_body(captured)): + token = _REQUEST_ZDR_ONLY.set(True) + try: + client._send_raw(agent, "chat/completions", {"model": agent.model, "messages": []}) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert captured["body"]["provider"] == {"zdr": True} + + +def test_send_does_not_pin_zdr_outside_zdr_only_context() -> None: + """A normal (non-zdr_only) request to OpenRouter is sent unmodified.""" + agent = _openrouter_agent() + client = ModelClient() + captured: dict[str, Any] = {} + with patch.object(client, "_open_provider", side_effect=_capture_request_body(captured)): + client._send(agent, {"model": agent.model, "messages": []}) + assert "provider" not in captured["body"] + + +def test_proxy_send_bytes_pins_openrouter_zdr_only_in_policy_scope() -> None: + """Binary speech transport applies the same request-time ZDR boundary.""" + agent = _openrouter_agent() + client = ModelClient() + captured: dict[str, Any] = {} + with patch.object(client, "_validate_provider", return_value=None), patch.object( + client, "_open_provider", side_effect=_capture_binary_request_body(captured) + ): + token = _REQUEST_ZDR_ONLY.set(True) + try: + client.proxy_send_bytes(agent, "audio/speech", {"input": "hello"}) + finally: + _REQUEST_ZDR_ONLY.reset(token) + assert captured["body"]["provider"] == {"zdr": True} + + with patch.object(client, "_validate_provider", return_value=None), patch.object( + client, "_open_provider", side_effect=_capture_binary_request_body(captured) + ): + client.proxy_send_bytes(agent, "audio/speech", {"input": "hello"}) + assert "provider" not in captured["body"] + + # -- batch success paths ------------------------------------------------------------ @@ -562,6 +833,36 @@ def batch_json(_agent, method, _path, payload=None, destination=None): assert results["task_0"]["usage"] == {"prompt_tokens": 3} +def test_batch_run_pins_openrouter_zdr_in_uploaded_jsonl() -> None: + agent = _openrouter_agent() + client = ModelClient() + captured: dict[str, Any] = {} + raw = b'{"custom_id":"task_0","response":{"body":{"choices":[{"message":{"content":"ok"}}]}}}\n' + + def capture_upload(_agent, content, _destination): + captured["line"] = json.loads(content.decode("utf-8")) + return "file_1" + + def batch_json(_agent, method, _path, payload=None, destination=None): + del payload, destination + return {"id": "batch_1"} if method == "POST" else { + "status": "completed", "output_file_id": "file_9" + } + + with patch.object(client, "_batch_upload", side_effect=capture_upload), patch.object( + client, "_batch_json", side_effect=batch_json + ), patch.object(client, "_batch_raw", return_value=raw): + token = _REQUEST_ZDR_ONLY.set(True) + try: + client._batch_run( + agent, {"task_0": [{"role": "user", "content": "hi"}]}, None, 0.01, 5.0 + ) + finally: + _REQUEST_ZDR_ONLY.reset(token) + + assert captured["line"]["body"]["provider"] == {"zdr": True} + + # -- Responses input coercion shapes ------------------------------------------------- diff --git a/tests/test_provider_error_taxonomy.py b/tests/test_provider_error_taxonomy.py index 9aba81f39..7e17bfb3b 100644 --- a/tests/test_provider_error_taxonomy.py +++ b/tests/test_provider_error_taxonomy.py @@ -20,8 +20,14 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +import pytest # noqa: E402 + from contextual_orchestrator import ModelAgent, TaskOrchestrator # noqa: E402 -from contextual_orchestrator.orchestrator import ModelClient, is_transient_error # noqa: E402 +from contextual_orchestrator.orchestrator import ( # noqa: E402 + ModelClient, + _REQUEST_ZDR_ONLY, + is_transient_error, +) from contextual_orchestrator.provider_errors import ( # noqa: E402 MAX_PROVIDER_ERROR_BODY_BYTES, MAX_SAFE_MESSAGE_CHARS, @@ -272,6 +278,26 @@ def test_binary_passthrough_classifies_provider_transport_failure() -> None: raise AssertionError("binary provider failure must be classified") +def test_speech_passthrough_rejects_malformed_zdr_provider_routing() -> None: + """A non-object ``provider`` on the speech/audio bytes path fails closed. + + Devin review on #953: under ``zdr_only`` scope, ``_pin_openrouter_zdr`` + used to build ``dict(payload["provider"])`` unconditionally, so a + caller-supplied non-mapping ``provider`` (an int, bool, list, or string) + raised an uncaught ``TypeError`` from inside ``proxy_send_bytes`` before + any provider transport or failure classification ran. It must instead + raise the same named validation error the helper raises everywhere else. + """ + client = ModelClient(max_retries=0) + agent = ModelAgent("audio_agent", "audio-model", provider_name="openrouter") + token = _REQUEST_ZDR_ONLY.set(True) + try: + with pytest.raises(ValueError, match="provider must be an object"): + client.proxy_send_bytes(agent, "audio/speech", {"input": "hello", "provider": 5}) + finally: + _REQUEST_ZDR_ONLY.reset(token) + + def test_detail_and_transport_are_preserved_for_callers() -> None: """The structured detail names agent/model/status/retryability/transport.""" classified = classify_provider_failure( diff --git a/tests/test_review_gateway.py b/tests/test_review_gateway.py index 2c324cc5e..249106178 100644 --- a/tests/test_review_gateway.py +++ b/tests/test_review_gateway.py @@ -109,12 +109,12 @@ def test_build_review_orchestrator_uses_model_group_diversity(monkeypatch): def test_build_review_orchestrator_never_routes_evidence_only_models(monkeypatch): - """OpenRouter catalog rows are evidence, never review upstreams.""" + """A row explicitly marked evidence_only is never a review upstream.""" discovered = [ _discovered( - "openrouter", + "bytez", "router-review", - "OPENROUTER_API_KEY", + "BYTEZ_API_KEY", 0.01, evidence_only=True, ) @@ -122,7 +122,7 @@ def test_build_review_orchestrator_never_routes_evidence_only_models(monkeypatch monkeypatch.setattr(review_gateway, "discover_all_models", lambda: (discovered, [])) with pytest.raises(NotConfigured, match="general chat models"): - review_gateway.build_review_orchestrator({"OPENROUTER_API_KEY": "router-secret"}) + review_gateway.build_review_orchestrator({"BYTEZ_API_KEY": "router-secret"}) def test_build_review_orchestrator_excludes_endpoint_only_models(monkeypatch):