Skip to content

Commit 3cca741

Browse files
committed
fix(desktop): complete native computer tool lifecycle
1 parent c99237a commit 3cca741

10 files changed

Lines changed: 159 additions & 52 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/hooks/message-reconcile.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,15 @@ export function getReplayCompletedWorkflowToolCallIds(events: StreamBatchEvent[]
344344
const payload = event.payload
345345
if (!('phase' in payload)) continue
346346
if (payload.phase !== MothershipStreamV1ToolPhase.result) continue
347-
// Client-executed tools (workflow runs, browser actions) must never
348-
// re-fire when their completed call replays after reconnect/reload.
347+
/**
348+
* Client-executed tools (workflow runs, browser and computer actions) must never
349+
* re-fire when their completed call replays after reconnect/reload.
350+
*/
349351
if (
350352
typeof payload.toolCallId === 'string' &&
351-
(isWorkflowToolName(payload.toolName) || isBrowserToolName(payload.toolName))
353+
(isWorkflowToolName(payload.toolName) ||
354+
isBrowserToolName(payload.toolName) ||
355+
payload.toolName === 'computer')
352356
) {
353357
completedToolCallIds.add(payload.toolCallId)
354358
}

‎apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-tool-event.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ vi.mock(
1313
)
1414

1515
import type { PersistedStreamEventEnvelope } from '@/lib/mothership/request/session/contract'
16+
import { toStreamBatchEvent } from '@/lib/mothership/request/session/types'
17+
import { getReplayCompletedWorkflowToolCallIds } from '@/app/workspace/[workspaceId]/home/hooks/message-reconcile'
1618
import { dispatchStreamEvent } from './dispatch-stream-event'
1719
import { createStreamLoopContext, type StreamLoopContext } from './stream-context'
1820
import { makeStreamLoopDeps, ref } from './stream-test-helpers'
@@ -96,6 +98,38 @@ describe('tool events (dispatch → model + side effects)', () => {
9698
expect(deps.startClientComputerTool).not.toHaveBeenCalled()
9799
})
98100

101+
it.each([true, false])(
102+
'does not redispatch a completed computer call from a fresh replay batch (success=%s)',
103+
(success) => {
104+
const call = (id: string) =>
105+
toolEnv({
106+
phase: 'call',
107+
executor: 'client',
108+
mode: 'async',
109+
toolCallId: id,
110+
toolName: 'computer',
111+
arguments: { action: 'list_apps' },
112+
})
113+
const events = [
114+
call('computer-complete'),
115+
toolResult('computer-complete', success, 'computer'),
116+
call('computer-unfinished'),
117+
].map(toStreamBatchEvent)
118+
const deps = makeStreamLoopDeps()
119+
deps.options.suppressedWorkflowToolStartIds = getReplayCompletedWorkflowToolCallIds(events)
120+
const ctx = createStreamLoopContext(deps)
121+
122+
for (const entry of events) dispatchStreamEvent(ctx, entry.event)
123+
124+
expect(deps.startClientComputerTool).toHaveBeenCalledExactlyOnceWith(
125+
'computer-unfinished',
126+
{ action: 'list_apps' },
127+
''
128+
)
129+
expect(toolNode(ctx, 'computer-complete').result).toBeDefined()
130+
}
131+
)
132+
99133
it('redelivers an unsettled computer call through the replay-safe native executor after reconnect', () => {
100134
const deps = makeStreamLoopDeps()
101135
const ctx = createStreamLoopContext(deps)

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,15 @@ describe('getReplayCompletedWorkflowToolCallIds', () => {
190190

191191
expect(result).toEqual(new Set(['workflow-complete']))
192192
})
193+
194+
it('suppresses completed computer and browser calls while keeping unfinished calls eligible', () => {
195+
const result = getReplayCompletedWorkflowToolCallIds([
196+
toolBatchEvent(1, 'computer-complete', 'computer', MothershipStreamV1ToolPhase.call),
197+
toolBatchEvent(2, 'computer-complete', 'computer', MothershipStreamV1ToolPhase.result),
198+
toolBatchEvent(3, 'computer-active', 'computer', MothershipStreamV1ToolPhase.call),
199+
toolBatchEvent(4, 'browser-complete', 'browser_click', MothershipStreamV1ToolPhase.result),
200+
])
201+
202+
expect(result).toEqual(new Set(['computer-complete', 'browser-complete']))
203+
})
193204
})

‎apps/sim/lib/mothership/request/handlers/handlers.test.ts‎

Lines changed: 58 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -315,43 +315,64 @@ describe('sse-handlers tool lifecycle', () => {
315315
)
316316
})
317317

318-
it('pre-persists browser tools as pending for the desktop authorization claim', async () => {
319-
isSimExecuted.mockReturnValue(false)
320-
context.runId = 'run-1'
321-
322-
await prePersistClientExecutableToolCall(
323-
{
324-
type: MothershipStreamV1EventType.tool,
325-
payload: {
326-
toolCallId: 'browser-tool-1',
327-
toolName: 'browser_list_tabs',
328-
arguments: {},
329-
executor: MothershipStreamV1ToolExecutor.client,
330-
mode: MothershipStreamV1ToolMode.async,
331-
phase: MothershipStreamV1ToolPhase.call,
332-
},
333-
} satisfies StreamEvent,
334-
context,
335-
{},
336-
execContext
337-
)
338-
339-
expect(upsertAsyncToolCall).toHaveBeenCalledWith({
340-
runId: 'run-1',
341-
toolCallId: 'browser-tool-1',
342-
toolName: 'browser_list_tabs',
343-
args: {},
344-
sealedContext: { __sealedClientToolContextV1: 'sealed-context' },
345-
status: MothershipStreamV1AsyncToolRecordStatus.pending,
346-
})
347-
expect(sealClientToolContext).toHaveBeenCalledWith({
348-
toolCallId: 'browser-tool-1',
349-
runId: 'run-1',
350-
userId: 'user-1',
351-
registry: execContext.resolvedSecretTraceRegistry,
352-
toolInput: {},
353-
})
354-
})
318+
describe.each(['browser_list_tabs', 'terminal', 'import_local_files', 'computer'])(
319+
'native %s pre-persistence',
320+
(toolName) => {
321+
it.each([false, true])(
322+
'keeps the call pending for the desktop claim when approval gating is %s',
323+
async (gated) => {
324+
isSimExecuted.mockReturnValue(false)
325+
toolRequiresApproval.mockReturnValue(gated)
326+
context.runId = 'run-1'
327+
context.toolPermissions.enabled = gated
328+
const args =
329+
toolName === 'computer'
330+
? { action: 'status' }
331+
: toolName === 'terminal'
332+
? { operation: 'run', command: 'pwd' }
333+
: {}
334+
const event = {
335+
type: MothershipStreamV1EventType.tool,
336+
payload: {
337+
toolCallId: 'native-tool-1',
338+
toolName,
339+
arguments: args,
340+
executor: MothershipStreamV1ToolExecutor.client,
341+
mode: MothershipStreamV1ToolMode.async,
342+
phase: MothershipStreamV1ToolPhase.call,
343+
},
344+
} satisfies StreamEvent
345+
346+
await prePersistClientExecutableToolCall(event, context, {}, execContext)
347+
348+
expect(upsertAsyncToolCall).toHaveBeenCalledExactlyOnceWith({
349+
runId: 'run-1',
350+
toolCallId: 'native-tool-1',
351+
toolName,
352+
args,
353+
sealedContext: { __sealedClientToolContextV1: 'sealed-context' },
354+
status: MothershipStreamV1AsyncToolRecordStatus.pending,
355+
})
356+
expect(sealClientToolContext).toHaveBeenCalledWith({
357+
toolCallId: 'native-tool-1',
358+
runId: 'run-1',
359+
userId: 'user-1',
360+
registry: execContext.resolvedSecretTraceRegistry,
361+
toolInput: args,
362+
})
363+
expect(event.payload).toEqual({
364+
toolCallId: 'native-tool-1',
365+
toolName,
366+
arguments: args,
367+
executor: MothershipStreamV1ToolExecutor.client,
368+
mode: MothershipStreamV1ToolMode.async,
369+
phase: MothershipStreamV1ToolPhase.call,
370+
...(gated ? { status: 'awaiting_approval' } : {}),
371+
})
372+
}
373+
)
374+
}
375+
)
355376

356377
it('persists a gated sim tool and stamps the frame so a reload can still answer it', async () => {
357378
toolRequiresApproval.mockReturnValue(true)

‎apps/sim/lib/mothership/request/handlers/tool.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -304,17 +304,17 @@ export async function prePersistClientExecutableToolCall(
304304
toolName: data.toolName,
305305
args: data.arguments,
306306
sealedContext,
307-
// Browser and terminal actions cross a second, native authorization
308-
// boundary. Leave those rows pending until Electron atomically claims
309-
// them — the authorize endpoint only hands over a pending call, so a row
310-
// that arrives already running can never be executed natively. All other
311-
// client tools retain the established "already dispatched" running state.
312-
// A gated tool is likewise pending: nothing has been dispatched yet.
307+
/**
308+
* Native desktop actions remain pending until Electron atomically claims
309+
* them at authorization. Gated tools also await dispatch; other client
310+
* tools retain their established already-dispatched running state.
311+
*/
313312
status:
314313
gated ||
315314
isCurrentBrowserToolName(data.toolName) ||
316315
isTerminalToolName(data.toolName) ||
317-
data.toolName === 'import_local_files'
316+
data.toolName === 'import_local_files' ||
317+
data.toolName === 'computer'
318318
? MothershipStreamV1AsyncToolRecordStatus.pending
319319
: MothershipStreamV1AsyncToolRecordStatus.running,
320320
}).catch((err) => {

‎apps/sim/lib/mothership/request/tools/executor.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,13 @@ describe('pendingToolWaitBudgetMs', () => {
271271
).toBe(195_000)
272272
})
273273

274+
it('reserves the whole computer action budget before the lifecycle adds delivery grace', () => {
275+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'executing' })).toBe(90_000)
276+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'awaiting_approval' })).toBe(
277+
TOOL_WATCHDOG_LONG_RUNNING_MS
278+
)
279+
})
280+
274281
it('falls back to the tool\u2019s own watchdog once it is actually executing', () => {
275282
expect(pendingToolWaitBudgetMs({ name: 'terminal_run', status: 'executing' })).toBe(
276283
TOOL_WATCHDOG_DEFAULT_MS

‎apps/sim/lib/mothership/request/tools/executor.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { browserToolRendererTimeoutMs, isCurrentBrowserToolName } from '@sim/browser-protocol'
2+
import { COMPUTER_USE_TOOL_TIMEOUT_MS } from '@sim/desktop-bridge'
23
import { createLogger } from '@sim/logger'
34
import { toError } from '@sim/utils/errors'
45
import { isRecordLike } from '@sim/utils/object'
@@ -250,8 +251,8 @@ export function toolWatchdogTimeoutMs(toolName: string | undefined): number {
250251

251252
/**
252253
* How long the resume gate may wait on one pending tool call. Permission
253-
* prompts receive the long-running budget. Browser calls share the renderer's
254-
* budget so authorization and native queueing cannot outlive the resume gate.
254+
* prompts receive the long-running budget. Native calls share the renderer's
255+
* budget so authorization and native queueing leave the full resume grace for result delivery.
255256
*/
256257
export function pendingToolWaitBudgetMs(
257258
toolCall:
@@ -260,6 +261,7 @@ export function pendingToolWaitBudgetMs(
260261
): number {
261262
if (toolCall?.status === 'awaiting_approval') return TOOL_WATCHDOG_LONG_RUNNING_MS
262263
const executableName = toolCall?.execName ?? toolCall?.name
264+
if (executableName === 'computer') return COMPUTER_USE_TOOL_TIMEOUT_MS
263265
if (executableName && isCurrentBrowserToolName(executableName)) {
264266
return browserToolRendererTimeoutMs(executableName, toolCall?.params)
265267
}

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/** @vitest-environment jsdom */
2-
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
33

44
const mocks = vi.hoisted(() => ({
55
execute: vi.fn(),
@@ -29,6 +29,31 @@ describe('computer action delivery', () => {
2929
mocks.complete.mockResolvedValue(undefined)
3030
mocks.pageExit.mockResolvedValue(undefined)
3131
})
32+
afterEach(() => {
33+
vi.useRealTimers()
34+
})
35+
36+
it('allows the full action budget, then cancels and reports an uncertain result', async () => {
37+
vi.useFakeTimers()
38+
mocks.execute.mockImplementationOnce(() => new Promise(() => {}))
39+
const id = nextId()
40+
const execution = executeComputerToolOnClient(id, { action: 'list_apps' }, now())
41+
42+
await vi.advanceTimersByTimeAsync(89_999)
43+
expect(mocks.cancel).not.toHaveBeenCalled()
44+
expect(mocks.complete).not.toHaveBeenCalled()
45+
46+
await vi.advanceTimersByTimeAsync(1)
47+
await execution
48+
expect(mocks.cancel).toHaveBeenCalledExactlyOnceWith(id)
49+
expect(mocks.complete).toHaveBeenCalledExactlyOnceWith(
50+
id,
51+
'cancelled',
52+
expect.stringContaining('timed out'),
53+
{ doNotRetry: true, outcomeUnknown: true }
54+
)
55+
})
56+
3257
it('strips UI activity and runs each action only once across redelivery', async () => {
3358
const id = nextId()
3459
await executeComputerToolOnClient(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { COMPUTER_USE_TOOL_TIMEOUT_MS } from '@sim/desktop-bridge'
12
import { ComputerUseSchema } from '@sim/desktop-bridge/computer-use'
23
import { createLogger } from '@sim/logger'
34
import { getErrorMessage } from '@sim/utils/errors'
@@ -17,7 +18,6 @@ import { computerToolResultForModel } from '@/lib/mothership/tools/client/comput
1718
const logger = createLogger('ComputerToolExecution')
1819
const MAX_EVENT_AGE_MS = 120_000
1920
const MAX_UNDELIVERED_RESULTS = 8
20-
const MAX_ACTION_MS = 90_000
2121
const replayLedger = new BrowserToolReplayLedger({
2222
storageKey: 'sim:computer-tool-ledger:v1',
2323
legacyStoragePrefix: 'sim:computer-tool-executed:',
@@ -147,7 +147,7 @@ export async function executeComputerToolOnClient(
147147
timer = setTimeout(() => {
148148
cancel()
149149
rejectTimeout(new Error('Computer action timed out; its effect may be incomplete'))
150-
}, MAX_ACTION_MS)
150+
}, COMPUTER_USE_TOOL_TIMEOUT_MS)
151151
}),
152152
])
153153
execution.completion = cancelled

‎packages/desktop-bridge/src/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ import type {
3636

3737
export const PENDING_DESKTOP_SCOPE_PREFIX = 'pending:' as const
3838

39+
/** Renderer budget for native authorization, app approval, queueing, and execution. */
40+
export const COMPUTER_USE_TOOL_TIMEOUT_MS = 90_000
41+
3942
/** Native work is bound to the server-authorized chat and tool call. */
4043
export interface ComputerUseActivity {
4144
toolCallId: string

0 commit comments

Comments
 (0)