mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-21 16:18:55 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(cli,gateway): sync base_url/api_mode on global model switch persist

Browse source 
Same bug family as #47828, at the config-persistence layer instead of
the in-memory agent layer:

- cli.py (#25106): the --global /model handlers (both the typed-name
  path in _handle_model_switch and the picker path in
  _apply_model_switch_result) wrote model.default/model.provider to
  config.yaml but never touched base_url/api_mode at all. A provider
  switch left the OLD endpoint on disk; the next launch reconnected to
  the previous provider's host under the new model name.

- gateway/slash_commands.py (#25107): both persist-global blocks (the
  picker-tap callback and the typed /model --global path) guarded the
  write with two INDEPENDENT ifs — `if result.base_url: ...` and
  `if target_provider != "custom": clear_model_endpoint_credentials(...)`.
  For named providers the second if always cleared stale values, masking
  the bug. For a custom provider with an empty resolved base_url, neither
  branch fired, so the previous custom endpoint's base_url/api_key/
  api_mode survived untouched in config.yaml.

Fix: explicit set-if-truthy/clear-if-falsy for base_url and api_mode at
all four call sites, matching the already-correct pattern in
tui_gateway/server.py:_persist_model_switch (fixed for #48305).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
joaomarcos 2026-07-08 11:52:12 -03:00 • committed by Teknium
parent c7aa01ff06
commit 398634eeda
4 changed files with 353 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

12
cli.py
View file

@ -7979,6 +7979,14 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
save_config_value("model.default", result.new_model)
if result.provider_changed:
save_config_value("model.provider", result.target_provider)
# base_url/api_mode were previously never persisted here, so a
# global switch left the OLD provider's endpoint/wire-protocol in
# config.yaml. result.base_url/api_mode are always freshly
# resolved for the target provider (see model_switch.py), so sync
# them every time; None clears a value the new provider doesn't
# need (#25106).
save_config_value("model.base_url", result.base_url or None)
save_config_value("model.api_mode", result.api_mode or None)
_cprint(" Saved to config.yaml (--global)")
else:
_cprint(" (session only — add --global to persist)")
@ -8291,6 +8299,10 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
save_config_value("model.default", result.new_model)
if result.provider_changed:
save_config_value("model.provider", result.target_provider)
# See _apply_model_switch_result above for why base_url/api_mode
# must be synced on every global switch (#25106).
save_config_value("model.base_url", result.base_url or None)
save_config_value("model.api_mode", result.api_mode or None)
_cprint(" Saved to config.yaml")
else:
_cprint(" (session only — add --global to persist)")

29
gateway/slash_commands.py
View file

@ -1723,9 +1723,24 @@ class GatewaySlashCommandsMixin:
_persist_cfg["model"] = _persist_model_cfg
_persist_model_cfg["default"] = result.new_model
_persist_model_cfg["provider"] = result.target_provider
# Named providers always resolve base_url/api_mode fresh,
# so any leftover is cleared unconditionally below. Custom
# providers have no registry entry to re-derive from, so
# they need an explicit set-or-clear here — the previous
# lone `if result.base_url:` left a stale base_url behind
# when switching to a custom provider whose resolver
# returned an empty base_url (#25107).
_is_custom_target = str(result.target_provider or "").strip().lower() == "custom"
if result.base_url:
_persist_model_cfg["base_url"] = result.base_url
if str(result.target_provider or "").strip().lower() != "custom":
elif _is_custom_target:
_persist_model_cfg.pop("base_url", None)
if _is_custom_target:
if result.api_mode:
_persist_model_cfg["api_mode"] = result.api_mode
else:
_persist_model_cfg.pop("api_mode", None)
else:
clear_model_endpoint_credentials(_persist_model_cfg, clear_base_url=True)
from hermes_cli.config import save_config
save_config(_persist_cfg)
@ -1988,9 +2003,19 @@ class GatewaySlashCommandsMixin:
cfg["model"] = model_cfg
model_cfg["default"] = result.new_model
model_cfg["provider"] = result.target_provider
# See the picker handler above for why custom providers need an
# explicit set-or-clear instead of the old lone truthy check (#25107).
_is_custom_target = str(result.target_provider or "").strip().lower() == "custom"
if result.base_url:
model_cfg["base_url"] = result.base_url
if str(result.target_provider or "").strip().lower() != "custom":
elif _is_custom_target:
model_cfg.pop("base_url", None)
if _is_custom_target:
if result.api_mode:
model_cfg["api_mode"] = result.api_mode
else:
model_cfg.pop("api_mode", None)
else:
clear_model_endpoint_credentials(model_cfg, clear_base_url=True)
from hermes_cli.config import save_config
save_config(cfg)

194
tests/gateway/test_25107_stale_base_url_api_mode.py Normal file
View file

@ -0,0 +1,194 @@
"""Regression tests for #25107: gateway /model switch left a stale
``base_url``/never persisted ``api_mode`` in config.yaml when switching to a
custom provider whose resolver returned an empty ``base_url``.
Root cause: both the picker-tap path (``_on_model_selected`` in
``gateway/slash_commands.py``) and the typed ``/model X --global`` path
(``_finish_switch``) guarded the persist block with two *independent*
``if``s:
if result.base_url:
model_cfg["base_url"] = result.base_url
if target_provider != "custom":
clear_model_endpoint_credentials(model_cfg, clear_base_url=True)
For a NAMED provider the second ``if`` always clears any stale value, so the
bug was invisible there. But for a ``custom`` provider with an empty
resolved ``base_url``, NEITHER branch fires: the old base_url survives
untouched, and ``api_mode`` was never written to ``model_cfg`` at all in
either branch (only present in the in-memory session override).
Fix: an explicit set-or-clear for both fields when the target provider is
custom, so a genuine switch always leaves config.yaml coherent with the
resolved result.
"""
import types
import yaml
import pytest
from gateway.config import Platform
from gateway.platforms.base import MessageEvent, MessageType
from gateway.run import GatewayRunner
from gateway.session import SessionSource
class _FakePickerAdapter:
def __init__(self):
self.captured_callback = None
async def send_model_picker(self, *, on_model_selected, **kwargs):
self.captured_callback = on_model_selected
return types.SimpleNamespace(success=True)
def _make_runner(adapter=None):
runner = object.__new__(GatewayRunner)
runner.adapters = {Platform.TELEGRAM: adapter} if adapter else {}
runner._voice_mode = {}
runner._session_model_overrides = {}
runner._running_agents = {}
return runner
def _make_event(text):
return MessageEvent(
text=text,
message_type=MessageType.TEXT,
source=SessionSource(platform=Platform.TELEGRAM, chat_id="12345", chat_type="dm"),
)
def _fake_switch_result(*, base_url="", api_mode=""):
from hermes_cli.model_switch import ModelSwitchResult
return ModelSwitchResult(
success=True,
new_model="local-llama",
target_provider="custom",
provider_changed=True,
api_key="sk-local",
base_url=base_url,
api_mode=api_mode,
provider_label="Custom",
is_global=True,
)
def _setup_isolated_home(tmp_path, monkeypatch, model_yaml_value, *, base_url="", api_mode=""):
import gateway.run as gateway_run
hermes_home = tmp_path / ".hermes"
hermes_home.mkdir()
cfg_path = hermes_home / "config.yaml"
cfg_path.write_text(
yaml.safe_dump({"model": model_yaml_value, "providers": {}}),
encoding="utf-8",
)
monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home)
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
monkeypatch.setattr(
"hermes_cli.model_switch.list_picker_providers",
lambda **kw: [{"slug": "custom", "name": "Custom", "models": ["local-llama"]}],
)
monkeypatch.setattr(
"hermes_cli.model_switch.switch_model",
lambda **kw: _fake_switch_result(base_url=base_url, api_mode=api_mode),
)
monkeypatch.setattr(
"hermes_cli.model_switch.resolve_display_context_length",
lambda *a, **k: 8192,
)
monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home)
monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: hermes_home)
return cfg_path
_STALE_MODEL_CFG = {
"default": "old-custom-model",
"provider": "custom",
"base_url": "https://old-stale-endpoint.example/v1",
"api_key": "sk-stale",
"api_mode": "anthropic_messages",
}
# ---------------------------------------------------------------------------
# Typed `/model X --global` path (_finish_switch)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_typed_switch_to_custom_clears_stale_base_url_and_api_mode(tmp_path, monkeypatch):
"""Switching to a custom provider whose resolver returns no base_url/
api_mode must clear the previous custom endpoint's leftovers, not keep
routing at the old host/protocol (#25107)."""
cfg_path = _setup_isolated_home(tmp_path, monkeypatch, dict(_STALE_MODEL_CFG))
result = await _make_runner()._handle_model_command(
_make_event("/model local-llama --provider custom --global")
)
assert result is not None
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
assert written["model"]["default"] == "local-llama"
assert "base_url" not in written["model"], (
"stale base_url from the old custom endpoint must be cleared"
)
assert "api_mode" not in written["model"], (
"stale api_mode from the old custom endpoint must be cleared"
)
@pytest.mark.asyncio
async def test_typed_switch_to_custom_persists_resolved_base_url_and_api_mode(tmp_path, monkeypatch):
"""The normal case: a custom-provider switch that DOES resolve a fresh
base_url/api_mode must persist both (api_mode was never written here
before the fix)."""
cfg_path = _setup_isolated_home(
tmp_path,
monkeypatch,
dict(_STALE_MODEL_CFG),
base_url="https://new-endpoint.example/v1",
api_mode="anthropic_messages",
)
result = await _make_runner()._handle_model_command(
_make_event("/model local-llama --provider custom --global")
)
assert result is not None
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
assert written["model"]["base_url"] == "https://new-endpoint.example/v1"
assert written["model"]["api_mode"] == "anthropic_messages"
# ---------------------------------------------------------------------------
# Picker-tap path (_on_model_selected)
# ---------------------------------------------------------------------------
async def _drive_picker(runner, event):
sent = await runner._handle_model_command(event)
assert sent is None
adapter = runner.adapters[Platform.TELEGRAM]
assert adapter.captured_callback is not None, "picker callback was not wired"
return await adapter.captured_callback("12345", "local-llama", "custom")
@pytest.mark.asyncio
async def test_picker_tap_to_custom_clears_stale_base_url_and_api_mode(tmp_path, monkeypatch):
"""Same bug, picker-tap path: tapping a custom-provider model with an
empty resolved base_url must clear the previous custom endpoint (#25107).
"""
adapter = _FakePickerAdapter()
cfg_path = _setup_isolated_home(tmp_path, monkeypatch, dict(_STALE_MODEL_CFG))
confirmation = await _drive_picker(_make_runner(adapter), _make_event("/model"))
assert confirmation is not None
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
assert "base_url" not in written["model"]
assert "api_mode" not in written["model"]

120
tests/hermes_cli/test_25106_global_switch_persists_base_url_api_mode.py Normal file
View file

@ -0,0 +1,120 @@
"""Regression tests for #25106: CLI `/model <name> --global` never persisted
``model.base_url``/``model.api_mode`` to config.yaml, so a global provider
switch left the PREVIOUS provider's endpoint/wire-protocol on disk. The next
`hermes` launch re-read the stale base_url and routed the new model at the
old host.
Both ``_handle_model_switch`` (typed ``/model <name>``) and
``_apply_model_switch_result`` (interactive picker) shared the same gap: the
persistence block wrote ``model.default``/``model.provider`` but never
touched ``base_url``/``api_mode`` at all. Fix: sync both on every global
switch, clearing to ``None`` when the resolved result doesn't need them —
mirroring the already-correct ``tui_gateway/server.py:_persist_model_switch``
pattern (fixed for #48305).
"""
from unittest.mock import MagicMock, patch
import pytest
from hermes_cli.model_switch import ModelSwitchResult
def _make_result(*, base_url="https://api.minimax.io/v1", api_mode="chat_completions", provider_changed=True):
return ModelSwitchResult(
success=True,
new_model="MiniMax-M3",
target_provider="custom:minimax",
provider_changed=provider_changed,
api_key="sk-minimax",
base_url=base_url,
api_mode=api_mode,
warning_message="",
provider_label="MiniMax (custom)",
resolved_via_alias=False,
capabilities=None,
model_info=None,
is_global=True,
)
class _StubCLI:
"""Minimum attrs/methods `_handle_model_switch` reads or calls on self."""
agent = None
model = "old-model"
provider = "copilot"
requested_provider = "copilot"
api_key = "sk-old"
base_url = "https://api.githubcopilot.com"
api_mode = "chat_completions"
_explicit_api_key = ""
_explicit_base_url = ""
conversation_history = []
_pending_model_switch_note = ""
def _confirm_expensive_model_switch(self, result) -> bool:
return True
def _open_model_picker(self, *a, **k):
raise AssertionError("picker should not open when a model name is given")
def _run_switch(monkeypatch, result, cmd="/model MiniMax-M3 --global"):
import cli as cli_mod
monkeypatch.setattr(cli_mod, "_cprint", lambda *a, **k: None)
saved: dict[str, object] = {}
def _fake_save(key, value):
saved[key] = value
monkeypatch.setattr(cli_mod, "save_config_value", _fake_save)
monkeypatch.setattr("hermes_cli.model_switch.switch_model", lambda **kw: result)
monkeypatch.setattr(
"hermes_cli.inventory.load_picker_context",
lambda: (_ for _ in ()).throw(RuntimeError("no picker context in test")),
)
cli_mod.HermesCLI._handle_model_switch(_StubCLI(), cmd)
return saved
def test_global_switch_persists_base_url_and_api_mode(monkeypatch):
"""The core #25106 fix: a --global switch to a new provider/endpoint must
write the newly resolved base_url and api_mode, not just default/provider."""
saved = _run_switch(monkeypatch, _make_result())
assert saved["model.default"] == "MiniMax-M3"
assert saved["model.provider"] == "custom:minimax"
assert saved["model.base_url"] == "https://api.minimax.io/v1"
assert saved["model.api_mode"] == "chat_completions"
def test_global_switch_clears_base_url_and_api_mode_when_unresolved(monkeypatch):
"""When the resolver returns no base_url/api_mode for the new provider
(e.g. a named provider needing neither), any previous value must be
cleared (None) rather than silently left in config.yaml."""
result = _make_result(base_url="", api_mode="")
saved = _run_switch(monkeypatch, result)
assert saved["model.base_url"] is None
assert saved["model.api_mode"] is None
def test_session_only_switch_does_not_touch_config(monkeypatch):
"""--session must not call save_config_value at all — persistence stays
entirely in-memory."""
import cli as cli_mod
monkeypatch.setattr(cli_mod, "_cprint", lambda *a, **k: None)
save_calls = []
monkeypatch.setattr(cli_mod, "save_config_value", lambda *a, **k: save_calls.append(a))
monkeypatch.setattr("hermes_cli.model_switch.switch_model", lambda **kw: _make_result())
monkeypatch.setattr(
"hermes_cli.inventory.load_picker_context",
lambda: (_ for _ in ()).throw(RuntimeError("no picker context in test")),
)
cli_mod.HermesCLI._handle_model_switch(_StubCLI(), "/model MiniMax-M3 --session")
assert save_calls == []