fix(desktop): multi-profile message routing sends messages to wrong profile (#67097) - #67254
fix(desktop): multi-profile message routing sends messages to wrong profile (#67097)#67254webtecnica wants to merge 1 commit into
Conversation
…rofile Fix NousResearch#67097 — four root causes addressed: 1. store/gateway.ts: activeGateway() no longer silently falls back to primaryGateway when the active profile's secondary doesn't exist. Returns null instead, preventing messages meant for a remote/secondary profile from routing to the local primary backend. 2. store/gateway.ts: ensureGatewayForProfile() now closes all secondary WebSocket connections before activating the primary profile. Stale secondary connections with wantOpen=true could receive messages if active key state was confused. 3. electron/main.ts: Set HERMES_SESSION_PROFILE in spawn environment for both startHermes() (primary backend: activeProfile || 'default') and spawnPoolBackend() (pool: profile). Previously the env var was empty, causing subprocess tools to query an empty string instead of the correct profile name. 4. electron/main.ts: Create active-profile.json on first boot in app.whenReady(). Previously the file was only written on explicit profile switch via UI, leaving no persisted state on fresh install.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the fallback route; that part addresses a real current-main defect in apps/desktop/src/store/gateway.ts:74.
Problems
closeSecondaryGateways()in the primary-profile path would tear down live background streams. Current main explicitly says background sockets are never closed in this path (apps/desktop/src/store/gateway.ts:205-236), and profile switching relies on keeping them alive for concurrent streaming (apps/desktop/src/store/profile.ts:253-295).- The new
HERMES_SESSION_PROFILEspawn value is not the per-turn source of truth: gateway turns bind it fromcontext.source.profile(gateway/run.py:15558-15569), andget_session_env()prefers that ContextVar over process environment (gateway/session_context.py:304-327). - The boot write persists
{ "profile": null }, whichreadActiveDesktopProfile()reads as the samenullvalue as a missing file (apps/desktop/electron/main.ts:5863-5892). - Please add a regression test for the non-primary active-key/absent-secondary case; #67190 demonstrates the focused test shape.
Suggested changes
- Keep the null fallback, remove the unconditional secondary teardown, and re-scope the profile propagation/persistence additions to verified behavior.
Automated hermes-sweeper review.
| if (key === primaryProfile) { | ||
| // Close all secondary WebSocket connections before activating the primary. | ||
| // Leaving secondaries open with wantOpen=true while the active key flips to | ||
| // primary creates stale connections that can receive messages if the active |
There was a problem hiding this comment.
This closes every live background-profile socket when returning to the primary profile, contradicting the current concurrent-streaming contract: ensureGatewayForProfile() intentionally preserves background sockets (apps/desktop/src/store/gateway.ts:205-206) and ensureGatewayProfile() relies on them staying live (apps/desktop/src/store/profile.ts:253-295). Please remove this call from the primary fast path.
SummaryOne PR, #67254, addresses issue #67097. Its diff fixes the demonstrated wrong-gateway fallback, while the contributor review identifies design or effectiveness problems with the secondary-socket cleanup, spawn-time profile propagation, and null-profile boot write. Related pull requests
Duplicates#67254 overlaps open #67190 on the Suggested consolidationKeep #67254 open with a salvage path: preserve the null-return fallback fix and add the focused regression test, while explicitly resolving the contributor review's lifecycle, session-context, and persistence objections. Do not close it as a duplicate because its diff extends beyond #67190, and no listed PR can be closed as a duplicate on the supplied evidence. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I67097(["issue #67097 (open)"])
P67254["PR #67254 (open)"]
P67254 -->|best fix| I67097
class I67097 open
class P67254 open
class P67254 best
class P67254 target
click I67097 "https://github.com/NousResearch/hermes-agent/issues/67097"
click P67254 "https://github.com/NousResearch/hermes-agent/pull/67254"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 4 kB of PR diffs, 5 kB of issue/PR text, 5 kB of discussion (5 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
The core fix here (activeGateway's silent primary fallback) landed on main via #87600, which also syncs $connection/$activeGatewayProfile on every activation and serializes switches through a shared mutex. Closing as implemented on main — thanks for the analysis and the PR. |
Summary
Fixes #67097 — Desktop multi-profile message routing intermittently sends messages to the wrong profile's gateway.
Root Causes & Fixes
1.
activeGateway()silent fallback to primary gateway (store/gateway.ts)Problem: When
activeKeypointed to a non-primary profile and the secondary didn't exist (transient state),activeGateway()silently fell back toprimaryGateway, routing messages meant for a remote profile to the local backend.Fix: Return
nullinstead ofprimaryGateway— the callers already handle null.2. Secondary WebSockets not closed on primary switch (
store/gateway.ts)Problem:
ensureGatewayForProfile("default")calledsetActive(key)and returned immediately for the primary profile, but left all secondary WebSocket connections open withwantOpen = true. Stale connections could receive messages if active key state was confused.Fix: Call
closeSecondaryGateways()beforesetActive()when switching to the primary profile.3.
HERMES_SESSION_PROFILEenv var empty (electron/main.ts)Problem: Both
startHermes()andspawnPoolBackend()inheritedHERMES_SESSION_PROFILE=""fromprocess.env. Subprocess tools that queryget_session_env("HERMES_SESSION_PROFILE")got an empty string instead of the correct profile name.Fix: Set
HERMES_SESSION_PROFILEexplicitly:startHermes():activeProfile || 'default'spawnPoolBackend(): the profile name4.
active-profile.jsonnever created on boot (electron/main.ts)Problem:
active-profile.jsonwas only written when the user explicitly switched profiles via the UI. On first boot (missing file), there was no persisted record of the profile state.Fix: Initialize
active-profile.jsoninapp.whenReady()by callingwriteActiveDesktopProfile(null)when the file is missing.Files Changed
apps/desktop/src/store/gateway.ts— Fixes 1 & 2apps/desktop/electron/main.ts— Fixes 3 & 4