Skip to content

Commit 9012d95

Browse files
Sg312Vikhyath Mondreti
authored andcommitted
fix(desktop): complete native computer tool lifecycle
1 parent 053aca3 commit 9012d95

9 files changed

Lines changed: 152 additions & 44 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
@@ -348,11 +348,15 @@ export function getReplayCompletedWorkflowToolCallIds(events: StreamBatchEvent[]
348348
const payload = event.payload
349349
if (!('phase' in payload)) continue
350350
if (payload.phase !== MothershipStreamV1ToolPhase.result) continue
351-
// Client-executed tools (workflow runs, browser actions) must never
352-
// re-fire when their completed call replays after reconnect/reload.
351+
/**
352+
* Client-executed tools (workflow runs, browser and computer actions) must never
353+
* re-fire when their completed call replays after reconnect/reload.
354+
*/
353355
if (
354356
typeof payload.toolCallId === 'string' &&
355-
(isWorkflowToolName(payload.toolName) || isBrowserToolName(payload.toolName))
357+
(isWorkflowToolName(payload.toolName) ||
358+
isBrowserToolName(payload.toolName) ||
359+
payload.toolName === 'computer')
356360
) {
357361
completedToolCallIds.add(payload.toolCallId)
358362
}

‎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
@@ -12,6 +12,8 @@ vi.mock(
1212
)
1313

1414
import type { PersistedStreamEventEnvelope } from '@/lib/mothership/request/session/contract'
15+
import { toStreamBatchEvent } from '@/lib/mothership/request/session/types'
16+
import { getReplayCompletedWorkflowToolCallIds } from '@/app/workspace/[workspaceId]/home/hooks/message-reconcile'
1517
import { dispatchStreamEvent } from './dispatch-stream-event'
1618
import { createStreamLoopContext, type StreamLoopContext } from './stream-context'
1719
import { makeStreamLoopDeps, ref } from './stream-test-helpers'
@@ -99,6 +101,38 @@ describe('tool events (dispatch → model + side effects)', () => {
99101
expect(deps.startClientComputerTool).not.toHaveBeenCalled()
100102
})
101103

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

187187
expect(result).toEqual(new Set(['workflow-complete']))
188188
})
189+
190+
it('suppresses completed computer and browser calls while keeping unfinished calls eligible', () => {
191+
const result = getReplayCompletedWorkflowToolCallIds([
192+
toolBatchEvent(1, 'computer-complete', 'computer', MothershipStreamV1ToolPhase.call),
193+
toolBatchEvent(2, 'computer-complete', 'computer', MothershipStreamV1ToolPhase.result),
194+
toolBatchEvent(3, 'computer-active', 'computer', MothershipStreamV1ToolPhase.call),
195+
toolBatchEvent(4, 'browser-complete', 'browser_click', MothershipStreamV1ToolPhase.result),
196+
])
197+
198+
expect(result).toEqual(new Set(['computer-complete', 'browser-complete']))
199+
})
189200
})

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

Lines changed: 59 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -326,44 +326,65 @@ describe('sse-handlers tool lifecycle', () => {
326326
)
327327
})
328328

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

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

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,14 @@ describe('pendingToolWaitBudgetMs', () => {
281281
)
282282
})
283283

284+
it('reserves the whole computer action budget before the lifecycle adds delivery grace', () => {
285+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'executing' })).toBe(90_000)
286+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'executing' }, 'device-1')).toBe(CLIENT_TOOL_RESULT_TIMEOUT_MS)
287+
expect(pendingToolWaitBudgetMs({ name: 'computer', status: 'awaiting_approval' })).toBe(
288+
TOOL_WATCHDOG_LONG_RUNNING_MS
289+
)
290+
})
291+
284292
it('falls back to the tool\u2019s own watchdog once it is actually executing', () => {
285293
expect(pendingToolWaitBudgetMs({ name: 'terminal_run', status: 'executing' })).toBe(
286294
TOOL_WATCHDOG_DEFAULT_MS

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

Lines changed: 2 additions & 0 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 { isTerminalToolName, resolveRunWaitMs } from '@sim/terminal-protocol'
45
import { toError } from '@sim/utils/errors'
@@ -271,6 +272,7 @@ export function pendingToolWaitBudgetMs(
271272
const executableName = toolCall?.execName ?? toolCall?.name
272273
if (desktopDeviceId && executableName && isDesktopToolCall(executableName, toolCall?.params))
273274
return CLIENT_TOOL_RESULT_TIMEOUT_MS
275+
if (executableName === 'computer') return COMPUTER_USE_TOOL_TIMEOUT_MS
274276
if (executableName && isCurrentBrowserToolName(executableName)) {
275277
return browserToolRendererTimeoutMs(executableName, toolCall?.params)
276278
}

‎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
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)