From 010ac29c4de4f84a0d6bbd75d20b87d84d3a31a7 Mon Sep 17 00:00:00 2001 From: Kewe63 Date: Sun, 2 Aug 2026 10:16:47 +0300 Subject: [PATCH 1/2] fix(auxiliary_client): preserve named user-defined provider on explicit base_url (#76602) When async_call_llm(task='vision') (and any other auxiliary path that re-passes the config-resolved provider) forwards provider= + base_url=... + api_key=None to _resolve_task_provider_model, the helper consulted only the built-in provider registry via hermes_cli.providers.get_provider. A name defined in the providers: section of config.yaml (e.g. agnes-ai.cn, nvidia-nim) missed that lookup and fell through to the anonymous "custom" downgrade. The downgrade routed the call through the bare-custom branch in resolve_provider_client, which has no key, sent Authorization: Bearer no-key-required and produced a 401 from any auth-required provider. The fix adds a second lookup against hermes_cli.runtime_provider._get_named_custom_provider (the same helper call_llm already uses for main-runtime resolution) so a named user-defined provider + explicit base_url stays named through to the named-custom-provider key-resolution branch, which then picks up providers..api_key or the configured key_env. Both lookups run inside the existing try / except guard, so a catalog-load failure (early import, missing config) still falls back to the hardcoded allowlist (xai-oauth / qwen-oauth / etc.). The "custom" / "auto" / "custom:" short-circuit is unchanged; the only behavioral change is that the second lookup widens the True set without ever widening a False. Tests cover: - user-defined provider from providers: section is preserved (the repro from #76602) - built-in registry hit still wins (no built-in regression) - unknown provider with explicit base_url still downgrades to "custom" (pre-existing behavior preserved) - hardcoded allowlist still works when both registries are unavailable (early-import path) --- agent/auxiliary_client.py | 24 ++++- tests/agent/test_auxiliary_client.py | 146 +++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 9d26d1bf611b4..6efffc4bcf921 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -7080,7 +7080,29 @@ def _preserve_provider_with_base_url(prov: Optional[str]) -> bool: try: from hermes_cli.providers import get_provider - return get_provider(normalized) is not None + if get_provider(normalized) is not None: + return True + # #76602 — also preserve when *normalized* names a user-defined + # provider from the ``providers:`` section of config.yaml. + # Without this, an explicit provider + base_url (the shape the + # auxiliary vision path passes after resolving the task config) + # falls through to the anonymous ``"custom"`` downgrade, the + # downstream ``resolve_provider_client`` named-custom-provider + # branch never runs, and the call lands with ``no-key-required`` + # → 401 from any auth-required provider (e.g. agnes-ai.cn, + # nvidia-nim with key_env, etc.). Mirrors the keep-naming + # behavior ``call_llm`` uses for main-runtime resolution. + try: + from hermes_cli.runtime_provider import _get_named_custom_provider + + if _get_named_custom_provider(normalized) is not None: + return True + except Exception: + # Config not loaded yet (early import paths, tests) — fall + # back to the hardcoded allowlist below; never widen a True + # just because the import failed. + pass + return False except Exception: # Keep the high-risk provider-backed routes safe even if provider # catalog loading is unavailable during early import/test paths. diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 0e60ea3dd7064..f31ab70793148 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -4213,3 +4213,149 @@ def test_third_party_anthropic_fallback_keeps_message_markers_without_tool_marke for part in (message.get("content") if isinstance(message.get("content"), list) else []) ) + +# --------------------------------------------------------------------------- +# Regression coverage for #76602 — auxiliary vision with a custom provider +# defined in the ``providers:`` section of config.yaml plus an explicit +# ``base_url`` was being silently downgraded to ``"custom"`` because +# ``_preserve_provider_with_base_url`` only consulted the built-in +# provider registry (``hermes_cli.providers.get_provider``). The +# downgrade routed the call through the bare-custom branch in +# ``resolve_provider_client`` with no key, producing 401s from +# auth-required providers (e.g. agnes-ai.cn, nvidia-nim with key_env). +# The fix also checks ``_get_named_custom_provider`` so a named +# user-defined provider + explicit base_url stays named through to the +# named-custom-provider key-resolution branch. +# --------------------------------------------------------------------------- + + +class TestPreserveNamedCustomProviderWithBaseUrl: + """#76602 — _resolve_task_provider_model must keep a user-defined + provider named when the call site passes ``provider=`` + + ``base_url=...`` (the shape async_call_llm takes after resolving the + auxiliary.vision task config). The bare-custom downgrade to the + ``"custom"`` string loses the key and produces 401s. + """ + + def test_user_defined_provider_named_in_config_is_preserved(self, monkeypatch): + """A provider name from ``providers:`` survives explicit base_url. + + Before the fix this returned ``("custom", ...)`` → bare-custom + branch in ``resolve_provider_client`` → ``no-key-required`` → 401. + After the fix it returns ``("agnes-ai.cn", ...)`` so the + named-custom-provider branch picks up + ``providers..api_key`` (or the configured ``key_env``). + """ + import agent.auxiliary_client as ac + + fake_entry = { + "name": "AgnesAI", + "base_url": "https://api.agnes-ai.cn/v1", + "api_key": "sk-agnes-test", + "model": "agnes-2.5-flash", + } + with patch( + "hermes_cli.runtime_provider._get_named_custom_provider", + return_value=fake_entry, + ), patch( + "hermes_cli.providers.get_provider", return_value=None, + ): + resolved_provider, _model, base_url, _api_key, _api_mode = ( + ac._resolve_task_provider_model( + task="vision", + provider="agnes-ai.cn", + model="agnes-2.5-flash", + base_url="https://api.agnes-ai.cn/v1", + api_key=None, + ) + ) + + assert resolved_provider == "agnes-ai.cn", ( + "User-defined provider from providers: section must be preserved " + "instead of being downgraded to 'custom' (issue #76602)" + ) + assert base_url == "https://api.agnes-ai.cn/v1" + + def test_built_in_provider_with_base_url_still_preserved(self, monkeypatch): + """Built-in registry hit still wins — user-defined fallback only + fires when the built-in registry missed. This guards against + regressing the pre-existing built-in provider behavior. + """ + import agent.auxiliary_client as ac + + with patch( + "hermes_cli.providers.get_provider", + return_value={"name": "Anthropic", "api_key": "sk-anthropic"}, + ): + resolved_provider, _model, base_url, _api_key, _api_mode = ( + ac._resolve_task_provider_model( + task="moa_reference", + provider="anthropic", + model="claude-sonnet-4-6", + base_url="https://api.anthropic.com/v1", + api_key="sk-anthropic", + ) + ) + + assert resolved_provider == "anthropic" + assert base_url == "https://api.anthropic.com/v1" + + def test_unknown_provider_with_base_url_falls_back_to_custom_downgrade(self, monkeypatch): + """Provider name not in either registry → keep the existing + ``"custom"`` downgrade behavior. Nothing changes for truly + anonymous custom endpoints (no name → no key → bare-custom branch + handles ``no-key-required`` itself). + """ + import agent.auxiliary_client as ac + + with patch( + "hermes_cli.runtime_provider._get_named_custom_provider", + return_value=None, + ), patch( + "hermes_cli.providers.get_provider", return_value=None, + ): + resolved_provider, _model, base_url, _api_key, _api_mode = ( + ac._resolve_task_provider_model( + task="vision", + provider="some-unknown-gateway", + model="custom-model", + base_url="https://example.com/v1", + api_key="some-token", + ) + ) + + # Pre-existing behavior: unknown name + explicit base_url → + # downgrade to "custom" so the caller routes through the + # bare-custom branch (which uses the explicit api_key). + assert resolved_provider == "custom" + assert base_url == "https://example.com/v1" + + def test_hardcoded_allowlist_still_works_when_both_registries_unavailable(self, monkeypatch): + """If both the built-in registry and the user-defined config are + unavailable (e.g. early import path before config is loaded), the + existing hardcoded allowlist still returns True for known names — + so xai-oauth / qwen-oauth / etc. aren't regressed by the fix. + """ + import agent.auxiliary_client as ac + + def _boom(_name): + raise RuntimeError("catalog unavailable") + + with patch( + "hermes_cli.runtime_provider._get_named_custom_provider", + side_effect=_boom, + ), patch( + "hermes_cli.providers.get_provider", side_effect=_boom, + ): + resolved_provider, _model, _base_url, _api_key, _api_mode = ( + ac._resolve_task_provider_model( + task="moa_reference", + provider="xai-oauth", + model="grok-3", + base_url="https://api.x.ai/v1", + api_key="xai-token", + ) + ) + + assert resolved_provider == "xai-oauth" + From 2b261338c6d730281880061834ce20ea974b11fa Mon Sep 17 00:00:00 2001 From: Kewe63 Date: Sun, 2 Aug 2026 12:18:11 +0300 Subject: [PATCH 2/2] fix(auxiliary_client): isolate builtin + named-custom lookups in preserve check (#76602 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback from @teknium1 and @pestoura on PR #76668. The previous shape nested the new `_get_named_custom_provider` lookup inside the outer try-block that also called `get_provider()`. A partial catalog-load failure in the built-in registry therefore jumped the outer `except` straight to the hardcoded allowlist fallback and never consulted the user-defined providers: entry — so a configured named provider was still downgraded to `"custom"` whenever the built-in catalog failed to load. This contradicts the stated goal of preserving named custom providers through partial catalog-load failures and re-introduces the very 401 path the PR exists to fix. Refactor: extract two module-level helpers — `_builtin_provider_present` and `_named_custom_provider_present` — each with its own try/except. The preserve check is now a flat `if/return True / if/return True / return False` so a failure in one lookup cannot suppress the other. Two new tests: - `test_builtin_registry_raises_does_not_suppress_named_custom_lookup`: `get_provider` raises, `_get_named_custom_provider` returns an entry → the named provider is preserved (the partial-load regression the reviewer flagged). - `test_resolve_vision_provider_client_preserves_named_provider_via_config`: integration test through the real `resolve_vision_provider_client` entry point with a real `HERMES_HOME` config.yaml, mirroring the pattern from `tests/agent/test_auxiliary_named_custom_providers.py`. Exercises the actual credential-resolution path (the inline `api_key` from the providers: entry reaches the client) instead of only asserting on the resolved provider name. Test results: ``` pytest tests/agent/test_auxiliary_client.py ``` ✅ 168/168 passed (162 existing + 6 new), no regressions. --- agent/auxiliary_client.py | 97 ++++++++++++++----------- tests/agent/test_auxiliary_client.py | 101 +++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 39 deletions(-) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 6efffc4bcf921..69b8cd969b1e8 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -6960,6 +6960,52 @@ def _get_cached_client( } +def _builtin_provider_present(name: str) -> bool: + """Look up *name* in the built-in provider registry, returning False + (not raising) when the catalog fails to load. + + Used by ``_preserve_provider_with_base_url`` so a built-in lookup + exception cannot suppress the parallel user-defined provider lookup + (#76602). + """ + try: + from hermes_cli.providers import get_provider + + return get_provider(name) is not None + except Exception: + # Keep the high-risk provider-backed routes safe even if provider + # catalog loading is unavailable during early import/test paths. + return name in { + "anthropic", + "copilot", + "copilot-acp", + "minimax-oauth", + "nous", + "openai-codex", + "qwen-oauth", + "xai-oauth", + } + + +def _named_custom_provider_present(name: str) -> bool: + """Look up *name* in the user-defined ``providers:`` section of + config.yaml, returning False when the config is unavailable or + fails to load. + + Used by ``_preserve_provider_with_base_url`` so a user-defined + provider remains preserved even when the built-in registry raises + (parallel lookup; each side fails independently — #76602). + """ + try: + from hermes_cli.runtime_provider import _get_named_custom_provider + + return _get_named_custom_provider(name) is not None + except Exception: + # Config not loaded yet (early import paths, tests) — fail closed: + # never widen True just because the import / load failed. + return False + + def _resolve_task_provider_model( task: str = None, provider: str = None, @@ -7077,45 +7123,18 @@ def _preserve_provider_with_base_url(prov: Optional[str]) -> bool: normalized = str(prov or "").strip().lower() if normalized in {"", "auto", "custom"} or normalized.startswith("custom:"): return False - try: - from hermes_cli.providers import get_provider - - if get_provider(normalized) is not None: - return True - # #76602 — also preserve when *normalized* names a user-defined - # provider from the ``providers:`` section of config.yaml. - # Without this, an explicit provider + base_url (the shape the - # auxiliary vision path passes after resolving the task config) - # falls through to the anonymous ``"custom"`` downgrade, the - # downstream ``resolve_provider_client`` named-custom-provider - # branch never runs, and the call lands with ``no-key-required`` - # → 401 from any auth-required provider (e.g. agnes-ai.cn, - # nvidia-nim with key_env, etc.). Mirrors the keep-naming - # behavior ``call_llm`` uses for main-runtime resolution. - try: - from hermes_cli.runtime_provider import _get_named_custom_provider - - if _get_named_custom_provider(normalized) is not None: - return True - except Exception: - # Config not loaded yet (early import paths, tests) — fall - # back to the hardcoded allowlist below; never widen a True - # just because the import failed. - pass - return False - except Exception: - # Keep the high-risk provider-backed routes safe even if provider - # catalog loading is unavailable during early import/test paths. - return normalized in { - "anthropic", - "copilot", - "copilot-acp", - "minimax-oauth", - "nous", - "openai-codex", - "qwen-oauth", - "xai-oauth", - } + # #76602 — two independent lookups, each guarded by its own + # try/except so a partial catalog-load failure in either path + # doesn't suppress the other. The previous shape nested the + # named-custom lookup inside the built-in try-block, so an + # exception from get_provider() jumped straight to the outer + # allowlist fallback and never consulted the user's configured + # named provider (the very bug this PR exists to fix). + if _builtin_provider_present(normalized): + return True + if _named_custom_provider_present(normalized): + return True + return False if provider: provider, base_url = _expand_direct_api_alias(provider, base_url) diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index f31ab70793148..076a75a1af6aa 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -4359,3 +4359,104 @@ def _boom(_name): assert resolved_provider == "xai-oauth" + def test_builtin_registry_raises_does_not_suppress_named_custom_lookup(self, monkeypatch): + """#76602 (review feedback) — a partial catalog-load failure in the + built-in registry must not short-circuit the user-defined + provider lookup. The two lookups are now parallel and + independent; an exception in one does not suppress the other. + + Before the refactor, ``get_provider`` raising jumped the outer + ``except`` to the hardcoded allowlist fallback and never + consulted ``_get_named_custom_provider`` — so a configured + named provider was still downgraded to ``"custom"`` whenever the + built-in catalog failed to load. The user's repro path + (Hermes desktop on Windows with a partial / early-startup + catalog state) hits this branch. + """ + import agent.auxiliary_client as ac + + def _catalog_raises(_name): + raise RuntimeError("built-in catalog unavailable") + + fake_entry = { + "name": "AgnesAI", + "base_url": "https://api.agnes-ai.cn/v1", + "api_key": "sk-agnes-from-config", + "model": "agnes-2.5-flash", + } + with patch( + "hermes_cli.runtime_provider._get_named_custom_provider", + return_value=fake_entry, + ), patch( + "hermes_cli.providers.get_provider", side_effect=_catalog_raises, + ): + resolved_provider, _model, base_url, _api_key, _api_mode = ( + ac._resolve_task_provider_model( + task="vision", + provider="agnes-ai.cn", + model="agnes-2.5-flash", + base_url="https://api.agnes-ai.cn/v1", + api_key=None, + ) + ) + + assert resolved_provider == "agnes-ai.cn", ( + "Built-in registry raising must not suppress the named-custom " + "lookup; the user-defined provider must remain named (review " + "feedback on #76602 — partial-load failure path)" + ) + assert base_url == "https://api.agnes-ai.cn/v1" + + def test_resolve_vision_provider_client_preserves_named_provider_via_config(self, tmp_path): + """#76602 (review feedback) — integration test through the real + ``resolve_vision_provider_client`` entry point with a real + ``HERMES_HOME`` config.yaml, mirroring the pattern from + ``tests/agent/test_auxiliary_named_custom_providers.py``. + + Before the fix the repro in the issue body returned + ``('custom', 'no-key-required')`` for this exact config shape; + after the fix the named provider is preserved and the inline + ``api_key`` from the providers: entry reaches the client. + """ + import yaml + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text(yaml.dump({ + "model": {"default": "test-model"}, + "providers": { + "agnes-ai.cn": { + "name": "AgnesAI", + "base_url": "https://api.agnes-ai.cn/v1", + "api_key": "sk-agnes-test", + "model": "agnes-2.5-flash", + }, + }, + })) + import os + old_home = os.environ.get("HERMES_HOME") + os.environ["HERMES_HOME"] = str(hermes_home) + try: + from agent.auxiliary_client import resolve_vision_provider_client + + resolved_provider, client, _model = resolve_vision_provider_client( + provider="agnes-ai.cn", + model="agnes-2.5-flash", + base_url="https://api.agnes-ai.cn/v1", + api_key=None, + ) + finally: + if old_home is None: + os.environ.pop("HERMES_HOME", None) + else: + os.environ["HERMES_HOME"] = old_home + + assert resolved_provider == "agnes-ai.cn", ( + "resolve_vision_provider_client must preserve a named " + "user-defined provider + explicit base_url rather than " + "downgrading to 'custom' (issue #76602 repro)" + ) + # The inline api_key from the providers: entry must reach the + # client — this is what the 'custom' downgrade was losing. + assert client.api_key == "sk-agnes-test" +