diff --git a/EvoScientist/model_registry/adapters.py b/EvoScientist/model_registry/adapters.py index 56ebe8c..0c0893d 100644 --- a/EvoScientist/model_registry/adapters.py +++ b/EvoScientist/model_registry/adapters.py @@ -531,9 +531,16 @@ def _resolve_nullable_parameter( if not rule.supported: if value is None or value == "auto": return OMIT - raise _reject_parameter( - name, f"{name} is not supported by this adapter contract." - ) + # A concrete value for an unsupported parameter is handed to the + # contract-declared normalizer: reject_non_auto raises here. Any + # normalizer that would let the value through still rejects, because + # an unsupported parameter must never enter the request. + resolved = _NORMALIZERS[rule.normalizer](name, rule, value) + if resolved is not OMIT: + raise _reject_parameter( + name, f"{name} is not supported by this adapter contract." + ) + return OMIT if value is None or (name == "reasoning_effort" and value == "auto"): if rule.nullable == "forbidden": raise _reject_parameter(name, f"{name} must have a concrete value.") diff --git a/EvoScientist/model_registry/factory.py b/EvoScientist/model_registry/factory.py index 2e06b72..cb2d0d9 100644 --- a/EvoScientist/model_registry/factory.py +++ b/EvoScientist/model_registry/factory.py @@ -11,16 +11,22 @@ never reads the credential store and never falls back to provider API-key environment variables such as ``OPENAI_API_KEY`` — a missing credential fails with ``CREDENTIAL_NOT_CONFIGURED`` before any model is constructed. -Every builder injects the Task 2 safe HTTP client so provider egress keeps -passing the EndpointPolicy/SSRF defenses: +Every builder injects the Task 2 safe HTTP clients so provider egress keeps +passing the EndpointPolicy/SSRF defenses. Callers pass the sync client +(``build_safe_http_client``), the async client +(``build_safe_async_http_client``), or both; at least one is required: -- ``ChatOpenAI`` (openai, openai-compatible) takes ``http_client`` directly. -- ``ChatAnthropic`` builds its SDK client from its own ``_client_params`` - plus the safe client (LangChain exposes no constructor hook for it). -- ``ChatOllama`` routes through the safe transport via ``client_kwargs``; - the ollama SDK also reads ``OLLAMA_API_KEY`` from the environment, so the - ``Authorization`` header it may inject is stripped after construction — - a ``mode=none`` adapter must not read, write, or fabricate API keys. +- ``ChatOpenAI`` (openai, openai-compatible) takes ``http_client`` / + ``http_async_client`` directly. +- ``ChatAnthropic`` builds its SDK clients from its own ``_client_params`` + plus the safe clients (LangChain exposes no constructor hook for them). +- ``ChatOllama`` routes through the safe transports via + ``sync_client_kwargs`` / ``async_client_kwargs`` — never the shared + ``client_kwargs``, which langchain-ollama merges into both clients and + would poison the async client with a sync transport. The ollama SDK also + reads ``OLLAMA_API_KEY`` from the environment, so the ``Authorization`` + header it may inject is stripped after construction — a ``mode=none`` + adapter must not read, write, or fabricate API keys. """ from __future__ import annotations @@ -39,49 +45,70 @@ from langchain_openai import ChatOpenAI from .adapters import get_adapter from .schemas import ResolvedModelConfig -ChatModelBuilder = Callable[[dict[str, Any], httpx.Client], BaseChatModel] +ChatModelBuilder = Callable[ + [dict[str, Any], httpx.Client | None, httpx.AsyncClient | None], BaseChatModel +] def _build_chat_openai( - options: dict[str, Any], http_client: httpx.Client + options: dict[str, Any], + http_client: httpx.Client | None, + http_async_client: httpx.AsyncClient | None, ) -> BaseChatModel: - return ChatOpenAI(**options, http_client=http_client) + kwargs = dict(options) + if http_client is not None: + kwargs["http_client"] = http_client + if http_async_client is not None: + kwargs["http_async_client"] = http_async_client + return ChatOpenAI(**kwargs) def _build_chat_anthropic( - options: dict[str, Any], http_client: httpx.Client + options: dict[str, Any], + http_client: httpx.Client | None, + http_async_client: httpx.AsyncClient | None, ) -> BaseChatModel: model = ChatAnthropic(**options) # ChatAnthropic exposes no http_client constructor argument; it builds - # ``_client`` (a cached_property) from ``_client_params``. Seeding the - # cached property with an SDK client wrapped around the safe transport - # keeps anthropic egress inside the EndpointPolicy defenses. + # ``_client``/``_async_client`` (cached_property) from ``_client_params``. + # Seeding the cached properties with SDK clients wrapped around the safe + # transports keeps anthropic egress inside the EndpointPolicy defenses. client_params = model._client_params - model.__dict__["_client"] = anthropic.Client( - **client_params, http_client=http_client - ) + if http_client is not None: + model.__dict__["_client"] = anthropic.Client( + **client_params, http_client=http_client + ) + if http_async_client is not None: + model.__dict__["_async_client"] = anthropic.AsyncClient( + **client_params, http_client=http_async_client + ) return model -def _transport_of(http_client: httpx.Client) -> httpx.BaseTransport: +def _transport_of(http_client: httpx.Client | httpx.AsyncClient) -> Any: transport = getattr(http_client, "_transport", None) if transport is None: # pragma: no cover - defensive raise TypeError( - "http_client must be an httpx.Client built by build_safe_http_client." + "http_client must be built by build_safe_http_client or " + "build_safe_async_http_client." ) return transport def _build_chat_ollama( - options: dict[str, Any], http_client: httpx.Client + options: dict[str, Any], + http_client: httpx.Client | None, + http_async_client: httpx.AsyncClient | None, ) -> BaseChatModel: options = copy.deepcopy(options) - client_kwargs = dict(options.get("client_kwargs") or {}) - # The ollama SDK forwards client_kwargs to its internal httpx client, so - # the safe transport (URL + IP layers) carries over; retries live in the - # transport built by Task 2. - client_kwargs["transport"] = _transport_of(http_client) - options["client_kwargs"] = client_kwargs + # langchain-ollama merges the shared client_kwargs into BOTH clients, so + # the safe transports must go through the per-direction kwargs: a sync + # transport in client_kwargs would poison the async client (its httpx + # async calls would hit handle_async_request on a sync transport). + if http_client is not None: + options["sync_client_kwargs"] = {"transport": _transport_of(http_client)} + if http_async_client is not None: + options["async_client_kwargs"] = {"transport": _transport_of(http_async_client)} model = ChatOllama(**options) # ollama-python silently adds an Authorization header from OLLAMA_API_KEY. # Phase-1 ollama contracts only allow auth mode "none", which must not @@ -102,7 +129,8 @@ CHAT_MODEL_BUILDERS: dict[str, ChatModelBuilder] = { def build_chat_model( resolved_config: ResolvedModelConfig, - http_client: httpx.Client, + http_client: httpx.Client | None = None, + http_async_client: httpx.AsyncClient | None = None, *, credential: str | None = None, ) -> BaseChatModel: @@ -117,19 +145,37 @@ def build_chat_model( "resolved_config must be a ResolvedModelConfig, got " f"{type(resolved_config).__name__}." ) - if not isinstance(http_client, httpx.Client): + if http_client is None and http_async_client is None: + raise TypeError( + "At least one of http_client / http_async_client is required; " + "provider egress must go through the safe transports." + ) + if http_client is not None and not isinstance(http_client, httpx.Client): raise TypeError( f"http_client must be an httpx.Client, got {type(http_client).__name__}." ) + if http_async_client is not None and not isinstance( + http_async_client, httpx.AsyncClient + ): + raise TypeError( + "http_async_client must be an httpx.AsyncClient, got " + f"{type(http_async_client).__name__}." + ) adapter = get_adapter( resolved_config.adapter_id, resolved_config.upstream_model_id, spec_revision=resolved_config.adapter_spec_revision, ) built = adapter.build_request(resolved_config, credential=credential) + overlap = set(built.client_options) & set(built.request_options) + if overlap: + raise ValueError( + "Adapter contract client_options/request_options keys overlap: " + f"{sorted(overlap)}." + ) options = {**built.client_options, **built.request_options} connection = adapter.spec.connection if connection is None: # pragma: no cover - built-in contracts always set it raise TypeError(f"Adapter {adapter.spec.adapter_id!r} has no connection spec.") builder = CHAT_MODEL_BUILDERS[connection.chat_model] - return builder(options, http_client) + return builder(options, http_client, http_async_client) diff --git a/tests/test_adapter_contracts.py b/tests/test_adapter_contracts.py index 433b844..570eec9 100644 --- a/tests/test_adapter_contracts.py +++ b/tests/test_adapter_contracts.py @@ -650,3 +650,45 @@ class TestEffectiveCapabilities: def test_adapter_spec_type_is_schema_model(self): for spec in adapter_specs(): assert isinstance(spec, AdapterParameterSpec) + + +class TestUnsupportedRuleNormalizer: + def test_declared_reject_non_auto_normalizer_executes(self, monkeypatch): + import EvoScientist.model_registry.adapters as adapters + + calls = [] + original = adapters._NORMALIZERS["reject_non_auto"] + + def spy(name, rule, value): + calls.append((name, value)) + return original(name, rule, value) + + monkeypatch.setitem(adapters._NORMALIZERS, "reject_non_auto", spy) + provider = _provider() + model = _model(provider, reasoning_effort="high") + spec = find_adapter_spec("openai-compatible", "glm-5.2") + assert spec is not None + with pytest.raises(ModelRegistryError) as excinfo: + resolve_parameters(provider, model, spec) + assert excinfo.value.code == UNSUPPORTED_RUNTIME_PARAMETER + assert calls == [("reasoning_effort", "high")] + + def test_unsupported_rule_with_passthrough_normalizer_still_rejected(self): + spec = find_adapter_spec("openai-compatible", "glm-5.2") + assert spec is not None + spec = spec.model_copy(deep=True) + spec.parameters["reasoning_effort"].normalizer = "identity" + provider = _provider() + model = _model(provider, reasoning_effort="low") + with pytest.raises(ModelRegistryError) as excinfo: + resolve_parameters(provider, model, spec) + assert excinfo.value.code == UNSUPPORTED_RUNTIME_PARAMETER + + def test_unsupported_rule_still_omits_inherited_values(self): + spec = find_adapter_spec("openai-compatible", "glm-5.2") + assert spec is not None + spec = spec.model_copy(deep=True) + spec.parameters["reasoning_effort"].normalizer = "identity" + provider = _provider() + resolved = resolve_parameters(provider, provider.models[0], spec) + assert resolved.reasoning_effort == "auto" diff --git a/tests/test_model_factory.py b/tests/test_model_factory.py index 3be5927..ead9544 100644 --- a/tests/test_model_factory.py +++ b/tests/test_model_factory.py @@ -68,6 +68,10 @@ def _http_client() -> httpx.Client: return httpx.Client(transport=httpx.MockTransport(lambda request: None)) +def _http_async_client() -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(lambda request: None)) + + class _FakeChatModel: """Captures construction arguments instead of opening a connection.""" @@ -76,13 +80,13 @@ class _FakeChatModel: class _FakeBuilder: - """Records the options and HTTP client a builder receives.""" + """Records the options and HTTP clients a builder receives.""" def __init__(self): self.calls = [] - def __call__(self, options, http_client): - self.calls.append((dict(options), http_client)) + def __call__(self, options, http_client, http_async_client): + self.calls.append((dict(options), http_client, http_async_client)) return _FakeChatModel(**options) @@ -153,7 +157,9 @@ class TestEnvironmentIsolation: base_url="http://localhost:11434", auth_ref={"mode": "none", "credential_id": None}, ) - model = build_chat_model(resolved, _http_client()) + model = build_chat_model( + resolved, _http_client(), http_async_client=_http_async_client() + ) headers = model._client._client.headers assert "authorization" not in headers async_headers = model._async_client._client.headers @@ -164,8 +170,9 @@ class TestPerAdapterConstruction: def test_openai_compatible_glm_options(self, fake_builders): client = _http_client() build_chat_model(_resolved_config(), client, credential="test-secret") - options, seen_client = fake_builders["ChatOpenAI"].calls[0] + options, seen_client, seen_async = fake_builders["ChatOpenAI"].calls[0] assert seen_client is client + assert seen_async is None assert options == { "model": "glm-5.2", "base_url": "https://open.bigmodel.cn/api/paas/v4", @@ -184,7 +191,7 @@ class TestPerAdapterConstruction: base_url="https://api.openai.com/v1", ) build_chat_model(resolved, _http_client(), credential="sk-test") - options, _ = fake_builders["ChatOpenAI"].calls[0] + options, _, _ = fake_builders["ChatOpenAI"].calls[0] assert options["model"] == "gpt-x" assert options["max_tokens"] == 32768 assert options["api_key"] == "sk-test" @@ -197,7 +204,7 @@ class TestPerAdapterConstruction: ) client = _http_client() build_chat_model(resolved, client, credential="sk-ant") - options, seen_client = fake_builders["ChatAnthropic"].calls[0] + options, seen_client, _ = fake_builders["ChatAnthropic"].calls[0] assert seen_client is client assert options["model"] == "claude-x" assert options["max_tokens"] == 32768 @@ -211,7 +218,7 @@ class TestPerAdapterConstruction: base_url="https://gateway.example.com", ) build_chat_model(resolved, _http_client(), credential="sk-ant") - options, _ = fake_builders["ChatAnthropic"].calls[0] + options, _, _ = fake_builders["ChatAnthropic"].calls[0] assert options["base_url"] == "https://gateway.example.com" assert options["max_tokens"] == 32768 @@ -224,7 +231,7 @@ class TestPerAdapterConstruction: ) client = _http_client() build_chat_model(resolved, client) - options, seen_client = fake_builders["ChatOllama"].calls[0] + options, seen_client, _ = fake_builders["ChatOllama"].calls[0] assert seen_client is client assert options["model"] == "llama3" assert options["base_url"] == "http://localhost:11434" @@ -310,3 +317,148 @@ class TestOutboundRequestCapture: assert body["top_p"] == 0.95 assert "reasoning_effort" not in body assert "reasoning" not in body + + +class TestAsyncClientInjection: + def _ollama_resolved(self): + return _resolved_config( + adapter_id="ollama", + upstream_model_id="llama3", + base_url="http://localhost:11434", + auth_ref={"mode": "none", "credential_id": None}, + ) + + def test_requires_at_least_one_client(self): + with pytest.raises(TypeError): + build_chat_model(_resolved_config()) + + def test_rejects_non_httpx_async_client(self): + with pytest.raises(TypeError): + build_chat_model(_resolved_config(), http_async_client=object()) + + def test_chat_openai_receives_async_client(self): + async_client = _http_async_client() + model = build_chat_model( + _resolved_config(), http_async_client=async_client, credential="key" + ) + assert model.http_async_client is async_client + + def test_chat_anthropic_async_egress_uses_safe_client(self): + async_client = _http_async_client() + resolved = _resolved_config( + adapter_id="anthropic", + upstream_model_id="claude-x", + base_url="https://api.anthropic.com", + ) + model = build_chat_model( + resolved, http_async_client=async_client, credential="sk-ant" + ) + assert model._async_client._client is async_client + + def test_chat_ollama_sync_and_async_transports_are_split(self): + sync_client = _http_client() + async_client = _http_async_client() + model = build_chat_model( + self._ollama_resolved(), sync_client, http_async_client=async_client + ) + sync_transport = model._client._client._transport + async_transport = model._async_client._client._transport + assert sync_transport is sync_client._transport + assert async_transport is async_client._transport + assert async_transport is not sync_client._transport + + async def test_ollama_ainvoke_uses_async_transport(self): + captured = [] + + def handler(request: httpx.Request) -> httpx.Response: + captured.append(request) + return httpx.Response( + 200, + json={ + "model": "llama3", + "created_at": "2026-01-01T00:00:00Z", + "message": {"role": "assistant", "content": "hello"}, + "done": True, + "done_reason": "stop", + "total_duration": 1, + "prompt_eval_count": 3, + "eval_count": 1, + }, + ) + + sync_client = httpx.Client(transport=httpx.MockTransport(handler)) + async_client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + model = build_chat_model( + self._ollama_resolved(), sync_client, http_async_client=async_client + ) + response = await model.ainvoke([HumanMessage(content="hi")]) + assert response.content == "hello" + assert len(captured) == 1 + assert captured[0].url.path == "/api/chat" + + +class TestOptionKeyOverlap: + def test_overlapping_client_and_request_keys_rejected(self, monkeypatch): + from EvoScientist.model_registry.adapters import Adapter + from EvoScientist.model_registry.schemas import ( + AdapterParameterSpec, + AuthSpec, + Capabilities, + ConnectionSpec, + ParameterRule, + ) + + spec = AdapterParameterSpec( + adapter_id="ollama", + spec_revision=1, + model_selector="*", + auth_specs={ + "none": AuthSpec( + credential_required=False, + credential_kind="none", + target="adapter_internal", + target_name=None, + ) + }, + parameters={ + "timeout_seconds": ParameterRule( + supported=True, + value_type="integer", + nullable="forbidden", + target="client_option", + target_name="shared", + normalizer="identity", + ), + "max_retries": ParameterRule( + supported=True, + value_type="integer", + nullable="forbidden", + target="client_option", + target_name="max_retries", + normalizer="identity", + ), + "max_output_tokens": ParameterRule( + supported=True, + value_type="integer", + nullable="forbidden", + target="request_option", + target_name="shared", + normalizer="identity", + ), + }, + protocol_capabilities=Capabilities(), + connection=ConnectionSpec( + chat_model="ChatOllama", model_field="model", base_url_field="base_url" + ), + ) + monkeypatch.setattr( + factory, "get_adapter", lambda *args, **kwargs: Adapter(spec) + ) + resolved = _resolved_config( + adapter_id="ollama", + upstream_model_id="llama3", + base_url="http://localhost:11434", + auth_ref={"mode": "none", "credential_id": None}, + ) + with pytest.raises(ValueError, match="overlap"): + build_chat_model(resolved, _http_client())