Skip to content

Commit 7aa719f

Browse files
committed
fix(mothership): bind desktop tools to the session the chat surface mounted in
A send or reconnect still in flight at sign-out reaches the stream reader after the stop, so a per-reader capture took the new session. The surface now takes its session once at mount; signing out leaves or reloads every chat surface.
1 parent 8208336 commit 7aa719f

4 files changed

Lines changed: 46 additions & 29 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes.test.ts‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { describe, expect, it } from 'vitest'
22
import {
3-
desktopToolTurn,
3+
desktopToolSession,
44
stopAllDesktopTools,
55
stopDesktopTools,
66
} from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
77

88
describe('desktop tool leases', () => {
99
it('cancels every running tool of the stopped turn and no other turn', () => {
10-
const first = desktopToolTurn('turn-a').lease()
11-
const second = desktopToolTurn('turn-a').lease()
12-
const other = desktopToolTurn('turn-b').lease()
10+
const first = desktopToolSession().turn('turn-a').lease()
11+
const second = desktopToolSession().turn('turn-a').lease()
12+
const other = desktopToolSession().turn('turn-b').lease()
1313

1414
stopDesktopTools('turn-a', 'user_stop')
1515

@@ -21,9 +21,10 @@ describe('desktop tool leases', () => {
2121
})
2222

2323
it('keeps a turn reachable by Stop while any of its tools still runs', () => {
24-
const settled = desktopToolTurn('turn-c').lease()
25-
const running = desktopToolTurn('turn-c').lease()
26-
for (let turn = 0; turn < 500; turn++) desktopToolTurn(`busy-${turn}`).lease().release()
24+
const settled = desktopToolSession().turn('turn-c').lease()
25+
const running = desktopToolSession().turn('turn-c').lease()
26+
for (let turn = 0; turn < 500; turn++)
27+
desktopToolSession().turn(`busy-${turn}`).lease().release()
2728

2829
settled.release()
2930
settled.release()
@@ -33,11 +34,11 @@ describe('desktop tool leases', () => {
3334
})
3435

3536
it('gives a turn whose tools all settled a fresh lifetime for its next tool', () => {
36-
const settled = desktopToolTurn('turn-d').lease()
37+
const settled = desktopToolSession().turn('turn-d').lease()
3738
settled.release()
3839
stopDesktopTools('turn-d', 'user_stop')
3940

40-
const next = desktopToolTurn('turn-d').lease()
41+
const next = desktopToolSession().turn('turn-d').lease()
4142

4243
expect(settled.signal.aborted).toBe(false)
4344
expect(next.signal).not.toBe(settled.signal)
@@ -46,9 +47,9 @@ describe('desktop tool leases', () => {
4647
})
4748

4849
it('does not let a tool that settles after Stop release a newer lease on the turn', () => {
49-
const stopped = desktopToolTurn('turn-e').lease()
50+
const stopped = desktopToolSession().turn('turn-e').lease()
5051
stopDesktopTools('turn-e', 'user_stop')
51-
const next = desktopToolTurn('turn-e').lease()
52+
const next = desktopToolSession().turn('turn-e').lease()
5253

5354
stopped.release()
5455
stopDesktopTools('turn-e', 'user_stop')
@@ -57,8 +58,8 @@ describe('desktop tool leases', () => {
5758
})
5859

5960
it('cancels the running tools of every turn when the session ends', () => {
60-
const first = desktopToolTurn('turn-f').lease()
61-
const second = desktopToolTurn('turn-g').lease()
61+
const first = desktopToolSession().turn('turn-f').lease()
62+
const second = desktopToolSession().turn('turn-g').lease()
6263

6364
stopAllDesktopTools('signed_out')
6465

@@ -69,21 +70,25 @@ describe('desktop tool leases', () => {
6970
second.release()
7071
})
7172

72-
it('cancels a tool a turn of the ended session starts after sign-out', () => {
73-
const turn = desktopToolTurn('turn-h')
73+
it('cancels tools of a surface mounted before sign-out, even on a stream it reads later', () => {
74+
const surface = desktopToolSession()
75+
const running = surface.turn('turn-h')
7476
stopAllDesktopTools('signed_out')
7577

76-
const late = turn.lease()
78+
const late = running.lease()
79+
const reconnected = surface.turn('turn-j').lease()
7780

7881
expect(late.signal.aborted).toBe(true)
7982
expect(late.signal.reason).toBe('signed_out')
83+
expect(reconnected.signal.aborted).toBe(true)
8084
late.release()
85+
reconnected.release()
8186
})
8287

83-
it('runs the tools of a turn started after the session ended', () => {
88+
it('runs the tools of a surface mounted after the session ended', () => {
8489
stopAllDesktopTools('signed_out')
8590

86-
const next = desktopToolTurn('turn-i').lease()
91+
const next = desktopToolSession().turn('turn-i').lease()
8792

8893
expect(next.signal.aborted).toBe(false)
8994
next.release()

‎apps/sim/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,30 @@ interface DesktopToolLease {
2626
release(): void
2727
}
2828

29-
/** A turn's stream, bound to the session it started in. */
29+
/** A turn's desktop tools, in the session of the chat surface that runs the turn. */
3030
export interface DesktopToolTurn {
3131
/** Starts one desktop tool for the turn. */
3232
lease(): DesktopToolLease
3333
}
3434

35+
/** The desktop tools a chat surface starts, bound to the session the surface mounted in. */
36+
export interface DesktopToolSession {
37+
turn(streamId: string): DesktopToolTurn
38+
}
39+
3540
/**
36-
* Binds a turn's stream to the current session. Take it once, when the stream starts: its tool
37-
* events can still arrive after a sign-out, and each of them then gets an already-aborted lease.
41+
* Binds a chat surface to the current session. Take it once, when the surface mounts: a send or
42+
* reconnect still in flight at sign-out can deliver tool events after the stop, and each of them
43+
* then gets an already-aborted lease. Signing out leaves or reloads every chat surface, so a
44+
* surface mounted after sign-in binds to the new session.
3845
*/
39-
export function desktopToolTurn(streamId: string): DesktopToolTurn {
46+
export function desktopToolSession(): DesktopToolSession {
4047
const startedIn = session.signal
4148
return {
42-
lease: () =>
43-
startedIn.aborted ? { signal: startedIn, release() {} } : leaseDesktopTool(streamId),
49+
turn: (streamId) => ({
50+
lease: () =>
51+
startedIn.aborted ? { signal: startedIn, release() {} } : leaseDesktopTool(streamId),
52+
}),
4453
}
4554
}
4655

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,9 @@ import { initTerminalTransport } from '@/lib/terminal/transport'
9393
import { getQueryClient } from '@/app/_shell/providers/get-query-client'
9494
import { chatUrl } from '@/app/workspace/[workspaceId]/home/hooks/chat-url'
9595
import {
96+
type DesktopToolSession,
9697
type DesktopToolTurn,
97-
desktopToolTurn,
98+
desktopToolSession,
9899
stopDesktopTools,
99100
} from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
100101
import { useFilePreviewController } from '@/app/workspace/[workspaceId]/home/hooks/preview'
@@ -1004,6 +1005,8 @@ export function useChat(
10041005
const chatIdRef = useRef<string | undefined>(initialChatId)
10051006
/** Cleared on unmount, so late async work cannot act on a surface the user left. */
10061007
const surfaceMountedRef = useRef(true)
1008+
const desktopToolsRef = useRef<DesktopToolSession | null>(null)
1009+
const desktopTools = (desktopToolsRef.current ??= desktopToolSession())
10071010
useEffect(() => {
10081011
surfaceMountedRef.current = true
10091012
return () => {
@@ -2298,7 +2301,7 @@ export function useChat(
22982301
shouldContinue?: () => boolean
22992302
}
23002303
) => {
2301-
const desktopTurn = streamIdRef.current ? desktopToolTurn(streamIdRef.current) : undefined
2304+
const desktopTurn = streamIdRef.current ? desktopTools.turn(streamIdRef.current) : undefined
23022305
const activityTracker = getResourceActivityTracker(
23032306
expectedGen ?? streamGenRef.current,
23042307
options?.targetChatId

‎apps/sim/stores/index.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ vi.mock('@/stores/reset-all-stores', () => {
1313
return { resetAllStores: mockResetAllStores }
1414
})
1515

16-
import { desktopToolTurn } from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
16+
import { desktopToolSession } from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
1717
import { clearUserData, RECENT_IMPERSONATIONS_STORAGE_KEY } from '@/stores'
1818

1919
expect(mockModuleLoaded).not.toHaveBeenCalled()
@@ -98,8 +98,8 @@ describe('clearUserData', () => {
9898
})
9999

100100
it('cancels desktop tools still running for the signed-out identity', async () => {
101-
const localRead = desktopToolTurn('turn-before-sign-out').lease()
102-
const browserAction = desktopToolTurn('other-turn-before-sign-out').lease()
101+
const localRead = desktopToolSession().turn('turn-before-sign-out').lease()
102+
const browserAction = desktopToolSession().turn('other-turn-before-sign-out').lease()
103103
mockResetAllStores.mockImplementationOnce(() => {
104104
throw new Error('Chunk unavailable')
105105
})

0 commit comments

Comments
 (0)