From 362996e269bd67922fc0ca6fbc4a1f022f4f8fb6 Mon Sep 17 00:00:00 2001 From: Sebastian B Date: Thu, 30 Apr 2026 10:12:27 +0100 Subject: [PATCH] fix(runtime_provider): _get_named_custom_provider must honour transport field on v12+ providers dict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v11→v12 migrate_config step writes the API mode for every entry under the new transport: field (per the v12+ schema in _normalize_custom_provider_entry). _get_named_custom_provider read the legacy api_mode: spelling only, so for every migrated config the lookup returned None for the api mode. Downstream, _resolve_named_custom_runtime then falls back through custom_provider.get("api_mode") or _detect_api_mode_for_url(base_url) or "chat_completions". For loopback URLs (proxies, local servers) or unknown hostnames, the URL detector returns None and the resolver silently downgrades the configured codex_responses / anthropic_messages transport to chat_completions. Requests get sent to /v1/chat/completions instead of /v1/responses or /v1/messages and the provider 404s — or worse, returns a usable chat_completions response while skipping the model's reasoning / caching surface. Fix: read both field names — entry.get("api_mode") or entry.get("transport") — at the two match-by-key + match-by-name branches in _get_named_custom_provider. The runtime normaliser _normalize_custom_provider_entry already accepts both spellings; this lifts the same compat into the direct-dict reader so v12+ configs work without going through the shim. Adds three regression tests under tests/hermes_cli/test_user_providers_model_switch.py: - transport field is read on the match-by-key branch - legacy api_mode spelling still works for hand-edited configs - transport is read on the match-by-display-name branch --- hermes_cli/runtime_provider.py | 11 ++- .../test_user_providers_model_switch.py | 90 ++++++++++++++++++- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index c46ebf3991..3afd67e1cc 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -391,7 +391,14 @@ def _get_named_custom_provider(requested_provider: str) -> Optional[Dict[str, An "api_key": resolved_api_key, "model": entry.get("default_model", ""), } - api_mode = _parse_api_mode(entry.get("api_mode")) + # The v11→v12 migration writes the API mode under the new + # ``transport`` field, but hand-edited configs may still + # use the legacy ``api_mode`` spelling. Accept both — + # the runtime normaliser ``_normalize_custom_provider_entry`` + # already does, so without this lift every migrated config + # silently downgrades codex_responses / anthropic_messages + # providers to chat_completions in the resolved runtime. + api_mode = _parse_api_mode(entry.get("api_mode") or entry.get("transport")) if api_mode: result["api_mode"] = api_mode return result @@ -409,7 +416,7 @@ def _get_named_custom_provider(requested_provider: str) -> Optional[Dict[str, An "api_key": resolved_api_key, "model": entry.get("default_model", ""), } - api_mode = _parse_api_mode(entry.get("api_mode")) + api_mode = _parse_api_mode(entry.get("api_mode") or entry.get("transport")) if api_mode: result["api_mode"] = api_mode return result diff --git a/tests/hermes_cli/test_user_providers_model_switch.py b/tests/hermes_cli/test_user_providers_model_switch.py index 0a357c21fc..0a97509f7c 100644 --- a/tests/hermes_cli/test_user_providers_model_switch.py +++ b/tests/hermes_cli/test_user_providers_model_switch.py @@ -748,6 +748,94 @@ def test_switch_model_resolves_user_provider_credentials(monkeypatch, tmp_path): is_global=False, user_providers=config["providers"], ) - + assert result.success is True assert result.error_message == "" + + +# ============================================================================= +# Regression: providers: dict ``transport`` field must be honored +# ============================================================================= + + +def test_get_named_custom_provider_reads_transport_field(monkeypatch): + """v12+ ``providers:`` dict stores api mode under ``transport:`` (not the + legacy ``api_mode:``). ``_get_named_custom_provider`` must accept both + field names. + + Bug: this function read only ``entry.get("api_mode")`` for v12+ entries. + After ``migrate_config()`` writes ``transport`` on every entry, the + lookup returns None and ``_resolve_named_custom_runtime`` falls back + through ``_detect_api_mode_for_url(base_url) or "chat_completions"`` + — silently downgrading every codex_responses / anthropic_messages + provider to chat_completions. + """ + config = { + "_config_version": 12, + "providers": { + "my-codex-provider": { + "name": "my-codex-provider", + "api": "http://127.0.0.1:4000/v1", + "api_key": "test-key", + "default_model": "gpt-5", + "transport": "codex_responses", + }, + }, + } + + monkeypatch.setattr(rp, "load_config", lambda: config) + + result = rp._get_named_custom_provider("my-codex-provider") + assert result is not None + assert result["api_mode"] == "codex_responses" + assert result["base_url"] == "http://127.0.0.1:4000/v1" + assert result["model"] == "gpt-5" + + +def test_get_named_custom_provider_legacy_api_mode_field_still_works(monkeypatch): + """Hand-edited configs that used ``api_mode:`` (legacy spelling) inside + the v12+ providers: dict shape must keep working — the migration writer + produces ``transport:`` but human-edited configs may carry the older + spelling forward.""" + config = { + "_config_version": 12, + "providers": { + "anthropic-proxy": { + "name": "anthropic-proxy", + "api": "http://127.0.0.1:8082", + "api_key": "test-key", + "default_model": "claude-opus-4-7", + "api_mode": "anthropic_messages", # legacy spelling + }, + }, + } + + monkeypatch.setattr(rp, "load_config", lambda: config) + + result = rp._get_named_custom_provider("anthropic-proxy") + assert result is not None + assert result["api_mode"] == "anthropic_messages" + + +def test_get_named_custom_provider_transport_resolves_via_display_name(monkeypatch): + """When the requested name matches the entry's ``name:`` field rather + than its dict key, the same transport-vs-api_mode logic must apply + (second branch in ``_get_named_custom_provider``).""" + config = { + "_config_version": 12, + "providers": { + "slug-different-from-name": { + "name": "Codex Provider", # display name + "api": "http://127.0.0.1:4000/v1", + "api_key": "test-key", + "default_model": "gpt-5", + "transport": "codex_responses", + }, + }, + } + + monkeypatch.setattr(rp, "load_config", lambda: config) + + result = rp._get_named_custom_provider("Codex Provider") + assert result is not None + assert result["api_mode"] == "codex_responses"