Skip to content

fix(desktop): gate session restore on profile initialization - #74388

Open
DavidMetcalfe wants to merge 2 commits into
NousResearch:mainfrom
DavidMetcalfe:fix/session-restore-profile-race
Open

fix(desktop): gate session restore on profile initialization#74388
DavidMetcalfe wants to merge 2 commits into
NousResearch:mainfrom
DavidMetcalfe:fix/session-restore-profile-race

Conversation

@DavidMetcalfe

@DavidMetcalfe DavidMetcalfe commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

After c4212b945 scoped getRememberedRoute/setRememberedRoute to per-profile localStorage keys, the cold-start restore effect in use-desktop-integrations.ts still reads $activeGatewayProfile.get() inside the effect body without subscribing to it. On mount, $activeGatewayProfile is 'default' (its initial value). The real profile is set asynchronously by adoptPrimaryProfile(). The restore effect fires first, reads from the global unsuffixed key hermes.desktop.lastRoute, and finds the stale value from before per-profile scoping was introduced. The write side correctly writes to the scoped key — but that key is never read at startup, so every restart restores the stale global key.

The fix

A $profileInitialized atom (initialized false, set true in adoptPrimaryProfile / adoptBoot after the real profile is resolved) gates the restore effect. The restore waits until the profile is known, then reads from the correct scoped key.

Simply adding $activeGatewayProfile to the dependency array is insufficient — the first render still runs with 'default', and restoredRef blocks the second run with the correct profile. A separate readiness signal is needed.

Changes

  • store/profile.ts: Add $profileInitialized atom
  • app/gateway/hooks/use-gateway-boot.ts: Set $profileInitialized after profile resolution in adoptPrimaryProfile() (finally block) and adoptBoot()
  • app/contrib/hooks/use-desktop-integrations.ts: Subscribe to $profileInitialized via useStore, gate the restore effect on profileInitialized, add to dependency array

Reviewer Notes

  • The restoredRef logic is adjusted: when !profileInitialized, we return WITHOUT setting restoredRef.current = true, so the effect re-runs when profileInitialized becomes true. The original behavior (set restoredRef when not at NEW_CHAT_ROUTE) is preserved.
  • For the default profile, rememberedRouteKey('default') returns the global unsuffixed key — the same key used before scoping. No behavioral change for single-profile users.
  • The adoptBoot() HMR path also sets $profileInitialized to cover the case where the Desktop window persists across a hot-reload update.
  • The finally block in adoptPrimaryProfile guarantees $profileInitialized is set even when the IPC call throws or profile setup fails — the error path falls back to 'default' in the catch, and the finally unblocks the restore effect regardless.

Closes #74387

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) area/sessions Session lifecycle, resume, persistence, history area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 29, 2026
@DavidMetcalfe
DavidMetcalfe force-pushed the fix/session-restore-profile-race branch from d729bc7 to 7128f77 Compare July 29, 2026 21:31
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the profile-scoped storage read through the renderer boot sequence. The race is present on current main: useDesktopIntegrations sets restoredRef and reads $activeGatewayProfile.get() at apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts:101-103, while adoptPrimaryProfile() awaits IPC before setting that profile at apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:238-247.

Problems

  • This change adds no regression coverage for the async ordering. Current tests in apps/desktop/src/store/session.test.ts:521-562 prove profile-key selection only; they do not exercise the restore effect that is racing here.

Suggested changes

  • Add a hook/component test that begins on NEW_CHAT_ROUTE with default selected, resolves a named profile after mount, and verifies restoration reads the named profile's remembered route.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
After c4212b9 scoped remembered routes to per-profile localStorage
keys, the cold-start restore effect reads $activeGatewayProfile.get()
inside the effect body without subscribing. On mount the atom is
'default' (initial value), while the real profile is set asynchronously
by adoptPrimaryProfile(). The restore fires first, reads the global
unsuffixed key hermes.desktop.lastRoute, and finds the stale value from
before per-profile scoping was introduced.

Add $profileInitialized — set true after adoptPrimaryProfile (finally)
and adoptBoot resolve the real profile — and gate the restore effect on
it. The restoredRef logic is adjusted so a not-yet-initialized profile
does not consume the ref, allowing the effect to retry once the flag
lifts. Single-profile (default) users are unaffected: the global key
IS the correct key when the profile is default.
…tion

Regression coverage for the async ordering the fix gates: the restore
effect must not read $activeGatewayProfile until adoptPrimaryProfile()
resolves the real profile, or it restores the stale global localStorage
keys instead of the named profile's scoped route/session.
@DavidMetcalfe
DavidMetcalfe force-pushed the fix/session-restore-profile-race branch from 7128f77 to 578e5cd Compare August 5, 2026 06:39
@DavidMetcalfe

Copy link
Copy Markdown
Contributor Author

@teknium1 Good catch — the PR shipped without regression coverage for the async ordering, and the existing session.test.ts cases only prove key selection, not the restore effect's behavior. I've added apps/desktop/src/app/contrib/hooks/use-desktop-integrations.test.tsx with three cases:

  1. Cold start at NEW_CHAT_ROUTE with the placeholder profile: no navigation until the profile resolves, then restores the named profile's scoped route (hermes.desktop.lastRoute.atlas) — not the stale global key.
  2. Profile already resolved at mount (HMR survivor path): restore runs immediately and reads the named profile's scoped session id, not the stale global one.
  3. Window started away from the new-chat route: never restores, even after profile resolution.

Mutation-verified: removing the !profileInitialized gate from the effect makes case 1 fail (it navigates to the stale global value), confirming the test exercises the race. All 3 pass with the fix; tsc --noEmit is clean and the sibling use-gateway-boot + session suites pass. Also rebased onto current main — the PR had drifted into CONFLICTING.

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/*) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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 forgets last session on restart after per-profile route scoping

3 participants