diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index fb9a348c8149..d0071c7489ad 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -30,6 +30,7 @@ from hermes_cli.providers import ( custom_provider_slug, determine_api_mode, get_label, + host_mandated_api_mode, is_aggregator, resolve_provider_full, ) @@ -1255,6 +1256,21 @@ def switch_model( if not api_key: api_key = "no-key-required" + # --- Resolve api_mode from the final (provider, base_url) before validation --- + # Two cases this closes, both surfaced when the switched model's reasoning + # is actually applied (post the reasoning-unification refactor): + # 1. api_mode empty (e.g. alias cleared it above) → fill from the endpoint. + # 2. api_mode carried a STALE value from the previous session state + # (e.g. a same-provider /model switch to gpt-5.x on api.openai.com that + # kept the prior openrouter/chat_completions mode). A host that mandates + # one wire protocol must override the stale value — otherwise the request + # goes out on chat_completions and OpenAI 400s on tools+reasoning_effort. + _mandated_mode = host_mandated_api_mode(base_url) + if _mandated_mode is not None: + api_mode = _mandated_mode + elif not api_mode: + api_mode = determine_api_mode(target_provider, base_url) + # --- Normalize model name for target provider --- new_model = normalize_model_for_provider(new_model, target_provider) diff --git a/hermes_cli/providers.py b/hermes_cli/providers.py index 2880b33fc4d0..c3e8efd22290 100644 --- a/hermes_cli/providers.py +++ b/hermes_cli/providers.py @@ -549,45 +549,61 @@ def is_routing_aggregator(provider: str) -> bool: return is_aggregator(provider_norm) +def host_mandated_api_mode(base_url: str = "") -> Optional[str]: + """Return the wire protocol a specific endpoint *requires*, or None. + + Some hosts only accept one API mode and reject the others outright: + - api.openai.com only accepts the Responses API for its (reasoning) + models when tools + reasoning are in play (chat/completions 400s). + - api.anthropic.com / ``…/anthropic`` suffixes speak native Messages. + - Kimi's ``/coding`` endpoint speaks native Messages. + - AWS Bedrock runtime hosts speak Converse. + + These are *mandatory* — a session carrying a stale api_mode (e.g. a + /model switch that kept the previous provider's ``chat_completions``) + must be overridden to the host's required mode, not merely filled in + when empty. Generic / unknown endpoints return None so an explicitly + configured api_mode on them is never clobbered. + """ + if not base_url: + return None + url_lower = base_url.rstrip("/").lower() + hostname = base_url_hostname(base_url) + # Exact-hostname matching only — never bare substring — so lookalike hosts + # (api.openai.com.attacker.test) and path-segment spoofs + # (proxy.test/api.openai.com/v1) are NOT treated as the real endpoint. (#32243) + if hostname == "api.kimi.com" and "/coding" in url_lower: + return "anthropic_messages" + if hostname == "api.anthropic.com" or url_lower.endswith("/anthropic"): + return "anthropic_messages" + if hostname == "api.openai.com": + return "codex_responses" + if hostname.startswith("bedrock-runtime.") and base_url_host_matches(base_url, "amazonaws.com"): + return "bedrock_converse" + return None + + def determine_api_mode(provider: str, base_url: str = "") -> str: """Determine the API mode (wire protocol) for a provider/endpoint. Resolution order: - 1. Known provider → transport → TRANSPORT_TO_API_MODE. - 2. URL heuristics for unknown / custom providers. - 3. Default: 'chat_completions'. + 1. Host-mandated mode (special endpoints that only accept one protocol). + 2. Known provider → transport → TRANSPORT_TO_API_MODE. + 3. Direct provider checks (bedrock). + 4. Default: 'chat_completions'. """ + mandated = host_mandated_api_mode(base_url) + if mandated is not None: + return mandated + pdef = get_provider(provider) if pdef is not None: - # Even for known providers, check URL heuristics for special endpoints - # (e.g. kimi /coding endpoint needs anthropic_messages even on 'custom') - if base_url: - url_lower = base_url.rstrip("/").lower() - if "api.kimi.com/coding" in url_lower: - return "anthropic_messages" - if url_lower.endswith("/anthropic") or "api.anthropic.com" in url_lower: - return "anthropic_messages" - if "api.openai.com" in url_lower: - return "codex_responses" return TRANSPORT_TO_API_MODE.get(pdef.transport, "chat_completions") # Direct provider checks for providers not in HERMES_OVERLAYS if provider == "bedrock": return "bedrock_converse" - # URL-based heuristics for custom / unknown providers - if base_url: - url_lower = base_url.rstrip("/").lower() - hostname = base_url_hostname(base_url) - if url_lower.endswith("/anthropic") or hostname == "api.anthropic.com": - return "anthropic_messages" - if hostname == "api.kimi.com" and "/coding" in url_lower: - return "anthropic_messages" - if hostname == "api.openai.com": - return "codex_responses" - if hostname.startswith("bedrock-runtime.") and base_url_host_matches(base_url, "amazonaws.com"): - return "bedrock_converse" - return "chat_completions" diff --git a/tests/hermes_cli/test_model_switch_openai_api_mode.py b/tests/hermes_cli/test_model_switch_openai_api_mode.py new file mode 100644 index 000000000000..e8eb4004ac7d --- /dev/null +++ b/tests/hermes_cli/test_model_switch_openai_api_mode.py @@ -0,0 +1,113 @@ +"""Regression tests for OpenAI-direct api_mode recomputation during /model switch. + +api.openai.com only accepts the Responses API (codex_responses) for its +reasoning models when tools + reasoning are in play — /v1/chat/completions +returns HTTP 400 ("Function tools with reasoning_effort are not supported"). + +When a session switches to a GPT-5.x model on api.openai.com while carrying a +stale ``chat_completions`` api_mode from the previous provider (e.g. an +openrouter default), the switch must override the stale value with the +host-mandated ``codex_responses``. Filling only when empty (the earlier fix) +was insufficient: the carried value was a *non-empty but wrong* mode. + +This surfaced after the reasoning-unification refactor made the switched +model's reasoning effort actually apply, turning a silently-dropped +reasoning_effort into a live one that OpenAI 400s on the chat_completions path. +""" + +from unittest.mock import patch + +from hermes_cli.model_switch import switch_model + +_MOCK_VALIDATION = { + "accepted": True, + "persist": True, + "recognized": True, + "message": None, +} + + +def _run_openai_switch( + raw_input: str, + current_provider: str = "openrouter", + current_model: str = "anthropic/claude-opus-4.8", + explicit_provider: str = "openai-api", + runtime_api_mode: str = "chat_completions", + runtime_base_url: str = "https://api.openai.com/v1", +): + """Run switch_model with OpenAI-direct mocks and return the result.""" + with ( + patch("hermes_cli.model_switch.resolve_alias", return_value=None), + patch("hermes_cli.model_switch.list_provider_models", return_value=[]), + patch( + "hermes_cli.runtime_provider.resolve_runtime_provider", + return_value={ + "api_key": "sk-test", + "base_url": runtime_base_url, + "api_mode": runtime_api_mode, + }, + ), + patch( + "hermes_cli.models.validate_requested_model", + return_value=_MOCK_VALIDATION, + ), + patch("hermes_cli.model_switch.get_model_info", return_value=None), + patch("hermes_cli.model_switch.get_model_capabilities", return_value=None), + patch("hermes_cli.models.detect_provider_for_model", return_value=None), + ): + return switch_model( + raw_input=raw_input, + current_provider=current_provider, + current_model=current_model, + explicit_provider=explicit_provider, + ) + + +def test_stale_chat_completions_overridden_on_openai_direct(): + """The incident: stale chat_completions on api.openai.com → codex_responses. + + Switching from an openrouter/chat_completions session to gpt-5.6-sol on + api.openai.com must flip the wire protocol to the Responses API so tools + + reasoning are preserved, instead of 400ing on chat/completions. + """ + result = _run_openai_switch( + raw_input="gpt-5.6-sol", + current_provider="openrouter", + current_model="anthropic/claude-opus-4.8", + explicit_provider="openai-api", + runtime_api_mode="chat_completions", # stale value carried over + ) + + assert result.success, f"switch_model failed: {result.error_message}" + assert result.target_provider == "openai-api" + assert result.new_model == "gpt-5.6-sol" + assert result.api_mode == "codex_responses" + + +def test_empty_api_mode_filled_on_openai_direct(): + """An empty runtime api_mode on api.openai.com resolves to codex_responses.""" + result = _run_openai_switch( + raw_input="gpt-5.6-sol", + runtime_api_mode="", # empty — the earlier fill-if-empty subcase + ) + + assert result.success, f"switch_model failed: {result.error_message}" + assert result.api_mode == "codex_responses" + + +def test_generic_endpoint_keeps_explicit_api_mode(): + """A generic (non-host-mandated) endpoint must NOT have its api_mode clobbered. + + Only hosts that mandate one wire protocol override a carried value; a + generic OpenAI-compatible relay returns None from host_mandated_api_mode, + so the switch path leaves the resolver's api_mode untouched. + """ + from hermes_cli.providers import host_mandated_api_mode + + assert host_mandated_api_mode("https://generic.example.com/v1") is None + # Lookalike / path-spoof hosts must also NOT be treated as mandated (#32243). + assert host_mandated_api_mode("https://api.openai.com.attacker.test/v1") is None + assert host_mandated_api_mode("https://proxy.test/api.openai.com/v1") is None + # The real endpoints are mandated. + assert host_mandated_api_mode("https://api.openai.com/v1") == "codex_responses" + assert host_mandated_api_mode("https://api.anthropic.com") == "anthropic_messages"