diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index ffdc79e65b751..8c8aedef27ac8 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -445,6 +445,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { onFreshDraftRouteIntent: clearRoutedSessionIntent, requestGateway, resetViewSync, + routedSessionId, runtimeIdByStoredSessionIdRef, selectedStoredSessionId, selectedStoredSessionIdRef, diff --git a/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx b/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx index ae7055ff22ad7..12a9084c7019f 100644 --- a/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx @@ -36,6 +36,150 @@ describe('useRouteResume', () => { vi.restoreAllMocks() }) + it('does not resume stale routed session A while the create guard holds selection on B (#66057)', () => { + // createBackendSessionForSend updates refs/atoms to B and navigates, but the + // router can still report A for a tick. While creatingSessionRef is true, + // stuckOnRoutedSession must NOT treat that as "stranded on A" and call + // resumeSession(A) (jump-back bug). + const resumeSession = vi.fn(async () => undefined) + const startFreshSessionDraft = vi.fn() + const activeSessionIdRef: MutableRefObject = { current: 'runtime-B' } + const creatingSessionRef = { current: true } + const runtimeIdByStoredSessionIdRef = { current: new Map([['session-B', 'runtime-B']]) } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'session-B' } + + const { rerender } = render( + + ) + + expect(resumeSession).not.toHaveBeenCalled() + + // Simulate post-create: refs/atoms already on B, route still on A, create + // guard still held until the router catches up. + rerender( + + ) + + expect(resumeSession).not.toHaveBeenCalled() + }) + + it('holds the create guard until the route catches up to the created session (#66057)', () => { + // While creatingSessionRef is true, even the stale-route + moved-selection + // shape must not resume. (Belt + guard: selectionMovedAheadOfRoute alone + // also blocks; this asserts the creatingSessionRef gate still works.) + const resumeSession = vi.fn(async () => undefined) + const startFreshSessionDraft = vi.fn() + const creatingSessionRef = { current: true } + const activeSessionIdRef: MutableRefObject = { current: 'runtime-B' } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'session-B' } + + render( + + ) + + expect(resumeSession).not.toHaveBeenCalled() + }) + + it('recovers by resuming A after create timeout when the route never catches up to B', () => { + // Post-timeout shape: creatingSessionRef false, selection/active on B, route + // still on A. selectionMovedAheadOfRoute must NOT keep blocking once the + // pending-create hold is gone — stuckOnRoutedSession should resume A so + // ChatView leaves its route/selection mismatch loading state. + const resumeSession = vi.fn(async () => undefined) + const startFreshSessionDraft = vi.fn() + const activeSessionIdRef: MutableRefObject = { current: 'runtime-A' } + const creatingSessionRef = { current: false } + const selectedStoredSessionIdRef: MutableRefObject = { current: 'session-A' } + + const { rerender } = render( + + ) + + expect(resumeSession).not.toHaveBeenCalled() + + // Create moved selection/runtime to B; safety timeout already released the + // guard; router never left A. + activeSessionIdRef.current = 'runtime-B' + selectedStoredSessionIdRef.current = 'session-B' + creatingSessionRef.current = false + rerender( + + ) + + expect(resumeSession).toHaveBeenCalledTimes(1) + expect(resumeSession).toHaveBeenCalledWith('session-A', true) + }) + it('does not re-resume the old session during a /:sid -> /new transition', () => { const resumeSession = vi.fn(async () => undefined) const startFreshSessionDraft = vi.fn() diff --git a/apps/desktop/src/app/session/hooks/use-route-resume.ts b/apps/desktop/src/app/session/hooks/use-route-resume.ts index ed12a91da4210..4ac68d9a1d0dd 100644 --- a/apps/desktop/src/app/session/hooks/use-route-resume.ts +++ b/apps/desktop/src/app/session/hooks/use-route-resume.ts @@ -135,7 +135,26 @@ export function useRouteResume({ // pathname flips to / (same null+/:sid signature). freshDraftReady is the // discriminator: it's true while heading into a blank new chat, false when // genuinely stranded on a routed session. - const stuckOnRoutedSession = routedSessionId !== selectedStoredSessionIdRef.current && !freshDraftReady + // + // Also must NOT fire when create/fork already moved selection + runtime to + // a new session B while the router still shows stale A (#66057). That looks + // "stuck on A" but resuming A yanks the UI back off the new chat. + // + // Scope this suppression to an active pending-create hold only. Once + // creatingSessionRef drops (route caught up, user left, or the safety + // timeout), a lingering A-route / B-selection mismatch must be able to + // self-heal via stuckOnRoutedSession — otherwise ChatView stays in its + // route/selection loading state forever after a stuck navigate. + const selectionMovedAheadOfRoute = + creatingSessionRef.current && + Boolean(selectedStoredSessionIdRef.current) && + selectedStoredSessionIdRef.current !== routedSessionId && + Boolean(activeSessionIdRef.current) + + const stuckOnRoutedSession = + routedSessionId !== selectedStoredSessionIdRef.current && + !freshDraftReady && + !selectionMovedAheadOfRoute // Resume when the route meaningfully changed, the gateway just opened, or // we're stranded on a routed session that never loaded. The first two 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 603b45a1a2bda..986d68f2a4604 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 @@ -113,6 +113,7 @@ function Harness({ navigate: navigate as never, requestGateway, resetViewSync: vi.fn(), + routedSessionId: null, runtimeIdByStoredSessionIdRef: ref(new Map()), selectedStoredSessionId: null, selectedStoredSessionIdRef: ref(null), @@ -152,6 +153,7 @@ function StoredIdRotationHarness({ navigate: navigate as never, requestGateway: async () => ({}) as never, resetViewSync: vi.fn(), + routedSessionId: getRoutedStoredSessionId(), runtimeIdByStoredSessionIdRef: ref(new Map()), selectedStoredSessionId: selectedStoredSessionIdRef.current, selectedStoredSessionIdRef, @@ -591,6 +593,7 @@ function ResumeHarness({ navigate: vi.fn() as never, requestGateway, resetViewSync: vi.fn(), + routedSessionId: null, runtimeIdByStoredSessionIdRef: runtimeIdByStoredSessionIdRef ?? ref(new Map()), selectedStoredSessionId, selectedStoredSessionIdRef: ref(selectedStoredSessionId), @@ -998,6 +1001,7 @@ function BranchHarness({ navigate: navigate as never, requestGateway, resetViewSync: vi.fn(), + routedSessionId: null, runtimeIdByStoredSessionIdRef: ref(new Map()), selectedStoredSessionId: null, selectedStoredSessionIdRef: ref(null), @@ -1535,3 +1539,243 @@ describe('createBackendSessionForSend workspace target', () => { expect(params).toMatchObject({ cwd: '/clicked-workspace' }) }) }) + +describe('createBackendSessionForSend creatingSessionRef hold (#66057)', () => { + afterEach(() => { + cleanup() + vi.useRealTimers() + $newChatProfile.set(null) + $activeGatewayProfile.set('default') + setCurrentCwd('') + setNewChatWorkspaceTarget(undefined) + vi.restoreAllMocks() + }) + + function GuardHarness({ + creatingSessionRef, + navigate, + onReady, + requestGateway, + routeId, + selectedStoredSessionIdRef + }: { + creatingSessionRef: MutableRefObject + navigate: (...args: never[]) => unknown + onReady: (create: () => Promise) => void + requestGateway: (method: string, params?: Record) => Promise + routeId: null | string + selectedStoredSessionIdRef: MutableRefObject + }) { + const ref = (value: T): MutableRefObject => ({ current: value }) + const actions = useSessionActions({ + activeSessionId: null, + activeSessionIdRef: ref(null), + busyRef: ref(false), + creatingSessionRef, + ensureSessionState: () => ({}) as ClientSessionState, + getRouteToken: () => 'token', + getRoutedStoredSessionId: () => routeId, + navigate: navigate as never, + requestGateway, + resetViewSync: vi.fn(), + routedSessionId: routeId, + runtimeIdByStoredSessionIdRef: ref(new Map()), + selectedStoredSessionId: selectedStoredSessionIdRef.current, + selectedStoredSessionIdRef, + sessionStateByRuntimeIdRef: ref(new Map()), + syncSessionStateToView: vi.fn(), + updateSessionState: () => ({}) as ClientSessionState + }) + + useEffect(() => { + onReady(() => actions.createBackendSessionForSend()) + }, [actions, onReady]) + + return null + } + + it('keeps creatingSessionRef true until routedSessionId catches up to the created stored id', async () => { + const creatingSessionRef: MutableRefObject = { current: false } + const selectedStoredSessionIdRef: MutableRefObject = { current: null } + const navigate = vi.fn() + let routedSessionId: null | string = 'session-A' + + const requestGateway = async (method: string): Promise => { + if (method === 'session.create') { + return { session_id: 'rt-new', stored_session_id: 'stored-new' } as T + } + + return {} as T + } + + let create: (() => Promise) | null = null + const { rerender } = render( + (create = fn)} + requestGateway={requestGateway} + routeId={routedSessionId} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + await waitFor(() => expect(create).not.toBeNull()) + + await act(async () => { + await create!() + }) + + expect(navigate).toHaveBeenCalled() + expect(creatingSessionRef.current).toBe(true) + expect(selectedStoredSessionIdRef.current).toBe('stored-new') + + // Route still stale on A — guard must stay up (not a user navigation away). + rerender( + undefined} + requestGateway={requestGateway} + routeId="session-A" + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + expect(creatingSessionRef.current).toBe(true) + + // Router catches up to the created stored id — release the guard. + routedSessionId = 'stored-new' + rerender( + undefined} + requestGateway={requestGateway} + routeId={routedSessionId} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + expect(creatingSessionRef.current).toBe(false) + }) + + it('clears creatingSessionRef when navigate throws', async () => { + const creatingSessionRef: MutableRefObject = { current: false } + const selectedStoredSessionIdRef: MutableRefObject = { current: null } + const navigate = vi.fn(() => { + throw new Error('navigate failed') + }) + + let create: (() => Promise) | null = null + render( + (create = fn)} + requestGateway={async method => { + if (method === 'session.create') { + return { session_id: 'rt-new', stored_session_id: 'stored-new' } as never + } + + return {} as never + }} + routeId="session-A" + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + await waitFor(() => expect(create).not.toBeNull()) + + await act(async () => { + await create!() + }) + + expect(navigate).toHaveBeenCalled() + expect(creatingSessionRef.current).toBe(false) + }) + + it('clears creatingSessionRef when the route moves to a different session than pending', async () => { + const creatingSessionRef: MutableRefObject = { current: false } + const selectedStoredSessionIdRef: MutableRefObject = { current: null } + const navigate = vi.fn() + + const requestGateway = async (method: string): Promise => { + if (method === 'session.create') { + return { session_id: 'rt-new', stored_session_id: 'stored-new' } as T + } + + return {} as T + } + + let create: (() => Promise) | null = null + const { rerender } = render( + (create = fn)} + requestGateway={requestGateway} + routeId="session-A" + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + await waitFor(() => expect(create).not.toBeNull()) + + await act(async () => { + await create!() + }) + expect(creatingSessionRef.current).toBe(true) + + // User clicked another session while create navigate was still pending. + rerender( + undefined} + requestGateway={requestGateway} + routeId="session-C" + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + expect(creatingSessionRef.current).toBe(false) + }) + + it('clears creatingSessionRef via safety timeout if the route never catches up', async () => { + const creatingSessionRef: MutableRefObject = { current: false } + const selectedStoredSessionIdRef: MutableRefObject = { current: null } + const navigate = vi.fn() + + let create: (() => Promise) | null = null + render( + (create = fn)} + requestGateway={async method => { + if (method === 'session.create') { + return { session_id: 'rt-new', stored_session_id: 'stored-new' } as never + } + + return {} as never + }} + routeId="session-A" + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + /> + ) + await waitFor(() => expect(create).not.toBeNull()) + + // Arm the pending timeout under fake timers so we can advance deterministically. + vi.useFakeTimers() + await act(async () => { + await create!() + }) + expect(creatingSessionRef.current).toBe(true) + expect(navigate).toHaveBeenCalledTimes(1) + + // Route stays on A forever — safety timeout must retry navigate (reconcile) + // and drop the guard so use-route-resume can self-heal if the route still + // never moves. + await act(async () => { + await vi.advanceTimersByTimeAsync(3_000) + }) + expect(creatingSessionRef.current).toBe(false) + expect(navigate).toHaveBeenCalledTimes(2) + expect(navigate).toHaveBeenLastCalledWith('/stored-new', { replace: true }) + }) +}) 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 b0202e722a4db..f56739954eca9 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 @@ -103,6 +103,10 @@ interface SessionActionsOptions { onFreshDraftRouteIntent?: () => void requestGateway: (method: string, params?: Record) => Promise resetViewSync: () => void + // Live route session id from the router. Used to drop creatingSessionRef only + // after navigate to a freshly created/forked stored id has actually landed + // (setTimeout(0) cleared the guard before the route caught up — #66057). + routedSessionId: string | null runtimeIdByStoredSessionIdRef: MutableRefObject> selectedStoredSessionId: string | null selectedStoredSessionIdRef: MutableRefObject @@ -124,6 +128,10 @@ interface SessionActionsOptions { // (NOT in this set) still legitimately drops to a draft. const createdThisRun = new Set() +// How long we keep creatingSessionRef after create/fork navigate before giving up +// if the router never lands on the pending stored id (stuck navigate / lost race). +const CREATE_GUARD_RELEASE_MS = 3_000 + // Reflect a stored row's persisted token counts into the live usage atom // (total is derived, so callers can't drift it out of sync with input/output). function applyStoredUsage(stored: { input_tokens?: number | null; output_tokens?: number | null }) { @@ -207,6 +215,7 @@ export function useSessionActions({ onFreshDraftRouteIntent, requestGateway, resetViewSync, + routedSessionId, runtimeIdByStoredSessionIdRef, selectedStoredSessionId, selectedStoredSessionIdRef, @@ -217,6 +226,96 @@ export function useSessionActions({ const { t } = useI18n() const copy = t.desktop const resumeRequestRef = useRef(0) + // Stored id we just created/forked and navigated to. creatingSessionRef stays + // true until routedSessionId + selection both agree on this id — clearing via + // setTimeout(0) let use-route-resume resume the stale route as "stuck" (#66057). + const pendingCreatedStoredSessionIdRef = useRef(null) + // Route id at the moment we armed pending (often the stale previous session). + // Distinguishes "router still lagging on A" from "user navigated to C". + const pendingCreatedFromRouteRef = useRef(null) + const pendingGuardTimeoutRef = useRef | null>(null) + + const releaseCreatingSessionGuard = useCallback(() => { + if (pendingGuardTimeoutRef.current != null) { + clearTimeout(pendingGuardTimeoutRef.current) + pendingGuardTimeoutRef.current = null + } + + pendingCreatedStoredSessionIdRef.current = null + pendingCreatedFromRouteRef.current = null + creatingSessionRef.current = false + }, [creatingSessionRef]) + + // Arm the create/fork hold: keep creatingSessionRef until the route lands on + // `storedId`, the user leaves for another route, navigate throws, or the + // safety timeout fires (so a stuck router can't block resumes forever). + const armPendingCreatedSession = useCallback( + (storedId: string) => { + pendingCreatedStoredSessionIdRef.current = storedId + pendingCreatedFromRouteRef.current = routedSessionId + + if (pendingGuardTimeoutRef.current != null) { + clearTimeout(pendingGuardTimeoutRef.current) + } + + pendingGuardTimeoutRef.current = setTimeout(() => { + pendingGuardTimeoutRef.current = null + + if (pendingCreatedStoredSessionIdRef.current !== storedId) { + return + } + + // Route never caught up. Retry navigate so ChatView can leave the + // route/selection mismatch loading state; then drop the guard so + // use-route-resume can self-heal to the URL if navigate still fails. + try { + navigate(sessionRoute(storedId), { replace: true }) + } catch { + // Ignore — release below still unblocks recovery. + } + + releaseCreatingSessionGuard() + }, CREATE_GUARD_RELEASE_MS) + }, + [navigate, releaseCreatingSessionGuard, routedSessionId] + ) + + useEffect( + () => () => { + if (pendingGuardTimeoutRef.current != null) { + clearTimeout(pendingGuardTimeoutRef.current) + } + }, + [] + ) + + // Drop the create/fork guard once the router catches up — or if the user + // navigates somewhere other than the pending id (left the pre-create route). + useEffect(() => { + const pending = pendingCreatedStoredSessionIdRef.current + + if (!creatingSessionRef.current || !pending) { + return + } + + if (routedSessionId === pending && selectedStoredSessionIdRef.current === pending) { + releaseCreatingSessionGuard() + + return + } + + const fromRoute = pendingCreatedFromRouteRef.current + + if (routedSessionId !== fromRoute && routedSessionId !== pending) { + releaseCreatingSessionGuard() + } + }, [ + creatingSessionRef, + releaseCreatingSessionGuard, + routedSessionId, + selectedStoredSessionId, + selectedStoredSessionIdRef + ]) // Follow auto-compression's stored-id rotation only while the exact runtime, // selection, and route intent still belong to the rotating conversation. @@ -406,7 +505,17 @@ export function useSessionActions({ // "Untitled session" until the turn persists and auto-title runs. The // server later returns its own preview/title and supersedes this. upsertOptimisticSession(created, stored, null, preview?.trim() || null) - navigate(sessionRoute(stored), { replace: true }) + // Hold creatingSessionRef until the route lands on `stored` (release + // effect above). setTimeout(0) raced use-route-resume back onto the + // previous session (#66057). + armPendingCreatedSession(stored) + + try { + navigate(sessionRoute(stored), { replace: true }) + } catch { + releaseCreatingSessionGuard() + } + // Other windows (e.g. the main window when this is the pop-out) can't // see this session until they re-pull the shared list. broadcastSessionsChanged() @@ -432,17 +541,21 @@ export function useSessionActions({ return created.session_id } finally { - window.setTimeout(() => { + // Keep the guard up while a navigate to the new stored id is pending; + // otherwise clear immediately (abort, error, or create without stored id). + if (!pendingCreatedStoredSessionIdRef.current) { creatingSessionRef.current = false - }, 0) + } } }, [ activeSessionIdRef, + armPendingCreatedSession, creatingSessionRef, ensureSessionState, getRouteToken, navigate, + releaseCreatingSessionGuard, requestGateway, resetViewSync, selectedStoredSessionIdRef, @@ -1180,13 +1293,16 @@ export function useSessionActions({ return true } catch (err) { + // Navigate throw or earlier failure after arming pending — never leave + // creatingSessionRef stuck true. + releaseCreatingSessionGuard() notifyError(err, copy.branchFailed) return false } finally { - window.setTimeout(() => { + if (!pendingCreatedStoredSessionIdRef.current) { creatingSessionRef.current = false - }, 0) + } } }, [copy, creatingSessionRef, ensureSessionState, requestGateway, updateSessionState]