ouyangbo/EvoScientist
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 3 Wiki Activity

fix(model-registry): inject async safe clients and split ollama transports

Browse Source 
Review fixes for the Task 3 contract layer:

- build_chat_model now accepts http_async_client alongside http_client
  (at least one required) and wires it into ChatOpenAI
  (http_async_client), ChatAnthropic (seeded _async_client), and
  ChatOllama (async_client_kwargs transport), closing the unsafe
  default-async-client gap.
- ChatOllama safe transports move from the shared client_kwargs to
  sync_client_kwargs/async_client_kwargs; langchain-ollama merges shared
  kwargs into both clients, which poisoned the async client with a sync
  transport and crashed ainvoke.
- Unsupported parameters now actually execute the contract-declared
  normalizer (reject_non_auto) instead of a hardcoded raise, with a
  fallback rejection if a normalizer would let a value through.
- build_chat_model rejects overlapping client_options/request_options
  keys instead of silently overwriting.
This commit is contained in:
m4
2026-07-20 22:40:33 +08:00
parent af4ae1aef5
commit b2e28249fd
4 changed files with 290 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
EvoScientist/model_registry/adapters.py
+10 -3
View File
@@ -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.")
EvoScientist/model_registry/factory.py
+77 -31
View File
@@ -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)
tests/test_adapter_contracts.py
+42
View File
@@ -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"
tests/test_model_factory.py
+161 -9
View File
@@ -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())