Skip to content

fix(desktop): stop a spawned session's model from moving the composer pick - #318

Merged
OmarB97 merged 1 commit into
mainfrom
fix/spawn-model-composer-leak-fork-20260802
Aug 2, 2026
Merged

fix(desktop): stop a spawned session's model from moving the composer pick#318
OmarB97 merged 1 commit into
mainfrom
fix/spawn-model-composer-leak-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What

hermes desktop spawn -m <model> no longer moves the model the composer is set to. The spawned session runs on the model it was given; the next chat you start still comes up on the model you picked.

Why

#298 added per-session overrides specifically so a spawn would not touch the composer's selection — setCurrentModel persists to localStorage, so steering one spawn through it would silently change the model for every chat started afterwards. Two paths still did exactly that:

  • At create. applyRuntimeInfo called setCurrentModel(info.model). The backend puts the spawn's override into that field (tui_gateway/server.py, session.create), so the override was written straight into the persisted pick.
  • For the session's whole life. syncRuntimeMetadataToView called setCurrentModel(state.model ?? ''), and state.model is stamped from every session.info. Every heartbeat rewrote the pick. use-message-stream/gateway-event.ts explicitly refuses to call setCurrentModel on the direct path for this exact reason — and the value then reached the same atom by routing around that guard through the state cache.

The test meant to lock the guarantee could not catch either one: it stubbed session.create as {session_id, stored_session_id} with no info key, so applyRuntimeInfo(undefined) returned early and the assertion ran against an untouched atom.

How

$currentModel/$currentProvider go back to meaning only one thing: the composer's sticky selection, written by a user pick or the profile-default seed, and read by desktopSessionCreateParams when creating the next chat.

Runtime metadata lands in a new unpersisted mirror, $activeSessionModel/$activeSessionProvider. The primary chat surface displays $primaryModel — the open session's model, falling back to the composer's pick on a fresh draft (null mirror). That is the same shape a session tile already computes from its own $sessionStates slice, so both surfaces now derive their display the same way instead of the primary one aliasing a persisted atom.

Because the primary surface no longer reads $currentModel for display, picking a model for a live primary session pushes an optimistic paint through the session-state delegate, the way a tile already did. Selection and live model can now differ, so selectModel rolls each back to its own previous value.

Testing

  • npm run typecheck, npm run lint on the changed files, and the full ui project (230 files / 1935 tests) pass.

  • Both leak sites were reproduced before fixing, and each new assertion was re-run against reverted production code to confirm it genuinely fails there — no repeat of the vacuous lock.

  • Live, in a sandboxed HERMES_HOME (scripts/dev-sandbox.sh), using a spawn without -m as the probe since it takes the same desktopSessionCreateParams path a typed new chat does:

    spawn -m glm-5.2-2bit-pool next chat (no -m)
    without this fix glm-5.2-2bit-pool glm-5.2-2bit-pool ← leaked
    with this fix glm-5.2-2bit-pool deepseek-v4-flash-0731-ds4 ← the user's pick

Notes for the reviewer

  • Two existing tests in use-session-state-cache.test.tsx asserted $currentModel as the mirror target. The display behaviour they protect is correct and is kept — they now assert it through $primaryModel, plus a new assertion that the persisted selection is untouched. That retarget is the point of the change, not a refreshed expectation.
  • applyStoredSessionPreviewRuntimeInfo writes the mirror too. It runs while a resume is in flight and $activeSessionId is still null, which is why the mirror is gated on its own null rather than on the active session id.
  • The mirror is cleared on the two paths that return to a draft: startFreshSessionDraft and wipeSessionListsForGatewaySwitch.

Risk

Renderer-only, no backend or schema change. The blast radius is what the model pill, model menu, and model picker display on the primary surface; tiles are untouched. The behaviour change users can see is the intended one — a spawned override stops following them into their next chat.

🤖 Generated with Claude Code

… pick

PR #298 promised that `hermes desktop spawn -m <model>` would steer one
session without touching the composer's persisted selection, because
`setCurrentModel` writes localStorage and would otherwise re-aim every
chat the user starts afterwards. Two paths broke that promise, and the
test meant to lock it was passing vacuously.

The backend echoes the spawn's override back as `info.model` on the
session.create response, and stamps `model` on every session.info after
that. Both echoes were being fed into the composer's sticky selection:
`applyRuntimeInfo` wrote it once at create, and `syncRuntimeMetadataToView`
rewrote it on every heartbeat for the session's whole life. The second one
is the irony — use-message-stream/gateway-event.ts explicitly refuses to
call setCurrentModel on the direct path, then routed the same value
around itself through the state cache.

Runtime metadata now lands in a separate, unpersisted mirror
($activeSessionModel/$activeSessionProvider). The primary chat surface
displays $primaryModel — the open session's model, falling back to the
composer's pick on a fresh draft — which is the same shape a session tile
already computes from its own $sessionStates slice. Only a user pick or
the profile-default seed writes $currentModel, so
desktopSessionCreateParams can never see a spawned override.

The existing lock stubbed session.create as `{session_id,
stored_session_id}` with no `info` key, so `applyRuntimeInfo(undefined)`
returned early and the assertion never reached the leak. The stub now
echoes `info` the way the backend does, and a new case follows the
guarantee end to end: spawn with an override, then start the next chat
and assert it is created on the user's own model.

Verified live in a sandboxed HERMES_HOME. On this build a spawn with
`-m glm-5.2-2bit-pool` runs on glm and the next chat is still created on
deepseek-v4-flash-0731-ds4; on a build without the fix that next chat
came up on glm-5.2-2bit-pool.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 787754f into main Aug 2, 2026
25 of 26 checks passed
@OmarB97
OmarB97 deleted the fix/spawn-model-composer-leak-fork-20260802 branch August 2, 2026 15:06
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…er pick (#322)

#318 stopped a spawned session's MODEL from overwriting the composer's
persisted selection. Reasoning effort and fast mode are sticky in exactly
the same way and were left with exactly the same bug.

Both persist to localStorage, and `desktopSessionCreateParams` reads both as
what the NEXT new chat runs — so writing a session's runtime values into them
re-aims every chat the user starts afterwards. Four paths did that, all with
the persisting setters: `syncRuntimeMetadataToView` on every state flush,
`applyRuntimeInfo` at session create, `applyStoredSessionPreviewRuntimeInfo`
on a sidebar preview, and the `session.info` handler in gateway-event.ts.

That last one is worth a reviewer's attention: it sits inside the `if (apply)`
block whose own comment explains why model/provider must not be written there,
and `apply` is also true for a global broadcast when NO session is active — so
a heartbeat could rewrite the stored effort while the user sat on a fresh
draft, with no session on screen to explain it.

The fix is #318's, applied to its two remaining fields:
$activeSessionReasoningEffort / $activeSessionFastMode hold what the open
session runs, `$primaryReasoningEffort` / `$primaryFastMode` compute
`live ?? picked` for the primary chat surface, and only a deliberate pick
(model menu, presets, Settings) or the profile-default seed still writes
$currentReasoningEffort / $currentFastMode. `clearActiveSessionModel` became
`clearActiveSessionRuntime` since it now hands back all four.

Two existing state-cache assertions moved from $currentReasoningEffort to
$primaryReasoningEffort. They are testing what the composer DISPLAYS, and the
atom holding that answer changed identity — the same edit #318 made to the
model assertions beside them. Their meaning is unchanged: a background
session's metadata still must not paint the foreground.

Proven both ways rather than assumed. Reverting just the state-cache swap
fails three tests (`expected 'high' to be 'low'`); reverting just the
gateway-event swap fails two (`expected 'low' to be 'high'`). End to end: with
a stored pick of effort=high/fast=true, a session reporting low/false leaves
the next chat's session.create params on high/true.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…endpoint" (#330)

`hermes desktop spawn --provider <name>` bound a correct per-session
override, but the model picker rendered the session under a separate
"Custom endpoint" section with its own thinking/effort badge, duplicating
the provider's real entry.

resolve_runtime_provider() reports EVERY user-defined provider as
provider="custom" + base_url — bare "custom" is the resolved billing
class, not a routable identity — so `--provider ai-router` reaches the
picker substrate looking exactly like a one-off endpoint. Two sites then
mishandled it:

- model_switch.py section 3b synthesized the "Custom endpoint" row for
  any bare-custom session. Its dedup guard scanned only custom_providers:
  and never the rows section 3 had already emitted from providers:, so an
  endpoint declared there got a second anonymous row aliasing it. That is
  the shape every profiles/*/config.yaml uses.
- inventory._single_named_custom_match() matched only `custom:`-prefixed
  slugs. providers: entries own their endpoint under a bare slug, so the
  payload's top-level provider stayed "custom"; the renderer marks a row
  live with `provider.slug === payload.provider`, so the real entry never
  got the check mark and its badge fell back to the stored preset instead
  of live session state.

Both now key on endpoint identity, and both require exactly one owner.
Endpoint identity rather than model membership because a row's model list
is one live /models probe away from changing, which made sections flicker
with endpoint reachability. Exactly one because rows with distinct
credentials can share an endpoint (tenants behind one proxy URL) and none
can claim the session alone — the two sites must agree, or suppressing
the synthesized row would leave only rows that do not own it.

_hide_shadow_bare_custom_row() also drops the `configured-current`
placeholder that the explicit-only filter adds, once the attributed row
carries that job.

Display attribution only: no renderer change, and the composer's
persisted selection is untouched — spawn overrides still write only the
unpersisted active-session mirror (#318).

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant