diff --git a/cli.py b/cli.py index 336b25177457..600aff62aa35 100644 --- a/cli.py +++ b/cli.py @@ -8189,16 +8189,16 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): Supports: /model — show current model + usage hints - /model — switch model (persists by default) + /model — switch model (this session only) /model --once — switch for the next turn only - /model --session — switch for this session only - /model --global — switch and persist (explicit) + /model --session — switch for this session only (explicit) + /model --global — switch and persist to config.yaml /model --provider — switch provider + model /model --provider — switch to provider, auto-detect model - Persistence defaults to on (``model.persist_switch_by_default`` in - config.yaml, default True). Use ``--session`` for this CLI session or - ``--once`` for the next turn only. + Persistence defaults to off (``model.persist_switch_by_default`` in + config.yaml, default False — switches are session-scoped). Use + ``--global`` to persist, or ``--once`` for the next turn only. """ from hermes_cli.model_switch import ( switch_model, @@ -8225,10 +8225,10 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): if one_turn and not model_input and not explicit_provider: _cprint(" ✗ /model --once requires a model or provider.") return - # Resolve the effective persistence once: --session overrides the - # config-gated default, --global forces persist, otherwise defer to - # model.persist_switch_by_default (defaults to True so /model survives - # across sessions). + # Resolve the effective persistence once: --global forces persist, + # --session/--once force session-scope, otherwise defer to + # model.persist_switch_by_default (defaults to False so /model is + # session-scoped unless the user opts in). persist_global = resolve_persist_behavior( is_global_flag, is_session, is_once=one_turn, explicit_provider=explicit_provider, @@ -13160,8 +13160,8 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): # --- /model picker modal --- if self._model_picker_state: try: - # Picker selections persist by default (same default as - # /model ); honour model.persist_switch_by_default. + # Picker selections follow the same session-scoped default + # as /model ; honour model.persist_switch_by_default. from hermes_cli.model_switch import resolve_persist_behavior self._handle_model_picker_selection( diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 25261156c355..480a2e37ffdc 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -1437,10 +1437,10 @@ class GatewaySlashCommandsMixin: Supports: /model — interactive picker (Telegram/Discord) or text list - /model — switch model (persists by default) + /model — switch model (this session only) /model --once — switch for the next turn only - /model --session — switch for this session only - /model --global — switch and persist (explicit) + /model --session — switch for this session only (explicit) + /model --global — switch and persist to config.yaml /model --provider — switch provider + model /model --provider — switch to provider, auto-detect model """ diff --git a/hermes_cli/commands.py b/hermes_cli/commands.py index 8fb6926d8671..aab738c3ea55 100644 --- a/hermes_cli/commands.py +++ b/hermes_cli/commands.py @@ -131,7 +131,7 @@ COMMAND_REGISTRY: list[CommandDef] = [ # Configuration CommandDef("config", "Show current configuration", "Configuration", cli_only=True), - CommandDef("model", "Switch model (persists by default)", "Configuration", + CommandDef("model", "Switch model (session-scoped; --global to persist)", "Configuration", args_hint="[model] [--provider name] [--global|--session] [--refresh]"), CommandDef("codex-runtime", "Toggle codex app-server runtime for OpenAI/Codex models", "Configuration", aliases=("codex_runtime",), diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index f5cc07d65e38..0c4d8c0148e2 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -570,12 +570,14 @@ def resolve_persist_behavior( user is trying a different backend for this conversation, not reconfiguring the default. ``--global`` can still force persist. 5. Otherwise defer to ``model.persist_switch_by_default`` in - ``config.yaml`` (defaults to ``True``, so a plain ``/model `` - survives across sessions — the behavior users expect). + ``config.yaml`` (defaults to ``False``: a plain ``/model `` + affects only the current session). Users who want the old + persist-by-default behavior can set the key to ``true``; a one-off + ``--global`` always persists. The config read is defensive: on a fresh install ``model`` may be a flat string rather than a dict, in which case the built-in default - (``True``) applies. + (``False``) applies. """ if is_once: return False @@ -590,10 +592,10 @@ def resolve_persist_behavior( model_cfg = load_config().get("model") if isinstance(model_cfg, dict): - return bool(model_cfg.get("persist_switch_by_default", True)) + return bool(model_cfg.get("persist_switch_by_default", False)) except Exception: pass - return True + return False # --------------------------------------------------------------------------- diff --git a/tests/gateway/test_25107_stale_base_url_api_mode.py b/tests/gateway/test_25107_stale_base_url_api_mode.py index 781fc3c4bf68..9cbe06217488 100644 --- a/tests/gateway/test_25107_stale_base_url_api_mode.py +++ b/tests/gateway/test_25107_stale_base_url_api_mode.py @@ -186,7 +186,7 @@ async def test_picker_tap_to_custom_clears_stale_base_url_and_api_mode(tmp_path, adapter = _FakePickerAdapter() cfg_path = _setup_isolated_home(tmp_path, monkeypatch, dict(_STALE_MODEL_CFG)) - confirmation = await _drive_picker(_make_runner(adapter), _make_event("/model")) + confirmation = await _drive_picker(_make_runner(adapter), _make_event("/model --global")) assert confirmation is not None written = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) diff --git a/tests/gateway/test_model_command_flat_string_config.py b/tests/gateway/test_model_command_flat_string_config.py index 8de92e9e57dd..e90b340335a3 100644 --- a/tests/gateway/test_model_command_flat_string_config.py +++ b/tests/gateway/test_model_command_flat_string_config.py @@ -159,11 +159,11 @@ async def test_model_global_persists_when_config_has_proper_dict_model(tmp_path, @pytest.mark.asyncio -async def test_model_no_flag_persists_by_default(tmp_path, monkeypatch): - """A plain ``/model X`` (no --global) now persists to config.yaml. +async def test_model_no_flag_is_session_scoped_by_default(tmp_path, monkeypatch): + """A plain ``/model X`` (no --global) does NOT persist to config.yaml. - This is the user-facing fix: switching models in one session survives - into the next without re-typing the switch every time. + This is the user-facing fix: switches are session-scoped unless the user + opts in with ``--global`` or sets ``model.persist_switch_by_default: true``. """ cfg_path = _setup_isolated_home( tmp_path, @@ -178,7 +178,7 @@ async def test_model_no_flag_persists_by_default(tmp_path, monkeypatch): assert result is not None assert "gpt-5.5" in result written = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) - assert written["model"]["default"] == "gpt-5.5" + assert written["model"]["default"] == "old-model" @pytest.mark.asyncio diff --git a/tests/gateway/test_model_picker_persist.py b/tests/gateway/test_model_picker_persist.py index ddba56d9084b..4cb5c6e9ad42 100644 --- a/tests/gateway/test_model_picker_persist.py +++ b/tests/gateway/test_model_picker_persist.py @@ -8,9 +8,10 @@ model in the Telegram/Discord picker silently reverted on the next launch while *typing* the same model persisted — a contradiction the same PR introduced. After the fix (#49176), the picker callback honors the resolved -``persist_global`` (defaults to ``True``, still respects ``--session``) and runs -the same read-modify-write block the text path uses, so a tapped model survives -across sessions like a typed one. +``persist_global`` and runs the same read-modify-write block the text path +uses, so a tapped model behaves exactly like a typed one. Since the +session-scope-by-default change, both default to session-only and persist +only with ``--global`` (or ``model.persist_switch_by_default: true``). These tests drive the real ``_handle_model_command`` with a fake picker-capable adapter that captures the ``on_model_selected`` callback, then invoke that @@ -176,14 +177,14 @@ async def _drive_picker(runner, event): ], ids=["nested-dict", "flat-string"], ) -async def test_picker_tap_persists_by_default(tmp_path, monkeypatch, seed_model): - """Tapping a model in the picker (bare /model) persists to config.yaml, - matching the typed ``/model`` default — this is the #49176 fix. The written - ``model:`` must always end up a nested dict regardless of the seed shape.""" +async def test_picker_tap_global_flag_persists(tmp_path, monkeypatch, seed_model): + """Tapping a model in a ``/model --global`` picker persists to config.yaml, + matching the typed ``/model --global`` path. The written ``model:`` must + always end up a nested dict regardless of the seed shape.""" adapter = _FakePickerAdapter() cfg_path = _setup_isolated_home(tmp_path, monkeypatch, seed_model) - confirmation = await _drive_picker(_make_runner(adapter), _make_event("/model")) + confirmation = await _drive_picker(_make_runner(adapter), _make_event("/model --global")) assert confirmation is not None assert "gpt-5.5" in confirmation @@ -198,6 +199,34 @@ async def test_picker_tap_persists_by_default(tmp_path, monkeypatch, seed_model) assert "api_mode" not in written["model"] +@pytest.mark.asyncio +async def test_picker_tap_is_session_scoped_by_default(tmp_path, monkeypatch): + """Tapping a model in a bare ``/model`` picker applies an in-memory session + override and does NOT touch config.yaml — switches are session-scoped + unless the user opts in with ``--global`` (or sets + ``model.persist_switch_by_default: true``).""" + adapter = _FakePickerAdapter() + cfg_path = _setup_isolated_home( + tmp_path, monkeypatch, {"default": "old-model", "provider": "openrouter"} + ) + runner = _make_runner(adapter) + + confirmation = await _drive_picker(runner, _make_event("/model")) + + assert confirmation is not None + assert "gpt-5.5" in confirmation + # The session override IS applied in-memory (the switch worked). + assert runner._session_model_overrides, "session override should be set" + assert any( + ov.get("model") == "gpt-5.5" + for ov in runner._session_model_overrides.values() + ) + # But config.yaml is untouched — session-scoped by default. + written = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) + assert written["model"]["default"] == "old-model" + assert written["model"]["provider"] == "openrouter" + + @pytest.mark.asyncio async def test_picker_tap_session_flag_does_not_persist(tmp_path, monkeypatch): """``/model --session`` then a picker tap stays in-memory only — config diff --git a/tests/hermes_cli/test_model_switch_persist_default.py b/tests/hermes_cli/test_model_switch_persist_default.py index 7e42ce6cb93c..02e4521ffb8e 100644 --- a/tests/hermes_cli/test_model_switch_persist_default.py +++ b/tests/hermes_cli/test_model_switch_persist_default.py @@ -1,11 +1,12 @@ -"""Tests for persist-by-default model switching. +"""Tests for session-scoped-by-default model switching. Covers: - ``parse_model_flags`` recognises ``--session`` (and keeps ``--global``). - ``resolve_persist_behavior`` applies the config-gated default and the ``--session`` / ``--global`` overrides. -- The default (no flags) persists, which is the user-facing fix: a plain - ``/model `` survives across sessions. +- The default (no flags) is session-only, which is the user-facing fix: a + plain ``/model `` affects only the current session unless the user + passes ``--global`` or sets ``model.persist_switch_by_default: true``. """ from unittest.mock import patch @@ -73,10 +74,10 @@ class TestResolvePersistBehavior: with _config({"model": {"persist_switch_by_default": False}}): assert resolve_persist_behavior(True, False) is True - def test_default_persists_when_config_missing(self): - # No model section at all → built-in default (True). + def test_default_session_only_when_config_missing(self): + # No model section at all → built-in default (False, session-only). with _config({}): - assert resolve_persist_behavior(False, False) is True + assert resolve_persist_behavior(False, False) is False def test_default_persists_when_key_true(self): with _config({"model": {"persist_switch_by_default": True}}): @@ -87,9 +88,9 @@ class TestResolvePersistBehavior: assert resolve_persist_behavior(False, False) is False def test_default_when_model_is_flat_string(self): - # Fresh install: ``model: ""`` (not a dict) → built-in default True. + # Fresh install: ``model: ""`` (not a dict) → built-in default False. with _config({"model": ""}): - assert resolve_persist_behavior(False, False) is True + assert resolve_persist_behavior(False, False) is False def test_session_overrides_global_when_both_set(self): # --session is the explicit opt-out and wins over --global.