Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion gateway/slash_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 16 additions & 6 deletions tests/gateway/test_model_command_flat_string_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,26 +159,36 @@ 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,
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
Expand Down
68 changes: 39 additions & 29 deletions tests/gateway/test_model_picker_persist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading