Skip to content

fix(desktop): scope composer model selection and cold resume to active session - #117122

Open
edosulai wants to merge 3 commits into
NousResearch:mainfrom
edosulai:fix/desktop-session-scoped-composer-model
Open

edosulai wants to merge 3 commits into
NousResearch:mainfrom
edosulai:fix/desktop-session-scoped-composer-model

Conversation

@edosulai

@edosulai edosulai commented Sep 20, 2026 •

Copy link
Copy Markdown

Summary

Fixes an issue in Hermes Desktop where resuming a stored chat session or switching models leaks the global composer sticky model across sessions.

Problem

  1. Sticky model leak on cold resume: When a stored session without an active slice is resumed (after app relaunch, gateway reconnect, or Cmd+N), PRIMARY_SESSION_VIEW.$model and $provider fell back to $currentModel and $currentProvider (the composer sticky). As a result, a model selected in one chat (e.g. Gemini) leaked onto another thread (e.g. a Grok session).
  2. Missing --session scope on picker changes: Changing the model from the active primary session picker sent config.set without --session, allowing the switch to alter global defaults.
  3. Turn-start config sync leak: _sync_agent_model_with_config on the gateway automatically pulled config.yaml changes into active desktop sessions at turn start, overriding session-specific models.
  4. Same leak for non-Desktop clients (ACP / TUI / resumed rebuilds): a chat attached over ACP (e.g. a Paseo mobile client proxying to the owner gateway) is not source == "desktop", so after a resume that lost the in-RAM pin (rebuild, reload race) the next turn read "agent ≠ config" as a profile edit and rewrote it onto model.default. Because the OpenAI client is process-shared, sibling chats snapped to the default together. Observed: several ACP-attached chats flipped from their persisted model to the profile's Gemini default after a client reload.

Solution

  • apps/desktop/src/app/chat/session-view.tsx: Add primaryIdentityField so stored sessions without an active slice resolve to empty string instead of falling back to the draft sticky atoms ($currentModel / $currentProvider). True drafts (no stored ID) still follow the last picked model.
  • apps/desktop/src/app/session/hooks/use-model-controls.ts: Append --session to config.set for primary session picker selections so model changes remain scoped to the active session.
  • tui_gateway/model_switch.py: Exempt desktop sessions (session.get("source") == "desktop") from turn-start config sync unless opted in via follow_profile_config.
  • tui_gateway/model_switch.py (source-agnostic backstop): _session_db_model() reads the chat's persisted model; when it already differs from model.default the chat is a session pick, so config sync records the target but does not switch. A chat still on the default (or with no row) keeps adopting profile edits as before.

Test plan

  • Vitest: npx vitest run src/app/chat/session-view.test.ts src/app/session/hooks/use-model-controls.test.tsx (33 passed)
  • Pytest: uv run --with pytest pytest tests/tui_gateway/test_tui_gateway_server.py -k "test_config_sync" (11 passed)
  • Pytest: tests/tui_gateway/test_desktop_model_sync_guard.py (new: skips when persisted model differs; still adopts when persisted matches default / row missing). Rebased on current main; the tui_gateway suites show the same failure set before and after this branch (no new failures).

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 20, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

This PR fixes the sticky composer model leaking across chats (cold-resumed grok thread inheriting another chat's Gemini pick) via three coordinated changes: identity-scoped model/provider atoms in the primary session view, always---session composer picks, and a gateway exemption from config auto-adopt for desktop sessions. The direction is sound and tests cover the new behavior, but the always---session switch silently drops two previously deliberate persistence paths, and the reasoning-effort/fast atoms keep the old sticky behavior.

  1. Always---session drops first-pick persistence and persist_switch_by_default — apps/desktop/src/app/session/hooks/use-model-controls.ts:249 (const scope = ' --session') — non-blocking. The removed block documents two intentional cases for an unscoped primary pick: the first-ever pick (so resolve_provider never falls through to a leftover OPENAI_API_KEY, fix(desktop): the main agent's model pick persists as the profile default #86414) and model.persist_switch_by_default: true users. With a hardcoded scope those users' primary picks silently stop persisting with no Settings affordance change. If deliberate, call it out in the PR description and docs; otherwise preserve '' scope when no default was ever configured or persist_switch_by_default is set.
  2. Reasoning effort/fast still use the leaky sticky — apps/desktop/src/app/chat/session-view.tsx:131 ($reasoningEffort: primaryField(...)) — non-blocking. Only $model/$provider moved to primaryIdentityField (session-view.tsx:129-130); $reasoningEffort (and $fast if present on the view) still inherit the global sticky on a cold-resumed stored session, so the same class of leak persists for effort pills. Either scope them the same way or state why model/provider are special.
  3. Hunk does not apply cleanly on current main — apps/desktop/src/app/chat/session-view.tsx:101 — non-blocking. git apply --check against hermes-agent-fix@d6dd884b12 fails on session-view.tsx (upstream gained $reasoningEffortWire/$turnStartedAt entries in PRIMARY_SESSION_VIEW), so the PR base is stale. Rebase before merge; no logic conflict expected.

Minor: primaryIdentityField returns '' for a live slice with an empty-string model even when no stored session is selected — intended per the comment, but the value || (selected ? '' : draft) fallback means a transient empty model on a live session reads as blank instead of draft; fine for display, just confirm no downstream config.set path consumes it.

@edosulai

Copy link
Copy Markdown
Author

Updated with resolutions for review feedback:

  • Preserve Gateway Persistence Resolution: In use-model-controls.ts, restored touchesPrimary && !isSessionOnlyPreset ? '' : ' --session'. This preserves first-ever pick persistence (fix(desktop): the main agent's model pick persists as the profile default #86414) and model.persist_switch_by_default: true, while letting the gateway's resolve_persist_behavior decide persistence and leaving cross-session isolation to the view and gateway config-sync bypass.
  • Scope Reasoning Effort: In session-view.tsx, switched $reasoningEffort to primaryIdentityField, so cold-resumed stored sessions without an active slice do not inherit leftover reasoning effort from other chats. Verified with dedicated test in session-view.test.ts.

@Enough1122

Copy link
Copy Markdown

AI code review — automated follow-up for reference; not a maintainer.

Re-checked head 0aec6963 against the two findings:

  • Finding 2 (reasoning-effort sticky) — resolved. $reasoningEffort now goes through primaryIdentityField (session-view.tsx) with a dedicated cold-resume test in session-view.test.ts. Verified in the diff.
  • Finding 1 (always---session persistence) — moot on current head. use-model-controls.ts is not in this PR's file list; the touchesPrimary && !isSessionOnlyPreset ? '' : ' --session' conditional is already on main, so the rebase picked it up. End state preserves first-pick persistence (fix(desktop): the main agent's model pick persists as the profile default #86414) and persist_switch_by_default — no action needed, just noting the fix arrived via main rather than this branch.

No blockers from this review.

@edosulai
edosulai force-pushed the fix/desktop-session-scoped-composer-model branch 3 times, most recently from 8b8954e to afae49c Compare September 26, 2026 07:02
…e session

Cold-resuming a stored chat session that had no active slice yet fell
back to the global composer sticky atoms ($currentModel / $currentProvider).
That sticky is the last pick from ANY chat in the window, causing models
selected in one chat to leak onto other threads after app restart, gateway
reconnect, or Cmd+N.

Additionally, choosing a model from the active primary session picker
did not specify --session, allowing the gateway default to potentially
rewrite other sessions, and turn-start config sync automatically adopted
config.yaml defaults onto existing desktop sessions.

- In session-view.tsx, introduce primaryIdentityField to isolate model/provider
  for stored sessions from the draft sticky atom.
- In use-model-controls.ts, append --session when changing model in an
  active primary session so the switch remains session-scoped.
- In tui_gateway/model_switch.py, exempt desktop sessions from auto-adopting
  config.yaml model changes at turn start (unless opted-in via follow_profile_config).
- Add tests covering cold resume, live slice preference, session scoping,
  and desktop session config-sync bypass.
A desktop turn whose RAM model pin is missing still has the picked model
in the session row. Config sync treated that as an unpinned chat and
rewrote the process-shared client onto model.default, so sibling chats
snapped to gemini together.
@edosulai
edosulai force-pushed the fix/desktop-session-scoped-composer-model branch from afae49c to 86d36b3 Compare September 28, 2026 15:50

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants