Skip to content

fix(desktop): scope restored navigation by profile (#67709) - #67823

Closed
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/67709-profile-scoped-session-restore
Closed

fix(desktop): scope restored navigation by profile (#67709)#67823
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/67709-profile-scoped-session-restore

Conversation

@Tranquil-Flow

Copy link
Copy Markdown
Contributor

Closes #67709

What changed

  • scope remembered desktop route and session keys by the encoded active profile
  • discard ambiguous legacy global navigation keys instead of assigning them to an arbitrary profile
  • delay cold-start restoration until the primary profile and session list have reached renderer.ready
  • preserve explicit deep-link and hidden-window destinations
  • restore and persist session routes only when a direct ID or lineage root is explicitly owned by the active profile
  • clear both profile-scoped route and session state after resume exhaustion

Why

The previous global keys could restore one profile's session into another. Restoration also raced primary-profile adoption, while the initial / route could overwrite remembered history before the correct profile was ready.

Verification

  • RED proof on unmodified upstream/main: 6 failed, 26 passed with the two regression files copied over unchanged
  • focused regression suite: 32 passed
  • TypeScript renderer + Electron typecheck: passed
  • ESLint and Prettier on all five changed files: passed
  • production desktop build, including assert-dist-built: passed
  • full desktop UI suite: 1,632 passed, 1 skipped, 1 unrelated order-sensitive failure; immediate isolated rerun of that unrelated file passed 25/25
  • git diff --check: passed
  • branch shape: exactly 1 commit ahead of current upstream/main

Related PR audit

Timeline-linked PRs #48473 and #59608 address profile forwarding to secondary/detached windows, not primary-window remembered navigation. PR #58138 contains runtime profile-switch session memory but does not repair the global localStorage keys or the boot/profile-adoption race. No open PR references #67709, and no same-author duplicate exists.

Auto-published by Moonsong via Path B automated pipeline.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Implements #67709. Related to open #60202, #63590, and #66672: each overlaps profile/session restoration but has a distinct restoration scope or state-layer change.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The underlying issue remains on current main: apps/desktop/src/store/session.ts:75-78 still keeps remembered routes global, while apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts:73-83 persists routes and unresolved routed sessions without the PR's ownership gate.

Problems

  • Current main contains the later partial fix 143942d497 for remembered session IDs. It modifies the same session store, integration hook, and store tests, so this needs manual conflict resolution rather than a clean cherry-pick.

Suggested changes

  • Preserve the current per-profile session behavior while integrating this PR's route scoping, renderer.ready restore gate, and explicit ownership validation; reconcile the incompatible legacy-key policies deliberately.

Automated hermes-sweeper review.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Fifteen PRs address or reference this Desktop issue complex; their diffs cover pin hydration, transcript virtualization, queue affinity and migration, messaging polling, stale-session recovery, new-window isolation, profile-scoped restoration, and composer/session races. For #67709, merged #74277 supplies route-key scoping, while recorded best fix #67823 additionally gates restoration on profile readiness and validates active-profile session ownership.

Related pull requests

Duplicates

#66270 is substantially duplicated by #67049, which has the stronger hook-boundary regression; #70610 was superseded by #70986; #60607 and #68181 overlap on stale cold-boot fallback, but #60607 uses the stronger terminal-resume signal; #67823 overlaps #74277 on route scoping while retaining distinct profile-readiness and ownership-validation work.

Suggested consolidation

Author action: rebase #67823 onto main, or split out the part that can merge, preserving the current per-profile session behavior from 143942d and route scoping from #74277 while carrying forward the renderer.ready restore gate, explicit active-profile ownership validation, and profile-scoped exhaustion cleanup. Close #66270 as duplicate of #67049, keep #70610 closed as superseded by #70986, and handle the remaining PRs in their distinct pin, polling, stale-session, window-isolation, composer-race, or queue-migration tracks.

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
    I67709(["issue #67709 (open)"])
    P67823["PR #67823 (open)"]
    P67823 -->|best fix| I67709
    class I67709 open
    class P67823 open
    class P67823 best
    class P67823 target
    click I67709 "https://github.com/NousResearch/hermes-agent/issues/67709"
    click P67823 "https://github.com/NousResearch/hermes-agent/pull/67823"
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 15 pull requests and 21 issues in this complex. Each diff was read against this issue; Assessment working set: 192 kB of PR diffs, 82 kB of issue/PR text, 53 kB of discussion (62 comments), 60 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@monerostar monerostar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Native Windows multi-profile live check on this PR (monerostar).

Env: Win11 10.0.26200 · Hermes Agent v0.20.0 (2026.8.3) · install tree %LOCALAPPDATA%\hermes\hermes-agent · Git Bash · head d491170f0 vs current origin/main f5be9236e

What still bites on current main (this box)

  • Cold-start restore in use-desktop-integrations still fires before primary profile adopt (profileReady / renderer.ready not gated).
  • default still uses the unsuffixed hermes.desktop.lastRoute / lastSessionId keys. On a tech-primary Desktop, we watched that global key hold an old tech session id and reopen it on every tech login until cleared/archived.
  • Fleet here: 6 profiles with state.db (main/tech/house/lifestyle/x/play-free-win). Sticky primary is tech via active-profile.json.

PR unit tests on this host
Swapped the PR’s 5 files into the install apps/desktop tree (worktree has no node_modules), ran:

npm run test -- --run src/app/contrib/hooks/use-desktop-integrations.test.tsx src/store/session.test.ts

Result: 32/32 passed (2 files). Install sources restored after.

Rebase status
git cherry-pick --no-commit d491170f0 onto current origin/main conflicts in:

  • apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts
  • apps/desktop/src/app/contrib/wiring.tsx
  • apps/desktop/src/store/session.ts

Main already has a partial per-profile key scheme (lastRoute.<profile>, unsuffixed for default). This PR’s stronger shape (.profile.<encoded>, discard legacy globals, wait for profile ready, sessionBelongsToProfile) is still the right end state for #67709, but needs a clean rebase before merge.

Verdict: direction LGTM for the lived Windows multi-profile sticky-restore class; blocking only on rebase/conflict resolution, not on the approach. Happy to re-run the 32 after a refreshed tip.

@Tranquil-Flow
Tranquil-Flow force-pushed the fix/67709-profile-scoped-session-restore branch from d491170 to 11669f3 Compare August 4, 2026 21:40

@monerostar monerostar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick delta on our 15:33 note: the 08-04 rebase landed the blocking item we flagged. GitHub now reports CLEAN/MERGEABLE and all CI is green including Desktop E2E, so the three-file conflict on our earlier cherry-pick test is resolved.

Both main partials are confirmed ancestors (143942d session-id scoping, #74277 route scoping merge b84389c), so this PR's delta is the collision-proof .profile.<encoded> namespace, the legacy-global discard, and the sessionBelongsToProfile ownership gate. Still the right end state for #67709 from the Windows side.

One more for reviewers: #69572 (clear stale chat before profile switch) is a separate open PR on the sibling symptom, different mechanism, no diff overlap.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvaged as #79142 — rebased cleanly onto current with zero conflicts (the reported conflicts were from a dirty working tree, not actual content divergence; the PR's base and current main have identical content in all 5 affected files).

The salvage also folds three findings from /simplify-code review:

  1. Non-reactive profileReady gate (HIGH): \.get() in wiring.tsx didn't subscribe, so profileReady could stay false in production. Fixed with useStore(\).
  2. Non-reactive activeProfile (MEDIUM): \.get() replaced with the already-subscribed activeGatewayProfile value.
  3. Redundant localStorage reads (HIGH): discardLegacyRememberedNavigation() was called on every get/set, causing 12+ sync localStorage reads per effect run. Gated with a module-level flag + storage event listener for cross-window safety.

All credit to @Tranquil-Flow for the original implementation. Closing in favor of #79142.

@Tranquil-Flow

Copy link
Copy Markdown
Contributor Author

Closed via salvage #79142 — items satisfied. kshitijk4poor merged #79142 (commit 8770754) on Aug 5 with the same profile-scoped restoration semantics (active-profile ownership, legacy global-key discard, profile-ready gate before restore, profile-scoped session/route storage). Tranquil-Flow author preserved on the merged commit.

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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

Desktop cold-start restore can resume a session into the wrong profile and create duplicate session IDs

6 participants