From e82692f6ac2ffebf840c19c238075afc73d8e004 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 12 Jul 2026 17:09:34 +0800 Subject: [PATCH] fix(gateway): make /model session-scoped by default on messaging platforms Previously, /model on messaging platforms (gateway) would persist to global config.yaml by default, matching CLI behavior via the model.persist_switch_by_default config setting. This caused unintended cross-session pollution when users switched models in one chat, as the global config change affected all other sessions. On messaging platforms, sessions are typically ephemeral and per-chat, so /model now defaults to session-scoped behavior: the switch applies to the current chat only and survives gateway restart via the session DB, but does not write to config.yaml unless --global is explicitly passed. This differs from CLI behavior (where /model still persists by default) to align with user expectations for messaging platform UX. Fixes #63083 --- gateway/slash_commands.py | 14 +++- .../test_model_command_flat_string_config.py | 22 ++++-- tests/gateway/test_model_picker_persist.py | 68 +++++++++++-------- 3 files changed, 68 insertions(+), 36 deletions(-) diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 38ab051c8e47..865eabdd68a4 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -1430,7 +1430,19 @@ async def _handle_model_command(self, event: MessageEvent) -> Optional[str]: force_refresh, is_session, ) = parse_model_flags(raw_args) - persist_global = resolve_persist_behavior(is_global_flag, is_session) + + # In messaging platforms (gateway), /model defaults to session-scope + # unless --global is explicitly specified. This differs from CLI + # behavior (which respects model.persist_switch_by_default config) because + # messaging platform sessions are typically ephemeral and per-chat, + # and unintended global config writes cause cross-session pollution. + # See #63083. + if is_session: + persist_global = False + elif is_global_flag: + persist_global = True + else: + persist_global = False # --refresh: bust the disk cache so the picker shows live data. if force_refresh: diff --git a/tests/gateway/test_model_command_flat_string_config.py b/tests/gateway/test_model_command_flat_string_config.py index 8de92e9e57dd..8e5e1dba9413 100644 --- a/tests/gateway/test_model_command_flat_string_config.py +++ b/tests/gateway/test_model_command_flat_string_config.py @@ -159,11 +159,12 @@ 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_does_not_persist_by_default(tmp_path, monkeypatch): + """A plain ``/model X`` (no --global) is session-scoped by default on messaging platforms. - This is the user-facing fix: switching models in one session survives - into the next without re-typing the switch every time. + This differs from CLI (which persists by default) because messaging + platform sessions are typically ephemeral and per-chat, and unintended + global config writes cause cross-session pollution. See #63083. """ cfg_path = _setup_isolated_home( tmp_path, @@ -171,14 +172,23 @@ async def test_model_no_flag_persists_by_default(tmp_path, monkeypatch): {"default": "old-model", "provider": "openai-codex"}, ) - result = await _make_runner()._handle_model_command( + runner = _make_runner() + result = await runner._handle_model_command( _make_event("/model gpt-5.5") ) assert result is not None assert "gpt-5.5" in result + # The session override IS applied in-memory. + assert runner._session_model_overrides + assert any( + ov.get("model") == "gpt-5.5" + for ov in runner._session_model_overrides.values() + ) + # But config.yaml is untouched — the override is in-memory only. written = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) - assert written["model"]["default"] == "gpt-5.5" + assert written["model"]["default"] == "old-model" + assert written["model"]["provider"] == "openai-codex" @pytest.mark.asyncio diff --git a/tests/gateway/test_model_picker_persist.py b/tests/gateway/test_model_picker_persist.py index 95654030d826..e87edb2a2947 100644 --- a/tests/gateway/test_model_picker_persist.py +++ b/tests/gateway/test_model_picker_persist.py @@ -136,44 +136,54 @@ async def _drive_picker(runner, event): @pytest.mark.asyncio -@pytest.mark.parametrize( - "seed_model", - [ - # Already-nested dict (common case). - { - "default": "old-model", - "provider": "custom", - "base_url": "https://api.custom.example/v1", - "api_key": "sk-stale", - "api_mode": "anthropic_messages", - }, - # Flat-string model: must be coerced to a nested dict on a tap (same - # scalar-``model:`` guard the text path has) instead of raising - # ``TypeError`` on assignment. - "deepseek-v4-flash", - ], - 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_does_not_persist_by_default(tmp_path, monkeypatch): + """Tapping a model in the picker (bare /model) is session-scoped by default + on messaging platforms — config.yaml is untouched. Use --global to persist. + + This behavior differs from CLI (which persists by default) because + messaging platform sessions are typically ephemeral and per-chat, and + unintended global config writes cause cross-session pollution. See #63083. + """ adapter = _FakePickerAdapter() - cfg_path = _setup_isolated_home(tmp_path, monkeypatch, seed_model) + cfg_path = _setup_isolated_home( + tmp_path, monkeypatch, {"default": "old-model", "provider": "custom"} + ) + runner = _make_runner(adapter) - confirmation = await _drive_picker(_make_runner(adapter), _make_event("/model")) + 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 (proves the path didn't no-op). + 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 — the override is in-memory only. written = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) - assert isinstance(written["model"], dict), ( - "model: should be coerced to a dict, got %r" % (written["model"],) + assert written["model"]["default"] == "old-model" + assert written["model"]["provider"] == "custom" + + +@pytest.mark.asyncio +async def test_picker_tap_with_global_flag_persists(tmp_path, monkeypatch): + """``/model --global`` then a picker tap persists to config.yaml.""" + adapter = _FakePickerAdapter() + cfg_path = _setup_isolated_home( + tmp_path, monkeypatch, {"default": "old-model", "provider": "custom"} ) + runner = _make_runner(adapter) + + confirmation = await _drive_picker(runner, _make_event("/model --global")) + + assert confirmation is not None + assert "gpt-5.5" in confirmation + # With --global, the persist block runs and updates config.yaml. + written = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) assert written["model"]["default"] == "gpt-5.5" assert written["model"]["provider"] == "openrouter" - assert "base_url" not in written["model"] - assert "api_key" not in written["model"] - assert "api_mode" not in written["model"] + assert "base_url" not in written["model"] # openrouter, not custom @pytest.mark.asyncio