diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index b48e6c36ca9..a74c7d2a322 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -1416,7 +1416,11 @@ def test_config_sync_switches_unpinned_session(monkeypatch): ( "sid", "new/model --provider nous", - {"confirm_expensive_model": True, "pin_session_override": False}, + { + "confirm_expensive_model": True, + "pin_session_override": False, + "persist_override": False, + }, ) ] assert session["config_model_seen"] == ("new/model", "nous") @@ -1536,6 +1540,78 @@ def test_config_sync_config_wins_over_env_seed(monkeypatch): assert session["config_model_seen"] == ("new/model", "") +def test_config_sync_ignores_env_seed_without_config_model(monkeypatch): + # `hermes --tui -m ` sets HERMES_MODEL/HERMES_INFERENCE_MODEL as a + # launch-scoped seed. When config.yaml has NO model.default (typical + # custom-provider-only setup), the sync must NOT adopt the env seed as a + # config target — doing so replayed the -m flag as a /model switch and + # (with persist_switch_by_default=True) wrote it into config.yaml + # permanently. + monkeypatch.setenv("HERMES_MODEL", "one-shot/model") + monkeypatch.setenv("HERMES_INFERENCE_MODEL", "one-shot/model") + monkeypatch.setattr( + server, "_load_cfg", lambda: {"model": {"provider": "custom:mylocal"}} + ) + session = _sync_test_session() + monkeypatch.setattr( + server, + "_apply_model_switch", + lambda *a, **k: pytest.fail("env seed must not trigger a config sync switch"), + ) + + server._sync_agent_model_with_config("sid", session) + + +def test_config_model_target_never_reads_env(monkeypatch): + monkeypatch.setenv("HERMES_MODEL", "seed/model") + monkeypatch.setenv("HERMES_INFERENCE_MODEL", "seed/model") + monkeypatch.setattr(server, "_load_cfg", lambda: {"model": {"provider": "nous"}}) + + assert server._config_model_target() == ("", "nous") + + +def test_apply_model_switch_persist_override_false_never_persists(monkeypatch): + # Internal callers (config sync, /moa one-shot + restore) pass + # persist_override=False; even with persist_switch_by_default=True the + # switch must not write config.yaml. + import types as _types + + result = _types.SimpleNamespace( + success=True, + new_model="new/model", + target_provider="nous", + base_url="", + api_key="key", + api_mode="chat_completions", + warning_message="", + model_info=None, + error_message="", + ) + monkeypatch.setattr( + "hermes_cli.model_switch.switch_model", lambda **kw: result + ) + monkeypatch.setattr( + "hermes_cli.model_switch.resolve_persist_behavior", + lambda *a: pytest.fail("persist_override must bypass resolve_persist_behavior"), + ) + monkeypatch.setattr( + server, "_persist_model_switch", + lambda _r: pytest.fail("persist_override=False must not persist"), + ) + monkeypatch.setattr( + "hermes_cli.model_cost_guard.expensive_model_warning", + lambda *a, **k: None, + ) + session = {"agent": None} + + out = server._apply_model_switch( + "sid", session, "new/model --provider nous", persist_override=False + ) + + assert out["value"] == "new/model" + assert session["model_override"]["model"] == "new/model" + + def test_startup_runtime_uses_tui_provider_env(monkeypatch): monkeypatch.setenv("HERMES_MODEL", "nous/hermes-test") monkeypatch.setenv("HERMES_TUI_PROVIDER", "nous") diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 7293aa70ff5..1b2156f4c20 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1994,14 +1994,14 @@ def _resolve_model() -> str: def _config_model_target() -> tuple[str, str]: - """(model, provider) currently selected by config (env as fallback). + """(model, provider) currently selected by config.yaml — and ONLY config. - config.yaml wins over HERMES_MODEL / HERMES_INFERENCE_MODEL here, the - reverse of `_resolve_model()`'s startup order. Those env vars are a - provision-time seed (hosted instances set HERMES_INFERENCE_MODEL in the - container env); if they outranked config.yaml, the per-turn sync would - stay pinned to the seed forever and dashboard/CLI model changes would - never reach an open chat — the exact bug this sync exists to fix. + Unlike `_resolve_model()`, this never reads HERMES_MODEL / + HERMES_INFERENCE_MODEL. Those env vars are a launch-scoped seed + (`hermes --tui -m `, hosted-instance provisioning); if they + fed the per-turn sync, the seed would be replayed as a /model switch + and persisted globally, or would pin the session so dashboard/CLI + model changes never reach an open chat. """ cfg_model = _load_cfg().get("model") model = "" @@ -2013,8 +2013,15 @@ def _config_model_target() -> tuple[str, str]: provider = "" elif isinstance(cfg_model, str): model = cfg_model.strip() - if not model: - model = _resolve_model() + # No fallback to _resolve_model() here: that reads HERMES_MODEL / + # HERMES_INFERENCE_MODEL, which `hermes --tui -m ` sets as a + # session-scoped seed for THIS launch. When config.yaml has no + # model.default (custom-provider-only setups), falling back to the env + # seed made the per-turn sync treat the -m flag as "the configured + # model" and replay it as a /model switch — which then persisted the + # one-shot flag into config.yaml globally (#-m leak). An empty model + # simply means "config expresses no preference": the sync is a no-op + # and the agent keeps whatever it was built with. return model, provider @@ -2660,6 +2667,7 @@ def _apply_model_switch( confirm_expensive_model: bool = False, pin_session_override: bool = True, parsed_flags: tuple[str, str, bool, bool, bool] | None = None, + persist_override: bool | None = None, ) -> dict: from hermes_cli.model_switch import ( parse_model_flags, @@ -2677,7 +2685,11 @@ def _apply_model_switch( _force_refresh, is_session, ) = parsed_flags - persist_global = resolve_persist_behavior(is_global_flag, is_session) + persist_global = ( + persist_override + if persist_override is not None + else resolve_persist_behavior(is_global_flag, is_session) + ) if not model_input: raise ValueError("model value required") @@ -2869,6 +2881,12 @@ def _sync_agent_model_with_config(sid: str, session: dict) -> None: raw, confirm_expensive_model=True, pin_session_override=False, + # This sync ADOPTS a config.yaml change into the live session; it + # must never write config back. Without this, the flag/config + # default (persist_switch_by_default=True) re-persisted whatever + # target the sync computed — the path that leaked `hermes --tui -m` + # into config.yaml as the permanent global model. + persist_override=False, ) except Exception as e: _emit( @@ -8700,6 +8718,9 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None: _raw, confirm_expensive_model=False, pin_session_override=bool(_prev_override), + # Session-internal restore after the /moa + # one-shot — never persist to config.yaml. + persist_override=False, ) except Exception as _moa_restore_exc: logger.warning( @@ -11559,6 +11580,9 @@ def _(rid, params: dict) -> dict: f"{preset} --provider moa", confirm_expensive_model=False, pin_session_override=True, + # One-shot turn-scoped swap — never persist the MoA + # virtual provider to config.yaml. + persist_override=False, ) except Exception as exc: session.pop("moa_one_shot_restore", None)