From cbfe186da8fa82bbf5830ecc20031735503773dd Mon Sep 17 00:00:00 2001 From: SHL0MS Date: Fri, 14 Aug 2026 14:47:37 -0400 Subject: [PATCH] fix(config): canonicalize legacy api_mode spellings instead of silently discarding them Earlier releases accepted api_mode: openai on custom provider entries. The canonical transport set is now {chat_completions, codex_responses, anthropic_messages, bedrock_converse, codex_app_server}, and an unrecognized value was silently ignored at both consumption sites (_normalize_custom_provider_entry passes the raw string through and agent_init's accepted-set check drops it; _parse_api_mode returns None), falling through to hostname-based detection. For hosts with a detection rule the provider silently switches transports after an update. Observed live: a custom entry for api.actual.inc with api_mode: openai (valid when written) flipped to codex_responses via the hostname rule, and every reasoning-bearing request to the relay's /v1/responses failed with a wrapped non-JSON error while /v1/chat/completions worked throughout. Fix: one shared alias map (_canonical_api_mode) consulted by both sites. openai/openai_chat -> chat_completions, responses -> codex_responses, anthropic/messages -> anthropic_messages, bedrock -> bedrock_converse. Canonical names and unknown values pass through unchanged, so invalid-config behavior is untouched. Tests: alias map contract (every alias lands in _VALID_API_MODES), normalizer canonicalization incl. the transport: key alias, and the runtime gate accepting legacy spellings while still rejecting unknowns. --- hermes_cli/config.py | 38 ++++++- hermes_cli/runtime_provider.py | 12 ++- tests/hermes_cli/test_api_mode_aliases.py | 120 ++++++++++++++++++++++ 3 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/hermes_cli/test_api_mode_aliases.py diff --git a/hermes_cli/config.py b/hermes_cli/config.py index a5319c9170..1851d1a3fc 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1283,6 +1283,42 @@ def _warn_once_per_provider( logger.warning(msg, *args) +_API_MODE_ALIASES = { + # Values accepted by earlier releases (and natural spellings) mapped to + # the canonical transport names consumed by agent_init. Before this map + # existed, an unrecognized api_mode was silently ignored and the + # transport fell through to hostname-based guessing, so a config that + # said ``api_mode: openai`` (valid on older releases) could flip to + # ``codex_responses`` after an update and break the provider (#66543 + # discussion; observed live against api.actual.inc). + "openai": "chat_completions", + "openai_chat": "chat_completions", + "openai-chat": "chat_completions", + "chat-completions": "chat_completions", + "chatcompletions": "chat_completions", + "responses": "codex_responses", + "openai_responses": "codex_responses", + "openai-responses": "codex_responses", + "anthropic": "anthropic_messages", + "anthropic-messages": "anthropic_messages", + "messages": "anthropic_messages", + "bedrock": "bedrock_converse", + "bedrock-converse": "bedrock_converse", +} + + +def _canonical_api_mode(api_mode: str) -> str: + """Map legacy/alias ``api_mode`` spellings to canonical transport names. + + Unknown values pass through unchanged (callers keep their existing + fall-through behavior); known aliases are rewritten so downstream + consumers (``agent_init``'s accepted-set check, runtime resolution) + see a canonical name instead of silently discarding the user's intent. + """ + cleaned = api_mode.strip() + return _API_MODE_ALIASES.get(cleaned.lower(), cleaned) + + def _normalize_custom_provider_entry( entry: Any, *, @@ -1403,7 +1439,7 @@ def _normalize_custom_provider_entry( api_mode = entry.get("api_mode") or entry.get("transport") if isinstance(api_mode, str) and api_mode.strip(): - normalized["api_mode"] = api_mode.strip() + normalized["api_mode"] = _canonical_api_mode(api_mode) model_name = entry.get("model") or entry.get("default_model") if isinstance(model_name, str) and model_name.strip(): diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index bc8b25924b..c24c5c1425 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -397,9 +397,17 @@ _VALID_API_MODES = { def _parse_api_mode(raw: Any) -> Optional[str]: - """Validate an api_mode value from config. Returns None if invalid.""" + """Validate an api_mode value from config. Returns None if invalid. + + Legacy/alias spellings (``openai``, ``anthropic``, ``responses``, …) are + canonicalized via the shared alias map before validation, so configs + written against older releases keep selecting the transport they named + instead of silently falling through to hostname-based detection. + """ if isinstance(raw, str): - normalized = raw.strip().lower() + from hermes_cli.config import _canonical_api_mode + + normalized = _canonical_api_mode(raw).lower() if normalized in _VALID_API_MODES: return normalized return None diff --git a/tests/hermes_cli/test_api_mode_aliases.py b/tests/hermes_cli/test_api_mode_aliases.py new file mode 100644 index 0000000000..550e009a15 --- /dev/null +++ b/tests/hermes_cli/test_api_mode_aliases.py @@ -0,0 +1,120 @@ +"""Legacy ``api_mode`` spellings must keep selecting the transport they named. + +Regression coverage for the silent api_mode vocabulary break: earlier +releases accepted ``api_mode: openai`` on custom provider entries. The +canonical set consumed by ``agent_init`` is now {chat_completions, +codex_responses, anthropic_messages, bedrock_converse, codex_app_server}, +and an unrecognized value was silently ignored at BOTH consumption sites: + +* ``hermes_cli.config._normalize_custom_provider_entry`` passed the raw + string through, so ``agent_init``'s accepted-set check dropped it and + fell through to hostname detection. +* ``hermes_cli.runtime_provider._parse_api_mode`` returned None, with the + same fall-through. + +For a host with a detection rule (e.g. api.actual.inc -> codex_responses) +the provider silently switched transports after an update and broke: +observed live as every reasoning-bearing request to a relay's untested +/v1/responses endpoint failing while chat_completions worked. See the +#66543 discussion. + +The fix canonicalizes known legacy/alias spellings through one shared map +(``_canonical_api_mode``) at both sites. Unknown values still pass through +unchanged (normalizer) / return None (runtime gate) so existing invalid +config behavior is untouched. +""" + +from __future__ import annotations + +import pytest + +from hermes_cli.config import _canonical_api_mode, _normalize_custom_provider_entry +from hermes_cli.runtime_provider import _parse_api_mode, _VALID_API_MODES + + +class TestCanonicalApiMode: + """The shared alias map.""" + + @pytest.mark.parametrize( + "alias, canonical", + [ + ("openai", "chat_completions"), + ("OpenAI", "chat_completions"), + (" openai ", "chat_completions"), + ("openai_chat", "chat_completions"), + ("chat-completions", "chat_completions"), + ("responses", "codex_responses"), + ("openai_responses", "codex_responses"), + ("anthropic", "anthropic_messages"), + ("messages", "anthropic_messages"), + ("bedrock", "bedrock_converse"), + ], + ) + def test_alias_maps_to_canonical(self, alias, canonical): + assert _canonical_api_mode(alias) == canonical + + @pytest.mark.parametrize( + "canonical", + sorted(_VALID_API_MODES), + ) + def test_canonical_names_pass_through(self, canonical): + assert _canonical_api_mode(canonical) == canonical + + def test_unknown_value_passes_through_unchanged(self): + assert _canonical_api_mode("weird_thing") == "weird_thing" + + def test_every_alias_lands_in_the_valid_set(self): + """Contract: aliasing must never produce a value the runtime rejects.""" + from hermes_cli.config import _API_MODE_ALIASES + + for target in _API_MODE_ALIASES.values(): + assert target in _VALID_API_MODES + + +class TestNormalizedEntryCanonicalizes: + """Config-side consumption: _normalize_custom_provider_entry.""" + + def _entry(self, api_mode): + return { + "name": "relay", + "api": "https://relay.example.invalid/v1", + "api_mode": api_mode, + } + + def test_legacy_openai_becomes_chat_completions(self): + normalized = _normalize_custom_provider_entry( + self._entry("openai"), provider_key="relay" + ) + assert normalized["api_mode"] == "chat_completions" + + def test_canonical_value_unchanged(self): + normalized = _normalize_custom_provider_entry( + self._entry("codex_responses"), provider_key="relay" + ) + assert normalized["api_mode"] == "codex_responses" + + def test_transport_key_also_canonicalized(self): + entry = { + "name": "relay", + "api": "https://relay.example.invalid/v1", + "transport": "openai", + } + normalized = _normalize_custom_provider_entry(entry, provider_key="relay") + assert normalized["api_mode"] == "chat_completions" + + +class TestRuntimeParseApiMode: + """Runtime-side consumption: _parse_api_mode.""" + + def test_legacy_openai_is_valid_chat_completions(self): + assert _parse_api_mode("openai") == "chat_completions" + + def test_canonical_value_still_valid(self): + assert _parse_api_mode("anthropic_messages") == "anthropic_messages" + + def test_unknown_value_still_rejected(self): + assert _parse_api_mode("bogus") is None + + def test_non_string_still_rejected(self): + assert _parse_api_mode(None) is None + assert _parse_api_mode(42) is None