From a23d5073fbd2efa133f96fd60ad172799de062e7 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Wed, 8 Jul 2026 10:53:13 -0300 Subject: [PATCH] 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 --- agent/agent_runtime_helpers.py | 27 ++++-- .../test_switch_model_stale_base_url.py | 87 +++++++++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 tests/run_agent/test_switch_model_stale_base_url.py diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 3e5cd63536b..14c7c73321d 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -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"): diff --git a/tests/run_agent/test_switch_model_stale_base_url.py b/tests/run_agent/test_switch_model_stale_base_url.py new file mode 100644 index 00000000000..bd38eb6c498 --- /dev/null +++ b/tests/run_agent/test_switch_model_stale_base_url.py @@ -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"