Skip to content

Commit 94cba3b

Browse files
authored
fix(mothership): end desktop tools at sign-out and settle held reports as final (#8673)
* fix(mothership): end desktop tools at sign-out and settle held reports as final - Signing out cancels every desktop tool still running in the tab, so no local read or browser action outlives the session that started it. - Local file tools lease the turn captured at stream start, like browser actions. - The confirm route records a held_by_desktop outcome for its 409s, and the page-exit reporter treats a 409 as final. - A stale browser observation says it never started and not to retry it in this turn. * fix(mothership): accurate stale-observation wording and lease scope notes - A stale browser observation says it was not run this time, which stays true when an earlier delivery already ran it. - The lifetimes module says what a terminal call leaves running, and that sign-out ends leased tools only. - Tests check responses and recorded outcomes instead of mock calls. * revert(mothership): keep the confirm trace outcome vocabulary unchanged Drop the held_by_desktop outcome so this change needs no trace contract update: the 409 paths record tool_call_not_found as before. Keep a route test for a not-started report on a claimed call.
1 parent ab25c2a commit 94cba3b

10 files changed

Lines changed: 99 additions & 6 deletions

File tree

‎apps/sim/app/api/copilot/confirm/route.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,29 @@ describe('Copilot Confirm API Route', () => {
305305
}
306306
)
307307

308+
it('refuses a not-started report for a call the desktop already claimed', async () => {
309+
getAsyncToolCall.mockResolvedValue({
310+
...existingRow,
311+
toolName: 'browser_snapshot',
312+
status: 'running',
313+
claimedBy: 'desktop-browser',
314+
})
315+
316+
const response = await POST(
317+
createMockPostRequest({
318+
toolCallId: 'tool-call-123',
319+
status: 'error',
320+
message: 'The desktop action did not start.',
321+
data: { notStarted: true },
322+
})
323+
)
324+
325+
expect(response.status).toBe(409)
326+
expect(await response.json()).toEqual({
327+
error: 'The desktop app holds this tool call; only its own result settles it',
328+
})
329+
})
330+
308331
it('does not publish when another terminal transition wins indeterminate claim reconciliation', async () => {
309332
getAsyncToolCall.mockResolvedValue({
310333
...existingRow,

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from 'vitest'
22
import {
33
leaseDesktopTool,
4+
stopAllDesktopTools,
45
stopDesktopTools,
56
} from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
67

@@ -54,4 +55,19 @@ describe('desktop tool leases', () => {
5455

5556
expect(next.signal.aborted).toBe(true)
5657
})
58+
59+
it('cancels the running tools of every turn when the session ends', () => {
60+
const first = leaseDesktopTool('turn-f')
61+
const second = leaseDesktopTool('turn-g')
62+
63+
stopAllDesktopTools('signed_out')
64+
const next = leaseDesktopTool('turn-f')
65+
66+
expect(first.signal.aborted).toBe(true)
67+
expect(second.signal.aborted).toBe(true)
68+
expect(first.signal.reason).toBe('signed_out')
69+
expect(next.signal.aborted).toBe(false)
70+
first.release()
71+
next.release()
72+
})
5773
})

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ interface RunningTurnTools {
88
* Turns with a desktop tool still running in this tab, keyed by the turn's stream id. A tool
99
* outlives the chat view that started it (and any stream reader), so the turn, not the view, owns
1010
* its Stop. A turn is held only while one of its tools runs: each tool releases it as it settles.
11+
* Terminal calls take no lease. A terminal call returns once its operation does (a `run` after
12+
* its wait window), and the command it started keeps running in a terminal tab the user can see
13+
* and control. Stop settles the agent's marks on that tab through the turn's resource activity;
14+
* the process itself is the user's to end.
1115
*/
1216
const runningTurns = new Map<string, RunningTurnTools>()
1317

@@ -49,3 +53,12 @@ export function stopDesktopTools(streamId: string, reason: string): void {
4953
runningTurns.get(streamId)?.stop.abort(reason)
5054
runningTurns.delete(streamId)
5155
}
56+
57+
/**
58+
* Cancels every leased desktop tool running in this tab (browser actions, local file reads and
59+
* imports), so none outlives the session that started it.
60+
*/
61+
export function stopAllDesktopTools(reason: string): void {
62+
for (const turn of runningTurns.values()) turn.stop.abort(reason)
63+
runningTurns.clear()
64+
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,7 +1577,12 @@ export function useChat(
15771577
)
15781578

15791579
const startClientLocalFilesystemTool = useCallback(
1580-
(toolCallId: string, toolName: string, toolArgs: Record<string, unknown>) => {
1580+
(
1581+
toolCallId: string,
1582+
toolName: string,
1583+
toolArgs: Record<string, unknown>,
1584+
turnStreamId: string | undefined
1585+
) => {
15811586
if (
15821587
!isNativeFileTool(toolName) &&
15831588
(!workspaceId || !isUserLocalVfsToolCall(toolName, toolArgs))
@@ -1588,7 +1593,7 @@ export function useChat(
15881593
return
15891594
}
15901595
handledClientLocalFilesystemToolIds.add(toolCallId)
1591-
const lease = streamIdRef.current ? leaseDesktopTool(streamIdRef.current) : undefined
1596+
const lease = turnStreamId ? leaseDesktopTool(turnStreamId) : undefined
15921597
const options = {
15931598
workspaceId,
15941599
chatId: chatIdRef.current ?? selectedChatIdRef.current,
@@ -2226,7 +2231,8 @@ export function useChat(
22262231
addResource,
22272232
removeResource,
22282233
startClientWorkflowTool,
2229-
startClientLocalFilesystemTool,
2234+
startClientLocalFilesystemTool: (toolCallId, toolName, toolArgs) =>
2235+
startClientLocalFilesystemTool(toolCallId, toolName, toolArgs, turnStreamId),
22302236
startClientBrowserTool: startClientBrowserToolForStream,
22312237
startClientTerminalTool: startClientTerminalToolForStream,
22322238
startBrowserAgentRun: startBrowserAgentRunForStream,

‎apps/sim/lib/mothership/tools/client/browser-tool-execution.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1470,9 +1470,12 @@ describe('pre-dispatch drops still resolve the waiter', () => {
14701470
expect(mockReportCompletion).toHaveBeenCalledWith(
14711471
'stale-call-1',
14721472
'error',
1473-
expect.stringContaining('too late'),
1473+
expect.stringContaining('not run this time'),
14741474
expect.objectContaining({ staleEvent: true })
14751475
)
1476+
const [, , message] = mockReportCompletion.mock.calls[0] ?? []
1477+
expect(message).toContain('Do not retry it in this turn')
1478+
expect(message).toContain('keep this chat open in the Sim desktop app')
14761479
})
14771480

14781481
it('marks a stale stateful event outcome unknown and unsafe to retry', async () => {

‎apps/sim/lib/mothership/tools/client/browser-tool-execution.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ const OUTCOME_UNKNOWN_MESSAGE =
109109
'The Sim window closed while this browser action was in flight. It may already have taken effect. Do not retry it automatically; take a fresh browser snapshot before deciding what to do.'
110110
const REPLAY_OUTCOME_UNKNOWN_MESSAGE =
111111
'This browser action was recorded before the Sim page reloaded, but its terminal result could not be recovered. It may already have taken effect. Do not retry it automatically; take a fresh browser snapshot before deciding what to do.'
112+
const STALE_OBSERVATION_NOT_RUN_MESSAGE =
113+
'Not run: this browser observation reached the Sim desktop app too late to run safely, so it was not run this time and has no result. An observation changes nothing in the browser. Do not retry it in this turn; tell the user to keep this chat open in the Sim desktop app, or to ask again later.'
112114
const STALE_STATEFUL_OUTCOME_UNKNOWN_MESSAGE =
113115
'This browser action was delivered too late to recover its exact result. It may already have taken effect. Do not retry it automatically; take a fresh browser snapshot before deciding what to do.'
114116
const REPLAY_GUARD_CAPACITY_MESSAGE =
@@ -593,7 +595,7 @@ export async function executeBrowserToolOnClient(
593595
logger.info('Skipping stale browser tool event', { toolCallId, toolName, age })
594596
const observationOnly = OBSERVATION_ONLY_BROWSER_TOOLS[toolName]
595597
const message = observationOnly
596-
? 'This browser observation was delivered too late to run safely. Ask again to retry it.'
598+
? STALE_OBSERVATION_NOT_RUN_MESSAGE
597599
: STALE_STATEFUL_OUTCOME_UNKNOWN_MESSAGE
598600
retainAndReportTerminalCompletion(
599601
toolCallId,

‎apps/sim/lib/mothership/tools/client/completion.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,14 @@ describe('client tool completion reporting', () => {
115115
expect(signal?.aborted).toBe(true)
116116
})
117117

118+
it('treats a 409 as final: the desktop app holds the call', async () => {
119+
fetchMock.mockResolvedValue(new Response(null, { status: 409 }))
120+
121+
await expect(
122+
reportClientToolCompletionOnPageExit('tool-1', 'error', 'Browser failed')
123+
).resolves.toBeUndefined()
124+
})
125+
118126
it('rejects a non-success response', async () => {
119127
fetchMock.mockResolvedValue(new Response(null, { status: 503 }))
120128

‎apps/sim/lib/mothership/tools/client/completion.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export async function reportClientToolCompletionOnPageExit(
145145
}),
146146
keepalive: true,
147147
})
148-
if (!response.ok) {
148+
if (!isSettledDelivery(response, toolCallId)) {
149149
throw new CompletionReportError(`Page-exit completion failed with status ${response.status}`)
150150
}
151151
}

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

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

16+
import { leaseDesktopTool } from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
1617
import { clearUserData, RECENT_IMPERSONATIONS_STORAGE_KEY } from '@/stores'
1718

1819
expect(mockModuleLoaded).not.toHaveBeenCalled()
@@ -95,4 +96,19 @@ describe('clearUserData', () => {
9596
expect(inMemoryResetSucceeded).toBe(false)
9697
expect(localStorage.getItem('private-cache')).toBeNull()
9798
})
99+
100+
it('cancels desktop tools still running for the signed-out identity', async () => {
101+
const localRead = leaseDesktopTool('turn-before-sign-out')
102+
const browserAction = leaseDesktopTool('other-turn-before-sign-out')
103+
mockResetAllStores.mockImplementationOnce(() => {
104+
throw new Error('Chunk unavailable')
105+
})
106+
107+
await clearUserData()
108+
109+
expect(localRead.signal.aborted).toBe(true)
110+
expect(browserAction.signal.aborted).toBe(true)
111+
localRead.release()
112+
browserAction.release()
113+
})
98114
})

‎apps/sim/stores/index.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
'use client'
22

33
import { createLogger } from '@sim/logger'
4+
import { stopAllDesktopTools } from '@/app/workspace/[workspaceId]/home/hooks/desktop-tool-lifetimes'
45

56
const logger = createLogger('Stores')
67

78
export const RECENT_IMPERSONATIONS_STORAGE_KEY = 'recent-impersonations'
89

10+
/** Why desktop tools still running for the previous identity were cancelled. */
11+
const SIGNED_OUT_ABORT_REASON = 'identity_boundary:clearUserData'
12+
913
interface ClearUserDataOptions {
1014
preserveRecentImpersonations?: boolean
1115
}
@@ -21,6 +25,8 @@ export async function clearUserData(options: ClearUserDataOptions = {}): Promise
2125
let cleanupFailed = false
2226
let inMemoryResetSucceeded = true
2327

28+
stopAllDesktopTools(SIGNED_OUT_ABORT_REASON)
29+
2430
try {
2531
const keysToKeep = [
2632
'next-favicon',

0 commit comments

Comments
 (0)