Skip to content

Commit 2f9688d

Browse files
committed
fix(mothership): refuse desktop claims for unapproved or stopped calls
The desktop authorize route claimed any pending call, so a gated terminal run that was still awaiting approval, or a call on a run the user had stopped, could be claimed and executed. The claim now locks the run row and refuses once tool admission has closed (Stop, a newer turn, or the run's end), and refuses a call held for the user's decision until they allow it. Whether a call is gated depends on the turn, so pre-persist records it on the row (permission_requested_at). Stop now settles the stopped runs' open desktop calls in the transaction that closes admission: unclaimed calls as never started, claimed calls as outcome unknown. A result for an already-settled call (a retry, or one that lost to Stop) is acknowledged with the stored outcome instead of 404/500.
1 parent 6845fd9 commit 2f9688d

15 files changed

Lines changed: 30575 additions & 54 deletions

File tree

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,26 @@ describe('Copilot Confirm API Route', () => {
624624
)
625625
})
626626

627+
it.each([
628+
['client_tool', 'running', 'success', completeAsyncToolCall],
629+
['browser_snapshot', 'pending', 'error', completePendingAsyncToolCall],
630+
] as const)(
631+
'acknowledges a %s result whose write lost to a settlement that landed first',
632+
async (toolName, storedStatus, status, completion) => {
633+
const row = { ...existingRow, toolName, claimedBy: null }
634+
getAsyncToolCall
635+
.mockResolvedValueOnce({ ...row, status: storedStatus })
636+
.mockResolvedValueOnce({ ...row, status: 'failed' })
637+
completion.mockResolvedValueOnce(null)
638+
639+
const response = await POST(createMockPostRequest({ toolCallId: 'tool-call-123', status }))
640+
641+
expect(response.status).toBe(200)
642+
expect(await response.json()).toMatchObject({ toolCallId: 'tool-call-123', status: 'error' })
643+
expect(publishToolConfirmation).not.toHaveBeenCalled()
644+
}
645+
)
646+
627647
it('does not publish when another terminal confirmation already won', async () => {
628648
completeAsyncToolCall.mockResolvedValueOnce(null)
629649

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

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { isBrowserToolName, isCurrentBrowserToolName } from '@sim/browser-protocol'
1+
import type { Span } from '@opentelemetry/api'
2+
import { isBrowserToolName } from '@sim/browser-protocol'
23
import { createLogger } from '@sim/logger'
34
import { isTerminalToolName } from '@sim/terminal-protocol'
45
import { getErrorMessage, toError } from '@sim/utils/errors'
@@ -7,12 +8,13 @@ import { type NextRequest, NextResponse } from 'next/server'
78
import { copilotConfirmContract } from '@/lib/api/contracts/copilot'
89
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11+
import { getDesktopToolClaimOwner } from '@/lib/mothership/async-runs/desktop-tools'
1012
import {
1113
ASYNC_TOOL_CONFIRMATION_STATUS,
1214
ASYNC_TOOL_STATUS,
1315
type AsyncCompletionData,
1416
type AsyncConfirmationStatus,
15-
DESKTOP_TOOL_CLAIM_OWNER,
17+
type AsyncTerminalStatus,
1618
isDeliveredAsyncStatus,
1719
isTerminalAsyncStatus,
1820
isWorkflowToolExecutionClaimable,
@@ -83,6 +85,24 @@ function createConfirmationResponse(
8385
return NextResponse.json({ success: true, message, toolCallId, status })
8486
}
8587

88+
/**
89+
* A result for a call that is already settled — a retried delivery, or one that lost to the server
90+
* settling the call first (Stop, a fast "not started" failure) — answers with the stored outcome.
91+
* Nothing is written or published again, and the reporter stops retrying.
92+
*/
93+
function acknowledgeSettledToolCall(
94+
span: Span,
95+
toolCallId: string,
96+
storedStatus: AsyncTerminalStatus
97+
): NextResponse {
98+
const settledStatus = getWorkflowToolConfirmationStatus(storedStatus)
99+
span.setAttributes({
100+
[TraceAttr.ToolConfirmationStatus]: settledStatus,
101+
[TraceAttr.CopilotConfirmOutcome]: CopilotConfirmOutcome.Delivered,
102+
})
103+
return createConfirmationResponse(toolCallId, settledStatus, 'Tool call was already settled')
104+
}
105+
86106
/** Atomically finalize or detach a client tool before publishing its wakeup event. */
87107
async function updateToolCallStatus(
88108
existing: NonNullable<Awaited<ReturnType<typeof getAsyncToolCall>>>,
@@ -284,26 +304,22 @@ export const POST = withRouteHandler((req: NextRequest) => {
284304
)
285305
}
286306

307+
if (!isWorkflowTool && isTerminalAsyncStatus(existing.status)) {
308+
return acknowledgeSettledToolCall(span, toolCallId, existing.status)
309+
}
310+
287311
const isErrorOrCancelledOutcome =
288312
status === ASYNC_TOOL_CONFIRMATION_STATUS.error ||
289313
status === ASYNC_TOOL_CONFIRMATION_STATUS.cancelled
290314
const isNativeClientTool =
291315
isBrowserToolName(existing.toolName) ||
292316
isTerminalToolName(existing.toolName) ||
293317
existing.toolName === 'import_local_files'
318+
const nativeClaimOwner = getDesktopToolClaimOwner(existing.toolName)
294319
const isPreclaimNativeTerminalOutcome =
295-
(isCurrentBrowserToolName(existing.toolName) ||
296-
isTerminalToolName(existing.toolName) ||
297-
existing.toolName === 'import_local_files') &&
320+
nativeClaimOwner !== undefined &&
298321
existing.status === ASYNC_TOOL_STATUS.pending &&
299322
isErrorOrCancelledOutcome
300-
const nativeClaimOwner = isCurrentBrowserToolName(existing.toolName)
301-
? DESKTOP_TOOL_CLAIM_OWNER.browser
302-
: isTerminalToolName(existing.toolName)
303-
? DESKTOP_TOOL_CLAIM_OWNER.terminal
304-
: existing.toolName === 'import_local_files'
305-
? DESKTOP_TOOL_CLAIM_OWNER.files
306-
: undefined
307323
const isIndeterminateNativeExit =
308324
isPreclaimNativeTerminalOutcome &&
309325
status === ASYNC_TOOL_CONFIRMATION_STATUS.error &&
@@ -453,6 +469,13 @@ export const POST = withRouteHandler((req: NextRequest) => {
453469
)
454470
: updateOutcome
455471

472+
if (reconciledOutcome === 'conflict' && !isWorkflowTool) {
473+
const settled = await getAsyncToolCall(toolCallId).catch(() => null)
474+
if (settled && isTerminalAsyncStatus(settled.status)) {
475+
return acknowledgeSettledToolCall(span, toolCallId, settled.status)
476+
}
477+
}
478+
456479
if (reconciledOutcome === 'conflict' && isPreclaimNativeTerminalOutcome) {
457480
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
458481
return createNotFoundResponse('Pending client tool call not found')

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('desktop tool authorization', () => {
5050
userId: 'user-1',
5151
status: 'active',
5252
})
53-
claimPendingAsyncToolCall.mockResolvedValue({ toolCallId: 'browser-tool', status: 'running' })
53+
claimPendingAsyncToolCall.mockResolvedValue('claimed')
5454
})
5555

5656
it('never returns presentation activity as an executable browser argument', async () => {
@@ -198,7 +198,7 @@ describe('desktop tool authorization', () => {
198198
)
199199
expect((await POST(request('import-1', true))).status).toBe(404)
200200
expect(claimPendingAsyncToolCall).not.toHaveBeenCalled()
201-
claimPendingAsyncToolCall.mockResolvedValueOnce(null)
201+
claimPendingAsyncToolCall.mockResolvedValueOnce('not_pending')
202202
expect((await POST(request('import-1', true))).status).toBe(409)
203203
})
204204

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

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
claimPendingAsyncToolCall,
1313
getAsyncToolCall,
1414
getRunSegment,
15+
type PendingToolCallClaim,
1516
} from '@/lib/mothership/async-runs/repository'
1617
import {
1718
authenticateCopilotRequestSessionOnly,
@@ -20,12 +21,33 @@ import {
2021
} from '@/lib/mothership/request/http'
2122
import { isUserLocalVfsToolCall } from '@/lib/mothership/tools/local-filesystem'
2223

24+
const admissionClosedResponse = () =>
25+
NextResponse.json(
26+
{ error: 'This chat was stopped, so the tool call can no longer run' },
27+
{ status: 410 }
28+
)
29+
30+
/** A refused claim answers the same way for every tool, except how each reports a lost race. */
31+
function refusedClaimResponse(
32+
claim: Exclude<PendingToolCallClaim, 'claimed'>,
33+
notPending: () => NextResponse
34+
): NextResponse {
35+
if (claim === 'admission_closed') return admissionClosedResponse()
36+
if (claim === 'awaiting_permission')
37+
return NextResponse.json({ error: 'The user has not approved this tool call' }, { status: 403 })
38+
return notPending()
39+
}
40+
2341
/**
2442
* Electron calls this endpoint from the main process before every privileged
2543
* native model action. It returns only server-persisted canonical tool args;
2644
* Electron validates local-file requests against them and uses them directly
2745
* for browser and terminal tools. The presentation-only `activity` field is
2846
* dropped: desktop actions reject arguments they do not declare.
47+
*
48+
* Nothing is handed over once the run's tool admission has closed (Stop, a
49+
* newer turn, or the run's end), nor for a call held for the user's decision
50+
* that they have not allowed.
2951
*/
3052
export const POST = withRouteHandler(async (request: NextRequest) => {
3153
const { userId, isAuthenticated } = await authenticateCopilotRequestSessionOnly()
@@ -47,6 +69,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4769
if (run.status === 'complete' || run.status === 'error' || run.status === 'cancelled') {
4870
return createNotFoundResponse('Pending client tool call not found')
4971
}
72+
if (run.toolAdmissionClosedAt) return admissionClosedResponse()
5073

5174
const args = isRecordLike(toolCall.args) ? (toolCall.args as Record<string, unknown>) : {}
5275
const isBrowserTool = isCurrentBrowserToolName(toolCall.toolName)
@@ -84,14 +107,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
84107
return NextResponse.json(projected.body, { status: projected.status })
85108
}
86109
if (parsed.data.body.claim) {
87-
if (
88-
toolCall.status !== 'pending' ||
89-
!(await claimPendingAsyncToolCall(toolCall.toolCallId, DESKTOP_TOOL_CLAIM_OWNER.files))
90-
)
91-
return NextResponse.json(
110+
const alreadyStarted = () =>
111+
NextResponse.json(
92112
{ error: 'This import was already started; inspect its result before retrying' },
93113
{ status: 409 }
94114
)
115+
if (toolCall.status !== 'pending') return alreadyStarted()
116+
const claim = await claimPendingAsyncToolCall(
117+
toolCall.toolCallId,
118+
DESKTOP_TOOL_CLAIM_OWNER.files
119+
)
120+
if (claim !== 'claimed') return refusedClaimResponse(claim, alreadyStarted)
95121
} else if (
96122
toolCall.status !== 'running' ||
97123
toolCall.claimedBy !== DESKTOP_TOOL_CLAIM_OWNER.files
@@ -104,16 +130,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
104130
// the Electron boundary — a replayed renderer event must not run a command
105131
// or click a button twice.
106132
if (isBrowserTool || isTerminalTool) {
107-
if (toolCall.status !== 'pending') {
108-
return createNotFoundResponse('Pending client tool call not found')
109-
}
110-
const claimed = await claimPendingAsyncToolCall(
133+
const notPending = () => createNotFoundResponse('Pending client tool call not found')
134+
if (toolCall.status !== 'pending') return notPending()
135+
const claim = await claimPendingAsyncToolCall(
111136
toolCall.toolCallId,
112137
isBrowserTool ? DESKTOP_TOOL_CLAIM_OWNER.browser : DESKTOP_TOOL_CLAIM_OWNER.terminal
113138
)
114-
if (!claimed) {
115-
return createNotFoundResponse('Pending client tool call not found')
116-
}
139+
if (claim !== 'claimed') return refusedClaimResponse(claim, notPending)
117140
}
118141

119142
return NextResponse.json({
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { CURRENT_BROWSER_TOOL_NAMES, isCurrentBrowserToolName } from '@sim/browser-protocol'
2+
import { isTerminalToolName, TERMINAL_TOOL_NAME } from '@sim/terminal-protocol'
3+
import { DESKTOP_TOOL_CLAIM_OWNER } from '@/lib/mothership/async-runs/lifecycle'
4+
5+
type DesktopToolClaimOwner =
6+
(typeof DESKTOP_TOOL_CLAIM_OWNER)[keyof typeof DESKTOP_TOOL_CLAIM_OWNER]
7+
8+
/**
9+
* The owner Electron claims a call as before acting on it, for the desktop tools whose pending
10+
* call is claimed atomically through `/api/desktop/tool/authorize`. Undefined for every other tool.
11+
*/
12+
export function getDesktopToolClaimOwner(toolName: string): DesktopToolClaimOwner | undefined {
13+
if (isCurrentBrowserToolName(toolName)) return DESKTOP_TOOL_CLAIM_OWNER.browser
14+
if (isTerminalToolName(toolName)) return DESKTOP_TOOL_CLAIM_OWNER.terminal
15+
if (toolName === 'import_local_files') return DESKTOP_TOOL_CLAIM_OWNER.files
16+
return undefined
17+
}
18+
19+
/** Every tool that acts on the user's machine through the desktop app. */
20+
export const DESKTOP_TOOL_NAMES = [
21+
...CURRENT_BROWSER_TOOL_NAMES,
22+
TERMINAL_TOOL_NAME,
23+
'import_local_files',
24+
'read_local_file',
25+
] as const
26+
27+
/** What the model learns about a desktop call that Stop cancelled before the desktop picked it up. */
28+
export const STOPPED_BEFORE_START_MESSAGE =
29+
'Not run: the user stopped the chat before the Sim desktop app started this action. Nothing happened on their computer.'
30+
31+
/** What the model learns about a desktop call that Stop cancelled after the desktop picked it up. */
32+
export const STOPPED_WHILE_RUNNING_MESSAGE =
33+
'Stopped by the user while the Sim desktop app was running this action. It may already have taken effect; inspect the current state before repeating it.'

‎apps/sim/lib/mothership/async-runs/lifecycle.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ export function isExecutableToolPermissionDecision(
102102
return decision !== null && decision !== undefined && decision !== 'skip'
103103
}
104104

105+
/** A call held for the user's decision may run only once they allowed it. */
106+
export function isAwaitingToolPermission(call: {
107+
permissionRequestedAt: Date | null
108+
permissionDecision: CopilotToolPermissionDecision | null
109+
}): boolean {
110+
return (
111+
Boolean(call.permissionRequestedAt) &&
112+
!isExecutableToolPermissionDecision(call.permissionDecision)
113+
)
114+
}
115+
105116
export function isWorkflowToolExecutionClaimable(
106117
status: CopilotAsyncToolStatus,
107118
permissionDecision: CopilotToolPermissionDecision | null | undefined

0 commit comments

Comments
 (0)