Skip to content

fix(desktop): adopt saved primary profile before gateway boot - #75112

Open
weddle wants to merge 1 commit into
NousResearch:mainfrom
weddle:fix/desktop-primary-profile-boot
Open

fix(desktop): adopt saved primary profile before gateway boot#75112
weddle wants to merge 1 commit into
NousResearch:mainfrom
weddle:fix/desktop-primary-profile-boot

Conversation

@weddle

@weddle weddle commented Jul 31, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes a race at cold boot in the desktop app. The renderer connects the primary gateway before it reads which profile the desktop persisted, so fetches keyed on gatewayReady can run while $activeGatewayProfile still holds its initial 'default'.

I measured the order on unpatched main with atlas as the persisted profile. desktop.profile.get() runs once, after getConnection() has already resolved. At the moment the connection is obtained the atom reads "default". The shared gateway client publishes setState('open') inside connect() before the promise resolves (apps/shared/src/json-rpc-gateway.ts), and boot awaits adoptPrimaryProfile() later. The sidebar's first sessions and projects fetches race the adoption IPC and lose. Any machine whose primary profile is a named one paints its first sidebar under the wrong profile.

PR #50529 identified this ordering problem. Its review asked for a rebase onto the current adoptPrimaryProfile() helper with the seed applied before cold-boot getConnection(). This PR implements that shape, with credit to #50529's author.

  1. Read before connect. The IPC read of the persisted profile moves out of adoptPrimaryProfile() into readPrimaryProfile() and applyPrimaryProfile(). Cold boot applies it to the atom, to the registry's primaryProfile, and to event tagging before desktop.getConnection(). HMR-survivor adoption keeps its path, as does the catch that falls back to 'default'.
  2. Align the registry's active pointer during adoption. applyPrimaryProfile() also runs the primary fast path of ensureGatewayForProfile(), a synchronous setActive that is safe before connect. Skipping this step drops the open transition: state events tagged with the adopted primary fail the activeKey comparison in reportGatewayState and the UI sits at "connecting" forever. A test guards this.

If the preferred path is updating #50529 itself, the tests here can move there.

Related Issue

Addresses the boot-ordering defect tracked in PR #50529's review: adopt the persisted primary profile before getConnection(). In-flight work on the same hook covers different lifecycle paths: #74552 handles reconnect after sleep, #74388 gates session restore. I am glad to coordinate rebases with either.

Type of Change

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

Changes Made

  • apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts: split the read of the persisted profile from applying it. startBoot() reads and applies before getConnection(). setPrimaryGateway waits until the profile is known. Event tagging uses the adopted profile. applyPrimaryProfile() aligns the registry's active pointer.
  • apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx: an ordering test (the persisted atlas must be applied before getConnection(), and the profile IPC must precede the connection IPC) plus an open-state test (after the socket opens on a named-primary boot, $gatewayState must publish 'open').

How to Test

  1. cd apps/desktop && npx vitest run --project ui src/app/gateway/hooks/use-gateway-boot.test.tsx passes 7 tests.
  2. Red-on-old for the ordering test: overlay only the test file onto main and run it. One test fails, six pass. The mocked getConnection asserts the atom already carries the persisted profile; on old code adoption runs after connect, so the assertion rejects inside getConnection. The runner reports expected "vi.fn()" to be called once, but got 0 times on profileGet, the downstream trace of that rejection.
  3. The open-state test passes on old main by construction. It exists to guard the hazard described in change 2. I verified it by mutation: the fix with the ensureGatewayForProfile() line deleted fails it with expected 'idle' to be 'open'.
  4. Manual: put a named profile in the desktop's active-profile.json, cold-launch, and confirm the first sidebar population belongs to that profile and the app reaches "open".

Checklist

Code

Documentation & Housekeeping

  • Docs: N/A (bug fix, no user-facing surface change)
  • cli-config.yaml.example: N/A
  • CONTRIBUTING/AGENTS: N/A
  • Cross-platform: renderer and Electron TypeScript only, no platform-specific code paths
  • Tool descriptions/schemas: N/A

Screenshots / Logs

Instrumented boot on unpatched main, persisted primary atlas:

profileAtGetConnection = "default"    (atom still at initial value when the connection is obtained)
profileGetCalls = 1                   (resolves only after getConnection)
gatewayState = "open"                 (ready is published while adoption is still pending)

With this PR the atom carries atlas before getConnection() runs, and the open transition still publishes.

Cold boot published gatewayReady before adoptPrimaryProfile()'s IPC read
returned: the shared gateway client fires setState('open') before
connect() resolves, while boot only awaited the profile adoption
afterwards. Any profile-scoped fetch triggered by gatewayReady (sessions,
projects) therefore raced the $activeGatewayProfile atom at its initial
'default' -- on a machine whose primary profile is a named one, the first
sidebar population was scoped to the wrong profile. Same ordering family
as NousResearch#50529.

Fix: split the persisted-profile read out of adoptPrimaryProfile() and
apply it before desktop.getConnection(), so the atom, the registry's
primaryProfile, and event tagging all carry the persisted profile before
any connection state can be observed. Applying the profile also aligns
the registry's active pointer (ensureGatewayForProfile fast path):
without that, state events tagged with the adopted primary fail the
activeKey match in reportGatewayState, the 'open' transition is dropped,
and the UI hangs at 'connecting'. HMR-survivor adoption and the
catch->default fallback are preserved.

Tests: the boot-ordering test asserts the profile IPC resolves and the
atom carries the persisted profile before getConnection(), and that the
gateway still publishes 'open' after the socket opens (regression for
the dropped-open hang).
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation labels Jul 31, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for carrying the saved-primary-profile boot ordering fix forward from #50529. The premise remains valid on current main: cold boot calls desktop.getConnection() before profile adoption (apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:470,499), while the profile atom controls REST routing and sidebar scope (apps/desktop/src/store/profile.ts:174-186,329-331).

The proposed pre-connect applyPrimaryProfile() is consistent with the registry contract: primary state only publishes when its profile matches activeKey, and the primary fast path in ensureGatewayForProfile() synchronously calls setActive (apps/desktop/src/store/gateway.ts:139-160,271-278). The ordering test also directly addresses the gap identified in #50529's review.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 31, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Graph note (no action implied — a maintainer has already reviewed this thread).

Our triage graph places this PR in a complex with 3 related pull requests (#44443, #50529, #62624). They were checked against each other at the diff level and no consolidation is indicated — they address distinct causes.

Full neighbourhood: https://hermes-triage.gottz.de/?node=75112

This note exists so the relationship stays discoverable from the thread itself.

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/*) needs-decision Awaiting maintainer decision before any implementation 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.

4 participants