diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 19df05535d..22b69e3e30 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -4919,11 +4919,7 @@ def _resolve_api_key_branch(req: _ResolveRequest, pconfig: Any, resolve_creds: C if req.explicit_base_url and provider != "actual": base_url = _to_openai_base_url(req.explicit_base_url.strip().rstrip("/")) final_model = _normalize_resolved_model(req.model or _get_aux_model_for_provider(provider), provider) - # Provider-supplied client, consulted before the built-in gemini/OpenAI ladder so an - # out-of-tree provider registered with auth_type="api_key" keeps its native transport for - # auxiliary tasks — the same hook the main-agent path honors (#112384). ``None`` (the - # profile default) falls through to the standard construction, so built-in providers - # without a registered profile are untouched. + # Consulted before the built-in gemini/OpenAI ladder so a registered native transport wins (#112384). profile_client = _api_key_profile_supplied_client(provider, api_key=api_key, base_url=base_url) if profile_client is not None: logger.debug("resolve_provider_client: %s native client from provider profile (%s)", provider, final_model) diff --git a/tests/agent/test_auxiliary_provider_supplied_client.py b/tests/agent/test_auxiliary_provider_supplied_client.py index 0e0a6684ab..027bb7d778 100644 --- a/tests/agent/test_auxiliary_provider_supplied_client.py +++ b/tests/agent/test_auxiliary_provider_supplied_client.py @@ -5,7 +5,7 @@ so an out-of-tree provider registered with ``auth_type="api_key"`` lost its nati for auxiliary tasks even though the main-agent path (``agent_runtime_helpers._provider_supplied_client``) honors the same hook (#112384). These tests pin the seam through the real resolution entry point: the native client -(sync + async), the ``None`` fall-through for ordinary providers, and failure isolation +(sync + async), and the fall-through to the standard client for ordinary providers and for a broken plugin. """ @@ -56,95 +56,57 @@ def _probe_profile(cls, name: str) -> ProviderProfile: @pytest.fixture -def registered(tmp_path, monkeypatch): - """Register provider profiles for one test; restore both registries and the secret scope after. +def registered(monkeypatch): + """Register provider profiles for one test on copies of both registries. Mirrors the import-time synthesis ``hermes_cli.auth`` performs for plugin ``api_key`` profiles, which is what routes them into ``_resolve_api_key_branch`` in the first place. """ import hermes_cli.auth as _auth from agent import secret_scope as _secret_scope - from hermes_constants import reset_hermes_home_override, set_hermes_home_override _providers._discover_providers() - providers_snapshot = ( - dict(_providers._REGISTRY), - dict(_providers._ALIASES), - _providers._PROVIDER_LIST_CACHE, - ) - auth_snapshot = dict(_auth.PROVIDER_REGISTRY) - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - home_token = set_hermes_home_override(str(tmp_path)) + monkeypatch.setattr(_providers, "_REGISTRY", dict(_providers._REGISTRY)) + monkeypatch.setattr(_providers, "_ALIASES", dict(_providers._ALIASES)) + monkeypatch.setattr(_providers, "_PROVIDER_LIST_CACHE", None) + monkeypatch.setattr(_auth, "PROVIDER_REGISTRY", dict(_auth.PROVIDER_REGISTRY)) scope_token = _secret_scope.set_secret_scope({_PROBE_ENV_VAR: _PROBE_KEY}) - yield ( - lambda profile: ( - ( - _providers.register_provider(profile), - _auth._register_plugin_provider(profile), - ) - and profile - ) - ) + def _register(profile: ProviderProfile) -> None: + _providers.register_provider(profile) + _auth._register_plugin_provider(profile) + yield _register _secret_scope.reset_secret_scope(scope_token) - reset_hermes_home_override(home_token) - _providers._REGISTRY.clear() - _providers._REGISTRY.update(providers_snapshot[0]) - _providers._ALIASES.clear() - _providers._ALIASES.update(providers_snapshot[1]) - _providers._PROVIDER_LIST_CACHE = providers_snapshot[2] - _auth.PROVIDER_REGISTRY.clear() - _auth.PROVIDER_REGISTRY.update(auth_snapshot) -def test_native_profile_supplies_the_auxiliary_client(registered): +@pytest.mark.parametrize( + "extra", + [{"task": "title_generation"}, {"async_mode": True}], + ids=["sync", "async-skips-AsyncOpenAI-rewrap"], +) +def test_native_profile_supplies_the_auxiliary_client(registered, extra): from agent.auxiliary_client import resolve_provider_client registered(_probe_profile(_NativeProfile, "aux-seam-native")) - client, model = resolve_provider_client( - "aux-seam-native", "probe-model", task="title_generation" - ) + client, model = resolve_provider_client("aux-seam-native", "probe-model", **extra) assert isinstance(client, _FakeNativeClient) assert model == "probe-model" # The hook receives the same mapping the branch would have passed to openai.OpenAI. - assert client.kwargs["api_key"] == _PROBE_KEY - assert client.kwargs["base_url"] == "https://aux-seam-native.invalid" + assert client.kwargs == {"api_key": _PROBE_KEY, "base_url": "https://aux-seam-native.invalid"} -def test_native_profile_client_survives_the_async_route(registered): - from agent.auxiliary_client import resolve_provider_client - - registered(_probe_profile(_NativeProfile, "aux-seam-native")) - client, model = resolve_provider_client( - "aux-seam-native", "probe-model", async_mode=True - ) - - # HERMES_SKIP_ASYNC_WRAP: a native transport is not rebuilt as AsyncOpenAI. - assert isinstance(client, _FakeNativeClient) - assert model == "probe-model" - - -def test_profile_returning_none_falls_through_to_the_standard_client(registered): +@pytest.mark.parametrize( + "profile_cls", [_PassThroughProfile, _ExplodingProfile], ids=["returns-None", "raises"] +) +def test_profile_without_a_client_falls_back_to_the_standard_client(registered, profile_cls): from openai import OpenAI from agent.auxiliary_client import resolve_provider_client - registered(_probe_profile(_PassThroughProfile, "aux-seam-passthrough")) - client, model = resolve_provider_client("aux-seam-passthrough", "probe-model") - - assert isinstance(client, OpenAI) - assert model == "probe-model" - - -def test_a_broken_profile_falls_back_to_the_standard_client(registered): - from openai import OpenAI - - from agent.auxiliary_client import resolve_provider_client - - registered(_probe_profile(_ExplodingProfile, "aux-seam-boom")) - client, model = resolve_provider_client("aux-seam-boom", "probe-model") + registered(_probe_profile(profile_cls, "aux-seam-fallback")) + client, model = resolve_provider_client("aux-seam-fallback", "probe-model") # A raising plugin can only fail to provide a client, never break auxiliary resolution. assert isinstance(client, OpenAI)