Skip to content

Commit bd54c7c

Browse files
committed
fix(mothership): fail unclaimed desktop calls fast and stop dropping them silently
A desktop call reaches the user's machine only through the chat view showing that chat, so one issued while the user is elsewhere was never claimed and the turn waited out a 90-160 s watchdog that called it hung. One desktop wait now fails a call still unclaimed after a 15 s pickup grace as never started, with the inverse of the desktop's claim (pending -> failed) and a result sealed like any client completion. A call claimed in time keeps waiting, and a result that lands as the grace runs out is returned, not discarded. Local reads (read_local_file, user-local VFS reads) join them for a desktop that advertises localReadClaims: the desktop claims each read through authorize, as it already does for imports. Older desktops keep today's behaviour. The single "hung and was abandoned" result becomes two: notStarted for an unclaimed call, and outcomeUnknown/doNotRetry for one that started and lost its result. Both are settled through one sealed failure helper. A terminal run's server budget now follows the wait the desktop holds it for. A not-started report can settle only an unclaimed call. In the renderer, a running browser action is cancelled only by Stop: recovering the stream or leaving the chat view lets it finish and report. A terminal call delivered too late is reported as not started instead of being skipped.
1 parent 46cd710 commit bd54c7c

28 files changed

Lines changed: 1041 additions & 114 deletions

File tree

‎apps/desktop/src/main/ipc.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ describe('registerIpcHandlers', () => {
485485
expect(mounts).not.toHaveBeenCalled()
486486
expect(fetchAuthorization).toHaveBeenCalledWith(
487487
`${APP}/api/desktop/tool/authorize`,
488-
expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native' }) })
488+
expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native', claim: true }) })
489489
)
490490
expect(
491491
await handler?.(evilEvent, { operation: 'read', toolCallId: 'tool-native' })
@@ -569,7 +569,7 @@ describe('registerIpcHandlers', () => {
569569
expect.objectContaining({
570570
method: 'POST',
571571
credentials: 'include',
572-
body: JSON.stringify({ toolCallId: 'tool-1' }),
572+
body: JSON.stringify({ toolCallId: 'tool-1', claim: true }),
573573
})
574574
)
575575
})

‎apps/desktop/src/main/ipc.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,10 +578,13 @@ async function authorizeLocalFilesystemTool(
578578
request: unknown
579579
): Promise<boolean> {
580580
if (typeof request !== 'object' || request === null) return false
581+
// Claimed like an import: the server sees the read picked up, and refuses one it already
582+
// failed as never started.
581583
const authorization = await fetchDesktopToolAuthorization(
582584
event,
583585
deps,
584-
(request as { requestId?: unknown }).requestId
586+
(request as { requestId?: unknown }).requestId,
587+
true
585588
)
586589
return authorization
587590
? deps.localFilesystem.isAuthorizedClientToolRequest(request, authorization)
@@ -2113,7 +2116,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
21132116
event,
21142117
deps,
21152118
request.toolCallId,
2116-
request.operation === 'manifest',
2119+
request.operation === 'manifest' || request.operation === 'read',
21172120
(status) => {
21182121
failureStatus = status
21192122
}

‎apps/desktop/src/main/terminal/index.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,10 @@ import { homedir } from 'node:os'
1414
import type { TerminalShortcutCommand } from '@sim/desktop-bridge'
1515
import { createLogger } from '@sim/logger'
1616
import {
17-
DEFAULT_RUN_WAIT_MS,
1817
isTerminalControlKey,
1918
MAX_INPUT_KEYS,
20-
MAX_RUN_WAIT_MS,
2119
MAX_TOOL_OUTPUT_CHARS,
20+
resolveRunWaitMs,
2221
type TerminalCommandEvent,
2322
type TerminalControlKey,
2423
type TerminalCwdResult,
@@ -109,14 +108,6 @@ const HANDOFF_MAX_MS = 12 * 60 * 60 * 1000
109108
*/
110109
const HANDOFF_SETTLE_MS = 5_000
111110

112-
/** How long to hold the turn before handing a still-running command back. */
113-
function resolveWaitMs(waitSeconds: number | undefined): number {
114-
const requested = Number(waitSeconds)
115-
return Number.isFinite(requested) && requested > 0
116-
? Math.min(requested * 1000, MAX_RUN_WAIT_MS)
117-
: DEFAULT_RUN_WAIT_MS
118-
}
119-
120111
function elideOutput(value: string): { text: string; truncated: boolean } {
121112
return elide(value, MAX_TOOL_OUTPUT_CHARS)
122113
}
@@ -1097,7 +1088,7 @@ export class TerminalService {
10971088
const handle = await startRun(session, command, terminal.currentCwd, terminal.env)
10981089
if ('error' in handle) throw new TerminalError('SPAWN_FAILED', handle.error)
10991090

1100-
const waitMs = resolveWaitMs(args.waitSeconds)
1091+
const waitMs = resolveRunWaitMs(args.waitSeconds)
11011092
const outcome = await awaitRun(handle, waitMs)
11021093
if (outcome.done) {
11031094
await closeRunWindow(handle, terminal.env)
@@ -1149,7 +1140,7 @@ export class TerminalService {
11491140
)
11501141
}
11511142

1152-
return session.runCommand(command, toolCallId, resolveWaitMs(args.waitSeconds))
1143+
return session.runCommand(command, toolCallId, resolveRunWaitMs(args.waitSeconds))
11531144
}
11541145

11551146
private spawn(

‎apps/desktop/src/preload/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ const api: SimDesktopApi = {
156156
ipcRenderer.invoke('desktop:local-filesystem', request),
157157
localFiles: (request: DesktopLocalFileRequest): Promise<DesktopLocalFileResponse> =>
158158
ipcRenderer.invoke('desktop:local-files', request),
159+
localReadClaims: true,
159160
onCommand: (callback: (command: DesktopCommand) => void): (() => void) => {
160161
const listener = (_event: unknown, command: DesktopCommand) => callback(command)
161162
ipcRenderer.on('desktop:command', listener)

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,17 @@ export const POST = withRouteHandler((req: NextRequest) => {
330330
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
331331
return createNotFoundResponse('Running client tool call not found')
332332
}
333+
// A reporter that says the call never started (a stale replay, a closed view) cannot speak
334+
// for a call the desktop claimed: only the claim's own result may settle it.
335+
if (
336+
isNativeClientTool &&
337+
isPlainRecord(data) &&
338+
data.notStarted === true &&
339+
existing.status !== ASYNC_TOOL_STATUS.pending
340+
) {
341+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
342+
return createNotFoundResponse('Pending client tool call not found')
343+
}
333344

334345
let effectiveStatus = status
335346
let executionId = submittedExecutionId

‎apps/sim/app/api/desktop/tool/authorize/route.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
createNotFoundResponse,
2020
createUnauthorizedResponse,
2121
} from '@/lib/mothership/request/http'
22+
import { isLocalReadToolCall } from '@/lib/mothership/tools/desktop-tools'
2223
import { isUserLocalVfsToolCall } from '@/lib/mothership/tools/local-filesystem'
2324

2425
const admissionClosedResponse = () =>
@@ -129,6 +130,23 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
129130
return createNotFoundResponse('The import must be started before reading file bytes')
130131
}
131132

133+
// A desktop that claims local reads claims each one before its first read; its later reads of
134+
// the same call ride on that claim. A call persisted running (an older desktop's turn) is read
135+
// as before.
136+
if (parsed.data.body.claim && isLocalReadToolCall(toolCall.toolName, args)) {
137+
const notPending = () => createNotFoundResponse('Pending client tool call not found')
138+
if (toolCall.status === 'pending') {
139+
const { outcome } = await claimToolExecution({
140+
toolCallId: toolCall.toolCallId,
141+
runId: toolCall.runId,
142+
userId,
143+
claimedBy: DESKTOP_TOOL_CLAIM_OWNER.files,
144+
})
145+
if (outcome !== 'claimed') return refusedClaimResponse(outcome, notPending)
146+
} else if (toolCall.claimedBy !== null && toolCall.claimedBy !== DESKTOP_TOOL_CLAIM_OWNER.files)
147+
return notPending()
148+
}
149+
132150
// Browser and terminal actions are one-shot side effects on the user's own
133151
// machine, so the pending call is claimed here, atomically, before crossing
134152
// the Electron boundary — a replayed renderer event must not run a command

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

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@
2121

2222
import { act, type ReactNode, StrictMode, useEffect, useState } from 'react'
2323
import { authClientMock, authClientMockFns } from '@sim/testing/mocks/auth-client.mock'
24+
import { libDesktopMock, libDesktopMockFns } from '@sim/testing/mocks/lib-desktop.mock'
2425
import { nextNavigationMock, nextNavigationMockFns } from '@sim/testing/mocks/next-navigation.mock'
2526
import { sleep } from '@sim/utils/helpers'
2627
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
2728
import { createRoot, type Root } from 'react-dom/client'
2829
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2930

30-
const { mockRequestJson, mockExecuteWorkflow } = vi.hoisted(() => ({
31+
const { mockRequestJson, mockExecuteWorkflow, mockExecuteBrowserToolOnClient } = vi.hoisted(() => ({
32+
mockExecuteBrowserToolOnClient: vi.fn(),
3133
mockRequestJson: vi.fn(),
3234
mockExecuteWorkflow:
3335
vi.fn<
@@ -44,6 +46,10 @@ vi.mock('@/app/workspace/[workspaceId]/providers/feature-flags-provider', () =>
4446
}))
4547

4648
vi.mock('next/navigation', () => nextNavigationMock)
49+
vi.mock('@/lib/desktop', () => libDesktopMock)
50+
vi.mock('@/lib/mothership/tools/client/browser-tool-execution', () => ({
51+
executeBrowserToolOnClient: mockExecuteBrowserToolOnClient,
52+
}))
4753
vi.mock('@/lib/auth/auth-client', () => authClientMock)
4854
vi.unmock('@/stores/execution/store')
4955
vi.unmock('@/stores/terminal')
@@ -2039,4 +2045,109 @@ describe('useChat remount send recovery', () => {
20392045
?.messages.map((message) => message.id)
20402046
).toEqual(['saved-user', 'saved-assistant'])
20412047
})
2048+
describe('a desktop browser action in flight', () => {
2049+
const chatId = 'chat-browser-action'
2050+
const history: MothershipChatHistory = {
2051+
id: chatId,
2052+
mode: 'agent',
2053+
title: 'Browser action',
2054+
messages: [],
2055+
activeStreamId: null,
2056+
resources: [],
2057+
}
2058+
2059+
/** Opens a turn whose stream delivers one desktop browser call and stays open. */
2060+
async function startBrowserAction() {
2061+
let streamId: string | undefined
2062+
const replays: string[] = []
2063+
mockRequestJson.mockImplementation((contract: AnyApiRouteContract) =>
2064+
Promise.resolve(
2065+
contract.path === '/api/mothership/chats/[chatId]'
2066+
? { chat: { ...history, activeStreamId: streamId ?? null } }
2067+
: { chats: [] }
2068+
)
2069+
)
2070+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2071+
const url = String(input)
2072+
if (url.includes('/api/mothership/chat/stream')) replays.push(url)
2073+
if (url !== '/api/mothership/chat' || init?.method !== 'POST') {
2074+
return fetchStub(input, init)
2075+
}
2076+
streamId = JSON.parse(String(init.body)).userMessageId
2077+
const call: MothershipStreamV1EventEnvelope = {
2078+
v: 1,
2079+
seq: 1,
2080+
ts: new Date().toISOString(),
2081+
type: 'tool',
2082+
stream: { streamId: streamId ?? '' },
2083+
payload: {
2084+
phase: 'call',
2085+
executor: 'client',
2086+
mode: 'async',
2087+
toolName: 'browser_list_tabs',
2088+
toolCallId: 'browser-call',
2089+
arguments: {},
2090+
},
2091+
}
2092+
return new Response(
2093+
new ReadableStream<Uint8Array>({
2094+
start(controller) {
2095+
controller.enqueue(new TextEncoder().encode(`data: ${JSON.stringify(call)}\n\n`))
2096+
},
2097+
}),
2098+
{ headers: { 'Content-Type': 'text/event-stream', 'x-mothership-chat-id': chatId } }
2099+
)
2100+
})
2101+
const chat = renderUseChatInChat(chatId, history)
2102+
await act(async () => {
2103+
void chat.getResult().sendMessage('List my tabs')
2104+
})
2105+
await waitFor(() => mockExecuteBrowserToolOnClient.mock.calls.length === 1)
2106+
const toolSignal = mockExecuteBrowserToolOnClient.mock.calls[0]?.[5]
2107+
if (!(toolSignal instanceof AbortSignal))
2108+
throw new Error('The browser action has no lifetime')
2109+
return { ...chat, toolSignal, replays }
2110+
}
2111+
2112+
beforeEach(() => {
2113+
libDesktopMockFns.mockIsDesktopApp.mockReturnValue(true)
2114+
})
2115+
2116+
afterEach(() => {
2117+
libDesktopMockFns.mockIsDesktopApp.mockReset()
2118+
})
2119+
2120+
it('keeps running when the window returns to view and the stream is recovered', async () => {
2121+
const { toolSignal, replays } = await startBrowserAction()
2122+
2123+
Object.defineProperty(document, 'visibilityState', {
2124+
configurable: true,
2125+
get: () => 'visible',
2126+
})
2127+
await act(async () => {
2128+
document.dispatchEvent(new Event('visibilitychange'))
2129+
})
2130+
await waitFor(() => replays.length > 0)
2131+
2132+
expect(toolSignal.aborted).toBe(false)
2133+
})
2134+
2135+
it('keeps running when the chat view unmounts, so it finishes and reports its result', async () => {
2136+
const { toolSignal, unmount } = await startBrowserAction()
2137+
2138+
unmount()
2139+
2140+
expect(toolSignal.aborted).toBe(false)
2141+
})
2142+
2143+
it('is cancelled when the user stops the chat', async () => {
2144+
const { toolSignal, getResult } = await startBrowserAction()
2145+
2146+
await act(async () => {
2147+
await getResult().stopGeneration()
2148+
})
2149+
2150+
expect(toolSignal.aborted).toBe(true)
2151+
})
2152+
})
20422153
})

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

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,25 @@ export async function waitForDetachedChatResolution(
452452
}
453453
}
454454

455+
/** The abort reason of the user's Stop. */
456+
const USER_STOP_ABORT_REASON = 'user_stop:client_stopGeneration'
457+
458+
/**
459+
* The lifetime a browser action started from one stream observes: only the user's Stop cancels
460+
* it. Replacing the stream reader (the window returning to view, a history reconnect) or leaving
461+
* the chat view leaves it running, so it finishes and reports its own result.
462+
*/
463+
function browserToolLifetime(streamSignal: AbortSignal | undefined): AbortSignal | undefined {
464+
if (!streamSignal) return undefined
465+
const lifetime = new AbortController()
466+
const followStop = () => {
467+
if (streamSignal.reason === USER_STOP_ABORT_REASON) lifetime.abort(USER_STOP_ABORT_REASON)
468+
}
469+
if (streamSignal.aborted) followStop()
470+
else streamSignal.addEventListener('abort', followStop, { once: true })
471+
return lifetime.signal
472+
}
473+
455474
/**
456475
* Runs a browser tool on the desktop client. The agent's tab reaches the
457476
* resource strip through the desktop tab list, so nothing is opened here.
@@ -2151,7 +2170,7 @@ export function useChat(
21512170
shouldContinue?: () => boolean
21522171
}
21532172
) => {
2154-
const streamAbortSignal = abortControllerRef.current?.signal
2173+
const browserToolSignal = browserToolLifetime(abortControllerRef.current?.signal)
21552174
const activityTracker = getResourceActivityTracker(
21562175
expectedGen ?? streamGenRef.current,
21572176
options?.targetChatId
@@ -2172,7 +2191,7 @@ export function useChat(
21722191
eventTs?: string
21732192
) => {
21742193
const scopeId = activityScopeId()
2175-
startClientBrowserTool(toolCallId, toolName, toolArgs, scopeId, eventTs, streamAbortSignal)
2194+
startClientBrowserTool(toolCallId, toolName, toolArgs, scopeId, eventTs, browserToolSignal)
21762195
}
21772196
const startClientTerminalToolForStream = (
21782197
toolCallId: string,
@@ -4424,7 +4443,7 @@ export function useChat(
44244443
streamReaderRef.current = null
44254444
const stoppedController = abortControllerRef.current
44264445
if (stoppedController !== pendingAdmission?.controller) {
4427-
stoppedController?.abort('user_stop:client_stopGeneration')
4446+
stoppedController?.abort(USER_STOP_ABORT_REASON)
44284447
}
44294448
abortControllerRef.current = null
44304449
setTransportIdle()
@@ -4520,7 +4539,7 @@ export function useChat(
45204539
if (pendingAdmission && pendingAdmission.userMessageId === sid) {
45214540
const admittedChatId = await pendingAdmission.settled
45224541
resolvedChatId ??= admittedChatId
4523-
pendingAdmission.controller.abort('user_stop:client_stopGeneration')
4542+
pendingAdmission.controller.abort(USER_STOP_ABORT_REASON)
45244543
}
45254544
if (!resolvedChatId && sid) {
45264545
resolvedChatId = await resolveChatIdForStream(sid, { preferExistingChatId: false })

‎apps/sim/lib/desktop/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ export interface DesktopChatCapabilities {
156156
desktopCapabilities?: {
157157
localFiles?: true
158158
localFilesystem?: true
159+
localReadClaims?: true
159160
browser?: true
160161
terminal?: true
161162
browserSessions?: BrowserKnownSession[]
@@ -217,6 +218,7 @@ export async function getDesktopChatCapabilities(
217218
? {
218219
desktopCapabilities: {
219220
...(localFiles ? { localFiles: true as const } : {}),
221+
...(bridge?.localReadClaims === true ? { localReadClaims: true as const } : {}),
220222
...(localFilesystem ? { localFilesystem: true as const } : {}),
221223
...(browser ? { browser: true as const } : {}),
222224
...(terminal ? { terminal: true as const } : {}),

‎apps/sim/lib/mothership/chat/post.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ const ChatMessageSchema = z
312312
.object({
313313
localFilesystem: z.boolean().optional(),
314314
localFiles: z.boolean().optional(),
315+
localReadClaims: z.boolean().optional(),
315316
browser: z.boolean().optional(),
316317
terminal: z.boolean().optional(),
317318
terminals: z
@@ -1517,6 +1518,7 @@ export async function handleUnifiedChatPost(req: NextRequest) {
15171518
// Executor routing is decided HERE, once per turn, from the caller's declared
15181519
// capabilities — dispatch never discovers client absence by burning a grace timer.
15191520
clientToolPickupExpected,
1521+
desktopClaimsLocalReads: body.desktopCapabilities?.localReadClaims === true,
15201522
executionContext,
15211523
billingAttribution: executionContext.billingAttribution,
15221524
onComplete: buildOnComplete({

0 commit comments

Comments
 (0)