mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-15 14:22:43 +00:00
fix(agent): stop switch_model from pairing new provider with stale base_url
switch_model() unconditionally set agent.provider but only set agent.base_url when the resolved value was truthy. When a real provider change resolved an empty base_url (e.g. minimax after copilot), the agent ended up with provider="minimax" but base_url still pointing at api.githubcopilot.com. That incoherent pair then got snapshotted into agent._primary_runtime, so it kept re-applying on every subsequent turn via restore_primary_runtime() until the process restarted. try_activate_fallback() and _swap_credential() were audited and confirmed unaffected: both always derive base_url from an actually constructed client, never from a possibly-empty resolver hint. Fix: when base_url is empty AND the provider is genuinely changing, raise ValueError instead of silently keeping the old provider's URL. This routes through switch_model()'s existing snapshot/rollback path, and callers (tui_gateway/server.py's _apply_model_switch) already catch and surface a clean "switch failed, staying on X" message. Re-selecting the SAME provider with an empty base_url (credential-only refresh) still keeps the current URL, unchanged. Fixes #47828 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
fe25806a6b
commit
a23d5073fb
2 changed files with 109 additions and 5 deletions
|
|
@ -1816,13 +1816,30 @@ def switch_model(agent, new_model, new_provider, api_key='', base_url='', api_mo
|
|||
# ── Swap core runtime fields ──
|
||||
agent.model = new_model
|
||||
agent.provider = new_provider
|
||||
# Use new base_url when provided; only fall back to current when the
|
||||
# new provider genuinely has no endpoint (e.g. native SDK providers).
|
||||
# Without this guard the old provider's URL (e.g. Ollama's localhost
|
||||
# address) would persist silently after switching to a cloud provider
|
||||
# that returns an empty base_url string.
|
||||
# Use the new base_url when provided. When it's empty AND the
|
||||
# provider is actually changing, do NOT fall back to the current
|
||||
# (old provider's) URL — that silently pairs the new provider label
|
||||
# with the previous provider's endpoint (e.g. new_provider=minimax
|
||||
# paired with the leftover api.githubcopilot.com URL), and every
|
||||
# request after the switch 400s at the wrong host. This mismatched
|
||||
# pair also gets snapshotted into _primary_runtime below, so it
|
||||
# keeps re-applying on every subsequent turn until a full restart.
|
||||
# Fail loud instead: the caller (model_switch.switch_model())
|
||||
# already resolves base_url for every real provider, so an empty
|
||||
# value here means resolution failed upstream, not that the
|
||||
# provider genuinely has none. Re-selecting the SAME provider with
|
||||
# an empty base_url (e.g. a credential-only refresh) is still fine
|
||||
# to keep the current URL. See #47828.
|
||||
old_norm_provider = (old_provider or "").strip().lower()
|
||||
new_norm_provider = (new_provider or "").strip().lower()
|
||||
if base_url:
|
||||
agent.base_url = base_url
|
||||
elif old_norm_provider != new_norm_provider:
|
||||
raise ValueError(
|
||||
f"switch_model: no base_url resolved for provider "
|
||||
f"'{new_provider}' (switching from '{old_provider}'); "
|
||||
"refusing to keep the previous provider's endpoint"
|
||||
)
|
||||
agent.api_mode = api_mode
|
||||
# Invalidate transport cache — new api_mode may need a different transport
|
||||
if hasattr(agent, "_transport_cache"):
|
||||
|
|
|
|||
87
tests/run_agent/test_switch_model_stale_base_url.py
Normal file
87
tests/run_agent/test_switch_model_stale_base_url.py
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
"""Regression tests for #47828: switch_model must not pair a new provider
|
||||
label with the previous provider's base_url when the resolver returns no
|
||||
new base_url for a genuine provider change.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from run_agent import AIAgent
|
||||
from agent.context_compressor import ContextCompressor
|
||||
|
||||
|
||||
def _make_agent_with_compressor(provider="copilot", base_url="https://api.githubcopilot.com") -> AIAgent:
|
||||
"""Build a minimal AIAgent with a context_compressor, skipping __init__."""
|
||||
agent = AIAgent.__new__(AIAgent)
|
||||
|
||||
agent.model = "claude-opus-4.8"
|
||||
agent.provider = provider
|
||||
agent.base_url = base_url
|
||||
agent.api_key = "sk-primary"
|
||||
agent.api_mode = "chat_completions"
|
||||
agent.client = MagicMock()
|
||||
agent.quiet_mode = True
|
||||
agent._config_context_length = None
|
||||
|
||||
compressor = ContextCompressor(
|
||||
model=agent.model,
|
||||
threshold_percent=0.50,
|
||||
base_url=base_url,
|
||||
api_key="sk-primary",
|
||||
provider=provider,
|
||||
quiet_mode=True,
|
||||
config_context_length=None,
|
||||
)
|
||||
agent.context_compressor = compressor
|
||||
agent._primary_runtime = {}
|
||||
|
||||
return agent
|
||||
|
||||
|
||||
@patch("agent.model_metadata.get_model_context_length", return_value=131_072)
|
||||
def test_switch_model_rejects_stale_base_url_on_provider_change(mock_ctx_len):
|
||||
"""A provider change with no resolved base_url must fail loud instead of
|
||||
silently keeping the previous provider's endpoint (#47828)."""
|
||||
agent = _make_agent_with_compressor(provider="copilot", base_url="https://api.githubcopilot.com")
|
||||
|
||||
with pytest.raises(ValueError, match="no base_url resolved"):
|
||||
agent.switch_model("MiniMax-M3", "custom:minimax", api_key="sk-minimax", base_url="")
|
||||
|
||||
# Rollback must leave the agent fully on the old (provider, base_url) pair —
|
||||
# not a mismatched new-model/old-endpoint hybrid.
|
||||
assert agent.provider == "copilot"
|
||||
assert agent.base_url == "https://api.githubcopilot.com"
|
||||
assert agent.model == "claude-opus-4.8"
|
||||
|
||||
|
||||
@patch("agent.model_metadata.get_model_context_length", return_value=131_072)
|
||||
def test_switch_model_allows_empty_base_url_for_same_provider(mock_ctx_len):
|
||||
"""Re-selecting the SAME provider (e.g. a credential-only refresh) with no
|
||||
new base_url must keep the current URL — this is not a provider change."""
|
||||
agent = _make_agent_with_compressor(provider="openrouter", base_url="https://openrouter.ai/api/v1")
|
||||
|
||||
agent.switch_model("new-model", "openrouter", api_key="sk-new", base_url="")
|
||||
|
||||
assert agent.provider == "openrouter"
|
||||
assert agent.base_url == "https://openrouter.ai/api/v1"
|
||||
assert agent.model == "new-model"
|
||||
|
||||
|
||||
@patch("agent.model_metadata.get_model_context_length", return_value=131_072)
|
||||
def test_switch_model_applies_new_base_url_on_provider_change(mock_ctx_len):
|
||||
"""The normal, resolved-correctly path must still work: new provider +
|
||||
new base_url is applied as-is."""
|
||||
agent = _make_agent_with_compressor(provider="copilot", base_url="https://api.githubcopilot.com")
|
||||
|
||||
agent.switch_model(
|
||||
"MiniMax-M3", "custom:minimax", api_key="sk-minimax", base_url="https://api.minimax.io/v1"
|
||||
)
|
||||
|
||||
assert agent.provider == "custom:minimax"
|
||||
assert agent.base_url == "https://api.minimax.io/v1"
|
||||
assert agent.model == "MiniMax-M3"
|
||||
# _primary_runtime must snapshot the coherent pair so it survives every
|
||||
# subsequent restore_primary_runtime() call across turns.
|
||||
assert agent._primary_runtime["provider"] == "custom:minimax"
|
||||
assert agent._primary_runtime["base_url"] == "https://api.minimax.io/v1"
|
||||
Loading…
Add table
Add a link
Reference in a new issue