Skip to content

Commit 03d9ebd

Browse files
committed
fix(mothership): keep local file tools running when the chat view changes
Local file reads and imports now take the same Stop-only lifetime as browser actions: a history reconnect, a stream recovery or leaving the chat view leaves them running so they finish and report, and only the user's Stop cancels them. The desktop-tool classifier in client-executed-tools.ts is removed in favour of the single one in tools/desktop-tools.ts. When the desktop app holds a call, confirm now answers 409 to any other reporter (a stale replay, a lost race against the claim), and the client treats 409 as final instead of retrying a 404 five times. The not-started results now tell the model what it can act on: the action never started; don't retry it in this turn; ask the user to keep the chat open in the Sim desktop app. A local read the server cannot see picked up is no longer described as started. The obsolete admission probe script is deleted. Nothing ran it, it no longer type-checked against the repository API, and the integration suites cover the same admission behaviour against real Postgres.
1 parent a766a03 commit 03d9ebd

19 files changed

Lines changed: 133 additions & 483 deletions

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ describe('Copilot Confirm API Route', () => {
165165
})
166166
)
167167

168-
expect(response.status).toBe(404)
168+
expect(response.status).toBe(409)
169169
expect(completeAsyncToolCall).not.toHaveBeenCalled()
170170
expect(detachAsyncToolCall).not.toHaveBeenCalled()
171171
expect(encryptSecret).not.toHaveBeenCalled()
@@ -233,8 +233,10 @@ describe('Copilot Confirm API Route', () => {
233233
})
234234
)
235235

236-
expect(response.status).toBe(404)
237-
expect(await response.json()).toEqual({ error: 'Pending client tool call not found' })
236+
expect(response.status).toBe(409)
237+
expect(await response.json()).toEqual({
238+
error: 'The desktop app holds this tool call; only its own result settles it',
239+
})
238240
expect(completePendingAsyncToolCall).toHaveBeenCalledOnce()
239241
expect(completeClaimedAsyncToolCall).not.toHaveBeenCalled()
240242
expect(completeAsyncToolCall).not.toHaveBeenCalled()
@@ -300,7 +302,7 @@ describe('Copilot Confirm API Route', () => {
300302
})
301303
)
302304

303-
expect(response.status).toBe(404)
305+
expect(response.status).toBe(409)
304306
expect(completeClaimedAsyncToolCall).toHaveBeenCalledWith(expect.any(Object), 'desktop-browser')
305307
expect(publishToolConfirmation).not.toHaveBeenCalled()
306308
})

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ function acknowledgeSettledToolCall(
9898
return createConfirmationResponse(toolCallId, settledStatus, 'Tool call was already settled')
9999
}
100100

101+
/**
102+
* A desktop call this report may not settle: the desktop app holds it under its claim (or a
103+
* report raced that claim and lost), so only the claim's own result settles it. Final, not
104+
* retryable: the reporter stops.
105+
*/
106+
function heldByAnotherReporterResponse(): NextResponse {
107+
return NextResponse.json(
108+
{ error: 'The desktop app holds this tool call; only its own result settles it' },
109+
{ status: 409 }
110+
)
111+
}
112+
101113
/** Atomically finalize or detach a client tool before publishing its wakeup event. */
102114
async function updateToolCallStatus(
103115
existing: NonNullable<Awaited<ReturnType<typeof getAsyncToolCall>>>,
@@ -321,7 +333,11 @@ export const POST = withRouteHandler((req: NextRequest) => {
321333
const isMutableClientToolCall = isWorkflowTool
322334
? isWorkflowToolExecutionClaimable(existing.status, existing.permissionDecision)
323335
: existing.status === ASYNC_TOOL_STATUS.running || isPreclaimNativeTerminalOutcome
324-
if ((isNativeClientTool || isWorkflowTool) && !isMutableClientToolCall) {
336+
if (isNativeClientTool && !isMutableClientToolCall) {
337+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
338+
return heldByAnotherReporterResponse()
339+
}
340+
if (isWorkflowTool && !isMutableClientToolCall) {
325341
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
326342
return createNotFoundResponse('Running client tool call not found')
327343
}
@@ -334,7 +350,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
334350
existing.status !== ASYNC_TOOL_STATUS.pending
335351
) {
336352
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
337-
return createNotFoundResponse('Pending client tool call not found')
353+
return heldByAnotherReporterResponse()
338354
}
339355

340356
let effectiveStatus = status
@@ -478,7 +494,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
478494

479495
if (reconciledOutcome === 'conflict' && isPreclaimNativeTerminalOutcome) {
480496
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
481-
return createNotFoundResponse('Pending client tool call not found')
497+
return heldByAnotherReporterResponse()
482498
}
483499

484500
if (reconciledOutcome !== 'updated') {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ import {
1717
} from '@/lib/mothership/resources/extraction'
1818
import {
1919
isClientExecutedToolCall,
20-
isDesktopExecutedToolCall,
2120
isWorkflowToolName,
2221
} from '@/lib/mothership/tools/client-executed-tools'
22+
import { isDesktopToolCall } from '@/lib/mothership/tools/desktop-tools'
2323
import { invalidateResourceQueries } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry'
2424
import type { StreamLoopContext } from '@/app/workspace/[workspaceId]/home/hooks/stream/stream-context'
2525
import {
@@ -197,7 +197,7 @@ export function handleToolEvent(ctx: StreamLoopContext, parsed: ToolEvent): void
197197
// tools to it: its answer could only be an error, and that error would beat the real result.
198198
const shouldStartClientTool =
199199
isClientExecutedToolCall(name, args) &&
200-
(isDesktopApp() || !isDesktopExecutedToolCall(name, args)) &&
200+
(isDesktopApp() || !isDesktopToolCall(name, args)) &&
201201
!isPartial &&
202202
!deps.options.suppressedWorkflowToolStartIds?.has(rawId) &&
203203
node?.kind === 'tool' &&

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

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
2828
import { createRoot, type Root } from 'react-dom/client'
2929
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
3030

31-
const { mockRequestJson, mockExecuteWorkflow, mockExecuteBrowserToolOnClient } = vi.hoisted(() => ({
31+
const {
32+
mockRequestJson,
33+
mockExecuteWorkflow,
34+
mockExecuteBrowserToolOnClient,
35+
mockExecuteLocalFilesystemTool,
36+
} = vi.hoisted(() => ({
3237
mockExecuteBrowserToolOnClient: vi.fn(),
38+
mockExecuteLocalFilesystemTool: vi.fn(),
3339
mockRequestJson: vi.fn(),
3440
mockExecuteWorkflow:
3541
vi.fn<
@@ -47,6 +53,9 @@ vi.mock('@/app/workspace/[workspaceId]/providers/feature-flags-provider', () =>
4753

4854
vi.mock('next/navigation', () => nextNavigationMock)
4955
vi.mock('@/lib/desktop', () => libDesktopMock)
56+
vi.mock('@/lib/mothership/tools/client/local-filesystem', () => ({
57+
executeLocalFilesystemTool: mockExecuteLocalFilesystemTool,
58+
}))
5059
vi.mock('@/lib/mothership/tools/client/browser-tool-execution', () => ({
5160
executeBrowserToolOnClient: mockExecuteBrowserToolOnClient,
5261
}))
@@ -2129,8 +2138,21 @@ describe('useChat remount send recovery', () => {
21292138
?.messages.map((message) => message.id)
21302139
).toEqual(['saved-user', 'saved-assistant'])
21312140
})
2132-
describe('a desktop browser action in flight', () => {
2133-
const chatId = 'chat-browser-action'
2141+
describe.each([
2142+
{
2143+
kind: 'browser action',
2144+
toolName: 'browser_list_tabs',
2145+
arguments: {},
2146+
lifetimeOf: () => mockExecuteBrowserToolOnClient.mock.calls[0]?.[5],
2147+
},
2148+
{
2149+
kind: 'local file read',
2150+
toolName: 'read_local_file',
2151+
arguments: { path: '/Users/me/notes.txt' },
2152+
lifetimeOf: () => mockExecuteLocalFilesystemTool.mock.calls[0]?.[3]?.signal,
2153+
},
2154+
])('a desktop $kind in flight', ({ toolName, arguments: toolArguments, lifetimeOf }) => {
2155+
const chatId = 'chat-desktop-action'
21342156
const history: MothershipChatHistory = {
21352157
id: chatId,
21362158
mode: 'agent',
@@ -2140,8 +2162,8 @@ describe('useChat remount send recovery', () => {
21402162
resources: [],
21412163
}
21422164

2143-
/** Opens a turn whose stream delivers one desktop browser call and stays open. */
2144-
async function startBrowserAction() {
2165+
/** Opens a turn whose stream delivers one desktop tool call and stays open. */
2166+
async function startDesktopAction() {
21452167
let streamId: string | undefined
21462168
const replays: string[] = []
21472169
mockRequestJson.mockImplementation((contract: AnyApiRouteContract) =>
@@ -2168,9 +2190,9 @@ describe('useChat remount send recovery', () => {
21682190
phase: 'call',
21692191
executor: 'client',
21702192
mode: 'async',
2171-
toolName: 'browser_list_tabs',
2172-
toolCallId: 'browser-call',
2173-
arguments: {},
2193+
toolName,
2194+
toolCallId: 'desktop-call',
2195+
arguments: toolArguments,
21742196
},
21752197
}
21762198
return new Response(
@@ -2186,10 +2208,10 @@ describe('useChat remount send recovery', () => {
21862208
await act(async () => {
21872209
void chat.getResult().sendMessage('List my tabs')
21882210
})
2189-
await waitFor(() => mockExecuteBrowserToolOnClient.mock.calls.length === 1)
2190-
const toolSignal = mockExecuteBrowserToolOnClient.mock.calls[0]?.[5]
2211+
await waitFor(() => lifetimeOf() !== undefined)
2212+
const toolSignal = lifetimeOf()
21912213
if (!(toolSignal instanceof AbortSignal))
2192-
throw new Error('The browser action has no lifetime')
2214+
throw new Error('The desktop action has no lifetime')
21932215
return { ...chat, toolSignal, replays }
21942216
}
21952217

@@ -2202,7 +2224,7 @@ describe('useChat remount send recovery', () => {
22022224
})
22032225

22042226
it('keeps running when the window returns to view and the stream is recovered', async () => {
2205-
const { toolSignal, replays } = await startBrowserAction()
2227+
const { toolSignal, replays } = await startDesktopAction()
22062228

22072229
Object.defineProperty(document, 'visibilityState', {
22082230
configurable: true,
@@ -2217,15 +2239,15 @@ describe('useChat remount send recovery', () => {
22172239
})
22182240

22192241
it('keeps running when the chat view unmounts, so it finishes and reports its result', async () => {
2220-
const { toolSignal, unmount } = await startBrowserAction()
2242+
const { toolSignal, unmount } = await startDesktopAction()
22212243

22222244
unmount()
22232245

22242246
expect(toolSignal.aborted).toBe(false)
22252247
})
22262248

22272249
it('is cancelled when the user stops the chat', async () => {
2228-
const { toolSignal, getResult } = await startBrowserAction()
2250+
const { toolSignal, getResult } = await startDesktopAction()
22292251

22302252
await act(async () => {
22312253
await getResult().stopGeneration()

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -457,11 +457,12 @@ export async function waitForDetachedChatResolution(
457457
const USER_STOP_ABORT_REASON = 'user_stop:client_stopGeneration'
458458

459459
/**
460-
* The lifetime a browser action started from one stream observes: only the user's Stop cancels
461-
* it. Replacing the stream reader (the window returning to view, a history reconnect) or leaving
462-
* the chat view leaves it running, so it finishes and reports its own result.
460+
* The lifetime a desktop tool (a browser action, a local file read or import) started from one
461+
* stream observes: only the user's Stop cancels it. Replacing the stream reader (the window
462+
* returning to view, a history reconnect) or leaving the chat view leaves it running, so it
463+
* finishes and reports its own result.
463464
*/
464-
function browserToolLifetime(streamSignal: AbortSignal | undefined): AbortSignal | undefined {
465+
function desktopToolLifetime(streamSignal: AbortSignal | undefined): AbortSignal | undefined {
465466
if (!streamSignal) return undefined
466467
const lifetime = new AbortController()
467468
const followStop = () => {
@@ -1595,7 +1596,7 @@ export function useChat(
15951596
const options = {
15961597
workspaceId,
15971598
chatId: chatIdRef.current ?? selectedChatIdRef.current,
1598-
signal: abortControllerRef.current?.signal,
1599+
signal: desktopToolLifetime(abortControllerRef.current?.signal),
15991600
}
16001601
/**
16011602
* Dynamic on purpose: the local-filesystem executor only runs for desktop-local
@@ -2171,7 +2172,7 @@ export function useChat(
21712172
shouldContinue?: () => boolean
21722173
}
21732174
) => {
2174-
const browserToolSignal = browserToolLifetime(abortControllerRef.current?.signal)
2175+
const browserToolSignal = desktopToolLifetime(abortControllerRef.current?.signal)
21752176
const activityTracker = getResourceActivityTracker(
21762177
expectedGen ?? streamGenRef.current,
21772178
options?.targetChatId

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const COPILOT_WORKFLOW_TOOL_CLIENT_GRACE_MS = 30_000
7373
* Same cause as the workflow grace: only the chat view showing this chat starts the call, so a
7474
* call issued while the user is on another chat or page is claimed by nobody. A live view claims
7575
* 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
76+
* run instead, so the call fails with a "never started" result rather than parking
7777
* until a watchdog calls it hung.
7878
*/
7979
export const DESKTOP_TOOL_PICKUP_GRACE_MS = 15_000

‎apps/sim/lib/mothership/request/tools/desktop-wait.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secr
1111
const logger = createLogger('CopilotDesktopToolWait')
1212

1313
const DESKTOP_TOOL_NOT_STARTED_MESSAGE =
14-
'Not run: this chat is not open in the Sim desktop app, so nothing picked up this call and nothing happened on the user’s computer. It is safe to retry once the user opens this chat in the Sim desktop app.'
14+
'Not run: this action never started, because nothing in the Sim desktop app picked it up (this chat is not open there). Nothing happened on the user’s computer. Do not retry it in this turn; tell the user to keep this chat open in the Sim desktop app, or to ask again later.'
1515

1616
/** The model-facing result of a desktop call that was never picked up. */
1717
export function desktopToolNotStarted(): { message: string; data: Record<string, unknown> } {

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ describe('watchdog completion provenance', () => {
919919

920920
expect(toolCall.result).toEqual({
921921
success: false,
922-
output: { error: expect.stringContaining('safe to retry'), notStarted: true },
922+
output: { error: expect.stringContaining('never started'), notStarted: true },
923923
})
924924
})
925925

@@ -940,6 +940,24 @@ describe('watchdog completion provenance', () => {
940940
})
941941
})
942942

943+
it('does not claim a local read the server never saw picked up had started', async () => {
944+
const { toolCall, context, execContext } = createHungClient()
945+
toolCall.name = 'read_local_file'
946+
toolCall.params = { path: '/Users/me/notes.txt' }
947+
completePendingAsyncToolCall.mockResolvedValueOnce(null)
948+
949+
await failPendingToolCall(toolCall.id, context, execContext)
950+
951+
expect(toolCall.result).toEqual({
952+
success: false,
953+
output: {
954+
error: expect.stringContaining('may never have started'),
955+
outcomeUnknown: true,
956+
doNotRetry: true,
957+
},
958+
})
959+
})
960+
943961
it('preserves an actual completion that settles while encryption is pending', async () => {
944962
const { toolCall, context, execContext } = createHungClient()
945963
let finishEncryption: (value: { encrypted: string }) => void = () => {}

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ import {
8686
type ToolCallState,
8787
} from '@/lib/mothership/request/types'
8888
import { ensureHandlersRegistered, executeTool } from '@/lib/mothership/tool-executor'
89-
import { isDesktopToolCall } from '@/lib/mothership/tools/desktop-tools'
89+
import { isDesktopToolCall, isLocalReadToolCall } from '@/lib/mothership/tools/desktop-tools'
9090
import { withSandboxResourceScope } from '@/lib/mothership/tools/sandbox-resources'
9191
import { isMcpTool } from '@/executor/constants'
9292

@@ -414,6 +414,12 @@ const TOOL_RESULT_LOST_MESSAGE =
414414
'This tool started, but its result never came back, so it was abandoned to let the conversation continue. Its outcome is unknown: it may already have taken effect, so inspect the current state before repeating it, and do not retry it automatically.'
415415
const DESKTOP_TOOL_RESULT_LOST_MESSAGE =
416416
'The Sim desktop app started this action, but its result never came back (the chat view closed or the app stopped responding). Its outcome is unknown: it may already have taken effect, so inspect the current state before repeating it, and do not retry it automatically.'
417+
/**
418+
* A local read the server cannot see picked up (one an older desktop reads without claiming) may
419+
* never have started; either way, reading changed nothing.
420+
*/
421+
const DESKTOP_LOCAL_READ_RESULT_MISSING_MESSAGE =
422+
'No result came back from the Sim desktop app for this read: it may never have started (this chat may not be open there), or its result was lost. Reading changes nothing on the user’s computer. Do not retry it in this turn; tell the user to keep this chat open in the Sim desktop app, or to ask again later.'
417423
const UNAVAILABLE_TOOL_SETTLEMENT_MESSAGE =
418424
'The tool result could not be restored before the conversation resumed. Its outcome is unknown; do not retry it automatically.'
419425

@@ -423,8 +429,8 @@ const UNAVAILABLE_TOOL_SETTLEMENT_MESSAGE =
423429
* Execution ownership remains held while retained work cleans up.
424430
*
425431
* Without an explicit `failureMessage` the model learns which of two things happened: a desktop
426-
* call nothing claimed never started (`notStarted`, safe to retry), and anything else started and
427-
* lost its result (`outcomeUnknown`, `doNotRetry`).
432+
* call nothing claimed never started (`notStarted`), and anything else started (or could not be
433+
* seen starting) and lost its result (`outcomeUnknown`, `doNotRetry`).
428434
*/
429435
export async function failPendingToolCall(
430436
toolCallId: string,
@@ -444,7 +450,12 @@ export async function failPendingToolCall(
444450
if (settled) return
445451
}
446452
const message =
447-
failureMessage ?? (desktopCall ? DESKTOP_TOOL_RESULT_LOST_MESSAGE : TOOL_RESULT_LOST_MESSAGE)
453+
failureMessage ??
454+
(!desktopCall
455+
? TOOL_RESULT_LOST_MESSAGE
456+
: isLocalReadToolCall(toolCall.execName ?? toolCall.name, toolCall.params)
457+
? DESKTOP_LOCAL_READ_RESULT_MISSING_MESSAGE
458+
: DESKTOP_TOOL_RESULT_LOST_MESSAGE)
448459
await settleAbandonedToolCall(toolCall, context, execContext, {
449460
message,
450461
data: { error: message, outcomeUnknown: true, doNotRetry: true },
Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import { isCurrentBrowserToolName } from '@sim/browser-protocol'
2-
import { isTerminalToolName } from '@sim/terminal-protocol'
3-
import { isNativeFileTool, isUserLocalVfsToolCall } from '@/lib/mothership/tools/local-filesystem'
1+
import { isDesktopToolCall } from '@/lib/mothership/tools/desktop-tools'
42

53
const WORKFLOW_TOOL_NAMES = new Set<string>([
64
'run_workflow',
@@ -13,22 +11,6 @@ export function isWorkflowToolName(name: string): boolean {
1311
return WORKFLOW_TOOL_NAMES.has(name)
1412
}
1513

16-
/**
17-
* Client-executed calls only the desktop app can run: local file access, the agent browser, and
18-
* the terminal. A web tab watching the same chat must leave them to the desktop app.
19-
*/
20-
export function isDesktopExecutedToolCall(
21-
name: string,
22-
args: Record<string, unknown> | undefined
23-
): boolean {
24-
return (
25-
isNativeFileTool(name) ||
26-
isUserLocalVfsToolCall(name, args) ||
27-
isCurrentBrowserToolName(name) ||
28-
isTerminalToolName(name)
29-
)
30-
}
31-
3214
/**
3315
* Tool calls the browser starts from the call frame's own arguments: workflow
3416
* runs, local file access, browser actions, and terminal commands. The stream
@@ -38,5 +20,5 @@ export function isClientExecutedToolCall(
3820
name: string,
3921
args: Record<string, unknown> | undefined
4022
): boolean {
41-
return isWorkflowToolName(name) || isDesktopExecutedToolCall(name, args)
23+
return isWorkflowToolName(name) || isDesktopToolCall(name, args)
4224
}

0 commit comments

Comments
 (0)