fix(desktop): stop cold-start resume homing focus to the workspace tab (⌘R tab persistence)

The layout tree persists the active tab, but every reload landed on main
anyway: boot's route resume sets $selectedStoredSessionId, and the
selection listener in store/session-states.ts treats every selection
change as a navigation — noteActiveTreeGroup(null) +
revealTreePane('workspace') — fronting the workspace tab over the
persisted active tile and then persisting that clobber.

A cold-start restore is a re-attachment, not a navigation, so
use-route-resume now arms a one-shot (markSelectionRestore) before
dispatching the window's FIRST resume; the selection listener consumes
it and skips homing exactly once. Every later resume — sidebar click,
route change, reconnect — homes as before, and starting on /new
consumes boot status too so a subsequent session open still homes.
This commit is contained in:
Brooklyn Nicholson 2026-07-29 20:57:04 -05:00
parent 646761c783
commit 4aa672b23e
4 changed files with 114 additions and 1 deletions

View file

@ -3,9 +3,15 @@ import type { MutableRefObject } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { $resumeExhaustedSessionId, setResumeExhaustedSessionId } from '@/store/session'
import { markSelectionRestore } from '@/store/session-states'
import { useRouteResume } from './use-route-resume'
// The hook only arms the boot-restore one-shot; the listener consuming it lives
// in the real store (covered by session-states.test.ts). Mock the module so the
// store's side effects (persistence listeners) stay out of this harness.
vi.mock('@/store/session-states', () => ({ markSelectionRestore: vi.fn() }))
interface HarnessProps {
activeSessionId: null | string
activeSessionIdRef: MutableRefObject<null | string>
@ -192,6 +198,45 @@ describe('useRouteResume', () => {
expect(resumeSession).toHaveBeenCalledWith('session-2', true)
})
it('arms the boot-restore one-shot for the FIRST resume only (⌘R tab persistence)', () => {
// Factory mocks survive restoreAllMocks — drop calls earlier tests made.
vi.mocked(markSelectionRestore).mockClear()
const resumeSession = vi.fn(async () => undefined)
const startFreshSessionDraft = vi.fn()
const activeSessionIdRef: MutableRefObject<null | string> = { current: null }
const creatingSessionRef = { current: false }
const runtimeIdByStoredSessionIdRef = { current: new Map() }
const selectedStoredSessionIdRef: MutableRefObject<null | string> = { current: null }
const props = {
activeSessionId: null,
activeSessionIdRef,
creatingSessionRef,
currentView: 'chat',
freshDraftReady: false,
gatewayState: 'open',
resumeSession,
runtimeIdByStoredSessionIdRef,
selectedStoredSessionId: null,
selectedStoredSessionIdRef,
startFreshSessionDraft
}
// Cold start: the window mounts already routed at /session-1 (the reload).
const { rerender } = render(
<RouteResumeHarness {...props} locationPathname="/session-1" routedSessionId="session-1" />
)
expect(resumeSession).toHaveBeenCalledWith('session-1', true)
expect(markSelectionRestore).toHaveBeenCalledTimes(1)
// A later route change is a real navigation: resume fires, one-shot doesn't.
rerender(<RouteResumeHarness {...props} locationPathname="/session-2" routedSessionId="session-2" />)
expect(resumeSession).toHaveBeenCalledWith('session-2', true)
expect(markSelectionRestore).toHaveBeenCalledTimes(1)
})
it('resumes the selected route again when the gateway reconnects', () => {
const resumeSession = vi.fn(async () => undefined)
const startFreshSessionDraft = vi.fn()

View file

@ -2,6 +2,7 @@ import { type MutableRefObject, useEffect, useRef } from 'react'
import { isNewChatRoute } from '@/app/routes'
import { setResumeExhaustedSessionId } from '@/store/session'
import { markSelectionRestore } from '@/store/session-states'
interface RouteResumeOptions {
activeSessionId: string | null
@ -85,6 +86,12 @@ export function useRouteResume({
const lastPathnameRef = useRef<string | null>(null)
const seenGatewayStateRef = useRef(false)
const wasGatewayOpenRef = useRef(false)
// True until the FIRST resume this window dispatches. That resume is the
// cold-start restore of a route that was already loaded before the reload —
// a re-attachment, not a navigation — so it must not home focus/tabs to the
// workspace (which would clobber the persisted active tab; ⌘R always landed
// on main). Every later resume is a real navigation and homes as usual.
const bootResumeRef = useRef(true)
// Per-session retry bookkeeping for the bounded auto-retry effect below. Keyed
// by the session id we're retrying so switching chats resets the counter.
const retrySessionIdRef = useRef<string | null>(null)
@ -148,6 +155,16 @@ export function useRouteResume({
// rebinds/reaps the session on its side, and trusting it strands Desktop on
// a dead id ("session not found"). Otherwise keep skipping when already active.
if ((gatewayBecameOpen || !alreadyActive) && shouldResume && !creatingSessionRef.current) {
// The window's FIRST resume re-attaches the pre-reload route rather
// than navigating anywhere, so the selection listener must not home
// focus/tabs to the workspace over the persisted layout (see
// markSelectionRestore). One-shot: consumed by the selection change
// resumeSession makes synchronously at entry.
if (bootResumeRef.current) {
markSelectionRestore()
}
bootResumeRef.current = false
void resumeSession(routedSessionId, true)
}
@ -160,6 +177,8 @@ export function useRouteResume({
(selectedStoredSessionId || activeSessionId || !freshDraftReady) &&
!rawHashLooksLikeSession()
) {
// A fresh draft is a real navigation — any later resume homes normally.
bootResumeRef.current = false
startFreshSessionDraft(true)
}
}, [

View file

@ -2,10 +2,13 @@ import { describe, expect, it } from 'vitest'
import type { ClientSessionState } from '@/app/types'
import { group, split } from '@/components/pane-shell/tree/model'
import { $layoutTree } from '@/components/pane-shell/tree/store'
import { $selectedStoredSessionId } from '@/store/session'
import type { SessionTile } from '@/store/session-states'
import {
blankDraftTile,
focusedSessionNeedsRoute,
markSelectionRestore,
orderTilesByTree,
selectionHomesToWorkspace
} from '@/store/session-states'
@ -51,6 +54,36 @@ describe('selectionHomesToWorkspace', () => {
})
})
describe('boot-restore selection homing (⌘R tab persistence)', () => {
const mainGroup = () => group(['workspace', tilePane('t')], { active: tilePane('t'), id: 'main' })
const activePane = () => {
const tree = $layoutTree.get()
return tree?.type === 'group' ? tree.active : null
}
it('a normal selection change fronts the workspace tab over an active tile', () => {
$layoutTree.set(mainGroup())
$selectedStoredSessionId.set('nav-1')
expect(activePane()).toBe('workspace')
})
it('markSelectionRestore skips homing exactly once, so the persisted active tab survives a reload', () => {
$layoutTree.set(mainGroup())
markSelectionRestore()
$selectedStoredSessionId.set('boot-1')
// Boot restore: the tile tab the user reloaded on stays fronted.
expect(activePane()).toBe(tilePane('t'))
// One-shot consumed: the next selection change is a real navigation.
$selectedStoredSessionId.set('nav-2')
expect(activePane()).toBe('workspace')
})
})
describe('focusedSessionNeedsRoute', () => {
it('routes when the session is not on screen', () => {
expect(focusedSessionNeedsRoute(null, false)).toBe(true)

View file

@ -768,10 +768,26 @@ export const $focusedSessionState = computed([$focusedRuntimeId, $sessionStates]
export const selectionHomesToWorkspace = (selected: null | string, tiles: readonly SessionTile[]): boolean =>
!(selected && tiles.some(t => t.storedSessionId === selected))
// Cold-start restore is the one selection change that is NOT a navigation: the
// route already pointed at the primary session before the window loaded, and
// homing on it would front the workspace tab over the PERSISTED active tab —
// then persist that clobber, so the tab you reloaded on never comes back
// (⌘R always landing on main). use-route-resume arms this one-shot right
// before dispatching the boot resume; the very next selection change skips
// homing and the restored layout tree keeps its say.
let selectionRestoreInFlight = false
export function markSelectionRestore() {
selectionRestoreInFlight = true
}
// Homing also FRONTS the workspace tab: the resumed chat loads in the workspace
// pane, so a zone parked on a tile tab must switch back or the click looks dead.
$selectedStoredSessionId.listen(selected => {
if (!selectionHomesToWorkspace(selected, $sessionTiles.get())) {
const restoring = selectionRestoreInFlight
selectionRestoreInFlight = false
if (restoring || !selectionHomesToWorkspace(selected, $sessionTiles.get())) {
return
}