Skip to content

Commit 9b15231

Browse files
authored
fix(desktop): preserve side chats across session switches (#1514)
1 parent 1443e22 commit 9b15231

3 files changed

Lines changed: 56 additions & 13 deletions

File tree

‎desktop/src/components/sideChat/SideChatSurface.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ export function SideChatSurface({ parentSessionId, sideChatId, visible = true }:
1616
const empty = useChatStore(state => !state.sessions[sideChatId]?.messages.length && !state.sessions[sideChatId]?.streamingText && (state.sessions[sideChatId]?.chatState ?? 'idle') === 'idle')
1717
const released = useChatStore(state => (state.sessions[sideChatId]?.connectionState ?? 'disconnected') === 'disconnected')
1818
const owned = Boolean(entry && entry.parentSessionId === parentSessionId)
19-
// A side chat is released with its parent once the parent leaves the warm
20-
// set (hiddenSessionRelease); showing it again reconnects on top of the
21-
// transcript it kept.
19+
// Reconnect a lost transport when shown. Hidden-session cleanup keeps this
20+
// temporary runtime connected: its CLI context cannot be restored from the
21+
// cached transcript after the server reclaims it.
2222
useEffect(() => {
2323
if (owned && visible && released) {
2424
useChatStore.getState().connectToSession(sideChatId, { prewarm: false, applyRuntimeSelection: false })

‎desktop/src/stores/hiddenSessionRelease.test.tsx‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
99
import '@testing-library/jest-dom'
1010

1111
const notifyDesktopMock = vi.hoisted(() => vi.fn())
12+
const discardSideChatMock = vi.hoisted(() => vi.fn(async () => ({})))
1213
const sessionsApiMock = vi.hoisted(() => ({
1314
delete: vi.fn(() => Promise.resolve()),
1415
getSlashCommands: vi.fn(async () => ({ commands: [] })),
@@ -18,6 +19,7 @@ const sessionsApiMock = vi.hoisted(() => ({
1819
}))
1920

2021
vi.mock('../api/sessions', () => ({ sessionsApi: sessionsApiMock }))
22+
vi.mock('../api/sideChats', () => ({ sideChatsApi: { discard: discardSideChatMock } }))
2123
vi.mock('../api/teams', () => ({ teamsApi: { getWorkbenchForSession: vi.fn() } }))
2224
vi.mock('../lib/desktopNotifications', () => ({ notifyDesktop: notifyDesktopMock }))
2325

@@ -28,6 +30,7 @@ import { registerSideChatSession, unregisterSideChatSession } from '../lib/sideC
2830
import { WARM_SESSION_LIMIT, useTabStore, type Tab } from './tabStore'
2931
import { useTeamStore } from './teamStore'
3032
import { useSessionStore } from './sessionStore'
33+
import { useSideChatStore } from './sideChatStore'
3134
import { navigateMobileUp } from '../components/mobile/mobileNavigation'
3235
import type { ServerMessage } from '../types/chat'
3336

@@ -173,6 +176,8 @@ describe('releasing sessions that no tab shows (#1485)', () => {
173176
wsManager.disconnectAll()
174177
disconnectSpy = vi.spyOn(wsManager, 'disconnect')
175178
notifyDesktopMock.mockClear()
179+
discardSideChatMock.mockClear()
180+
useSideChatStore.setState({ entries: {} })
176181
uninstall = installHiddenSessionRelease(useChatStore)
177182

178183
class ResizeObserverMock {
@@ -427,28 +432,64 @@ describe('releasing sessions that no tab shows (#1485)', () => {
427432
expect(isHeld('source')).toBe(false)
428433
})
429434

430-
it('releases a side chat with its parent once both are done, keeping its transcript', async () => {
435+
it.each([
436+
{ scope: 'open-tabs' as const, finishesWhileHidden: false },
437+
{ scope: 'open-tabs' as const, finishesWhileHidden: true },
438+
{ scope: 'active-tab' as const, finishesWhileHidden: false },
439+
{ scope: 'active-tab' as const, finishesWhileHidden: true },
440+
])('keeps a temporary side chat usable until explicitly closed ($scope, finishes while hidden: $finishesWhileHidden)', async ({ scope, finishesWhileHidden }) => {
441+
uninstall()
442+
uninstall = installHiddenSessionRelease(useChatStore, { scope })
431443
registerSideChatSession('side-of-parent', 'parent')
432444
try {
445+
useSideChatStore.setState({ entries: {
446+
'side-of-parent': { sessionId: 'side-of-parent', parentSessionId: 'parent', workDir: '/repo', title: 'Side chat', ephemeral: true },
447+
} })
433448
useTabStore.setState({ tabs: [sessionTab('parent', 'Parent chat')], activeTabId: 'parent' })
434449
const parentSocket = connect('parent')
435450
const sideSocket = connect('side-of-parent')
436451
serverSays(parentSocket, TURN_COMPLETE)
452+
serverSays(sideSocket, cliRun('running'))
437453
serverSays(sideSocket, { type: 'status', state: 'thinking' })
454+
for (const message of replyText('Side chat context.')) serverSays(sideSocket, message)
455+
if (!finishesWhileHidden) {
456+
serverSays(sideSocket, TURN_COMPLETE)
457+
serverSays(sideSocket, cliRun('idle'))
458+
}
438459
await settle()
439-
act(() => { vi.advanceTimersByTime(EIGHTEEN_HOURS_MS) })
440-
// While the parent is in the warm set its workspace, side chat included, stays.
441-
expect(isHeld('side-of-parent')).toBe(true)
442460

443461
moveOnFrom('parent')
444462
await settle()
445463
expect(isHeld('parent')).toBe(false)
446-
// Still answering: it finishes first.
464+
if (finishesWhileHidden) {
465+
serverSays(sideSocket, TURN_COMPLETE)
466+
serverSays(sideSocket, cliRun('idle'))
467+
}
468+
await settle()
469+
// #1513 released this socket when the parent left the warm set. The
470+
// server then destroyed the non-persistent CLI after its idle grace,
471+
// and a later send failed with SESSION_DELETED despite cached messages.
472+
act(() => { vi.advanceTimersByTime(EIGHTEEN_HOURS_MS) })
473+
await settle()
474+
expect(sideSocket.closedByClient).toBe(false)
447475
expect(isHeld('side-of-parent')).toBe(true)
476+
expect(disconnectSpy).not.toHaveBeenCalledWith('side-of-parent')
448477

478+
act(() => useTabStore.getState().openTab('parent', 'Parent chat'))
479+
act(() => useChatStore.getState().sendMessage('side-of-parent', 'Continue the side chat'))
480+
expect(socketsFor('side-of-parent')).toEqual([sideSocket])
481+
expect(sideSocket.sent).toContainEqual(expect.objectContaining({ type: 'user_message', content: 'Continue the side chat' }))
482+
expect(useChatStore.getState().sessions['side-of-parent']?.messages).toContainEqual(expect.objectContaining({ type: 'assistant_text', content: 'Side chat context.' }))
449483
serverSays(sideSocket, TURN_COMPLETE)
484+
serverSays(sideSocket, cliRun('idle'))
450485
await settle()
451-
expectReleasedWithTranscript('side-of-parent')
486+
487+
await act(async () => { await useSideChatStore.getState().close('side-of-parent') })
488+
expect(discardSideChatMock).toHaveBeenCalledWith('parent', 'side-of-parent')
489+
expect(sideSocket.closedByClient).toBe(true)
490+
expect(isHeld('side-of-parent')).toBe(false)
491+
expect(useChatStore.getState().sessions['side-of-parent']).toBeUndefined()
492+
expect(useSideChatStore.getState().entries['side-of-parent']).toBeUndefined()
452493
} finally {
453494
unregisterSideChatSession('side-of-parent')
454495
}

‎desktop/src/stores/hiddenSessionRelease.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,15 @@ function isCliStillWorking(session: PerSessionState): boolean {
9494
* was kept for is done. Kept: a pending prompt (the user's move; its
9595
* notification or the waiting list brings them back), a CLI still running
9696
* background work, and a team lead, whose socket carries every member's
97-
* approvals and team events. Side chats are shown by their parent's workspace
98-
* and released with it.
97+
* approvals and team events. Temporary side chats also stay connected until
98+
* explicitly closed: their CLI context is not persisted, so reclaiming the
99+
* runtime would make the conversation impossible to resume.
99100
*
100101
* Releasing never drops the transcript, draft or queued messages. Chats leave
101102
* the desktop's warm set on their own (`WARM_SESSION_LIMIT`), not because
102103
* someone closed them, so going back has to find the conversation where it
103104
* was. The chat store's history budget bounds the cached messages, and
104105
* `RELEASED_TRANSCRIPT_LIMIT` bounds how many released records are kept.
105-
* A side chat is released with its parent and reconnects when shown again.
106106
*/
107107
export function installHiddenSessionRelease(
108108
chatStore: Pick<StoreApi<HiddenSessionChatState>, 'getState' | 'subscribe'>,
@@ -138,6 +138,7 @@ export function installHiddenSessionRelease(
138138
const session = sessions[sessionId]
139139
if (
140140
!session ||
141+
isSideChatSession(sessionId) ||
141142
isShown(sessionId, scope) ||
142143
!wsManager.getConnectedSessionIds().includes(sessionId) ||
143144
hasForegroundWork(session) ||
@@ -151,7 +152,8 @@ export function installHiddenSessionRelease(
151152
pruneReleased()
152153
}
153154

154-
// A parent leaving the screen takes its side chats with it.
155+
// Recheck workspace sessions with their parent; the release guard keeps
156+
// temporary side chats alive even when the parent is no longer shown.
155157
const scheduleWithSideChats = (sessionId: string) => {
156158
schedule(sessionId)
157159
for (const id in chatStore.getState().sessions) {

0 commit comments

Comments
 (0)