Skip to content

fix(apps): stop Desktop sleep/wake from wiring primary chat to the wrong profile backend - #74552

Closed
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/desktop-primary-reconnect-active-profile
Closed

fix(apps): stop Desktop sleep/wake from wiring primary chat to the wrong profile backend#74552
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/desktop-primary-reconnect-active-profile

Conversation

@fangliquanflq

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes Desktop sleep/wake (and other primary WebSocket drop) reconnect so the window-owned primary gateway always redials the primary backend, not whichever profile is currently active in the sidebar. After concurrent primary+secondary sockets landed, the old single-socket reconnect still passed $activeGatewayProfile into getConnection(), which could wire the primary socket (and $connection) to a secondary backend.

Bug Cause

attemptReconnect in use-gateway-boot.ts called desktop.getConnection($activeGatewayProfile.get()) and then connect/publish on the effect-local primary HermesGateway. That was correct under the old single swapping socket model, but wrong after multi-profile concurrent sockets: primary reconnect must match boot/softSwitch (no-arg getConnection()), while secondaries reconnect via reconnectSecondaryGateways().

Reproduction Steps

  1. Boot Desktop on the window primary profile (typically default).
  2. Switch the sidebar to another profile (e.g. coder) with its own backend so a secondary socket is active.
  3. Sleep/wake the machine, or drop the primary WebSocket while still on coder.
  4. Observe primary reconnect using the active profile's descriptor; switching back to default can keep talking to the wrong backend.

Expected: primary reconnect dials only the window backend; secondaries reconnect on their own sockets; $connection / active socket / primaryProfile agree.
Before fix: primary socket and publish() can target the secondary backend.

Fix

Related Issue

Fixes #74551

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts - primary sleep/wake reconnect dials the window backend; gate publish on isActivePrimary()
  • apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx - assert primary reconnect ignores the active secondary profile and preserves $connection

How to Test

  1. Manual: multi-profile Desktop, switch to a secondary profile, sleep/wake (or drop primary WS), confirm primary stays on the window backend and the secondary still reconnects on its own socket.
  2. Automated (already run locally, 7 passed):
npm run test --workspace apps/desktop -- src/app/gateway/hooks/use-gateway-boot.test.tsx

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run the relevant desktop vitest file and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) - N/A
  • I've updated cli-config.yaml.example if I added/changed config keys - N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior - N/A

Screenshots / Logs

N/A

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 30, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main confirms the premise: attemptReconnect resolves and publishes $activeGatewayProfile at apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:153-159, while the primary socket is explicitly window-owned and secondary reconnects are separately handled by apps/desktop/src/store/gateway.ts:8-16 and :322-333.

The PR's no-argument primary lookup matches existing boot and soft-switch behavior at use-gateway-boot.ts:281 and :470. Its active-primary publication guard preserves the secondary descriptor that ensureGatewayProfile writes at apps/desktop/src/store/profile.ts:289-297. The added regression covers a secondary activation followed by a primary socket drop.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

One PR addresses #74551. #74552 changes the primary reconnect path to resolve the window-owned backend instead of the active secondary profile, preserves the active secondary descriptor, and adds regression coverage for a primary socket drop while a secondary profile is selected.

Related pull requests

Suggested consolidation

Keep #74552 open with a salvage path: retain the focused reconnect correction, the active-primary publication guard, and the accompanying secondary-active/primary-drop regression test. It is the only PR in this complex, so there are no competing PRs to close as duplicates.

Complex graph

flowchart 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
    I74551(["issue #74551 (open)"])
    P74552["PR #74552 (open)"]
    P74552 -->|best fix| I74551
    class I74551 open
    class P74552 open
    class P74552 best
    class P74552 target
    click I74551 "https://github.com/NousResearch/hermes-agent/issues/74551"
    click P74552 "https://github.com/NousResearch/hermes-agent/pull/74552"
Loading

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: 6 kB of PR diffs, 7 kB of issue/PR text, 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via #87872 with your commit cherry-picked onto current main — authorship preserved in git history. The fix landed essentially as you wrote it (no-arg getConnection + isActivePrimary publish guard); we added one regression test on top and audited the sibling wake paths (reconnectNow already funnels all wake signals through the fixed seam). Thank you!

@teknium1 teknium1 closed this Aug 16, 2026
@fangliquanflq

Copy link
Copy Markdown
Contributor Author

Thanks for salvaging this. I verified that #87872 merged the fix as commit 30f3d00 with the original authorship and the same two-file reconnect/test scope; its merge commit c23605e is an ancestor of current main, and #74551 is closed as completed. No further change is needed on this closed PR.

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 comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

[Bug]: Desktop sleep/wake reconnect wires the primary chat to the wrong profile backend

4 participants