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
32 changes: 26 additions & 6 deletions gateway/slash_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,12 @@ async def _handle_model_command(self, event: MessageEvent) -> Optional[str]:
from hermes_cli.providers import get_label

raw_args = event.get_command_args().strip()
source = event.source

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capturing the routed home is not sufficient for the initial picker setup: production dispatch reaches this handler without _profile_runtime_scope (gateway/run.py:9930), so the subsequent _load_gateway_config() still reads the default home. Please scope the initial config/provider-list work with this captured home too; the deferred callback scope only fixes the later selection phase.

_command_profile_home = None
if getattr(getattr(self, "config", None), "multiplex_profiles", False):
_command_profile_home = getattr(
self, "_resolve_profile_home_for_source"
)(source)

# Parse --provider, --global, --session, and --refresh flags
(
Expand All @@ -1467,7 +1473,7 @@ async def _handle_model_command(self, event: MessageEvent) -> Optional[str]:
current_api_key = ""
user_provs = None
custom_provs = None
config_path = _hermes_home / "config.yaml"
config_path = (_command_profile_home or _hermes_home) / "config.yaml"
try:
cfg = _load_gateway_config()
if cfg:
Expand All @@ -1485,9 +1491,8 @@ async def _handle_model_command(self, event: MessageEvent) -> Optional[str]:
except Exception:
pass

# Check for session override
source = event.source
# Normalize the source the same way a normal message turn does
# Check for session override. Normalize the source the same way a normal
# message turn does
# (Telegram DM topic recovery) before deriving the override key, so
# the override is stored under the key the next message turn reads
# (#30479).
Expand All @@ -1503,7 +1508,7 @@ async def _handle_model_command(self, event: MessageEvent) -> Optional[str]:
# No args: show interactive picker (Telegram/Discord) or text list
if not model_input and not explicit_provider:
# Try interactive picker if the platform supports it
adapter = self.adapters.get(source.platform)
adapter = getattr(self, "_adapter_for_source")(source)
has_picker = (
adapter is not None
and getattr(type(adapter), "send_model_picker", None) is not None
Expand Down Expand Up @@ -1536,8 +1541,9 @@ async def _handle_model_command(self, event: MessageEvent) -> Optional[str]:
_cur_provider = current_provider
_cur_base_url = current_base_url
_cur_api_key = current_api_key
_picker_profile_home = _command_profile_home

async def _on_model_selected(
async def _on_model_selected_scoped(
_chat_id: str, model_id: str, provider_slug: str
) -> str:
"""Perform the model switch and return confirmation text."""
Expand Down Expand Up @@ -1736,6 +1742,20 @@ async def _on_model_selected(
lines.append(t("gateway.model.session_only_hint"))
return "\n".join(lines)

async def _on_model_selected(
_chat_id: str, model_id: str, provider_slug: str
) -> str:
if _picker_profile_home is None:
return await _on_model_selected_scoped(
_chat_id, model_id, provider_slug
)
from gateway.run import _profile_runtime_scope

with _profile_runtime_scope(_picker_profile_home):
return await _on_model_selected_scoped(
_chat_id, model_id, provider_slug
)

metadata = self._thread_metadata_for_source(source, self._reply_anchor_for_event(event))
result = await adapter.send_model_picker(
chat_id=source.chat_id,
Expand Down
168 changes: 145 additions & 23 deletions tests/gateway/test_model_picker_persist.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,22 @@ def _fake_switch_result():
)


def _stub_picker_dependencies(monkeypatch):
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
monkeypatch.setattr(
"hermes_cli.model_switch.list_picker_providers",
lambda **kw: [{"slug": "openrouter", "name": "OpenRouter", "models": ["gpt-5.5"]}],
)
monkeypatch.setattr(
"hermes_cli.model_switch.switch_model",
lambda **kw: _fake_switch_result(),
)
monkeypatch.setattr(
"hermes_cli.model_switch.resolve_display_context_length",
lambda *a, **k: 272000,
)


def _setup_isolated_home(tmp_path, monkeypatch, model_yaml_value):
"""Write a config.yaml with the given ``model:`` value and stub heavy bits."""
import gateway.run as gateway_run
Expand All @@ -93,35 +109,41 @@ def _setup_isolated_home(tmp_path, monkeypatch, model_yaml_value):
)

monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home)
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
# The picker-setup path calls list_picker_providers, which otherwise hits
# the network (OpenRouter model catalog). Stub it to a minimal list β€” these
# tests capture and fire the on_model_selected callback and don't assert on
# picker contents. The handler imports it as a local alias at call time, so
# patching the source-module attribute takes effect.
_stub_picker_dependencies(monkeypatch)
# save_config writes to ``get_hermes_home() / config.yaml`` β€” point it here.
monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home)
monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: hermes_home)
return cfg_path


def _make_named_runner(monkeypatch, default_adapter, named_adapter, named_home):
runner = _make_runner(default_adapter)
monkeypatch.setattr(
"hermes_cli.model_switch.list_picker_providers",
lambda **kw: [{"slug": "openrouter", "name": "OpenRouter", "models": ["gpt-5.5"]}],
runner, "config", types.SimpleNamespace(multiplex_profiles=True), raising=False
)
# switch_model is imported as a local alias inside the handler
# (`from hermes_cli.model_switch import switch_model as _switch_model`),
# so patching the source-module attribute takes effect at call time.
monkeypatch.setattr(
"hermes_cli.model_switch.switch_model",
lambda **kw: _fake_switch_result(),
runner,
"_profile_adapters",
{"named": {Platform.TELEGRAM: named_adapter}},
raising=False,
)
# The confirmation builder resolves context length for display, which
# otherwise makes real outbound HTTP calls (Ollama /api/show + the
# OpenRouter models catalog). Stub it β€” these tests don't assert on the
# displayed context, and the closure imports it lazily from this module.
monkeypatch.setattr(
"hermes_cli.model_switch.resolve_display_context_length",
lambda *a, **k: 272000,
runner, "_resolve_profile_home_for_source", lambda source: named_home
)
return runner


def _named_event(args):
return MessageEvent(
text=f"/model {args}".rstrip(),
message_type=MessageType.TEXT,
source=SessionSource(
platform=Platform.TELEGRAM,
chat_id="named-chat",
chat_type="dm",
profile="named",
),
)
# save_config writes to ``get_hermes_home() / config.yaml`` β€” point it here.
monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home)
monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: hermes_home)
return cfg_path


async def _drive_picker(runner, event):
Expand Down Expand Up @@ -201,3 +223,103 @@ async def test_picker_tap_session_flag_does_not_persist(tmp_path, monkeypatch):
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
assert written["model"]["default"] == "old-model"
assert written["model"]["provider"] == "openai-codex"


@pytest.mark.asyncio
async def test_multiplex_picker_keeps_profile_adapter_and_callback_scope(
tmp_path, monkeypatch
):
"""A named profile must present and execute its picker under one identity."""
from agent.secret_scope import get_secret, set_multiplex_active

default_adapter = _FakePickerAdapter()
named_adapter = _FakePickerAdapter()
named_home = tmp_path / "profiles" / "named"
named_home.mkdir(parents=True)
(named_home / ".env").write_text("PROFILE_MODEL_KEY=named-secret\n", encoding="utf-8")
runner = _make_named_runner(monkeypatch, default_adapter, named_adapter, named_home)
_setup_isolated_home(
tmp_path,
monkeypatch,
{"default": "old-model", "provider": "openai-codex"},
)
resolved = []

def _profile_switch(**kwargs):
resolved.append(get_secret("PROFILE_MODEL_KEY"))
return _fake_switch_result()

monkeypatch.setattr("hermes_cli.model_switch.switch_model", _profile_switch)
event = _named_event("--session")

set_multiplex_active(True)
try:
sent = await runner._handle_model_command(event)

assert sent is None
assert default_adapter.captured_callback is None
assert named_adapter.captured_callback is not None
assert resolved == []

confirmation = await named_adapter.captured_callback(
"named-chat", "gpt-5.5", "openrouter"
)
finally:
set_multiplex_active(False)

assert "gpt-5.5" in confirmation
assert resolved == ["named-secret"]


@pytest.mark.asyncio
async def test_multiplex_picker_global_persists_only_named_profile(
tmp_path, monkeypatch
):
"""A named picker must not seed its global write from the default profile."""
import gateway.run as gateway_run
from agent.secret_scope import set_multiplex_active

default_home = tmp_path / "default"
named_home = tmp_path / "profiles" / "named"
default_home.mkdir(parents=True)
named_home.mkdir(parents=True)
default_cfg = {
"marker": "default",
"model": {"default": "default-old", "provider": "openai-codex"},
}
named_cfg = {
"marker": "named",
"model": {"default": "named-old", "provider": "openai-codex"},
}
(default_home / "config.yaml").write_text(
yaml.safe_dump(default_cfg, sort_keys=False), encoding="utf-8"
)
(named_home / "config.yaml").write_text(
yaml.safe_dump(named_cfg, sort_keys=False), encoding="utf-8"
)

default_adapter = _FakePickerAdapter()
named_adapter = _FakePickerAdapter()
runner = _make_named_runner(monkeypatch, default_adapter, named_adapter, named_home)
monkeypatch.setattr(gateway_run, "_hermes_home", default_home)
_stub_picker_dependencies(monkeypatch)
event = _named_event("--global")

set_multiplex_active(True)
try:
with gateway_run._profile_runtime_scope(named_home):
sent = await runner._handle_model_command(event)
assert sent is None
assert named_adapter.captured_callback is not None
confirmation = await named_adapter.captured_callback(
"named-chat", "gpt-5.5", "openrouter"
)
finally:
set_multiplex_active(False)

assert "gpt-5.5" in confirmation
assert yaml.safe_load((default_home / "config.yaml").read_text()) == default_cfg
written = yaml.safe_load((named_home / "config.yaml").read_text())
assert written["marker"] == "named"
assert written["model"]["default"] == "gpt-5.5"
assert written["model"]["provider"] == "openrouter"
Loading