Skip to content

Commit 7cf241e

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. After a 15 s pickup grace the still-pending call is now settled as never started with the inverse of the desktop's claim (pending -> failed), so a late claim is refused and a call claimed in time keeps its full budget. The single 'hung and was abandoned' result becomes two: notStarted (safe to retry) for an unclaimed call, and outcomeUnknown/doNotRetry for a call that started and lost its result. A terminal run's server budget now follows the wait the desktop holds it for. In the renderer, a browser action whose chat view closes reports what became of it instead of returning silently, recovering the stream when the window returns to view no longer aborts running actions, and a terminal call delivered too late is reported as not started instead of skipped.
1 parent d4f26a7 commit 7cf241e

14 files changed

Lines changed: 870 additions & 57 deletions

File tree

‎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/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx‎

Lines changed: 105 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,11 @@ 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+
CHAT_VIEW_CLOSED_ABORT_REASON: 'chat_view_closed',
52+
executeBrowserToolOnClient: mockExecuteBrowserToolOnClient,
53+
}))
4754
vi.mock('@/lib/auth/auth-client', () => authClientMock)
4855
vi.unmock('@/stores/execution/store')
4956
vi.unmock('@/stores/terminal')
@@ -76,6 +83,7 @@ import { MothershipHandoffStorage } from '@/lib/core/utils/browser-storage'
7683
import { MOTHERSHIP_STREAM_REPLAY_HEADER } from '@/lib/mothership/constants'
7784
import type { MothershipStreamV1EventEnvelope } from '@/lib/mothership/generated/mothership-stream-v1'
7885
import { getChatResourceSelectionId } from '@/lib/mothership/resources/types'
86+
import { CHAT_VIEW_CLOSED_ABORT_REASON } from '@/lib/mothership/tools/client/browser-tool-execution'
7987
import { collectCitedMessageSources } from '@/app/workspace/[workspaceId]/home/components/message-content/message-sources'
8088
import {
8189
readQueuedSendHandoffState,
@@ -2039,4 +2047,100 @@ describe('useChat remount send recovery', () => {
20392047
?.messages.map((message) => message.id)
20402048
).toEqual(['saved-user', 'saved-assistant'])
20412049
})
2050+
describe('a desktop browser action in flight', () => {
2051+
const chatId = 'chat-browser-action'
2052+
const history: MothershipChatHistory = {
2053+
id: chatId,
2054+
mode: 'agent',
2055+
title: 'Browser action',
2056+
messages: [],
2057+
activeStreamId: null,
2058+
resources: [],
2059+
}
2060+
2061+
/** Opens a turn whose stream delivers one desktop browser call and stays open. */
2062+
async function startBrowserAction() {
2063+
let streamId: string | undefined
2064+
const replays: string[] = []
2065+
mockRequestJson.mockImplementation((contract: AnyApiRouteContract) =>
2066+
Promise.resolve(
2067+
contract.path === '/api/mothership/chats/[chatId]'
2068+
? { chat: { ...history, activeStreamId: streamId ?? null } }
2069+
: { chats: [] }
2070+
)
2071+
)
2072+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2073+
const url = String(input)
2074+
if (url.includes('/api/mothership/chat/stream')) replays.push(url)
2075+
if (url !== '/api/mothership/chat' || init?.method !== 'POST') {
2076+
return fetchStub(input, init)
2077+
}
2078+
streamId = JSON.parse(String(init.body)).userMessageId
2079+
const call: MothershipStreamV1EventEnvelope = {
2080+
v: 1,
2081+
seq: 1,
2082+
ts: new Date().toISOString(),
2083+
type: 'tool',
2084+
stream: { streamId: streamId ?? '' },
2085+
payload: {
2086+
phase: 'call',
2087+
executor: 'client',
2088+
mode: 'async',
2089+
toolName: 'browser_list_tabs',
2090+
toolCallId: 'browser-call',
2091+
arguments: {},
2092+
},
2093+
}
2094+
return new Response(
2095+
new ReadableStream<Uint8Array>({
2096+
start(controller) {
2097+
controller.enqueue(new TextEncoder().encode(`data: ${JSON.stringify(call)}\n\n`))
2098+
},
2099+
}),
2100+
{ headers: { 'Content-Type': 'text/event-stream', 'x-mothership-chat-id': chatId } }
2101+
)
2102+
})
2103+
const chat = renderUseChatInChat(chatId, history)
2104+
await act(async () => {
2105+
void chat.getResult().sendMessage('List my tabs')
2106+
})
2107+
await waitFor(() => mockExecuteBrowserToolOnClient.mock.calls.length === 1)
2108+
const toolSignal = mockExecuteBrowserToolOnClient.mock.calls[0]?.[5]
2109+
if (!(toolSignal instanceof AbortSignal))
2110+
throw new Error('The browser action has no lifetime')
2111+
return { ...chat, toolSignal, replays }
2112+
}
2113+
2114+
beforeEach(() => {
2115+
libDesktopMockFns.mockIsDesktopApp.mockReturnValue(true)
2116+
})
2117+
2118+
afterEach(() => {
2119+
libDesktopMockFns.mockIsDesktopApp.mockReset()
2120+
})
2121+
2122+
it('keeps running when the window returns to view and the stream is recovered', async () => {
2123+
const { toolSignal, replays } = await startBrowserAction()
2124+
2125+
Object.defineProperty(document, 'visibilityState', {
2126+
configurable: true,
2127+
get: () => 'visible',
2128+
})
2129+
await act(async () => {
2130+
document.dispatchEvent(new Event('visibilitychange'))
2131+
})
2132+
await waitFor(() => replays.length > 0)
2133+
2134+
expect(toolSignal.aborted).toBe(false)
2135+
})
2136+
2137+
it('ends as a closed chat view, not a Stop, when the chat view unmounts', async () => {
2138+
const { toolSignal, unmount } = await startBrowserAction()
2139+
2140+
unmount()
2141+
2142+
expect(toolSignal.aborted).toBe(true)
2143+
expect(toolSignal.reason).toBe(CHAT_VIEW_CLOSED_ABORT_REASON)
2144+
})
2145+
})
20422146
})

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

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,10 @@ import {
7878
reorderStoredChatResources,
7979
sanitizeChatResources,
8080
} from '@/lib/mothership/resources/types'
81-
import { executeBrowserToolOnClient } from '@/lib/mothership/tools/client/browser-tool-execution'
81+
import {
82+
CHAT_VIEW_CLOSED_ABORT_REASON,
83+
executeBrowserToolOnClient,
84+
} from '@/lib/mothership/tools/client/browser-tool-execution'
8285
import {
8386
bindRunToolToExecution,
8487
executeRunToolOnClient,
@@ -452,6 +455,35 @@ export async function waitForDetachedChatResolution(
452455
}
453456
}
454457

458+
/** Aborts that only replace the stream reader; the turn and its running tools carry on. */
459+
const SUPERSEDED_RECOVERY_ABORT_REASON = 'superseded_recovery'
460+
const SUPERSEDED_HISTORY_RECONNECT_ABORT_REASON = 'superseded_chat_history_reconnect'
461+
const REPLACED_RECOVERY_SUBJECT_ABORT_REASON = 'replaced_by_new_recovery_subject'
462+
const READER_REPLACEMENT_ABORT_REASONS: ReadonlySet<unknown> = new Set([
463+
SUPERSEDED_RECOVERY_ABORT_REASON,
464+
SUPERSEDED_HISTORY_RECONNECT_ABORT_REASON,
465+
REPLACED_RECOVERY_SUBJECT_ABORT_REASON,
466+
])
467+
const UNMOUNT_ABORT_REASON = 'unmount:client_cleanup'
468+
469+
/**
470+
* The lifetime a browser tool started from one stream observes. Replacing the reader (a window
471+
* returning to view, a history reconnect) leaves an in-flight action running; an unmount ends it
472+
* as a closed chat view, which still reports its outcome; a Stop ends it silently.
473+
*/
474+
function browserToolLifetime(streamSignal: AbortSignal | undefined): AbortSignal | undefined {
475+
if (!streamSignal) return undefined
476+
const lifetime = new AbortController()
477+
const follow = () => {
478+
const reason: unknown = streamSignal.reason
479+
if (READER_REPLACEMENT_ABORT_REASONS.has(reason)) return
480+
lifetime.abort(reason === UNMOUNT_ABORT_REASON ? CHAT_VIEW_CLOSED_ABORT_REASON : reason)
481+
}
482+
if (streamSignal.aborted) follow()
483+
else streamSignal.addEventListener('abort', follow, { once: true })
484+
return lifetime.signal
485+
}
486+
455487
/**
456488
* Runs a browser tool on the desktop client. The agent's tab reaches the
457489
* resource strip through the desktop tab list, so nothing is opened here.
@@ -1014,7 +1046,7 @@ export function useChat(
10141046
const cancelActiveStreamRecovery = useCallback(() => {
10151047
const recovery = activeStreamReturnRecoveryRef.current
10161048
if (!recovery) return
1017-
recovery.controller.abort('superseded_recovery')
1049+
recovery.controller.abort(SUPERSEDED_RECOVERY_ABORT_REASON)
10181050
activeStreamReturnRecoveryRef.current = null
10191051
}, [])
10201052

@@ -2073,7 +2105,7 @@ export function useChat(
20732105
cancelActiveStreamRecovery()
20742106
const replacedController = abortControllerRef.current
20752107
if (replacedController && !replacedController.signal.aborted) {
2076-
replacedController.abort('superseded_chat_history_reconnect')
2108+
replacedController.abort(SUPERSEDED_HISTORY_RECONNECT_ABORT_REASON)
20772109
}
20782110
cancelActiveStreamReader()
20792111
abortControllerRef.current = abortController
@@ -2151,7 +2183,7 @@ export function useChat(
21512183
shouldContinue?: () => boolean
21522184
}
21532185
) => {
2154-
const streamAbortSignal = abortControllerRef.current?.signal
2186+
const browserToolSignal = browserToolLifetime(abortControllerRef.current?.signal)
21552187
const activityTracker = getResourceActivityTracker(
21562188
expectedGen ?? streamGenRef.current,
21572189
options?.targetChatId
@@ -2172,7 +2204,7 @@ export function useChat(
21722204
eventTs?: string
21732205
) => {
21742206
const scopeId = activityScopeId()
2175-
startClientBrowserTool(toolCallId, toolName, toolArgs, scopeId, eventTs, streamAbortSignal)
2207+
startClientBrowserTool(toolCallId, toolName, toolArgs, scopeId, eventTs, browserToolSignal)
21762208
}
21772209
const startClientTerminalToolForStream = (
21782210
toolCallId: string,
@@ -2965,7 +2997,7 @@ export function useChat(
29652997
return existingRecovery.promise
29662998
}
29672999
if (existingRecovery) {
2968-
existingRecovery.controller.abort('replaced_by_new_recovery_subject')
3000+
existingRecovery.controller.abort(REPLACED_RECOVERY_SUBJECT_ABORT_REASON)
29693001
activeStreamReturnRecoveryRef.current = null
29703002
}
29713003

@@ -3005,7 +3037,7 @@ export function useChat(
30053037

30063038
const replacedController = abortControllerRef.current
30073039
if (replacedController && !replacedController.signal.aborted) {
3008-
replacedController.abort('superseded_recovery')
3040+
replacedController.abort(SUPERSEDED_RECOVERY_ABORT_REASON)
30093041
}
30103042

30113043
const replacedReader = streamReaderRef.current
@@ -3895,7 +3927,7 @@ export function useChat(
38953927
const sendWasAborted =
38963928
(err instanceof Error && err.name === 'AbortError') || sendAbortSignal?.aborted === true
38973929
if (sendWasAborted) {
3898-
if (sendAbortSignal?.reason === 'unmount:client_cleanup' && !sendReachedServer) {
3930+
if (sendAbortSignal?.reason === UNMOUNT_ABORT_REASON && !sendReachedServer) {
38993931
/* A remount ran the unmount cleanup before this send's response
39003932
arrived — a chat-route `key` change, or StrictMode's dev
39013933
double-mount. Nothing was rendered from it, so withdraw the
@@ -4971,7 +5003,7 @@ export function useChat(
49715003
clearQueueDispatchState()
49725004
streamGenRef.current++
49735005
cancelActiveStreamReader()
4974-
abortControllerRef.current?.abort('unmount:client_cleanup')
5006+
abortControllerRef.current?.abort(UNMOUNT_ABORT_REASON)
49755007
abortControllerRef.current = null
49765008
for (const controller of detachedChatResolutionControllers) {
49775009
controller.abort('unmount:detached_chat_resolution')

‎apps/sim/lib/mothership/constants.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ export const CHAT_RUN_DEADLINE_MS = 3_600_000
6666
*/
6767
export const COPILOT_WORKFLOW_TOOL_CLIENT_GRACE_MS = 30_000
6868

69+
/**
70+
* How long a desktop tool call (browser, terminal, local import) waits for the desktop app to
71+
* claim it before it fails as never started.
72+
*
73+
* Same cause as the workflow grace: only the chat view showing this chat starts the call, so a
74+
* call issued while the user is on another chat or page is claimed by nobody. A live view claims
75+
* within a second or two (stream frame -> IPC -> authorize), and there is no server fallback to
76+
* run instead, so the call fails with a "not started, safe to retry" result rather than parking
77+
* until a watchdog calls it hung.
78+
*/
79+
export const DESKTOP_TOOL_PICKUP_GRACE_MS = 15_000
80+
6981
/** SessionStorage key for persisting active stream metadata across page reloads. */
7082
export const STREAM_STORAGE_KEY = 'copilot_active_stream'
7183

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { upsertAsyncToolCall } from '@/lib/mothership/async-runs/repository'
1111
import {
1212
CLIENT_TOOL_RESULT_TIMEOUT_MS,
1313
COPILOT_WORKFLOW_TOOL_CLIENT_GRACE_MS,
14+
DESKTOP_TOOL_PICKUP_GRACE_MS,
1415
} from '@/lib/mothership/constants'
1516
import {
1617
MothershipStreamV1AsyncToolRecordStatus,
@@ -32,6 +33,10 @@ import { markToolResultSeen, wasToolResultSeen } from '@/lib/mothership/request/
3233
import { setTerminalToolCallState } from '@/lib/mothership/request/tool-call-state'
3334
import { waitForClientToolCompletion } from '@/lib/mothership/request/tools/client'
3435
import { sealClientToolContext } from '@/lib/mothership/request/tools/client-completion-seal.server'
36+
import {
37+
isDesktopPickupTool,
38+
waitForDesktopToolPickup,
39+
} from '@/lib/mothership/request/tools/desktop-pickup'
3540
import { executeToolAndReport } from '@/lib/mothership/request/tools/executor'
3641
import {
3742
runGatedToolExecution,
@@ -938,6 +943,16 @@ async function dispatchToolExecution(
938943
return race.signal ?? errorCompletion('Tool completion missing')
939944
}
940945
completion = race.completion ?? null
946+
} else if (isDesktopPickupTool(toolName)) {
947+
completion = await waitForDesktopToolPickup({
948+
toolCallId,
949+
runId: context.runId,
950+
userId: execContext.userId,
951+
timeoutMs,
952+
graceMs: DESKTOP_TOOL_PICKUP_GRACE_MS,
953+
abortSignal: options.abortSignal,
954+
registry: execContext.resolvedSecretTraceRegistry,
955+
})
941956
} else {
942957
completion = await waitForClientToolCompletion({
943958
toolCallId,

0 commit comments

Comments
 (0)