Skip to content

Commit c914f5f

Browse files
committed
fix(desktop): complete native computer tool lifecycle
1 parent b3fa140 commit c914f5f

10 files changed

Lines changed: 156 additions & 50 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: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ vi.mock(
1818
import type { PersistedStreamEventEnvelope } from '@/lib/mothership/request/session/contract'
1919
import type { FilePreviewSession } from '@/lib/mothership/request/session/file-preview-session-contract'
2020
import { toStreamBatchEvent } from '@/lib/mothership/request/session/types'
21+
import { getReplayCompletedWorkflowToolCallIds } from '@/app/workspace/[workspaceId]/home/hooks/message-reconcile'
2122
import { oauthCredentialKeys } from '@/hooks/queries/oauth/oauth-credentials'
2223
import { workspaceCredentialKeys } from '@/hooks/queries/utils/credential-keys'
2324
import { selectorQueryRoots } from '@/hooks/queries/utils/selector-keys'
@@ -132,6 +133,38 @@ describe('tool events (dispatch → model + side effects)', () => {
132133
expect(deps.startClientComputerTool).not.toHaveBeenCalled()
133134
})
134135

136+
it.each([true, false])(
137+
'does not redispatch a completed computer call from a fresh replay batch (success=%s)',
138+
(success) => {
139+
const call = (id: string) =>
140+
toolEnv({
141+
phase: 'call',
142+
executor: 'client',
143+
mode: 'async',
144+
toolCallId: id,
145+
toolName: 'computer',
146+
arguments: { action: 'list_apps' },
147+
})
148+
const events = [
149+
call('computer-complete'),
150+
toolResult('computer-complete', success, 'computer'),
151+
call('computer-unfinished'),
152+
].map(toStreamBatchEvent)
153+
const deps = makeStreamLoopDeps()
154+
deps.options.suppressedWorkflowToolStartIds = getReplayCompletedWorkflowToolCallIds(events)
155+
const ctx = createStreamLoopContext(deps)
156+
157+
for (const entry of events) dispatchStreamEvent(ctx, entry.event)
158+
159+
expect(deps.startClientComputerTool).toHaveBeenCalledExactlyOnceWith(
160+
'computer-unfinished',
161+
{ action: 'list_apps' },
162+
''
163+
)
164+
expect(toolNode(ctx, 'computer-complete').result).toBeDefined()
165+
}
166+
)
167+
135168
it('redelivers an unsettled computer call through the replay-safe native executor after reconnect', () => {
136169
const deps = makeStreamLoopDeps()
137170
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
@@ -326,4 +326,15 @@ describe('getReplayCompletedWorkflowToolCallIds', () => {
326326

327327
expect(result).toEqual(new Set(['workflow-complete']))
328328
})
329+
330+
it('suppresses completed computer and browser calls while keeping unfinished calls eligible', () => {
331+
const result = getReplayCompletedWorkflowToolCallIds([
332+
toolBatchEvent(1, 'computer-complete', 'computer', MothershipStreamV1ToolPhase.call),
333+
toolBatchEvent(2, 'computer-complete', 'computer', MothershipStreamV1ToolPhase.result),
334+
toolBatchEvent(3, 'computer-active', 'computer', MothershipStreamV1ToolPhase.call),
335+
toolBatchEvent(4, 'browser-complete', 'browser_click', MothershipStreamV1ToolPhase.result),
336+
])
337+
338+
expect(result).toEqual(new Set(['computer-complete', 'browser-complete']))
339+
})
329340
})

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

Lines changed: 56 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -342,43 +342,64 @@ describe('sse-handlers tool lifecycle', () => {
342342
)
343343
})
344344

345-
it('pre-persists browser tools as pending for the desktop authorization claim', async () => {
346-
isSimExecuted.mockReturnValue(false)
347-
context.runId = 'run-1'
345+
describe.each(['browser_list_tabs', 'terminal', 'import_local_files', 'computer'])(
346+
'native %s pre-persistence',
347+
(toolName) => {
348+
it.each([false, true])(
349+
'keeps the call pending for the desktop claim when approval gating is %s',
350+
async (gated) => {
351+
isSimExecuted.mockReturnValue(false)
352+
toolRequiresApproval.mockReturnValue(gated)
353+
context.runId = 'run-1'
354+
context.toolPermissions.enabled = gated
355+
const args =
356+
toolName === 'computer'
357+
? { action: 'status' }
358+
: toolName === 'terminal'
359+
? { operation: 'run', command: 'pwd' }
360+
: {}
361+
const event = {
362+
type: MothershipStreamV1EventType.tool,
363+
payload: {
364+
toolCallId: 'native-tool-1',
365+
toolName,
366+
arguments: args,
367+
executor: MothershipStreamV1ToolExecutor.client,
368+
mode: MothershipStreamV1ToolMode.async,
369+
phase: MothershipStreamV1ToolPhase.call,
370+
},
371+
} satisfies StreamEvent
348372

349-
await prePersistClientExecutableToolCall(
350-
{
351-
type: MothershipStreamV1EventType.tool,
352-
payload: {
353-
toolCallId: 'browser-tool-1',
354-
toolName: 'browser_list_tabs',
355-
arguments: {},
356-
executor: MothershipStreamV1ToolExecutor.client,
357-
mode: MothershipStreamV1ToolMode.async,
358-
phase: MothershipStreamV1ToolPhase.call,
359-
},
360-
} satisfies StreamEvent,
361-
context,
362-
{},
363-
execContext
364-
)
373+
await prePersistClientExecutableToolCall(event, context, {}, execContext)
365374

366-
expect(upsertAsyncToolCall).toHaveBeenCalledWith({
367-
runId: 'run-1',
368-
toolCallId: 'browser-tool-1',
369-
toolName: 'browser_list_tabs',
370-
args: {},
371-
sealedContext: { __sealedClientToolContextV1: 'sealed-context' },
372-
status: MothershipStreamV1AsyncToolRecordStatus.pending,
373-
})
374-
expect(sealClientToolContext).toHaveBeenCalledWith({
375-
toolCallId: 'browser-tool-1',
376-
runId: 'run-1',
377-
userId: 'user-1',
378-
registry: execContext.resolvedSecretTraceRegistry,
379-
toolInput: {},
380-
})
381-
})
375+
expect(upsertAsyncToolCall).toHaveBeenCalledExactlyOnceWith({
376+
runId: 'run-1',
377+
toolCallId: 'native-tool-1',
378+
toolName,
379+
args,
380+
sealedContext: { __sealedClientToolContextV1: 'sealed-context' },
381+
status: MothershipStreamV1AsyncToolRecordStatus.pending,
382+
})
383+
expect(sealClientToolContext).toHaveBeenCalledWith({
384+
toolCallId: 'native-tool-1',
385+
runId: 'run-1',
386+
userId: 'user-1',
387+
registry: execContext.resolvedSecretTraceRegistry,
388+
toolInput: args,
389+
})
390+
expect(event.payload).toEqual({
391+
toolCallId: 'native-tool-1',
392+
toolName,
393+
arguments: args,
394+
executor: MothershipStreamV1ToolExecutor.client,
395+
mode: MothershipStreamV1ToolMode.async,
396+
phase: MothershipStreamV1ToolPhase.call,
397+
...(gated ? { status: 'awaiting_approval' } : {}),
398+
})
399+
}
400+
)
401+
}
402+
)
382403

383404
it('persists a gated sim tool and stamps the frame so a reload can still answer it', async () => {
384405
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
@@ -365,6 +365,13 @@ describe('pendingToolWaitBudgetMs', () => {
365365
}
366366
)
367367

368+
it('reserves the whole computer action budget before the lifecycle adds delivery grace', () => {
369+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'executing' })).toBe(90_000)
370+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'awaiting_approval' })).toBe(
371+
TOOL_WATCHDOG_LONG_RUNNING_MS
372+
)
373+
})
374+
368375
it('falls back to the tool\u2019s own watchdog once it is actually executing', () => {
369376
expect(pendingToolWaitBudgetMs({ name: 'terminal_run', status: 'executing' })).toBe(
370377
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)