Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 90 additions & 10 deletions apps/desktop/src/app/session/hooks/use-session-actions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
$messages,
$newChatWorkspaceTarget,
$resumeFailedSessionId,
$selectedStoredSessionId,
setActiveSessionId,
setCurrentCwd,
setMessages,
Expand All @@ -25,6 +26,10 @@ import type { ClientSessionState } from '../../types'

import { useSessionActions } from './use-session-actions'

const { ensureGatewayProfileMock } = vi.hoisted(() => ({
ensureGatewayProfileMock: vi.fn()
}))

vi.mock('@/hermes', async importOriginal => ({
...(await importOriginal<Record<string, unknown>>()),
deleteSession: vi.fn(),
Expand All @@ -34,10 +39,15 @@ vi.mock('@/hermes', async importOriginal => ({
setSessionArchived: vi.fn()
}))

vi.mock('@/store/profile', async importOriginal => ({
...(await importOriginal<typeof import('@/store/profile')>()),
ensureGatewayProfile: ensureGatewayProfileMock
}))

const RUNTIME_SESSION_ID = 'rt-new-001'
type HarnessHandle = Pick<
ReturnType<typeof useSessionActions>,
'createBackendSessionForSend' | 'startFreshSessionDraft'
'createBackendSessionForSend' | 'resumeSession' | 'startFreshSessionDraft'
>

function storedSession(overrides: Partial<SessionInfo> = {}): SessionInfo {
Expand All @@ -59,6 +69,15 @@ function storedSession(overrides: Partial<SessionInfo> = {}): SessionInfo {
}
}

function deferred<T>() {
let resolve!: (value: T) => void
const promise = new Promise<T>(resolvePromise => {
resolve = resolvePromise
})

return { promise, resolve }
}

function Harness({
onReady,
requestGateway
Expand Down Expand Up @@ -217,7 +236,7 @@ function ResumeHarness({
runtimeIdByStoredSessionIdRef,
sessionStateByRuntimeIdRef
}: {
onReady: (resume: (storedSessionId: string, replaceRoute?: boolean) => Promise<unknown>) => void
onReady: (actions: HarnessHandle) => void
requestGateway: <T>(method: string, params?: Record<string, unknown>) => Promise<T>
runtimeIdByStoredSessionIdRef?: MutableRefObject<Map<string, string>>
sessionStateByRuntimeIdRef?: MutableRefObject<Map<string, ClientSessionState>>
Expand All @@ -243,8 +262,8 @@ function ResumeHarness({
})

useEffect(() => {
onReady(actions.resumeSession)
}, [actions.resumeSession, onReady])
onReady(actions)
}, [actions, onReady])

return null
}
Expand All @@ -256,6 +275,7 @@ describe('resumeSession failure recovery', () => {
setResumeFailedSessionId(null)
setMessages([])
setSessions([])
ensureGatewayProfileMock.mockReset()
vi.restoreAllMocks()
})

Expand All @@ -266,10 +286,10 @@ describe('resumeSession failure recovery', () => {
sessionStateByRuntimeIdRef?: MutableRefObject<Map<string, ClientSessionState>>
} = {}
): Promise<void> {
let resume: ((storedSessionId: string, replaceRoute?: boolean) => Promise<unknown>) | null = null
render(<ResumeHarness onReady={r => (resume = r)} requestGateway={requestGateway} {...options} />)
await waitFor(() => expect(resume).not.toBeNull())
await resume!('stored-1', true)
let actions: HarnessHandle | null = null
render(<ResumeHarness onReady={handle => (actions = handle)} requestGateway={requestGateway} {...options} />)
await waitFor(() => expect(actions).not.toBeNull())
await actions!.resumeSession('stored-1', true)
}

it('arms $resumeFailedSessionId when resume RPC and REST fallback both fail', async () => {
Expand Down Expand Up @@ -336,6 +356,66 @@ describe('resumeSession failure recovery', () => {
await expect(runResume(requestGateway)).resolves.toBeUndefined()
})

it('does not let an in-flight resume revive a fresh chat draft', async () => {
setSessions([storedSession()])

const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
if (method === 'session.resume') {
return { session_id: 'runtime-1', resumed: params?.session_id, messages: [], info: {} } as never
}

return {} as never
})
let actions: HarnessHandle | null = null

render(<ResumeHarness onReady={handle => (actions = handle)} requestGateway={requestGateway} />)
await waitFor(() => expect(actions).not.toBeNull())

await act(async () => {
const pendingResume = actions!.resumeSession('stored-1', true)

// resolveStoredSession yields before the gateway resume. New Chat must
// invalidate that stale continuation rather than let it reselect stored-1.
actions!.startFreshSessionDraft(true)
await pendingResume
})

expect(requestGateway).not.toHaveBeenCalledWith('session.resume', expect.anything())
expect($activeSessionId.get()).toBeNull()
expect($selectedStoredSessionId.get()).toBeNull()
expect($messages.get()).toEqual([])
})

it('does not let a resume continue after New Chat during its gateway profile swap', async () => {
setSessions([storedSession({ profile: 'other-profile' })])
const gatewaySwap = deferred<void>()
ensureGatewayProfileMock.mockReturnValueOnce(gatewaySwap.promise)

const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
if (method === 'session.resume') {
return { session_id: 'runtime-1', resumed: params?.session_id, messages: [], info: {} } as never
}

return {} as never
})
let actions: HarnessHandle | null = null

render(<ResumeHarness onReady={handle => (actions = handle)} requestGateway={requestGateway} />)
await waitFor(() => expect(actions).not.toBeNull())

const pendingResume = actions!.resumeSession('stored-1', true)
await waitFor(() => expect(ensureGatewayProfileMock).toHaveBeenCalledWith('other-profile'))

actions!.startFreshSessionDraft(true)
gatewaySwap.resolve(undefined)
await pendingResume

expect(requestGateway).not.toHaveBeenCalledWith('session.resume', expect.anything())
expect($activeSessionId.get()).toBeNull()
expect($selectedStoredSessionId.get()).toBeNull()
expect($messages.get()).toEqual([])
})

it('leaves the failure latch clear when resume succeeds', async () => {
// Pre-arm to prove a successful resume clears it (entry-clear path).
setResumeFailedSessionId('stored-1')
Expand Down Expand Up @@ -581,7 +661,7 @@ describe('resumeSession warm-cache mapping integrity', () => {
let resume: ((storedSessionId: string, replaceRoute?: boolean) => Promise<unknown>) | null = null
render(
<ResumeHarness
onReady={r => (resume = r)}
onReady={actions => (resume = actions.resumeSession)}
requestGateway={requestGateway}
runtimeIdByStoredSessionIdRef={runtimeIdByStoredSessionIdRef}
sessionStateByRuntimeIdRef={sessionStateByRuntimeIdRef}
Expand Down Expand Up @@ -623,7 +703,7 @@ describe('resumeSession warm-cache mapping integrity', () => {
let resume: ((storedSessionId: string, replaceRoute?: boolean) => Promise<unknown>) | null = null
render(
<ResumeHarness
onReady={r => (resume = r)}
onReady={actions => (resume = actions.resumeSession)}
requestGateway={requestGateway}
runtimeIdByStoredSessionIdRef={runtimeIdByStoredSessionIdRef}
sessionStateByRuntimeIdRef={sessionStateByRuntimeIdRef}
Expand Down
11 changes: 10 additions & 1 deletion apps/desktop/src/app/session/hooks/use-session-actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ export function useSessionActions({
? normalizeNewChatWorkspaceTarget(draftOptions.workspaceTarget)
: undefined

// A resume may be between awaits (profile lookup / gateway swap) when the
// user starts a new chat. Invalidate that request before clearing the
// view; otherwise its stale continuation can select and paint the old
// session over this fresh draft.
resumeRequestRef.current += 1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This invalidates the lookup phase, but resumeSession does not re-check the token after await ensureGatewayProfile(sessionProfile). A New Chat during that gateway-swap await can still enter the warm/cold paths and reselect the old session; add an isCurrentResume() guard immediately after that await and cover it with a deferred-swap test.

@Astyyym Astyyym Jul 19, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right: the prior invalidation covered only the profile lookup. I added isCurrentResume() immediately after await ensureGatewayProfile(sessionProfile), before either the warm or cold resume path. The new deferred-swap regression covers: resume A -> wait until its profile swap is pending -> New Chat -> resolve the swap -> assert A cannot issue session.resume or restore active, selected, or message state. The targeted use-session-actions suite now passes: 20/20 tests. Pushed in 93fea22.

resetViewSync()
busyRef.current = false
setBusy(false)
Expand Down Expand Up @@ -499,12 +504,16 @@ export function useSessionActions({
const storedForProfile = await resolveStoredSession(storedSessionId)
const sessionProfile = storedForProfile?.profile

if (resumeRequestRef.current !== requestId) {
if (!isCurrentResume()) {
return
}

await ensureGatewayProfile(sessionProfile)

if (!isCurrentResume()) {
return
}

// Re-check after the profile-resolve / gateway-swap awaits above: the
// cache may have changed, and takeWarmCache re-validates belongs-to and
// purges a cross-wired mapping before we trust the fast-path.
Expand Down