From f0b763c74feed699b2f75f4f237eb64fb8dfdd45 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 21 Apr 2026 12:23:17 -0500 Subject: [PATCH] fix(model-switch): drop stale provider from fallback chain and env after /model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported during the TUI v2 blitz test: switching from openrouter to anthropic via `/model --provider anthropic` appeared to succeed, but the next turn kept hitting openrouter — the provider the user was deliberately moving away from. Two gaps caused this: 1. `Agent.switch_model` reset `_fallback_activated` / `_fallback_index` but left `_fallback_chain` intact. The chain was seeded from `fallback_providers:` at agent init for the *original* primary, so when the new primary returned 401 (invalid/expired Anthropic key), `_try_activate_fallback()` picked the old provider back up without informing the user. Prune entries matching either the old primary (user is moving away) or the new primary (redundant) whenever the primary provider actually changes. 2. `_apply_model_switch` persisted `HERMES_MODEL` but never updated `HERMES_INFERENCE_PROVIDER`. Any ambient re-resolution of the runtime (credential pool refresh, compressor rebuild, aux clients) falls through to that env var in `resolve_requested_provider`, so it kept reporting the original provider even after an in-memory switch. Adds three regression tests: fallback-chain prune on primary change, no-op on same-provider model swap, and env-var sync on explicit switch. --- run_agent.py | 16 ++++ .../test_switch_model_fallback_prune.py | 93 +++++++++++++++++++ tests/test_tui_gateway_server.py | 43 +++++++++ tui_gateway/server.py | 6 ++ 4 files changed, 158 insertions(+) create mode 100644 tests/run_agent/test_switch_model_fallback_prune.py diff --git a/run_agent.py b/run_agent.py index c5881b87f..5c121a085 100644 --- a/run_agent.py +++ b/run_agent.py @@ -2005,6 +2005,22 @@ class AIAgent: self._fallback_activated = False self._fallback_index = 0 + # When the user deliberately swaps primary providers (e.g. openrouter + # → anthropic), drop any fallback entries that target the OLD primary + # or the NEW one. The chain was seeded from config at agent init for + # the original provider — without pruning, a failed turn on the new + # primary silently re-activates the provider the user just rejected, + # which is exactly what was reported during TUI v2 blitz testing + # ("switched to anthropic, tui keeps trying openrouter"). + old_norm = (old_provider or "").strip().lower() + new_norm = (new_provider or "").strip().lower() + if old_norm and new_norm and old_norm != new_norm: + self._fallback_chain = [ + entry for entry in self._fallback_chain + if (entry.get("provider") or "").strip().lower() not in {old_norm, new_norm} + ] + self._fallback_model = self._fallback_chain[0] if self._fallback_chain else None + logging.info( "Model switched in-place: %s (%s) -> %s (%s)", old_model, old_provider, new_model, new_provider, diff --git a/tests/run_agent/test_switch_model_fallback_prune.py b/tests/run_agent/test_switch_model_fallback_prune.py new file mode 100644 index 000000000..99af3579f --- /dev/null +++ b/tests/run_agent/test_switch_model_fallback_prune.py @@ -0,0 +1,93 @@ +"""Regression test for TUI v2 blitz bug: explicit /model --provider switch +silently fell back to the old primary provider on the next turn because the +fallback chain — seeded from config at agent __init__ — kept entries for the +provider the user just moved away from. + +Reported: "switched from openrouter provider to anthropic api key via hermes +model and the tui keeps trying openrouter". +""" + +from unittest.mock import MagicMock, patch + +from run_agent import AIAgent + + +def _make_agent(chain): + agent = AIAgent.__new__(AIAgent) + + agent.provider = "openrouter" + agent.model = "x-ai/grok-4" + agent.base_url = "https://openrouter.ai/api/v1" + agent.api_key = "or-key" + agent.api_mode = "chat_completions" + agent.client = MagicMock() + agent._client_kwargs = {"api_key": "or-key", "base_url": "https://openrouter.ai/api/v1"} + agent.context_compressor = None + agent._anthropic_api_key = "" + agent._anthropic_base_url = None + agent._anthropic_client = None + agent._is_anthropic_oauth = False + agent._cached_system_prompt = "cached" + agent._primary_runtime = {} + agent._fallback_activated = False + agent._fallback_index = 0 + agent._fallback_chain = list(chain) + agent._fallback_model = chain[0] if chain else None + + return agent + + +def _switch_to_anthropic(agent): + with ( + patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()), + patch("agent.anthropic_adapter.resolve_anthropic_token", return_value="sk-ant-xyz"), + patch("agent.anthropic_adapter._is_oauth_token", return_value=False), + patch("hermes_cli.timeouts.get_provider_request_timeout", return_value=None), + ): + agent.switch_model( + new_model="claude-sonnet-4-5", + new_provider="anthropic", + api_key="sk-ant-xyz", + base_url="https://api.anthropic.com", + api_mode="anthropic_messages", + ) + + +def test_switch_drops_old_primary_from_fallback_chain(): + agent = _make_agent([ + {"provider": "openrouter", "model": "x-ai/grok-4"}, + {"provider": "nous", "model": "hermes-4"}, + ]) + + _switch_to_anthropic(agent) + + providers = [entry["provider"] for entry in agent._fallback_chain] + + assert "openrouter" not in providers, "old primary must be pruned" + assert "anthropic" not in providers, "new primary is redundant in the chain" + assert providers == ["nous"] + assert agent._fallback_model == {"provider": "nous", "model": "hermes-4"} + + +def test_switch_with_empty_chain_stays_empty(): + agent = _make_agent([]) + + _switch_to_anthropic(agent) + + assert agent._fallback_chain == [] + assert agent._fallback_model is None + + +def test_switch_within_same_provider_preserves_chain(): + chain = [{"provider": "openrouter", "model": "x-ai/grok-4"}] + agent = _make_agent(chain) + + with patch("hermes_cli.timeouts.get_provider_request_timeout", return_value=None): + agent.switch_model( + new_model="openai/gpt-5", + new_provider="openrouter", + api_key="or-key", + base_url="https://openrouter.ai/api/v1", + ) + + assert agent._fallback_chain == chain diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 3909c3ed8..7a7f63284 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -1,4 +1,5 @@ import json +import os import sys import threading import time @@ -230,6 +231,48 @@ def test_config_set_model_global_persists(monkeypatch): assert saved["model"]["base_url"] == "https://api.anthropic.com" +def test_config_set_model_syncs_inference_provider_env(monkeypatch): + """After an explicit provider switch, HERMES_INFERENCE_PROVIDER must + reflect the user's choice so ambient re-resolution (credential pool + refresh, aux clients) picks up the new provider instead of the original + one persisted in config or shell env. + + Regression: a TUI user switched openrouter → anthropic and the TUI kept + trying openrouter because the env-var-backed resolvers still saw the old + provider. + """ + class _Agent: + provider = "openrouter" + model = "old/model" + base_url = "" + api_key = "sk-or" + + def switch_model(self, **_kwargs): + return None + + result = types.SimpleNamespace( + success=True, + new_model="claude-sonnet-4.6", + target_provider="anthropic", + api_key="sk-ant", + base_url="https://api.anthropic.com", + api_mode="anthropic_messages", + warning_message="", + ) + + server._sessions["sid"] = _session(agent=_Agent()) + monkeypatch.setenv("HERMES_INFERENCE_PROVIDER", "openrouter") + monkeypatch.setattr("hermes_cli.model_switch.switch_model", lambda **_kwargs: result) + monkeypatch.setattr(server, "_restart_slash_worker", lambda session: None) + monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None) + + server.handle_request( + {"id": "1", "method": "config.set", "params": {"session_id": "sid", "key": "model", "value": "claude-sonnet-4.6 --provider anthropic"}} + ) + + assert os.environ["HERMES_INFERENCE_PROVIDER"] == "anthropic" + + def test_config_set_personality_rejects_unknown_name(monkeypatch): monkeypatch.setattr(server, "_available_personalities", lambda cfg=None: {"helpful": "You are helpful."}) resp = server.handle_request( diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 20564af65..32886d3d3 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -529,6 +529,12 @@ def _apply_model_switch(sid: str, session: dict, raw_input: str) -> dict: _emit("session.info", sid, _session_info(agent)) os.environ["HERMES_MODEL"] = result.new_model + # Keep the process-level provider env var in sync with the user's explicit + # choice so any ambient re-resolution (credential pool refresh, compressor + # rebuild, aux clients) resolves to the new provider instead of the + # original one persisted in config or env. + if result.target_provider: + os.environ["HERMES_INFERENCE_PROVIDER"] = result.target_provider if persist_global: _persist_model_switch(result) return {"value": result.new_model, "warning": result.warning_message or ""}