Files
hermes-agent/tests/tui_gateway/test_make_agent_provider.py
T
teknium1 11576390fe refactor(model): one persist writer for /model across CLI, gateway, TUI, dashboard; ACP + dashboard validate through switch_model
One `/model --global` produced four config.yaml shapes. CLI wrote
default/provider/base_url/api_mode and cleared the context pin on a route
change; the gateway rewrote the whole `model:` block (whole-file save_config)
and only set api_mode for `custom`; the TUI wrote three keys and never
touched api_mode, so a switch off an Anthropic-wire endpoint left a stale
`api_mode: anthropic_messages` in config; the dashboard main slot had its own
switched-provider logic, wrote `base_url: ""` and always dropped
context_length. ACP `session/set_model` and `POST /api/model/set` accepted
any model string (parse_model_input + detect_provider_for_model) so a model
no catalog knows, or a provider with no credentials, was handed to the
session / persisted and only failed at inference time.

Canonical: `hermes_cli.model_switch.model_selection_config_updates` (the
shape) + `persist_model_selection(result, config_path=None)` (targeted
per-key `atomic_roundtrip_yaml_update` writes, so sibling
`model_slots`/`model_fallback` keys survive; explicit path for the
multiplexed gateway's profile config) + `apply_model_selection` (same shape
applied to an in-memory `model:` dict for callers that save a whole
document). `atomic_roundtrip_yaml_update(value=None)` now REMOVES the key
instead of writing `key: null`, so per-key and whole-document writers land
the same file. Shape = CLI/gateway semantics: default, provider, base_url
(cleared when the target has none), api_mode (cleared when unresolved),
context_length cleared only when `should_clear_context_pin` says the route
identity changed, inline api_key/api cleared for non-custom targets.

Sites -> canonical:
  hermes_cli/cli_model_switch_mixin.py::_persist_global_switch          -> deleted; _commit_model_switch calls persist_model_selection
  hermes_cli/cli_model_switch_mixin.py::_clear_persisted_context_for_model_switch -> deleted (folded into the shape)
  gateway/slash_commands_model.py::_persist_model_switch_to_config       -> to_thread forwarder: persist_model_selection(result, ctx.config_path)
  tui_gateway/model_switch.py::_persist_model_switch                     -> deleted; _apply_model_switch calls persist_model_selection
  hermes_cli/web_server_config.py::_apply_main_model_assignment          -> apply_model_selection(result) (+ explicit custom api_key)
  hermes_cli/web_server_config.py::_validated_main_model_selection       -> NEW: switch_model(--provider) gate; rejection -> HTTP 400
  hermes_cli/web_routers/{models,profiles,config_env}.py main-slot paths -> through _validated_main_model_selection
  acp_adapter/server.py::_resolve_model_selection                        -> deleted; _switch_model calls switch_model (provider:model -> --provider), rejection -> ValueError

Behavior changes: TUI --global now writes/clears model.api_mode and clears a
route-changed context pin; gateway --global no longer rewrites the whole
model block (sibling keys survive) and clears api_mode for every target;
dashboard main slot / profile-create model / custom-endpoint activate now
reject unknown/uncredentialed/unlisted models (HTTP 400) and persist the
resolved base_url/api_mode instead of `base_url: ""`; ACP rejects the same
(ValueError surfaced by the command/protocol handler). Gateway persist runs
on a worker thread against the routed profile's config_path (multiplex-safe).
Cleared keys are removed from config.yaml rather than left as `null`. ACP
still never persists.

Kept `_normalize_main_model_assignment`: switch_model rejects a vendor name
posing as a provider (`moonshotai` -> "Unknown provider"), so the
vendor->aggregator repair is not a duplicate; E2E verified both branches.
No config migration: readers already coalesce `base_url: ""` to absent
(`_config_base_url_for_provider`) and gate api_mode on provider match
(`_provider_supports_explicit_api_mode`), so no stale-shape reader bug.

Tests: tests/hermes_cli/test_model_persist_one_shape.py (four surfaces land
one block; same-route re-pick keeps the pin), tests/acp_adapter/
test_acp_dashboard_model_switch_validation.py (rejection + explicit
provider prefix). Replaces test_acp_set_model_explicit_provider.py and the
two TUI-only persist tests; tests that intercepted the old per-surface seams
(`cli.save_config_value`, `load_config_readonly`, `tui_gateway.server.
_persist_model_switch`) now intercept the canonical seam. Each fix
sabotage-verified red.
2026-09-13 05:21:02 -07:00

143 lines
5.5 KiB
Python

"""Regression test for #11884: _make_agent must resolve runtime provider.
Without resolve_runtime_provider(), bare-slug models in config
(e.g. ``claude-opus-4-6`` with ``model.provider: anthropic``) leave
provider/base_url/api_key empty in AIAgent, causing HTTP 404.
"""
import os
from unittest.mock import MagicMock, patch
def test_make_agent_passes_resolved_provider():
"""_make_agent forwards provider/base_url/api_key/api_mode from
resolve_runtime_provider to AIAgent."""
fake_runtime = {
"provider": "anthropic",
"base_url": "https://api.anthropic.com",
"api_key": "sk-test-key",
"api_mode": "anthropic_messages",
"command": None,
"args": None,
"credential_pool": None,
}
fake_cfg = {
"model": {"default": "claude-opus-4-6", "provider": "anthropic"},
"agent": {"system_prompt": "test"},
}
with (
patch("tui_gateway.server._load_cfg", return_value=fake_cfg),
patch("tui_gateway.server._get_db", return_value=MagicMock()),
patch("tui_gateway.server._load_tool_progress_mode", return_value="compact"),
patch("tui_gateway.server._load_reasoning_config", return_value=None),
patch("tui_gateway.server._load_service_tier", return_value=None),
patch("tui_gateway.server._load_enabled_toolsets", return_value=None),
patch(
"hermes_cli.runtime_provider.resolve_runtime_provider",
return_value=fake_runtime,
) as mock_resolve,
patch("run_agent.AIAgent") as mock_agent,
):
from tui_gateway.server import _make_agent
_make_agent("sid-1", "key-1")
# target_model comes from _resolve_startup_runtime() which reads
# _load_cfg(). Due to module-level caching in tui_gateway.server,
# the patched config may not take effect when the module was already
# imported by an earlier test. Assert the stable part of the call.
mock_resolve.assert_called_once()
assert mock_resolve.call_args.kwargs.get("requested") is None
call_kwargs = mock_agent.call_args
assert call_kwargs.kwargs["provider"] == "anthropic"
assert call_kwargs.kwargs["base_url"] == "https://api.anthropic.com"
assert call_kwargs.kwargs["api_key"] == "sk-test-key"
assert call_kwargs.kwargs["api_mode"] == "anthropic_messages"
def test_probe_config_health_flags_null_sections():
"""Bare YAML keys (`agent:` with no value) parse as None and silently
drop nested settings; probe must surface them so users can fix."""
from tui_gateway.server import _probe_config_health
assert _probe_config_health({"agent": {"x": 1}}) == ""
assert _probe_config_health({}) == ""
msg = _probe_config_health({"agent": None, "display": None, "model": {}})
assert "agent" in msg and "display" in msg
assert "model" not in msg
def test_apply_model_switch_does_not_leak_process_env():
"""Core fix for cross-session contamination: an in-session /model switch
must mutate only the target session (record a per-session override + switch
that session's agent in place) and must NOT write process-global env vars,
which the single-process desktop backend shares across every live session.
"""
from tui_gateway import server
class _FakeResult:
success = True
error_message = ""
warning_message = ""
new_model = "zai/glm-5.1"
target_provider = "zai"
base_url = "https://api.z.ai/v1"
api_key = "sk-glm"
api_mode = "chat_completions"
class _FakeAgent:
def __init__(self):
self.model = "minimax/m3"
self.provider = "minimax"
self.base_url = ""
self.api_key = ""
def switch_model(self, **kw):
self.model = kw["new_model"]
self.provider = kw["new_provider"]
env_keys = (
"HERMES_MODEL",
"HERMES_INFERENCE_MODEL",
"HERMES_TUI_PROVIDER",
"HERMES_INFERENCE_PROVIDER",
)
sess_b = {"agent": _FakeAgent(), "session_key": "k-B", "model_override": None}
sess_a = {"agent": _FakeAgent(), "session_key": "k-A", "model_override": None}
with (
patch("hermes_cli.model_switch.parse_model_flags",
return_value=("glm-5.1", None, False, False, True)),
patch("hermes_cli.model_switch.resolve_persist_behavior",
return_value=False),
patch("hermes_cli.model_switch.switch_model", return_value=_FakeResult()),
patch("tui_gateway.server._emit"),
patch("tui_gateway.server._restart_slash_worker"),
patch("tui_gateway.server._session_info", return_value={}),
patch("hermes_cli.model_switch.persist_model_selection") as mock_persist,
):
before = {k: os.environ.get(k) for k in env_keys}
result = server._apply_model_switch("sidB", sess_b, "glm-5.1")
after = {k: os.environ.get(k) for k in env_keys}
assert result["value"] == "zai/glm-5.1"
# No process-global env mutation (the contamination vector).
assert before == after
# persist_global was False → config untouched.
mock_persist.assert_not_called()
# Target session recorded a per-session override.
assert sess_b["model_override"]["model"] == "zai/glm-5.1"
assert sess_b["model_override"]["provider"] == "zai"
# The switched agent mutated in place.
assert sess_b["agent"].model == "zai/glm-5.1"
# Sibling session is completely untouched.
assert sess_a["model_override"] is None
assert sess_a["agent"].model == "minimax/m3"