From aaa34b0e08834d6f7811577782c836f4d79f1b48 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 04:40:34 -0700 Subject: [PATCH] fix(desktop): model picker no longer hardcodes --global; one persist policy for every surface (#90235) Symptom: picking a model in the Desktop composer for the primary chat silently rewrote config.yaml (model.default + model.provider) as the profile default, ignoring model.persist_switch_by_default. A throwaway pick that resolved to e.g. openai-api (no key) left the profile with an unusable default on the next launch (#90235). Root cause: 7d96537bc8 (#86414) made use-model-controls.ts send --global for every primary-tile pick so a fresh profile would get a persisted provider instead of falling through to a leftover OPENAI_API_KEY env var. That put a persistence policy in the client, contradicting the server-side rule /model uses (resolve_persist_behavior). Fix: - resolve_persist_behavior gains one rule, ahead of the --provider session-only rule: when neither model.default nor model.provider is configured yet, persist. This preserves #86414's first-pick motivation for CLI, gateway and Desktop alike. With a default configured, a plain pick is session-only unless --global / persist_switch_by_default. - Desktop primary-tile picks send no scope flag and let the gateway decide. Secondary tiles and MoA presets still send --session. - /model help text in cli.py said "(persists)"; it now matches reality and lists --global. - Docs: desktop.md picker note + slash-commands /model row. Tests: test_first_pick_persists_then_session_only (fails on main), and the existing use-model-controls vitest updated to assert the flag-less request. --- .../session/hooks/use-model-controls.test.tsx | 12 ++++----- .../app/session/hooks/use-model-controls.ts | 19 +++++++------- cli.py | 3 ++- hermes_cli/model_switch.py | 26 +++++++++++++------ .../test_model_switch_persist_default.py | 18 +++++++++++++ website/docs/reference/slash-commands.md | 2 +- website/docs/user-guide/desktop.md | 2 +- 7 files changed, 55 insertions(+), 27 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx index 7d6a610d16..42421589a1 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx @@ -275,7 +275,7 @@ describe('useModelControls', () => { }) }) - it('persists an active primary-session picker change as the profile default via config.set --global', async () => { + it('sends an active primary-session picker change without a scope flag so the gateway decides persistence', async () => { $activeSessionId.set('session-1') const requestGateway = vi.fn(async () => ({ key: 'model', value: 'claude-sonnet-4.6' }) as never) let controls!: Controls @@ -289,13 +289,13 @@ describe('useModelControls', () => { }) ).resolves.toBe(true) - // The primary main agent's pick IS the profile default, so it persists to - // config.yaml (model.default + model.provider) — which is what lets a - // chosen subscription provider outrank a leftover OPENAI_API_KEY env var. + // No hardcoded --global (#90235): resolve_persist_behavior on the gateway + // owns the policy — session-only unless model.persist_switch_by_default + // is set or no default has ever been configured (#86414's first pick). expect(requestGateway).toHaveBeenCalledWith('config.set', { session_id: 'session-1', key: 'model', - value: 'claude-sonnet-4.6 --provider anthropic --global' + value: 'claude-sonnet-4.6 --provider anthropic' }) expect(requestGateway).not.toHaveBeenCalledWith('slash.exec', expect.anything()) }) @@ -376,7 +376,7 @@ describe('useModelControls', () => { confirm_expensive_model: true, key: 'model', session_id: 'session-1', - value: 'muse-spark-1.2-contributor --provider opencode-go --global' + value: 'muse-spark-1.2-contributor --provider opencode-go' }) expect($currentModel.get()).toBe('muse-spark-1.2-contributor') expect($currentProvider.get()).toBe('opencode-go') diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.ts b/apps/desktop/src/app/session/hooks/use-model-controls.ts index 4d0d63b058..8f28e1cf8e 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.ts +++ b/apps/desktop/src/app/session/hooks/use-model-controls.ts @@ -256,13 +256,13 @@ export function useModelControls({ return true } - // The PRIMARY profile's main agent is the profile's default — its - // model/provider choice IS the default, so persist it to config.yaml - // (model.default + model.provider) via --global. This is what makes - // the selection "stick": a set model.provider outranks a leftover - // OPENAI_API_KEY env var in resolve_provider(), so the main agent - // keeps the chosen (e.g. subscription) provider across restarts - // instead of silently falling back to an env key. + // The PRIMARY profile's main agent lets the gateway decide persistence + // (resolve_persist_behavior): session-only by default, persisted when + // model.persist_switch_by_default is true or when no default has ever + // been configured (the first-ever pick, so resolve_provider never falls + // through to a leftover OPENAI_API_KEY env var — #86414). A plain pick + // no longer silently rewrites config.yaml (#90235); Settings → Model + // remains the explicit "set as default" door. // // Two things stay --session, deliberately: // - a SECONDARY chat tile: picking a model there must not rewrite the @@ -270,14 +270,13 @@ export function useModelControls({ // - MoA (mixture-of-agents) presets: a transient orchestration choice // that must never become the persisted global gateway default. const isSessionOnlyPreset = (selection.provider || '').toLowerCase() === 'moa' - const persistsAsDefault = touchesPrimary && !isSessionOnlyPreset - const scope = persistsAsDefault ? '--global' : '--session' + const scope = touchesPrimary && !isSessionOnlyPreset ? '' : ' --session' const requestSwitch = (confirmExpensiveModel = false) => requestGateway('config.set', { session_id: liveSessionId, key: 'model', - value: `${selection.model} --provider ${selection.provider} ${scope}`, + value: `${selection.model} --provider ${selection.provider}${scope}`, ...(confirmExpensiveModel ? { confirm_expensive_model: true } : {}) }) diff --git a/cli.py b/cli.py index 5797c81318..e7dfd7fddc 100644 --- a/cli.py +++ b/cli.py @@ -12088,7 +12088,8 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): if not providers: _cprint(" No authenticated providers found.") _cprint("") - _cprint(" /model switch model (persists)") + _cprint(" /model switch model (this session)") + _cprint(" /model --global switch model and persist as default") _cprint(" /model --once switch for the next turn only") _cprint(" /model --session switch for this session only") _cprint(" /model --provider switch provider") diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 7e8e25b6da..bb025c95a5 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1016,11 +1016,18 @@ def resolve_persist_behavior( 1. ``--once`` explicitly opts out → ``False`` (next turn only). 2. ``--session`` explicitly opts out → ``False`` (this session only). 3. ``--global`` explicitly opts in → ``True``. - 4. ``--provider`` given without an explicit persist flag → ``False`` + 4. No default configured yet (neither ``model.default`` nor + ``model.provider`` set — a fresh install whose first-ever pick this + is) → ``True``. Without a persisted provider, ``resolve_provider`` + falls through to whatever ``*_API_KEY`` env var is lying around on + the next launch (#86414), so the first pick becomes the default + instead of evaporating. Applies to every surface (CLI, gateway, + Desktop picker) so no client has to hardcode ``--global``. + 5. ``--provider`` given without an explicit persist flag → ``False`` (session only). Provider switches are typically exploratory — the user is trying a different backend for this conversation, not reconfiguring the default. ``--global`` can still force persist. - 5. Otherwise defer to ``model.persist_switch_by_default`` in + 6. Otherwise defer to ``model.persist_switch_by_default`` in ``config.yaml`` (defaults to ``False``: a plain ``/model `` affects only the current session). Users who want the old persist-by-default behavior can set the key to ``true``; a one-off @@ -1036,17 +1043,20 @@ def resolve_persist_behavior( return False if is_global: return True - if explicit_provider: - return False try: from hermes_cli.config import load_config model_cfg = load_config().get("model") - if isinstance(model_cfg, dict): - return bool(model_cfg.get("persist_switch_by_default", False)) except Exception: - pass - return False + return False + if isinstance(model_cfg, dict): + if not (model_cfg.get("default") or model_cfg.get("provider")): + return True + if explicit_provider: + return False + return bool(model_cfg.get("persist_switch_by_default", False)) + # Flat-string form: a non-empty string IS a configured default. + return not model_cfg # --------------------------------------------------------------------------- diff --git a/tests/hermes_cli/test_model_switch_persist_default.py b/tests/hermes_cli/test_model_switch_persist_default.py index 11394c4222..b53c8913e1 100644 --- a/tests/hermes_cli/test_model_switch_persist_default.py +++ b/tests/hermes_cli/test_model_switch_persist_default.py @@ -52,6 +52,24 @@ class TestResolvePersistBehavior: with _config({"model": {"persist_switch_by_default": True}}): assert resolve_persist_behavior(False, False, explicit_provider="") is True + def test_first_pick_persists_then_session_only(self): + # #90235 / #86414: the ONE policy every surface (CLI, gateway, Desktop + # picker) defers to. With no default ever configured, the first pick + # persists (even with --provider, which is how the Desktop picker + # always sends it) so resolve_provider never falls through to a stray + # env key on restart. Once a default exists, a plain pick is + # session-only unless --global / persist_switch_by_default. + with _config({"model": {}}): + assert resolve_persist_behavior(False, False, explicit_provider="anthropic") is True + with _config({"model": ""}): + assert resolve_persist_behavior(False, False) is True + with _config({"model": {"default": "gpt-5.6", "provider": "openai-codex"}}): + assert resolve_persist_behavior(False, False, explicit_provider="openai-api") is False + assert resolve_persist_behavior(False, False) is False + assert resolve_persist_behavior(True, False, explicit_provider="openai-api") is True + with _config({"model": "gpt-5.6"}): + assert resolve_persist_behavior(False, False) is False + # --------------------------------------------------------------------------- # helper diff --git a/website/docs/reference/slash-commands.md b/website/docs/reference/slash-commands.md index f7a281d44f..ef939b4699 100644 --- a/website/docs/reference/slash-commands.md +++ b/website/docs/reference/slash-commands.md @@ -76,7 +76,7 @@ Type `/` in the CLI to open the autocomplete menu. Built-in commands are case-in | Command | Description | |---------|-------------| | `/config` | Show current configuration | -| `/model [model-name]` | Show or change the current model. Supports: `/model claude-sonnet-4`, `/model provider:model` (switch providers), `/model custom:model` (custom endpoint), `/model custom:name:model` (named custom provider), `/model custom` (auto-detect from endpoint), and user-defined aliases (`/model fav`, `/model grok` — see [Custom model aliases](#custom-model-aliases)). Flags: `--global` persists the change to config.yaml; `--session` forces session-only; `--once` applies to the next turn only; `--refresh` re-fetches the provider's model list; `--provider ` switches backend (session-only unless `--global`). A plain `/model ` is session-only unless `model.persist_switch_by_default: true` is set. **Interactive picker:** running `/model` with no arguments opens the provider→model picker; on the model list you can **type to fuzzy-filter** the models (e.g. type `grok` to narrow to matching models), Backspace to trim the filter, Esc to clear it (or close the picker). Selection always resolves to one concrete model — the filter only narrows the list, it never guesses. **Note:** `/model` can only switch between already-configured providers. To add a new provider, exit the session and run `hermes model` from your terminal. **Cost note:** switching models mid-conversation resets the prompt cache — the cache key includes the model, so your next turn re-reads the entire conversation at full input price instead of the ~75%-discounted cached rate. Expected and unavoidable, but worth knowing on long sessions. | +| `/model [model-name]` | Show or change the current model. Supports: `/model claude-sonnet-4`, `/model provider:model` (switch providers), `/model custom:model` (custom endpoint), `/model custom:name:model` (named custom provider), `/model custom` (auto-detect from endpoint), and user-defined aliases (`/model fav`, `/model grok` — see [Custom model aliases](#custom-model-aliases)). Flags: `--global` persists the change to config.yaml; `--session` forces session-only; `--once` applies to the next turn only; `--refresh` re-fetches the provider's model list; `--provider ` switches backend (session-only unless `--global`). A plain `/model ` is session-only unless `model.persist_switch_by_default: true` is set — except when no `model.default`/`model.provider` is configured yet, in which case the first pick persists so the profile gets a real default. The same rule governs the desktop composer picker. **Interactive picker:** running `/model` with no arguments opens the provider→model picker; on the model list you can **type to fuzzy-filter** the models (e.g. type `grok` to narrow to matching models), Backspace to trim the filter, Esc to clear it (or close the picker). Selection always resolves to one concrete model — the filter only narrows the list, it never guesses. **Note:** `/model` can only switch between already-configured providers. To add a new provider, exit the session and run `hermes model` from your terminal. **Cost note:** switching models mid-conversation resets the prompt cache — the cache key includes the model, so your next turn re-reads the entire conversation at full input price instead of the ~75%-discounted cached rate. Expected and unavoidable, but worth knowing on long sessions. | | `/codex-runtime [auto\|codex_app_server\|on\|off]` | Toggle the optional [Codex app-server runtime](../user-guide/features/codex-app-server-runtime) for OpenAI/Codex models. `auto` (default) uses Hermes' standard chat completions; `codex_app_server` hands turns to a `codex app-server` subprocess for native shell, apply_patch, ChatGPT subscription auth, and migrated Codex plugins. Effective on next session. | | `/personality` | Set a predefined personality. `/personality none` (or `default` / `neutral`) clears the overlay and returns to base behavior. | | `/verbose` | Cycle tool progress display: off → new → all → verbose. Can be [enabled for messaging](#notes) via config. | diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md index 2028923d77..9e2afa1da9 100644 --- a/website/docs/user-guide/desktop.md +++ b/website/docs/user-guide/desktop.md @@ -81,7 +81,7 @@ Changing any of these values invalidates only that profile's disk-discovery cach The model picker lives in the **composer**, just left of the microphone. Click it to switch the model, reasoning effort, and fast mode from one dropdown. -- **The composer picker is sticky UI state and never touches your default.** It's remembered locally (per device) and **follows** across new chats and restarts instead of snapping back to the default — pick a model once and the next `Cmd/Ctrl+N` opens on it. With a live chat, switching models scopes the change to that **current chat**; either way the selection rides along when the session is created/switched and is **never** written to the profile default. (Switching [profiles](#sessions--profiles) reseeds to that profile's own default.) +- **The composer picker is sticky UI state and never touches your default.** It's remembered locally (per device) and **follows** across new chats and restarts instead of snapping back to the default — pick a model once and the next `Cmd/Ctrl+N` opens on it. With a live chat, switching models scopes the change to that **current chat**; either way the selection rides along when the session is created/switched and is **never** written to the profile default — with one exception: on a fresh profile that has no `model.default`/`model.provider` configured yet, the first pick is persisted so the app has a real default instead of falling through to a stray API-key env var on restart. Persistence follows the same rule as `/model` (`model.persist_switch_by_default`); use **Settings → Model** to change the default deliberately. (Switching [profiles](#sessions--profiles) reseeds to that profile's own default.) - **Set the default in Settings → Model.** That "main" model is your **per-profile global default** — it's what new chats, crons, subagents, and auxiliary tasks start from, and it's the only place that writes it. Each [profile](#sessions--profiles) keeps its own default. - **Per-model effort/fast presets.** Each model remembers its own reasoning effort and fast-mode choice in the desktop app, re-applied to the session whenever you pick that model. These presets are a desktop convenience and don't change crons or subagents. - **Mid-chat switches reset the prompt cache.** Switching the model inside a live chat means the next message re-reads the whole conversation at full input price (provider prompt caches are keyed to the model). Fine occasionally; on a long chat, a fresh chat on the new model is often cheaper than bouncing back and forth.