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.
This commit is contained in:
+37
-1
@@ -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():
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user