From b64c5aa49ea178620edd98fbaf055c8ccfd276d2 Mon Sep 17 00:00:00 2001 From: UnathiCodex Date: Fri, 17 Jul 2026 07:56:46 +0200 Subject: [PATCH] fix(desktop): prevent session rotation from stealing focus --- apps/desktop/src/app/contrib/wiring.tsx | 1 + .../hooks/use-session-actions.test.tsx | 168 ++++++++++++++++++ .../hooks/use-session-actions/index.ts | 58 ++++-- .../hooks/use-session-state-cache.test.tsx | 47 +++++ .../session/hooks/use-session-state-cache.ts | 17 +- apps/desktop/src/store/session.ts | 22 ++- 6 files changed, 279 insertions(+), 34 deletions(-) diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index f47a2301b2acc..a548e191bbb3f 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -394,6 +394,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { creatingSessionRef, ensureSessionState, getRouteToken, + getRoutedStoredSessionId, navigate, onFreshDraftRouteIntent: clearRoutedSessionIntent, requestGateway, diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index 1b725f65737e1..2a3f93a028ac5 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -9,18 +9,23 @@ import { $activeGatewayProfile, $newChatProfile } from '@/store/profile' import { $projectScope, $projectTree, ALL_PROJECTS } from '@/store/projects' import { $activeSessionId, + $activeSessionStoredIdRotation, $currentCwd, $messages, $newChatWorkspaceTarget, $resumeFailedSessionId, + $selectedStoredSessionId, setActiveSessionId, + setActiveSessionStoredIdRotation, setCurrentCwd, setMessages, setNewChatWorkspaceTarget, setResumeFailedSessionId, + setSelectedStoredSessionId, setSessions } from '@/store/session' +import { sessionRoute } from '../../routes' import type { ClientSessionState } from '../../types' import { useSessionActions } from './use-session-actions' @@ -75,6 +80,7 @@ function Harness({ creatingSessionRef: ref(false), ensureSessionState: () => ({}) as ClientSessionState, getRouteToken: () => 'token', + getRoutedStoredSessionId: () => null, navigate: vi.fn() as never, requestGateway, resetViewSync: vi.fn(), @@ -93,6 +99,166 @@ function Harness({ return null } +function StoredIdRotationHarness({ + activeSessionIdRef, + getRoutedStoredSessionId, + navigate, + selectedStoredSessionIdRef +}: { + activeSessionIdRef: MutableRefObject + getRoutedStoredSessionId: () => null | string + navigate: (to: string, options?: { replace?: boolean }) => void + selectedStoredSessionIdRef: MutableRefObject +}) { + const ref = (value: T): MutableRefObject => ({ current: value }) + + useSessionActions({ + activeSessionId: activeSessionIdRef.current, + activeSessionIdRef, + busyRef: ref(false), + creatingSessionRef: ref(false), + ensureSessionState: () => ({}) as ClientSessionState, + getRouteToken: () => 'token', + getRoutedStoredSessionId, + navigate: navigate as never, + requestGateway: async () => ({}) as never, + resetViewSync: vi.fn(), + runtimeIdByStoredSessionIdRef: ref(new Map()), + selectedStoredSessionId: selectedStoredSessionIdRef.current, + selectedStoredSessionIdRef, + sessionStateByRuntimeIdRef: ref(new Map()), + syncSessionStateToView: vi.fn(), + updateSessionState: () => ({}) as ClientSessionState + }) + + return null +} + +describe('active stored-session id rotation routing', () => { + afterEach(() => { + cleanup() + setActiveSessionId(null) + setActiveSessionStoredIdRotation(null) + setSelectedStoredSessionId(null) + vi.restoreAllMocks() + }) + + it('follows a rotation while the same conversation still owns the foreground route', async () => { + const activeSessionIdRef: MutableRefObject = { current: 'runtime-A' } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'stored-A' } + const navigate = vi.fn() + + setSelectedStoredSessionId('stored-A') + render( + 'stored-A'} + navigate={navigate} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + + act(() => { + setActiveSessionStoredIdRotation({ + nextStoredSessionId: 'stored-A-next', + previousStoredSessionId: 'stored-A', + runtimeSessionId: 'runtime-A' + }) + }) + + await waitFor(() => expect(selectedStoredSessionIdRef.current).toBe('stored-A-next')) + expect($selectedStoredSessionId.get()).toBe('stored-A-next') + expect(navigate).toHaveBeenCalledWith(sessionRoute('stored-A-next'), { replace: true }) + expect($activeSessionStoredIdRotation.get()).toBeNull() + }) + + it('does not overwrite a newer route intent before its resume effect has synchronized selection', async () => { + const activeSessionIdRef: MutableRefObject = { current: 'runtime-A' } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'stored-A' } + const navigate = vi.fn() + + setSelectedStoredSessionId('stored-A') + render( + 'stored-C'} + navigate={navigate} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + + act(() => { + setActiveSessionStoredIdRotation({ + nextStoredSessionId: 'stored-A-next', + previousStoredSessionId: 'stored-A', + runtimeSessionId: 'runtime-A' + }) + }) + + await waitFor(() => expect($activeSessionStoredIdRotation.get()).toBeNull()) + expect(selectedStoredSessionIdRef.current).toBe('stored-A') + expect($selectedStoredSessionId.get()).toBe('stored-A') + expect(navigate).not.toHaveBeenCalled() + }) + + it('does not let the previous runtime jump back after selection already moved', async () => { + const activeSessionIdRef: MutableRefObject = { current: 'runtime-A' } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'stored-C' } + const navigate = vi.fn() + + setSelectedStoredSessionId('stored-C') + render( + 'stored-C'} + navigate={navigate} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + + act(() => { + setActiveSessionStoredIdRotation({ + nextStoredSessionId: 'stored-A-next', + previousStoredSessionId: 'stored-A', + runtimeSessionId: 'runtime-A' + }) + }) + + await waitFor(() => expect($activeSessionStoredIdRotation.get()).toBeNull()) + expect(selectedStoredSessionIdRef.current).toBe('stored-C') + expect($selectedStoredSessionId.get()).toBe('stored-C') + expect(navigate).not.toHaveBeenCalled() + }) + + it('updates the underlying selection without navigating out of an overlay or page', async () => { + const activeSessionIdRef: MutableRefObject = { current: 'runtime-A' } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'stored-A' } + const navigate = vi.fn() + + setSelectedStoredSessionId('stored-A') + render( + null} + navigate={navigate} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + + act(() => { + setActiveSessionStoredIdRotation({ + nextStoredSessionId: 'stored-A-next', + previousStoredSessionId: 'stored-A', + runtimeSessionId: 'runtime-A' + }) + }) + + await waitFor(() => expect(selectedStoredSessionIdRef.current).toBe('stored-A-next')) + expect($selectedStoredSessionId.get()).toBe('stored-A-next') + expect(navigate).not.toHaveBeenCalled() + }) +}) + async function createWith( profileSetup: () => void, beforeCreate?: (handle: HarnessHandle) => Promise | void @@ -231,6 +397,7 @@ function ResumeHarness({ creatingSessionRef: ref(false), ensureSessionState: () => ({}) as ClientSessionState, getRouteToken: () => 'token', + getRoutedStoredSessionId: () => null, navigate: vi.fn() as never, requestGateway, resetViewSync: vi.fn(), @@ -480,6 +647,7 @@ function BranchHarness({ creatingSessionRef: ref(false), ensureSessionState: () => ({}) as ClientSessionState, getRouteToken: () => 'token', + getRoutedStoredSessionId: () => null, navigate: vi.fn() as never, requestGateway, resetViewSync: vi.fn(), diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 1e1e37405d088..3635e30f17a37 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -13,7 +13,7 @@ import { clearNotifications, notify, notifyError } from '@/store/notifications' import { $activeGatewayProfile, $newChatProfile, ensureGatewayProfile, normalizeProfileKey } from '@/store/profile' import { resolveNewSessionCwd, tombstoneSessions, untombstoneSessions } from '@/store/projects' import { - $activeSessionStoredId, + $activeSessionStoredIdRotation, $currentCwd, $currentFastMode, $currentModel, @@ -26,6 +26,7 @@ import { type NewChatWorkspaceTarget, sessionPinId, setActiveSessionId, + setActiveSessionStoredIdRotation, setAwaitingResponse, setBusy, setCurrentBranch, @@ -83,6 +84,7 @@ interface SessionActionsOptions { creatingSessionRef: MutableRefObject ensureSessionState: (sessionId: string, storedSessionId?: string | null) => ClientSessionState getRouteToken: () => string + getRoutedStoredSessionId: () => null | string navigate: NavigateFunction onFreshDraftRouteIntent?: () => void requestGateway: (method: string, params?: Record) => Promise @@ -163,6 +165,7 @@ export function useSessionActions({ creatingSessionRef, ensureSessionState, getRouteToken, + getRoutedStoredSessionId, navigate, onFreshDraftRouteIntent, requestGateway, @@ -178,32 +181,49 @@ export function useSessionActions({ const copy = t.desktop const resumeRequestRef = useRef(0) - // Follow auto-compression's stored-id rotation. When the active session's - // stored id changes (compression ends the SessionDB session and forks a - // continuation), re-anchor the URL route + selection to the new id so the - // next send doesn't hit a stale stored→runtime mapping and trigger a full - // thread reload. replace: true — it's the same conversation, not a new - // history entry. - const rotatedStoredId = useStore($activeSessionStoredId) + // Follow auto-compression's stored-id rotation only while the exact runtime, + // selection, and route intent still belong to the rotating conversation. + // The previous implementation carried only the next stored id and navigated + // unconditionally; a fast A → B → C switch could therefore be overwritten + // by A's delayed session.info event and visibly jump back to A. + const storedIdRotation = useStore($activeSessionStoredIdRotation) useEffect(() => { - if (!rotatedStoredId || rotatedStoredId === selectedStoredSessionIdRef.current) { + if (!storedIdRotation) { return } - const oldStoredId = selectedStoredSessionIdRef.current + // Consume the event even when it is stale. Rotation is an edge, not durable + // state; replaying it after a later remount/selection would steal focus. + setActiveSessionStoredIdRotation(current => (current === storedIdRotation ? null : current)) - setSelectedStoredSessionId(rotatedStoredId) - selectedStoredSessionIdRef.current = rotatedStoredId - navigate(sessionRoute(rotatedStoredId), { replace: true }) + const selectedStoredSessionId = selectedStoredSessionIdRef.current + const routedStoredSessionId = getRoutedStoredSessionId() - // Clean up the stale stored→runtime mapping so getRuntimeIdForStoredSession - // can't resolve the old id to this runtime (it would fail the storedSessionId - // check and return null, but leaving the stale key is sloppy). - if (oldStoredId) { - runtimeIdByStoredSessionIdRef.current.delete(oldStoredId) + if ( + activeSessionIdRef.current !== storedIdRotation.runtimeSessionId || + selectedStoredSessionId !== storedIdRotation.previousStoredSessionId || + (routedStoredSessionId !== null && routedStoredSessionId !== storedIdRotation.previousStoredSessionId) + ) { + return + } + + setSelectedStoredSessionId(storedIdRotation.nextStoredSessionId) + selectedStoredSessionIdRef.current = storedIdRotation.nextStoredSessionId + + // A route overlay/page has no routed session id, but the underlying selected + // chat still needs to follow the continuation. Update that selection in + // place without navigating out of the surface the user deliberately opened. + if (routedStoredSessionId === storedIdRotation.previousStoredSessionId) { + navigate(sessionRoute(storedIdRotation.nextStoredSessionId), { replace: true }) } - }, [rotatedStoredId, navigate, runtimeIdByStoredSessionIdRef, selectedStoredSessionIdRef]) + }, [ + activeSessionIdRef, + getRoutedStoredSessionId, + navigate, + selectedStoredSessionIdRef, + storedIdRotation + ]) const startFreshSessionDraft = useCallback( (options: boolean | FreshSessionDraftOptions = false) => { diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx index f875266dc2cc6..868f42519cca6 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ChatMessage } from '@/lib/chat-messages' import { + $activeSessionStoredIdRotation, $currentFastMode, $currentModel, $currentProvider, @@ -11,6 +12,8 @@ import { $currentServiceTier, $messages, $turnStartedAt, + setActiveSessionId, + setActiveSessionStoredIdRotation, setCurrentFastMode, setCurrentModel, setCurrentProvider, @@ -29,6 +32,50 @@ interface HarnessProps { selectedStoredSessionId: string | null } +describe('useSessionStateCache — stored-id rotation provenance', () => { + afterEach(() => { + cleanup() + setActiveSessionId(null) + setActiveSessionStoredIdRotation(null) + }) + + it('emits the previous, next, and runtime ids and removes the stale reverse mapping', () => { + let cache!: Cache + + setActiveSessionId('runtime-A') + render( (cache = value)} selectedStoredSessionId="stored-A" />) + + act(() => { + cache.ensureSessionState('runtime-A', 'stored-A') + cache.ensureSessionState('runtime-A', 'stored-A-next') + }) + + expect($activeSessionStoredIdRotation.get()).toEqual({ + nextStoredSessionId: 'stored-A-next', + previousStoredSessionId: 'stored-A', + runtimeSessionId: 'runtime-A' + }) + expect(cache.runtimeIdByStoredSessionIdRef.current.has('stored-A')).toBe(false) + expect(cache.runtimeIdByStoredSessionIdRef.current.get('stored-A-next')).toBe('runtime-A') + }) + + it('does not publish a foreground-navigation event for a background runtime rotation', () => { + let cache!: Cache + + setActiveSessionId('runtime-B') + render( (cache = value)} selectedStoredSessionId="stored-B" />) + + act(() => { + cache.ensureSessionState('runtime-A', 'stored-A') + cache.ensureSessionState('runtime-A', 'stored-A-next') + }) + + expect($activeSessionStoredIdRotation.get()).toBeNull() + expect(cache.runtimeIdByStoredSessionIdRef.current.has('stored-A')).toBe(false) + expect(cache.runtimeIdByStoredSessionIdRef.current.get('stored-A-next')).toBe('runtime-A') + }) +}) + function Harness({ activeSessionId, onReady, selectedStoredSessionId }: HarnessProps) { const busyRef: MutableRefObject = { current: false } diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts index 75b535835acc7..b343831a6a184 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts @@ -11,7 +11,7 @@ import { $messages, noteSessionActivity, onSessionWatchdogClear, - setActiveSessionStoredId, + setActiveSessionStoredIdRotation, setCurrentFastMode, setCurrentModel, setCurrentPersonality, @@ -117,14 +117,17 @@ export function useSessionStateCache({ if (previousStoredSessionId && previousStoredSessionId !== storedSessionId) { setSessionWorking(previousStoredSessionId, false) + runtimeIdByStoredSessionIdRef.current.delete(previousStoredSessionId) // Auto-compression rotated the stored id on the active session. Signal - // the route-following effect in use-session-actions so the URL + selection - // re-anchor to the continuation id — otherwise the next send hits a stale - // stored→runtime mapping (getRuntimeIdForStoredSession returns null) and - // triggers a full thread reload via resumeStoredSession. - if (sessionId === $activeSessionId.get()) { - setActiveSessionStoredId(storedSessionId) + // the route-following effect with enough provenance to reject the + // event if the user navigated elsewhere before React handles it. + if (storedSessionId && sessionId === $activeSessionId.get()) { + setActiveSessionStoredIdRotation({ + nextStoredSessionId: storedSessionId, + previousStoredSessionId, + runtimeSessionId: sessionId + }) } } } diff --git a/apps/desktop/src/store/session.ts b/apps/desktop/src/store/session.ts index 449ed6055f580..cbad935f535f6 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -248,13 +248,18 @@ export const $sessionsLoading = atom(true) export const $workingSessionIds = atom([]) export const $activeSessionId = atom(null) export const $selectedStoredSessionId = atom(null) -// Reactive signal for when the active session's stored id rotates (auto- -// compression ends the SessionDB session and forks a continuation). The -// route + selection must follow the rotation so the next send doesn't -// trigger a full thread reload (getRuntimeIdForStoredSession would return -// null for the old stored id, forcing resumeStoredSession). Set in -// ensureSessionState when the cache entry's storedSessionId changes. -export const $activeSessionStoredId = atom(null) +export interface ActiveSessionStoredIdRotation { + nextStoredSessionId: string + previousStoredSessionId: string + runtimeSessionId: string +} + +// One-shot event for when auto-compression rotates the active runtime's stored +// id. Carrying the runtime + previous id is load-bearing: a bare next id cannot +// tell whether the user has already navigated away while React is waiting to +// run the route-following effect, which lets a background session steal the +// foreground route. +export const $activeSessionStoredIdRotation = atom(null) export const $messages = atom([]) // Streaming-stable derivations of $messages. During a token stream the array @@ -328,7 +333,8 @@ export const setSessionProfileTotals = (next: Updater>) = export const setSessionsLoading = (next: Updater) => updateAtom($sessionsLoading, next) export const setWorkingSessionIds = (next: Updater) => updateAtom($workingSessionIds, next) export const setActiveSessionId = (next: Updater) => updateAtom($activeSessionId, next) -export const setActiveSessionStoredId = (next: Updater) => updateAtom($activeSessionStoredId, next) +export const setActiveSessionStoredIdRotation = (next: Updater) => + updateAtom($activeSessionStoredIdRotation, next) export const setSelectedStoredSessionId = (next: Updater) => { updateAtom($selectedStoredSessionId, next)