Skip to content

Commit bbd8193

Browse files
committed
Revert "fix(desktop): preserve side chats across session switches (#1514)"
This reverts commit 9b15231.
1 parent a547c5c commit bbd8193

3 files changed

Lines changed: 13 additions & 56 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-
// 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.
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.
2222
useEffect(() => {
2323
if (owned && visible && released) {
2424
useChatStore.getState().connectToSession(sideChatId, { prewarm: false, applyRuntimeSelection: false })

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

Lines changed: 6 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ 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 () => ({})))
1312
const sessionsApiMock = vi.hoisted(() => ({
1413
delete: vi.fn(() => Promise.resolve()),
1514
getSlashCommands: vi.fn(async () => ({ commands: [] })),
@@ -19,7 +18,6 @@ const sessionsApiMock = vi.hoisted(() => ({
1918
}))
2019

2120
vi.mock('../api/sessions', () => ({ sessionsApi: sessionsApiMock }))
22-
vi.mock('../api/sideChats', () => ({ sideChatsApi: { discard: discardSideChatMock } }))
2321
vi.mock('../api/teams', () => ({ teamsApi: { getWorkbenchForSession: vi.fn() } }))
2422
vi.mock('../lib/desktopNotifications', () => ({ notifyDesktop: notifyDesktopMock }))
2523

@@ -30,7 +28,6 @@ import { registerSideChatSession, unregisterSideChatSession } from '../lib/sideC
3028
import { WARM_SESSION_LIMIT, useTabStore, type Tab } from './tabStore'
3129
import { useTeamStore } from './teamStore'
3230
import { useSessionStore } from './sessionStore'
33-
import { useSideChatStore } from './sideChatStore'
3431
import { navigateMobileUp } from '../components/mobile/mobileNavigation'
3532
import type { ServerMessage } from '../types/chat'
3633

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

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

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 })
430+
it('releases a side chat with its parent once both are done, keeping its transcript', async () => {
443431
registerSideChatSession('side-of-parent', 'parent')
444432
try {
445-
useSideChatStore.setState({ entries: {
446-
'side-of-parent': { sessionId: 'side-of-parent', parentSessionId: 'parent', workDir: '/repo', title: 'Side chat', ephemeral: true },
447-
} })
448433
useTabStore.setState({ tabs: [sessionTab('parent', 'Parent chat')], activeTabId: 'parent' })
449434
const parentSocket = connect('parent')
450435
const sideSocket = connect('side-of-parent')
451436
serverSays(parentSocket, TURN_COMPLETE)
452-
serverSays(sideSocket, cliRun('running'))
453437
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-
}
459438
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)
460442

461443
moveOnFrom('parent')
462444
await settle()
463445
expect(isHeld('parent')).toBe(false)
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)
446+
// Still answering: it finishes first.
475447
expect(isHeld('side-of-parent')).toBe(true)
476-
expect(disconnectSpy).not.toHaveBeenCalledWith('side-of-parent')
477448

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.' }))
483449
serverSays(sideSocket, TURN_COMPLETE)
484-
serverSays(sideSocket, cliRun('idle'))
485450
await settle()
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()
451+
expectReleasedWithTranscript('side-of-parent')
493452
} finally {
494453
unregisterSideChatSession('side-of-parent')
495454
}

‎desktop/src/stores/hiddenSessionRelease.ts‎

Lines changed: 4 additions & 6 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. 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.
97+
* approvals and team events. Side chats are shown by their parent's workspace
98+
* and released with it.
10099
*
101100
* Releasing never drops the transcript, draft or queued messages. Chats leave
102101
* the desktop's warm set on their own (`WARM_SESSION_LIMIT`), not because
103102
* someone closed them, so going back has to find the conversation where it
104103
* was. The chat store's history budget bounds the cached messages, and
105104
* `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,7 +138,6 @@ export function installHiddenSessionRelease(
138138
const session = sessions[sessionId]
139139
if (
140140
!session ||
141-
isSideChatSession(sessionId) ||
142141
isShown(sessionId, scope) ||
143142
!wsManager.getConnectedSessionIds().includes(sessionId) ||
144143
hasForegroundWork(session) ||
@@ -152,8 +151,7 @@ export function installHiddenSessionRelease(
152151
pruneReleased()
153152
}
154153

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

0 commit comments

Comments
 (0)