fix(model-switch): override stale api_mode with host-mandated mode on OpenAI-direct switch
Switching to a GPT-5.x model on api.openai.com while the session carried a stale chat_completions api_mode (e.g. from a prior openrouter default) left the request on /v1/chat/completions, which 400s with "Function tools with reasoning_effort are not supported" once the switched model's reasoning is applied. switch_model() only re-derived api_mode inside the provider-changed branch, so a same-provider/carryover switch kept the wrong wire protocol. Add host_mandated_api_mode(base_url): the endpoints that accept exactly one protocol (api.openai.com -> codex_responses, api.anthropic.com / *…/anthropic* -> anthropic_messages, api.kimi.com /coding -> anthropic_messages, bedrock-runtime -> bedrock_converse), matched by EXACT hostname so lookalike hosts and path-segment spoofs are rejected (#32243). switch_model() now uses it to override a stale carried api_mode, not merely fill an empty one; determine_api_mode() shares the same helper. Credit sjiangtao2024 (#15880) for the recompute-before-validation approach; this strengthens it from fill-if-empty to a host-mandated override. Co-Authored-By: sjiangtao2024 <siage@139.com>
This commit is contained in:
parent
7498eae3f7
commit
c7aa01ff06
3 changed files with 171 additions and 26 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
||||
|
||||
|
|
|
|||
113
tests/hermes_cli/test_model_switch_openai_api_mode.py
Normal file
113
tests/hermes_cli/test_model_switch_openai_api_mode.py
Normal file
|
|
@ -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"
|
||||
Loading…
Add table
Add a link
Reference in a new issue