Skip to content

fix(desktop): isolate New Window from remembered sessions - #69815

Open
kyledempster7 wants to merge 1 commit into
NousResearch:mainfrom
kyledempster7:fix/desktop-new-window-session-isolation
Open

fix(desktop): isolate New Window from remembered sessions#69815
kyledempster7 wants to merge 1 commit into
NousResearch:mainfrom
kyledempster7:fix/desktop-new-window-session-isolation

Conversation

@kyledempster7

@kyledempster7 kyledempster7 commented Jul 23, 2026

Copy link
Copy Markdown

Fixes #65601.\n\nNew Window created a full peer renderer at the root route, but that renderer shares Chromium local storage with the primary window. Its cold-start restore therefore reopened the primary window's remembered route/session.\n\nThis marks peer New Window URLs as fresh instances and skips both the initial remembered-route write and cold-start restore only for that launch. Once the new window creates or opens a real chat, normal remembered-session behavior resumes.\n\nValidation run from apps/desktop:\n- npm run typecheck\n- npm run lint: 0 errors; 13 existing warnings\n- npm run test:desktop:platforms: 697 checks passed\n- npm run test:ui\n- focused Electron URL and Desktop restore regression tests

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Jul 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #66270, #67049, and #67091 are open Desktop new-window/session-restore repairs. This patch has overlapping fresh-window intent but a distinct guard arrangement; maintainers should choose or consolidate.

@teknium1 teknium1 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.

Thanks for isolating the peer-window lifecycle. The premise remains valid on current main: apps/desktop/electron/main.ts:8767 loads an unmarked root URL, and apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts:94-115 restores shared remembered state from that route.

Problems

  • The new test at apps/desktop/src/app/contrib/hooks/use-desktop-integrations.test.ts:5-22 only invokes shouldRestoreRememberedRoute; it does not prove the hook avoids remembered-state reads, writes, or navigation.
  • This branch conflicts with newer profile-scoped remembered-state code (apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts:74-85,102-114). Salvage must preserve those profile arguments while adding the fresh-instance guard.

Suggested changes

  • Add a hook-level regression that asserts a ?win=instance root launch performs no remembered-state access or navigation, then assert a real routed chat persists normally.

Automated hermes-sweeper review.


it("never restores another window's remembered chat into a new peer instance", () => {
expect(shouldRestoreRememberedRoute({ freshInstanceWindow: true, locationPathname: '/', restored: false })).toBe(
false

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.

Please add a hook-boundary regression alongside this helper test: mount useDesktopIntegrations for a fresh root instance and assert it neither reads nor writes remembered state and does not navigate. This helper assertion alone cannot catch a misplaced or removed effect guard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history 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-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.

[Bug]: Desktop new session window reuses existing agent context — no session isolation

3 participants