From 11669f31d610c299987d730c86040032f7901745 Mon Sep 17 00:00:00 2001 From: Tranquil-Flow Date: Tue, 4 Aug 2026 23:19:58 +0200 Subject: [PATCH] fix(desktop): scope restored navigation by profile (#67709) --- .../hooks/use-desktop-integrations.test.tsx | 413 ++++++++++++++++++ .../contrib/hooks/use-desktop-integrations.ts | 134 +++--- apps/desktop/src/app/contrib/wiring.tsx | 7 +- apps/desktop/src/store/session.test.ts | 82 +++- apps/desktop/src/store/session.ts | 76 +++- 5 files changed, 630 insertions(+), 82 deletions(-) create mode 100644 apps/desktop/src/app/contrib/hooks/use-desktop-integrations.test.tsx diff --git a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.test.tsx b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.test.tsx new file mode 100644 index 000000000000..dd0bf5941769 --- /dev/null +++ b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.test.tsx @@ -0,0 +1,413 @@ +import { renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { SessionInfo } from '@/types/hermes' + +import { useDesktopIntegrations } from './use-desktop-integrations' + +// Pure-jsdom localStorage (no nanostores persistence module needed — the +// production functions write directly to window.localStorage through the +// persistString/storedString helpers in @/lib/storage, which in jsdom resolves +// to the real localStorage global). +// We import the hook and drive it with explicit rx-stores/props to exercise the +// profile-ready gate, ownership validation, and legacy-key discard. + +const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] } +const initialHermesDesktop = desktopWindow.hermesDesktop + +const session = (over: Partial = {}): SessionInfo => ({ + archived: false, + cwd: null, + ended_at: null, + id: 'live', + input_tokens: 0, + is_active: false, + last_active: 0, + message_count: 0, + model: null, + output_tokens: 0, + preview: null, + source: null, + started_at: 0, + title: null, + tool_call_count: 0, + ...over +}) + +describe('useDesktopIntegrations', () => { + let navigate: ReturnType void>> + + beforeEach(() => { + window.localStorage.clear() + navigate = vi.fn() + + // Stub the desktop bridge so the hook's useEffect callbacks don't try to + // reach real Electron IPC. The established desktop-test pattern assigns a + // plain object to window.hermesDesktop rather than using vi.spyOn. + desktopWindow.hermesDesktop = { + setPreviewShortcutActive: vi.fn(), + onOpenUpdatesRequested: vi.fn(), + onFocusSession: vi.fn(), + onNotificationAction: vi.fn(), + onDeepLink: vi.fn(), + signalDeepLinkReady: vi.fn(), + onClosePreviewRequested: vi.fn(), + onOpenFolderRequested: vi.fn() + } as unknown as Window['hermesDesktop'] + }) + + afterEach(() => { + if (initialHermesDesktop) { + desktopWindow.hermesDesktop = initialHermesDesktop + } + + vi.restoreAllMocks() + }) + + function render({ + activeProfile = 'default', + locationPathname = '/', + profileReady = false, + resumeExhaustedSessionId = null as string | null, + routedSessionId = null as string | null, + sessions = [] as readonly SessionInfo[] + } = {}) { + return renderHook( + ({ + activeProfile, + locationPathname, + profileReady, + resumeExhaustedSessionId, + routedSessionId, + sessions + }: { + activeProfile: string + locationPathname: string + profileReady: boolean + resumeExhaustedSessionId: string | null + routedSessionId: string | null + sessions: readonly SessionInfo[] + }) => + useDesktopIntegrations({ + activeProfile, + chatOpen: false, + hasPreview: false, + locationPathname, + navigate, + profileReady, + refreshSessions: vi.fn(), + resumeExhaustedSessionId, + routedSessionId, + runtimeIdByStoredSessionId: { current: new Map() }, + sessions + }), + { + initialProps: { + activeProfile, + locationPathname, + profileReady, + resumeExhaustedSessionId, + routedSessionId, + sessions + } + } + ) + } + + describe('profile-ready gate', () => { + it('does NOT restore before profileReady is true', () => { + // Set remembered state, but profileReady=false. + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/remembered-session') + window.localStorage.setItem('hermes.desktop.lastSessionId.profile.default', 'remembered-session') + + render({ profileReady: false }) + + // no navigation should have occurred + expect(navigate).not.toHaveBeenCalled() + }) + + it('restores on profileReady when remembered route exists and owns the session', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/remembered-session') + + const sessions = [session({ id: 'remembered-session', profile: 'default' })] + + render({ profileReady: true, sessions }) + + expect(navigate).toHaveBeenCalledWith('/remembered-session', { replace: true }) + }) + + it('restores remembered session id when no remembered route exists', () => { + window.localStorage.setItem('hermes.desktop.lastSessionId.profile.default', 'remembered-session') + + const sessions = [session({ id: 'remembered-session', profile: 'default' })] + + render({ profileReady: true, sessions }) + + // sessionRoute('remembered-session') = '/remembered-session' + expect(navigate).toHaveBeenCalledWith('/remembered-session', { replace: true }) + }) + + it('waits for sessions before validating a remembered session route', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/remembered-session') + + const result = render({ profileReady: true, sessions: [] }) + + expect(navigate).not.toHaveBeenCalled() + expect(window.localStorage.getItem('hermes.desktop.lastRoute.profile.default')).toBe('/remembered-session') + + result.rerender({ + activeProfile: 'default', + locationPathname: '/', + profileReady: true, + resumeExhaustedSessionId: null, + routedSessionId: null, + sessions: [session({ id: 'remembered-session', profile: 'default' })] + }) + + expect(navigate).toHaveBeenCalledWith('/remembered-session', { replace: true }) + }) + }) + + describe('ownership validation', () => { + it('refuses to restore a session route owned by another profile', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/ai-session') + + const sessions = [session({ id: 'ai-session', profile: 'ai-engineer' })] + + // The route belongs to ai-engineer; active profile is default. + // No navigation should happen — wrong owner. + render({ activeProfile: 'default', profileReady: true, sessions }) + + expect(navigate).not.toHaveBeenCalled() + }) + + it('refuses to restore a session id owned by another profile', () => { + window.localStorage.setItem('hermes.desktop.lastSessionId.profile.default', 'ai-session') + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/ai-session') + + const sessions = [session({ id: 'ai-session', profile: 'ai-engineer' })] + + render({ activeProfile: 'default', profileReady: true, sessions }) + + // Both route and fallback session id are owned by another profile. + expect(navigate).not.toHaveBeenCalled() + }) + + it('clears stale remembered route owned by wrong profile', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.ai-engineer', '/ai-session') + + const sessions = [session({ id: 'ai-session', profile: 'ai-engineer' })] + + render({ activeProfile: 'ai-engineer', profileReady: true, sessions }) + + // The route and session match the active profile — should restore. + expect(navigate).toHaveBeenCalledWith('/ai-session', { replace: true }) + }) + }) + + describe('two profiles with distinct sessions', () => { + it('restores profile A session when profile A is active', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.coder', '/coder-session') + + const sessions = [ + session({ id: 'coder-session', profile: 'coder' }), + session({ id: 'ops-session', profile: 'ops' }) + ] + + render({ activeProfile: 'coder', profileReady: true, sessions }) + + expect(navigate).toHaveBeenCalledWith('/coder-session', { replace: true }) + }) + + it('does NOT bleed profile A session into profile B', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.coder', '/coder-session') + + const sessions = [session({ id: 'coder-session', profile: 'coder' })] + + // ops profile is active but has no own remembered route + render({ + activeProfile: 'ops', + profileReady: true, + sessions + }) + + // No navigation — coder's remembered route doesn't belong to ops. + expect(navigate).not.toHaveBeenCalled() + }) + }) + + describe('legacy key behavior', () => { + it('discards legacy global keys on read and does NOT restore from them', () => { + // Simulate a pre-per-profile install. + window.localStorage.setItem('hermes.desktop.lastSessionId', 'legacy-session') + window.localStorage.setItem('hermes.desktop.lastRoute', '/session/legacy-session') + + // Profile contexts without matching sessions. + const sessions = [session({ id: 'legacy-session', profile: 'default' })] + + render({ profileReady: true, sessions }) + + // Legacy keys must be discarded. + expect(window.localStorage.getItem('hermes.desktop.lastSessionId')).toBeNull() + expect(window.localStorage.getItem('hermes.desktop.lastRoute')).toBeNull() + + // And no navigation should happen (the per-profile keys were empty). + expect(navigate).not.toHaveBeenCalled() + }) + }) + + describe('stale-result suppression during profile switch', () => { + it('remembers route for the new profile after switch, not the old one', () => { + const sessions = [ + session({ id: 'coder-session', profile: 'coder' }), + session({ id: 'ops-session', profile: 'ops' }) + ] + + // Render with coder active and navigate to a session. + const { rerender } = render({ + activeProfile: 'coder', + locationPathname: '/coder-session', + profileReady: true, + routedSessionId: 'coder-session', + sessions + }) + + // The coder session should be persisted under coder's key. + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.coder')).toBe('coder-session') + + // Now switch to ops. + rerender({ + activeProfile: 'ops', + locationPathname: '/ops-session', + profileReady: true, + resumeExhaustedSessionId: null, + routedSessionId: 'ops-session', + sessions + }) + + // The ops session should now be persisted under ops's key. + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.ops')).toBe('ops-session') + + // Coder's remembered session should still be there. + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.coder')).toBe('coder-session') + }) + + it('does NOT overwrite remembered state when session ownership fails validation', () => { + // Simulate an async restore result arriving for a route that doesn't + // own the active profile. + const sessions = [session({ id: 'coder-session', profile: 'coder' })] + + // Active profile is ops, but the routed session belongs to coder. + render({ + activeProfile: 'ops', + locationPathname: '/', + profileReady: true, + routedSessionId: 'coder-session', // wrong profile! + sessions + }) + + // No session should be remembered for the active profile. + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.ops')).toBeNull() + }) + }) + + describe('route-scoped restoration', () => { + it('restores a non-session route like /skills', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/skills') + + const sessions = [session({ id: 'some-session', profile: 'default' })] + + render({ profileReady: true, sessions }) + + // /skills is not a session route — no ownership validation needed. + expect(navigate).toHaveBeenCalledWith('/skills', { replace: true }) + }) + + it('does NOT restore overlay routes (settings/command-center)', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/settings') + + render({ profileReady: true, sessions: [] }) + + // Overlay routes should not be restored. + expect(navigate).not.toHaveBeenCalled() + }) + + it('does NOT persist overlay routes for next boot', () => { + const { rerender } = render({ + activeProfile: 'default', + locationPathname: '/settings', + profileReady: true, + routedSessionId: null, + sessions: [] + }) + + // Remembering effect fires on route change. + rerender({ + activeProfile: 'default', + locationPathname: '/settings', + profileReady: true, + resumeExhaustedSessionId: null, + routedSessionId: null, + sessions: [] + }) + + // Overlay routes must NOT be persisted. + expect(window.localStorage.getItem('hermes.desktop.lastRoute.profile.default')).toBeNull() + }) + }) + + describe('exhausted session cleanup', () => { + it('clears remembered session id when the exhausted session matches', () => { + window.localStorage.setItem('hermes.desktop.lastSessionId.profile.default', 'exhausted') + + const sessions = [session({ id: 'exhausted', profile: 'default' })] + + render({ + profileReady: true, + resumeExhaustedSessionId: 'exhausted', + sessions + }) + + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.default')).toBeNull() + }) + + it('clears remembered route when it carries the exhausted session', () => { + window.localStorage.setItem('hermes.desktop.lastRoute.profile.default', '/exhausted') + + const sessions = [session({ id: 'exhausted', profile: 'default' })] + + render({ + profileReady: true, + resumeExhaustedSessionId: 'exhausted', + sessions + }) + + expect(window.localStorage.getItem('hermes.desktop.lastRoute.profile.default')).toBeNull() + }) + + it('does NOT clear exhausted when profileReady is false', () => { + window.localStorage.setItem('hermes.desktop.lastSessionId.profile.default', 'exhausted') + + render({ + profileReady: false, + resumeExhaustedSessionId: 'exhausted', + sessions: [] + }) + + // profileReady=false gates the cleanup effect. + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.default')).toBe('exhausted') + }) + + it('does NOT clear remembered state when exhausted id does not match', () => { + window.localStorage.setItem('hermes.desktop.lastSessionId.profile.default', 'other-session') + + render({ + profileReady: true, + resumeExhaustedSessionId: 'exhausted', + sessions: [session({ id: 'other-session', profile: 'default' })] + }) + + expect(window.localStorage.getItem('hermes.desktop.lastSessionId.profile.default')).toBe('other-session') + }) + }) +}) diff --git a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts index cff5d86f4d54..6a6638b1d1f2 100644 --- a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts +++ b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts @@ -4,32 +4,36 @@ import { closeActiveTab } from '@/app/chat/close-tab' import { openSession } from '@/app/open-session' import { storedSessionIdForNotification } from '@/lib/session-ids' import { respondToApprovalAction } from '@/store/native-notifications' -import { $activeGatewayProfile } from '@/store/profile' import { openFolderAsProject } from '@/store/projects' import { - $sessions, getRememberedRoute, getRememberedSessionId, - rememberedSessionProfile, + sessionBelongsToProfile, setRememberedRoute, setRememberedSessionId } from '@/store/session' import { onSessionsChanged } from '@/store/session-sync' import { openUpdatesWindow, startUpdatePoller, stopUpdatePoller } from '@/store/updates' import { isSecondaryWindow } from '@/store/windows' +import type { SessionInfo } from '@/types/hermes' import { requestComposerFocus, requestComposerInsert } from '../../chat/composer/focus' -import { appViewForPath, isOverlayView, NEW_CHAT_ROUTE, sessionRoute } from '../../routes' +import { appViewForPath, isOverlayView, NEW_CHAT_ROUTE, routeSessionId, sessionRoute } from '../../routes' + +type RememberedSession = Pick interface DesktopIntegrationsParams { + activeProfile: string chatOpen: boolean hasPreview: boolean locationPathname: string navigate: (to: string, options?: { replace?: boolean }) => void + profileReady: boolean refreshSessions: () => Promise | unknown resumeExhaustedSessionId: null | string routedSessionId: null | string runtimeIdByStoredSessionId: { readonly current: Map } + sessions: readonly RememberedSession[] } /** @@ -40,12 +44,15 @@ interface DesktopIntegrationsParams { * "talks to the desktop shell" surface reads as one unit. */ export function useDesktopIntegrations({ + activeProfile, locationPathname, navigate, + profileReady, refreshSessions, resumeExhaustedSessionId, routedSessionId, - runtimeIdByStoredSessionId + runtimeIdByStoredSessionId, + sessions }: DesktopIntegrationsParams): void { // Update polling — populates $desktopVersion/$updateStatus, which feed the // statusbar version pill and the update toasts. Also honors the main @@ -67,66 +74,95 @@ export function useDesktopIntegrations({ window.hermesDesktop?.setPreviewShortcutActive?.(true) }, []) - // Remember the open chat (session id for notifications/resume) AND the last - // non-overlay route (a page like /skills, or a session route) so a relaunch - // lands where you were. Overlays (settings/command-center/…) aren't stored — - // you don't want to boot into a modal. - useEffect(() => { - const routeProfile = rememberedSessionProfile($sessions.get(), routedSessionId, $activeGatewayProfile.get()) - - if (routedSessionId) { - setRememberedSessionId(routedSessionId, routeProfile) - } - - if (!isOverlayView(appViewForPath(locationPathname))) { - // Keyed by the same owner as the id above: a session route embeds a - // session id, so remembering it globally would restore another profile's - // conversation on cold start. - setRememberedRoute(locationPathname, routeProfile) - } - }, [locationPathname, routedSessionId]) - const restoredRef = useRef(false) - // Restore once on cold start — only when the renderer booted at the default - // route (a hidden-then-shown window keeps its own route). Prefer the full - // remembered route (covers pages); fall back to the last session id. - // eslint-disable-next-line no-restricted-syntax -- legitimate non-atom ref write (see eslint rule comment) + // Wait until boot has adopted the primary profile, then restore that profile's + // navigation exactly once. The same effect owns subsequent writes so the + // initial `/` cannot overwrite remembered history before it is read. + // This ref is a one-time lifecycle latch, not a mirror of reactive atom state. + // eslint-disable-next-line no-restricted-syntax useEffect(() => { - if (restoredRef.current || locationPathname !== NEW_CHAT_ROUTE) { - restoredRef.current = true - + if (!profileReady) { return } - restoredRef.current = true - const activeProfile = $activeGatewayProfile.get() - const route = getRememberedRoute(activeProfile) - - if (route && route !== NEW_CHAT_ROUTE && !isOverlayView(appViewForPath(route))) { - navigate(route, { replace: true }) - - return + if (!restoredRef.current) { + // Only cold-start navigation at the default route is replaceable; a deep + // link or hidden-then-shown window keeps its explicit destination. + if (locationPathname === NEW_CHAT_ROUTE) { + const route = getRememberedRoute(activeProfile) + const routeSession = route ? routeSessionId(route) : null + const last = getRememberedSessionId(activeProfile) + + const restorableNonSessionRoute = + !!route && route !== NEW_CHAT_ROUTE && !routeSession && !isOverlayView(appViewForPath(route)) + + // Boot adoption can publish renderer.ready before its async session + // refresh completes. Keep the restore latch open until ownership can be + // decided; treating an unloaded list as authoritative would erase valid + // remembered navigation permanently. + if (sessions.length === 0 && !restorableNonSessionRoute && (routeSession || last)) { + return + } + + restoredRef.current = true + + if ( + route && + route !== NEW_CHAT_ROUTE && + !isOverlayView(appViewForPath(route)) && + (!routeSession || sessionBelongsToProfile(sessions, routeSession, activeProfile)) + ) { + navigate(route, { replace: true }) + + return + } + + // A remembered route carried a session id we can no longer validate — + // clear the stale entry so the next cold start won't re-try it. + if (routeSession) { + setRememberedRoute(null, activeProfile) + } + + if (last && sessionBelongsToProfile(sessions, last, activeProfile)) { + navigate(sessionRoute(last), { replace: true }) + + return + } + + if (last) { + setRememberedSessionId(null, activeProfile) + } + } else { + restoredRef.current = true + } } - const last = getRememberedSessionId(activeProfile) - - if (last) { - navigate(sessionRoute(last), { replace: true }) + // Remember the open chat (session id for notifications/resume) AND the last + // non-overlay route (a page like /skills, or a session route) per profile. + // Session-shaped routes require an explicit matching owner; unresolved and + // wrong-profile rows must not replace known-safe navigation. + if (routedSessionId && sessionBelongsToProfile(sessions, routedSessionId, activeProfile)) { + setRememberedSessionId(routedSessionId, activeProfile) + setRememberedRoute(locationPathname, activeProfile) + } else if (!routedSessionId && !isOverlayView(appViewForPath(locationPathname))) { + setRememberedRoute(locationPathname, activeProfile) } - }, [locationPathname, navigate]) + }, [activeProfile, locationPathname, navigate, profileReady, routedSessionId, sessions]) useEffect(() => { - if (!resumeExhaustedSessionId) { + if (!profileReady || !resumeExhaustedSessionId) { return } - const owner = rememberedSessionProfile($sessions.get(), resumeExhaustedSessionId, $activeGatewayProfile.get()) + if (getRememberedSessionId(activeProfile) === resumeExhaustedSessionId) { + setRememberedSessionId(null, activeProfile) + } - if (getRememberedSessionId(owner) === resumeExhaustedSessionId) { - setRememberedSessionId(null, owner) + if (routeSessionId(getRememberedRoute(activeProfile) ?? '') === resumeExhaustedSessionId) { + setRememberedRoute(null, activeProfile) } - }, [resumeExhaustedSessionId]) + }, [activeProfile, profileReady, resumeExhaustedSessionId]) // Native-notification click -> jump to the session WHERE IT ALREADY IS (open // tile / main), else beside what's loaded rather than over it — the click diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index a39a16dd8615..47d24b88d17f 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -32,6 +32,7 @@ import { latestSessionTodos } from '@/lib/todos' import { activateWakeIndicator } from '@/lib/wake-indicator' import { playWakeSound } from '@/lib/wake-sound' import { $billingSettingsRequest } from '@/store/billing-block' +import { $desktopBoot } from '@/store/boot' import { requestVoiceConversationStart } from '@/store/composer' import { setCronFocusJobId } from '@/store/cron' import { $pinnedSessionIds, pinSession, restoreWorktree, unpinSession } from '@/store/layout' @@ -178,6 +179,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { const resumeExhaustedSessionId = useStore($resumeExhaustedSessionId) const selectedStoredSessionId = useStore($selectedStoredSessionId) const messagingSessions = useStore($messagingSessions) + const sessions = useStore($sessions) const activeGatewayProfile = useStore($activeGatewayProfile) const profileScope = useStore($profileScope) @@ -773,14 +775,17 @@ export function ContribWiring({ children }: { children: ReactNode }) { const previewTarget = useStore($previewTarget) useDesktopIntegrations({ + activeProfile: normalizeProfileKey($activeGatewayProfile.get()), chatOpen, hasPreview: Boolean(previewTarget), locationPathname: location.pathname, navigate, + profileReady: $desktopBoot.get().phase === 'renderer.ready', refreshSessions, resumeExhaustedSessionId, routedSessionId, - runtimeIdByStoredSessionId: runtimeIdByStoredSessionIdRef + runtimeIdByStoredSessionId: runtimeIdByStoredSessionIdRef, + sessions }) // Pin/unpin the selected session (statusbar keybind + chat header) — pinned diff --git a/apps/desktop/src/store/session.test.ts b/apps/desktop/src/store/session.test.ts index 415c167a195e..994c0ce08acb 100644 --- a/apps/desktop/src/store/session.test.ts +++ b/apps/desktop/src/store/session.test.ts @@ -17,6 +17,7 @@ import { mergeSessionPage, rememberedSessionProfile, resolveComposerSessionKey, + sessionBelongsToProfile, sessionPinId, setCurrentCwd, setRememberedRoute, @@ -517,15 +518,26 @@ describe('remembered session id (per profile)', () => { expect(getRememberedSessionId('research')).toBeNull() }) - it('keeps the default profile on the legacy unsuffixed key for back-compat', () => { + it('discards legacy unsuffixed keys on first read (zero-migration, refuse-to-guess)', () => { // An existing install remembered its session under the pre-per-profile key. localStorage.setItem('hermes.desktop.lastSessionId', 'legacy-session') - expect(getRememberedSessionId('default')).toBe('legacy-session') - // Absent/blank profile normalizes to the default key too. - expect(getRememberedSessionId(undefined)).toBe('legacy-session') - expect(getRememberedSessionId('')).toBe('legacy-session') - expect(getRememberedSessionId(null)).toBe('legacy-session') + // Reading from any profile discards the legacy key — ownership is unknowable. + expect(getRememberedSessionId('default')).toBeNull() + expect(getRememberedSessionId('coder')).toBeNull() + + // The legacy key must be cleared. + expect(localStorage.getItem('hermes.desktop.lastSessionId')).toBeNull() + }) + + it('uses encodeURIComponent so profile names with reserved chars are isolated', () => { + setRememberedSessionId('ops-session', 'research/ops') + + expect(getRememberedSessionId('research/ops')).toBe('ops-session') + // Verify the storage key uses encoded form. + expect(localStorage.getItem('hermes.desktop.lastSessionId.profile.research%2Fops')).toBe('ops-session') + // Another profile with a different encoding cannot read it. + expect(getRememberedSessionId('research')).toBeNull() }) it('clearing one profile leaves the others intact', () => { @@ -559,13 +571,22 @@ describe('remembered route (per profile)', () => { expect(getRememberedRoute('research')).toBeNull() }) - it('keeps the default profile on the legacy unsuffixed key for back-compat', () => { + it('discards legacy unsuffixed keys on first read (zero-migration, refuse-to-guess)', () => { localStorage.setItem('hermes.desktop.lastRoute', '/skills') - expect(getRememberedRoute('default')).toBe('/skills') - expect(getRememberedRoute(undefined)).toBe('/skills') - expect(getRememberedRoute('')).toBe('/skills') - expect(getRememberedRoute(null)).toBe('/skills') + // Reading from any profile discards the legacy key. + expect(getRememberedRoute('default')).toBeNull() + expect(getRememberedRoute('coder')).toBeNull() + + expect(localStorage.getItem('hermes.desktop.lastRoute')).toBeNull() + }) + + it('uses encodeURIComponent so profile names with reserved chars are isolated', () => { + setRememberedRoute('/cron', 'research/ops') + + expect(getRememberedRoute('research/ops')).toBe('/cron') + expect(localStorage.getItem('hermes.desktop.lastRoute.profile.research%2Fops')).toBe('/cron') + expect(getRememberedRoute('research')).toBeNull() }) it('clearing one profile leaves the others intact', () => { @@ -592,6 +613,45 @@ describe('remembered route (per profile)', () => { }) }) +describe('sessionBelongsToProfile', () => { + it('validates that a session row matches a stored id and target profile', () => { + const sessions = [ + session({ id: 's1', profile: 'ai-engineer' }), + session({ id: 's2', profile: 'default' }), + session({ id: 's3', profile: 'ai-engineer' }) + ] + + expect(sessionBelongsToProfile(sessions, 's1', 'ai-engineer')).toBe(true) + expect(sessionBelongsToProfile(sessions, 's3', 'ai-engineer')).toBe(true) + expect(sessionBelongsToProfile(sessions, 's2', 'default')).toBe(true) + // Wrong profile. + expect(sessionBelongsToProfile(sessions, 's1', 'default')).toBe(false) + // Missing session. + expect(sessionBelongsToProfile(sessions, 's-missing', 'ai-engineer')).toBe(false) + }) + + it('matches on lineage root so compressed tips validate their owner', () => { + const sessions = [session({ id: 'tip-2', _lineage_root_id: 'root-1', profile: 'work' })] + + expect(sessionBelongsToProfile(sessions, 'root-1', 'work')).toBe(true) + expect(sessionBelongsToProfile(sessions, 'tip-2', 'work')).toBe(true) + // Wrong profile even when lineage matches. + expect(sessionBelongsToProfile(sessions, 'root-1', 'personal')).toBe(false) + }) + + it('normalizes blank/empty profiles to default', () => { + const sessions = [session({ id: 's1', profile: '' }), session({ id: 's2', profile: null as unknown as string })] + + expect(sessionBelongsToProfile(sessions, 's1', 'default')).toBe(true) + expect(sessionBelongsToProfile(sessions, 's1', '')).toBe(true) + expect(sessionBelongsToProfile(sessions, 's2', 'default')).toBe(true) + }) + + it('returns false for an empty session list', () => { + expect(sessionBelongsToProfile([], 'any-id', 'default')).toBe(false) + }) +}) + describe('rememberedSessionProfile', () => { it('keys by the session row owning profile, not the active one', () => { const sessions = [session({ id: 'stored-1', profile: 'ai-engineer' })] diff --git a/apps/desktop/src/store/session.ts b/apps/desktop/src/store/session.ts index 19e56b2c1b12..a98d92128d0c 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -27,24 +27,58 @@ const COMPOSER_FAST_KEY = 'hermes.desktop.composer.fast' // The last chat the user had open, so a relaunch lands back on it instead of an // empty new-chat. Stored (not runtime) id — the route is keyed by stored id. // -// Scoped per profile: a single global key remembered ONE session across every -// profile, so relaunching (or a cold start) under profile B would try to -// restore a session that belongs to profile A — one of the ways a conversation -// appears to bleed between profiles (#63590). Each profile now remembers its -// own last session. The default profile keeps the original unsuffixed key so -// existing installs' remembered session survives the upgrade. +// Scoped per profile with an explicit namespace (`.profile.`) and +// encodeURIComponent so a profile name carrying `/` or other reserved chars +// cannot collide or leak across keys. Legacy global (unsuffixed) keys are +// discarded on first read to prevent cross-profile bleed — ownership of the old +// global values is unknowable, and guessing the owning profile is exactly the +// cross-profile corruption this storage boundary prevents (#67709). const LAST_SESSION_KEY = 'hermes.desktop.lastSessionId' -function rememberedSessionKey(profile?: null | string): string { - const key = (profile ?? '').trim() +function profileNavigationKey(base: string, profile: string): string { + const key = profile.trim() || 'default' - return !key || key === 'default' ? LAST_SESSION_KEY : `${LAST_SESSION_KEY}.${key}` + return `${base}.profile.${encodeURIComponent(key)}` } -export const getRememberedSessionId = (profile?: null | string): null | string => - storedString(rememberedSessionKey(profile)) -export const setRememberedSessionId = (id: null | string, profile?: null | string) => - persistString(rememberedSessionKey(profile), id) +function discardLegacyRememberedNavigation(): void { + // Ownership of the old global values is unknowable. Never migrate them into + // a profile: guessing is exactly the cross-profile corruption this storage + // boundary prevents. Re-check because an older concurrently open window can + // still write the legacy keys after this renderer has started. + if (storedString(LAST_SESSION_KEY) !== null) { + persistString(LAST_SESSION_KEY, null) + } + + if (storedString(LAST_ROUTE_KEY) !== null) { + persistString(LAST_ROUTE_KEY, null) + } +} + +export function getRememberedSessionId(profile: string): null | string { + discardLegacyRememberedNavigation() + + return storedString(profileNavigationKey(LAST_SESSION_KEY, profile)) +} + +export function setRememberedSessionId(id: null | string, profile: string): void { + discardLegacyRememberedNavigation() + persistString(profileNavigationKey(LAST_SESSION_KEY, profile), id) +} + +export function sessionBelongsToProfile( + sessions: readonly Pick[], + storedSessionId: string, + profile: string +): boolean { + const key = profile.trim() || 'default' + + return sessions.some(session => { + const owner = (session.profile ?? '').trim() || 'default' + + return owner === key && sessionMatchesStoredId(session, storedSessionId) + }) +} /** * The profile a routed session belongs to, for keying the remembered id. @@ -78,19 +112,19 @@ export function rememberedSessionProfile( // carries a session id in its path. Restoring under profile B would navigate to // a session owned by profile A — the remembered-id scoping above is bypassed // entirely, because the route is preferred over the id on cold start -// (#67603 family). The default profile keeps the original unsuffixed key so -// existing installs' remembered route survives the upgrade. +// (#67603 family). Legacy global values are discarded on first read. const LAST_ROUTE_KEY = 'hermes.desktop.lastRoute' -function rememberedRouteKey(profile?: null | string): string { - const key = (profile ?? '').trim() +export function getRememberedRoute(profile: string): null | string { + discardLegacyRememberedNavigation() - return !key || key === 'default' ? LAST_ROUTE_KEY : `${LAST_ROUTE_KEY}.${key}` + return storedString(profileNavigationKey(LAST_ROUTE_KEY, profile)) } -export const getRememberedRoute = (profile?: null | string): null | string => storedString(rememberedRouteKey(profile)) -export const setRememberedRoute = (path: null | string, profile?: null | string) => - persistString(rememberedRouteKey(profile), path) +export function setRememberedRoute(path: null | string, profile: string): void { + discardLegacyRememberedNavigation() + persistString(profileNavigationKey(LAST_ROUTE_KEY, profile), path) +} let configuredDefaultProjectDir = ''