fix(gateway): a --global /model pick leaves config.yaml as the only durable model authority
A `/model <m> --global` switch (typed or picker) used to persist twice: the profile config.yaml AND a per-session `model_override` in the session store. The session copy has higher precedence on rehydration, so after a later global change (CLI `hermes model`, another chat's `--global`) and a gateway restart the stale override silently won — #100314 saw an explicit `gpt-5.6-sol-900k` resume as the base 272K `gpt-5.6-sol`. Now `_record_model_switch` writes config.yaml FIRST and, on success, drops the redundant session override from memory and the store. If the config write fails the switch stays a truthful session override and the confirmation says "config.yaml not updated (...)" plus the session-only hint instead of claiming "Saved to config.yaml"; a riding `--reasoning` pin follows the same effective scope. `--session` and `--once` semantics are unchanged. Tests: two invariants in tests/gateway/test_model_picker_persist.py (typed+picker clear the durable override and a fresh SessionStore rehydrates nothing; failed config write keeps the override and an honest reply), red on base. test_model_command_request_overrides now points get_hermes_home at its own config so the --provider switch resolves session-scoped as intended instead of the sandbox's fresh-install first-pick rule. Fixes #100314 Supersedes #99825 (slim redo; the original wrapped the commit boundary through a sys.modules-swapped mixin). Co-authored-by: Andrex Ibiza, MBA <andrexibiza@gmail.com>
This commit is contained in:
@@ -195,8 +195,12 @@ class GatewayModelCommandsMixin:
|
||||
|
||||
async def _record_model_switch(
|
||||
self, result, ctx: _ModelSwitchContext, *, source, one_turn: bool, picker: bool
|
||||
) -> None:
|
||||
"""Persist a committed switch: session DB, next-turn note, override map, config write-through."""
|
||||
) -> Optional[str]:
|
||||
"""Persist a committed switch: session DB, next-turn note, config write-through, override map.
|
||||
|
||||
Returns the config-write error for a ``--global`` switch whose ``config.yaml`` write failed
|
||||
(the switch then stays a session override), else ``None``.
|
||||
"""
|
||||
from hermes_cli.model_switch import format_model_for_display
|
||||
|
||||
# Persist the new model to the session DB so the dashboard shows the updated model (#34850).
|
||||
@@ -237,6 +241,23 @@ class GatewayModelCommandsMixin:
|
||||
self._claim_one_turn_restore(ctx.session_key, ctx.restore_snapshot)
|
||||
elif not picker and hasattr(self, "_pending_one_turn_model_restores"):
|
||||
self._pending_one_turn_model_restores.pop(ctx.session_key, None)
|
||||
# A --global switch has ONE durable authority: config.yaml. Write it first; on success drop
|
||||
# the session override (memory + store) — a redundant copy would shadow every later global
|
||||
# change after a restart (#100314: a stale override resumed `gpt-5.6-sol-900k` as the base
|
||||
# 272K model). On failure keep the override so the switch truthfully survives as session-only.
|
||||
global_error: Optional[str] = None
|
||||
if ctx.persist_global:
|
||||
try:
|
||||
await _persist_model_switch_to_config(result, ctx.config_path)
|
||||
except Exception as e:
|
||||
logger.warning("Failed to persist model switch: %s", e)
|
||||
global_error = str(e) or type(e).__name__
|
||||
if ctx.persist_global and global_error is None:
|
||||
self._session_model_overrides.pop(ctx.session_key, None)
|
||||
try:
|
||||
await self.async_session_store.set_model_override(ctx.session_key, None)
|
||||
except Exception:
|
||||
logger.debug("Failed to clear persisted session model override", exc_info=True)
|
||||
# Non-secret write-through so the override survives a restart (api_key/api_mode are
|
||||
# re-resolved on rehydration); a --once override must NOT outlive a restart.
|
||||
# Write-through the non-secret parts (model/provider/base_url) to the session store so the override
|
||||
@@ -246,7 +267,7 @@ class GatewayModelCommandsMixin:
|
||||
# pre-once state (the prior session override, or nothing), which is exactly what the finally-restore
|
||||
# reverts the in-memory dict to. (#29923 review defect: the original implementation wrote through,
|
||||
# so a crash before the restore rehydrated the once-model permanently.)
|
||||
if not one_turn:
|
||||
elif not one_turn:
|
||||
try:
|
||||
await self.async_session_store.set_model_override(
|
||||
ctx.session_key, self._session_model_overrides[ctx.session_key]
|
||||
@@ -254,14 +275,11 @@ class GatewayModelCommandsMixin:
|
||||
except Exception:
|
||||
logger.debug("Failed to persist session model override", exc_info=True)
|
||||
self._evict_cached_agent(ctx.session_key) # next turn builds fresh from the override
|
||||
if ctx.persist_global:
|
||||
try:
|
||||
await _persist_model_switch_to_config(result, ctx.config_path)
|
||||
except Exception as e:
|
||||
logger.warning("Failed to persist model switch: %s", e)
|
||||
return global_error
|
||||
|
||||
async def _model_switch_confirmation(
|
||||
self, result, ctx: _ModelSwitchContext, *, one_turn: bool, picker: bool
|
||||
self, result, ctx: _ModelSwitchContext, *, one_turn: bool, picker: bool,
|
||||
global_error: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Confirmation text with full metadata (display form shortens opaque Palantir IDs)."""
|
||||
from gateway.run import _load_gateway_config
|
||||
@@ -303,7 +321,11 @@ class GatewayModelCommandsMixin:
|
||||
lines.append(t("gateway.model.prompt_caching_enabled"))
|
||||
if result.warning_message:
|
||||
lines.append(t("gateway.model.warning_prefix", warning=result.warning_message))
|
||||
if ctx.persist_global:
|
||||
if ctx.persist_global and global_error is not None:
|
||||
# Never claim a clean global commit the disk did not take (#100314).
|
||||
lines.append(t("gateway.model.warning_prefix", warning=f"config.yaml not updated ({global_error})"))
|
||||
lines.append(t("gateway.model.session_only_hint"))
|
||||
elif ctx.persist_global:
|
||||
lines.append(t("gateway.model.saved_global"))
|
||||
elif one_turn:
|
||||
lines.append(" (next turn only — restores after one response)")
|
||||
@@ -320,15 +342,17 @@ class GatewayModelCommandsMixin:
|
||||
error = self._switch_cached_agent_model(result, ctx, picker)
|
||||
if error is not None:
|
||||
return error
|
||||
await self._record_model_switch(result, ctx, source=source, one_turn=one_turn, picker=picker)
|
||||
reply = await self._model_switch_confirmation(result, ctx, one_turn=one_turn, picker=picker)
|
||||
global_error = await self._record_model_switch(result, ctx, source=source, one_turn=one_turn, picker=picker)
|
||||
reply = await self._model_switch_confirmation(
|
||||
result, ctx, one_turn=one_turn, picker=picker, global_error=global_error,
|
||||
)
|
||||
if ctx.reasoning_effort and not one_turn:
|
||||
# `/model X --reasoning <level>`: same applier as /reasoning, same scope as the pick.
|
||||
# The record step already evicted the cached agent, so the pin lands on the rebuild.
|
||||
from gateway.run import _platform_config_key
|
||||
reply += "\n" + self._apply_reasoning_selection(
|
||||
ctx.session_key, _platform_config_key(source.platform), ctx.reasoning_effort,
|
||||
persist_global=ctx.persist_global)
|
||||
persist_global=ctx.persist_global and global_error is None)
|
||||
return reply
|
||||
|
||||
async def _send_model_picker(self, event: MessageEvent, source, adapter, session_key: str, listing_kwargs: dict, on_model_selected) -> bool:
|
||||
|
||||
@@ -16,7 +16,7 @@ def _runner():
|
||||
runner = object.__new__(GatewayRunner)
|
||||
calls = {}
|
||||
runner._switch_cached_agent_model = lambda *_a, **_k: None
|
||||
runner._record_model_switch = AsyncMock()
|
||||
runner._record_model_switch = AsyncMock(return_value=None) # None = config write succeeded
|
||||
runner._model_switch_confirmation = AsyncMock(return_value="switched")
|
||||
runner._apply_reasoning_selection = (
|
||||
lambda session_key, platform_key, value, persist_global=False:
|
||||
|
||||
@@ -63,6 +63,9 @@ custom_providers:
|
||||
)
|
||||
|
||||
monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home)
|
||||
# resolve_persist_behavior() reads the profile config through get_hermes_home(); without this
|
||||
# the sandbox home looks like a fresh install and the --provider switch persists globally.
|
||||
monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: hermes_home)
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_switch.switch_model",
|
||||
|
||||
@@ -254,3 +254,80 @@ async def test_multiplex_picker_global_persists_only_named_profile(
|
||||
assert written["marker"] == "named"
|
||||
assert written["model"]["default"] == "gpt-5.5"
|
||||
assert written["model"]["provider"] == "openrouter"
|
||||
|
||||
|
||||
def _make_store_runner(adapter, sessions_dir, monkeypatch):
|
||||
"""Bare runner with a real JSONL SessionStore (the durable /model override lives there)."""
|
||||
import hermes_state
|
||||
from gateway.config import GatewayConfig
|
||||
from gateway.session import SessionStore
|
||||
|
||||
def _no_sqlite(*_a, **_k):
|
||||
raise RuntimeError("SQLite disabled in test")
|
||||
|
||||
monkeypatch.setattr(hermes_state, "SessionDB", _no_sqlite)
|
||||
runner = _make_runner(adapter)
|
||||
runner.session_store = SessionStore(sessions_dir=sessions_dir, config=GatewayConfig())
|
||||
return runner
|
||||
|
||||
|
||||
async def _typed_global(runner, event_text="/model gpt-5.5 --global"):
|
||||
return await runner._handle_model_command(_make_event(event_text))
|
||||
|
||||
|
||||
async def _picker_global(runner, event_text="/model --global"):
|
||||
return await _drive_picker(runner, _make_event(event_text))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("drive", [_typed_global, _picker_global], ids=["typed", "picker"])
|
||||
async def test_global_switch_clears_redundant_session_override(tmp_path, monkeypatch, drive):
|
||||
"""A ``--global`` pick (typed or picker) leaves config.yaml as the ONLY durable authority:
|
||||
the per-session override is dropped from memory and the session store, so a later global
|
||||
change is not shadowed after a gateway restart (#100314: a stale override resumed
|
||||
``gpt-5.6-sol-900k`` as the base 272K model)."""
|
||||
adapter = _FakePickerAdapter()
|
||||
cfg_path = _setup_isolated_home(tmp_path, monkeypatch, {"default": "old-model", "provider": "openrouter"})
|
||||
runner = _make_store_runner(adapter, tmp_path / "sessions", monkeypatch)
|
||||
source = _make_event("x").source
|
||||
session_key = runner._session_key_for_source(source)
|
||||
runner.session_store.get_or_create_session(source)
|
||||
stale = {"model": "old-model", "provider": "openrouter"}
|
||||
runner.session_store.set_model_override(session_key, stale)
|
||||
runner._session_model_overrides[session_key] = dict(stale)
|
||||
|
||||
confirmation = await drive(runner)
|
||||
|
||||
assert "config.yaml" in confirmation
|
||||
assert yaml.safe_load(cfg_path.read_text(encoding="utf-8"))["model"]["default"] == "gpt-5.5"
|
||||
assert runner._session_model_override(session_key) is None
|
||||
assert runner.session_store.get_model_override(session_key) is None
|
||||
# Restart: a fresh store + runner rehydrate nothing, so config.yaml decides the model.
|
||||
restarted = _make_store_runner(_FakePickerAdapter(), tmp_path / "sessions", monkeypatch)
|
||||
restarted._rehydrate_session_model_override(session_key)
|
||||
assert restarted._session_model_override(session_key) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_global_switch_keeps_session_override_when_config_write_fails(tmp_path, monkeypatch):
|
||||
"""When the config.yaml write fails the switch must stay a truthful session override (memory +
|
||||
store) and the confirmation must not claim a clean global save (#100314)."""
|
||||
adapter = _FakePickerAdapter()
|
||||
cfg_path = _setup_isolated_home(tmp_path, monkeypatch, {"default": "old-model", "provider": "openrouter"})
|
||||
runner = _make_store_runner(adapter, tmp_path / "sessions", monkeypatch)
|
||||
source = _make_event("x").source
|
||||
session_key = runner._session_key_for_source(source)
|
||||
runner.session_store.get_or_create_session(source)
|
||||
|
||||
async def _disk_full(result, config_path):
|
||||
raise OSError("disk full")
|
||||
|
||||
monkeypatch.setattr("gateway.slash_commands_model._persist_model_switch_to_config", _disk_full)
|
||||
|
||||
confirmation = await _typed_global(runner)
|
||||
|
||||
assert yaml.safe_load(cfg_path.read_text(encoding="utf-8"))["model"]["default"] == "old-model"
|
||||
assert "Saved to config.yaml" not in confirmation
|
||||
assert "disk full" in confirmation
|
||||
assert runner._session_model_override(session_key)["model"] == "gpt-5.5"
|
||||
assert runner.session_store.get_model_override(session_key)["model"] == "gpt-5.5"
|
||||
|
||||
@@ -245,7 +245,7 @@ The messaging gateway supports the following built-in commands inside Telegram,
|
||||
| `/new [name]` (alias: `/reset`) | Start a new session (fresh session ID + history). Optional `[name]` sets the initial session title. Append `now`, `--yes`, or `-y` to skip the confirmation modal — e.g. `/reset now`, `/new --yes my-experiment`. |
|
||||
| `/status` | Show session info, followed by a local **Session recap** block (recent turn counts, top tools used, files touched, latest prompt + reply). |
|
||||
| `/stop` | Kill all running background processes and interrupt the running agent. |
|
||||
| `/model [provider:model]` | Show or change the model. Supports provider switches (`/model zai:glm-5`), custom endpoints (`/model custom:model`), named custom providers (`/model custom:local:qwen`), auto-detect (`/model custom`), OpenRouter account presets (`/model @preset/<slug>` — account-scoped, skips the public model-listing check), and user-defined aliases (`/model fav`, `/model grok` — see [Custom model aliases](#custom-model-aliases)). Use `--global` to persist the change to config.yaml. **Note:** `/model` can only switch between already-configured providers. To add a new provider or set up API keys, use `hermes model` from your terminal (outside the chat session). **Cost note:** a mid-session model switch resets the prompt cache (the cache key includes the model), so the next message re-reads the whole conversation at full input price. |
|
||||
| `/model [provider:model]` | Show or change the model. Supports provider switches (`/model zai:glm-5`), custom endpoints (`/model custom:model`), named custom providers (`/model custom:local:qwen`), auto-detect (`/model custom`), OpenRouter account presets (`/model @preset/<slug>` — account-scoped, skips the public model-listing check), and user-defined aliases (`/model fav`, `/model grok` — see [Custom model aliases](#custom-model-aliases)). Use `--global` to persist the change to config.yaml; a successful `--global` pick (typed or picker) also drops this chat's session-only override, so config.yaml alone decides the model after a gateway restart. **Note:** `/model` can only switch between already-configured providers. To add a new provider or set up API keys, use `hermes model` from your terminal (outside the chat session). **Cost note:** a mid-session model switch resets the prompt cache (the cache key includes the model), so the next message re-reads the whole conversation at full input price. |
|
||||
| `/codex-runtime [auto\|codex_app_server\|on\|off]` | Toggle the optional [Codex app-server runtime](../user-guide/features/codex-app-server-runtime). Persists to `model.openai_runtime` in config.yaml and evicts the cached agent so the next message picks up the new runtime. Effective on next session. |
|
||||
| `/personality [name]` | Set a personality overlay for the session. `/personality none` (or `default` / `neutral`) clears it. |
|
||||
| `/fast [normal\|fast\|auto\|cold\|status]` | Fast mode — OpenAI Priority Processing / Anthropic Fast Mode. `auto`/`cold` open a bounded fast window per turn / per session. |
|
||||
|
||||
@@ -204,7 +204,7 @@ hermes config set model.aliases.grok x-ai/grok-4
|
||||
| `/reset` | 重置对话历史。 |
|
||||
| `/status` | 显示会话信息,随后显示本地**会话摘要**块(近期轮次数、最常用工具、访问的文件、最新 prompt + 回复)。 |
|
||||
| `/stop` | 终止所有正在运行的后台进程并中断运行中的 agent。 |
|
||||
| `/model [provider:model]` | 显示或更改模型。支持提供商切换(`/model zai:glm-5`)、自定义端点(`/model custom:model`)、命名自定义提供商(`/model custom:local:qwen`)、自动检测(`/model custom`),以及用户自定义别名(`/model fav`、`/model grok`——见[自定义模型别名](#custom-model-aliases))。使用 `--global` 将更改持久化到 config.yaml。**注意:** `/model` 只能在已配置的提供商之间切换。如需添加新提供商或设置 API 密钥,请在终端(聊天会话外)运行 `hermes model`。 |
|
||||
| `/model [provider:model]` | 显示或更改模型。支持提供商切换(`/model zai:glm-5`)、自定义端点(`/model custom:model`)、命名自定义提供商(`/model custom:local:qwen`)、自动检测(`/model custom`),以及用户自定义别名(`/model fav`、`/model grok`——见[自定义模型别名](#custom-model-aliases))。使用 `--global` 将更改持久化到 config.yaml;成功的 `--global` 选择(键入或选择器)还会清除本聊天的仅会话覆盖,因此网关重启后仅由 config.yaml 决定模型。**注意:** `/model` 只能在已配置的提供商之间切换。如需添加新提供商或设置 API 密钥,请在终端(聊天会话外)运行 `hermes model`。 |
|
||||
| `/codex-runtime [auto\|codex_app_server\|on\|off]` | 切换可选的 [Codex app-server runtime](../user-guide/features/codex-app-server-runtime)。持久化到 config.yaml 中的 `model.openai_runtime` 并驱逐缓存的 agent,使下一条消息使用新 runtime。下次会话生效。 |
|
||||
| `/personality [name]` | 为会话设置 personality 覆盖层。 |
|
||||
| `/fast [normal\|fast\|status]` | 切换快速模式——OpenAI Priority Processing / Anthropic Fast Mode。 |
|
||||
|
||||
Reference in New Issue
Block a user