From 1a595192448e2a1f978c54625f8c2d2ebbe729cd Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 19 Sep 2026 00:03:18 -0700 Subject: [PATCH] fix(gateway): a --global /model pick leaves config.yaml as the only durable model authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `/model --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 --- gateway/slash_commands_model.py | 50 ++++++++---- .../test_model_command_reasoning_flag.py | 2 +- .../test_model_command_request_overrides.py | 3 + tests/gateway/test_model_picker_persist.py | 77 +++++++++++++++++++ website/docs/reference/slash-commands.md | 2 +- .../current/reference/slash-commands.md | 2 +- 6 files changed, 120 insertions(+), 16 deletions(-) diff --git a/gateway/slash_commands_model.py b/gateway/slash_commands_model.py index b4804f0baf..bdf86281cb 100644 --- a/gateway/slash_commands_model.py +++ b/gateway/slash_commands_model.py @@ -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 `: 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: diff --git a/tests/gateway/test_model_command_reasoning_flag.py b/tests/gateway/test_model_command_reasoning_flag.py index 89a89030d5..306df0e0c1 100644 --- a/tests/gateway/test_model_command_reasoning_flag.py +++ b/tests/gateway/test_model_command_reasoning_flag.py @@ -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: diff --git a/tests/gateway/test_model_command_request_overrides.py b/tests/gateway/test_model_command_request_overrides.py index df0838dc1b..510c34dfe7 100644 --- a/tests/gateway/test_model_command_request_overrides.py +++ b/tests/gateway/test_model_command_request_overrides.py @@ -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", diff --git a/tests/gateway/test_model_picker_persist.py b/tests/gateway/test_model_picker_persist.py index d153256ead..5b4a336e44 100644 --- a/tests/gateway/test_model_picker_persist.py +++ b/tests/gateway/test_model_picker_persist.py @@ -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" diff --git a/website/docs/reference/slash-commands.md b/website/docs/reference/slash-commands.md index 1ecdcf5ab9..2d21128096 100644 --- a/website/docs/reference/slash-commands.md +++ b/website/docs/reference/slash-commands.md @@ -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/` — 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/` — 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. | diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/reference/slash-commands.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/reference/slash-commands.md index 00657198d8..e69f30697d 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/reference/slash-commands.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/reference/slash-commands.md @@ -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。 |