Skip to content

Commit 89626b1

Browse files
committed
refactor(mothership): one desktop-tool classifier and a typed claim for each claimant
The authorize and confirm routes classify desktop tools through lib/mothership/tools/desktop-tools.ts instead of inline checks. The Sim execution claim and the desktop claim share one run-admission lock but return their own outcome types, so a Sim caller can no longer receive the desktop-only awaiting_permission outcome. The terminal-status mapping moves to lifecycle as getTerminalConfirmationStatus, since confirm uses it for every client tool.
1 parent bd54c7c commit 89626b1

14 files changed

Lines changed: 203 additions & 159 deletions

File tree

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

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import type { Span } from '@opentelemetry/api'
2-
import { isBrowserToolName } from '@sim/browser-protocol'
32
import { createLogger } from '@sim/logger'
4-
import { isTerminalToolName } from '@sim/terminal-protocol'
53
import { getErrorMessage, toError } from '@sim/utils/errors'
64
import { isPlainRecord } from '@sim/utils/object'
75
import { type NextRequest, NextResponse } from 'next/server'
@@ -14,6 +12,7 @@ import {
1412
type AsyncCompletionData,
1513
type AsyncConfirmationStatus,
1614
type AsyncTerminalStatus,
15+
getTerminalConfirmationStatus,
1716
isDeliveredAsyncStatus,
1817
isTerminalAsyncStatus,
1918
isWorkflowToolExecutionClaimable,
@@ -41,12 +40,11 @@ import {
4140
import { withIncomingGoSpan } from '@/lib/mothership/request/otel'
4241
import { sealClientToolSettlement } from '@/lib/mothership/request/tools/client-completion-seal.server'
4342
import { isWorkflowToolName } from '@/lib/mothership/tools/client-executed-tools'
44-
import { getDesktopToolClaimOwner } from '@/lib/mothership/tools/desktop-tools'
43+
import { getDesktopToolClaimOwner, isNativeDesktopTool } from '@/lib/mothership/tools/desktop-tools'
4544
import {
4645
createStructuralWorkflowToolCompletionData,
4746
getWorkflowToolCompletionExecutionId,
4847
getWorkflowToolCompletionMessage,
49-
getWorkflowToolConfirmationStatus,
5048
getWorkflowToolLaunchError,
5149
resolveWorkflowToolTargetId,
5250
WORKFLOW_EXECUTION_BUSY,
@@ -92,7 +90,7 @@ function acknowledgeSettledToolCall(
9290
toolCallId: string,
9391
storedStatus: AsyncTerminalStatus
9492
): NextResponse {
95-
const settledStatus = getWorkflowToolConfirmationStatus(storedStatus)
93+
const settledStatus = getTerminalConfirmationStatus(storedStatus)
9694
span.setAttributes({
9795
[TraceAttr.ToolConfirmationStatus]: settledStatus,
9896
[TraceAttr.CopilotConfirmOutcome]: CopilotConfirmOutcome.Delivered,
@@ -264,7 +262,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
264262
return createNotFoundResponse('Completed workflow execution not found')
265263
}
266264

267-
const terminalStatus = getWorkflowToolConfirmationStatus(existing.status)
265+
const terminalStatus = getTerminalConfirmationStatus(existing.status)
268266
span.setAttributes({
269267
[TraceAttr.ToolConfirmationStatus]: terminalStatus,
270268
[TraceAttr.CopilotConfirmOutcome]: CopilotConfirmOutcome.Delivered,
@@ -308,10 +306,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
308306
const isErrorOrCancelledOutcome =
309307
status === ASYNC_TOOL_CONFIRMATION_STATUS.error ||
310308
status === ASYNC_TOOL_CONFIRMATION_STATUS.cancelled
311-
const isNativeClientTool =
312-
isBrowserToolName(existing.toolName) ||
313-
isTerminalToolName(existing.toolName) ||
314-
existing.toolName === 'import_local_files'
309+
const isNativeClientTool = isNativeDesktopTool(existing.toolName)
315310
const nativeClaimOwner = getDesktopToolClaimOwner(existing.toolName)
316311
const isPreclaimNativeTerminalOutcome =
317312
nativeClaimOwner !== undefined &&
@@ -376,7 +371,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
376371
executionId = claimedExecutionId
377372
if (status !== ASYNC_TOOL_CONFIRMATION_STATUS.background) {
378373
if (trustedExecution) {
379-
effectiveStatus = getWorkflowToolConfirmationStatus(trustedExecution.status)
374+
effectiveStatus = getTerminalConfirmationStatus(trustedExecution.status)
380375
} else if (!isErrorOrCancelledOutcome) {
381376
span.setAttribute(
382377
TraceAttr.CopilotConfirmOutcome,
@@ -389,7 +384,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
389384
executionId = submittedExecutionId
390385
} else if (trustedExecution) {
391386
executionId = trustedExecution.executionId
392-
effectiveStatus = getWorkflowToolConfirmationStatus(trustedExecution.status)
387+
effectiveStatus = getTerminalConfirmationStatus(trustedExecution.status)
393388
} else if (!isErrorOrCancelledOutcome) {
394389
effectiveStatus = ASYNC_TOOL_CONFIRMATION_STATUS.error
395390
executionId = undefined

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ vi.mock('@/lib/mothership/async-runs/repository', () => mothershipAsyncRunsMock)
1717

1818
import { POST } from './route'
1919

20-
const claimToolExecution = mothershipAsyncRunsMockFns.mockClaimToolExecution
20+
const claimDesktopToolCall = mothershipAsyncRunsMockFns.mockClaimToolExecution
2121
const getAsyncToolCall = mothershipAsyncRunsMockFns.mockGetAsyncToolCall
2222
const getRunSegment = mothershipAsyncRunsMockFns.mockGetRunSegment
2323
const resolveInvocationWorkspace = mothershipWorkspaceTargetMockFns.mockResolveInvocationWorkspace
@@ -50,7 +50,7 @@ describe('desktop tool authorization', () => {
5050
userId: 'user-1',
5151
status: 'active',
5252
})
53-
claimToolExecution.mockResolvedValue({ outcome: 'claimed' })
53+
claimDesktopToolCall.mockResolvedValue({ outcome: 'claimed' })
5454
})
5555

5656
it('never returns presentation activity as an executable browser argument', async () => {
@@ -193,7 +193,7 @@ describe('desktop tool authorization', () => {
193193
new OrchestrationError('not_found', 'Workspace not found')
194194
)
195195
expect((await POST(request('import-1', true))).status).toBe(404)
196-
claimToolExecution.mockResolvedValueOnce({ outcome: 'existing' })
196+
claimDesktopToolCall.mockResolvedValueOnce({ outcome: 'existing' })
197197
expect((await POST(request('import-1', true))).status).toBe(409)
198198
})
199199

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

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { isCurrentBrowserToolName } from '@sim/browser-protocol'
2-
import { isTerminalToolName } from '@sim/terminal-protocol'
31
import { isRecordLike, omit } from '@sim/utils/object'
42
import { type NextRequest, NextResponse } from 'next/server'
53
import { authorizeDesktopToolContract } from '@/lib/api/contracts/desktop-tool-authorization'
@@ -9,18 +7,21 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
97
import { resolveInvocationWorkspace } from '@/lib/mothership/application/workspace-target'
108
import { DESKTOP_TOOL_CLAIM_OWNER } from '@/lib/mothership/async-runs/lifecycle'
119
import {
12-
claimToolExecution,
10+
claimDesktopToolCall,
11+
type DesktopToolCallClaim,
1312
getAsyncToolCall,
1413
getRunSegment,
15-
type ToolExecutionClaim,
1614
} from '@/lib/mothership/async-runs/repository'
1715
import {
1816
authenticateCopilotRequestSessionOnly,
1917
createNotFoundResponse,
2018
createUnauthorizedResponse,
2119
} from '@/lib/mothership/request/http'
22-
import { isLocalReadToolCall } from '@/lib/mothership/tools/desktop-tools'
23-
import { isUserLocalVfsToolCall } from '@/lib/mothership/tools/local-filesystem'
20+
import {
21+
getDesktopToolClaimOwner,
22+
isDesktopToolCall,
23+
isLocalReadToolCall,
24+
} from '@/lib/mothership/tools/desktop-tools'
2425

2526
const admissionClosedResponse = () =>
2627
NextResponse.json(
@@ -30,7 +31,7 @@ const admissionClosedResponse = () =>
3031

3132
/** A refused claim answers the same way for every tool, except how each reports a lost race. */
3233
function refusedClaimResponse(
33-
claim: Exclude<ToolExecutionClaim['outcome'], 'claimed'>,
34+
claim: Exclude<DesktopToolCallClaim['outcome'], 'claimed'>,
3435
notPending: () => NextResponse
3536
): NextResponse {
3637
if (claim === 'closed') return admissionClosedResponse()
@@ -75,16 +76,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7576
}
7677

7778
const args = isRecordLike(toolCall.args) ? (toolCall.args as Record<string, unknown>) : {}
78-
const isBrowserTool = isCurrentBrowserToolName(toolCall.toolName)
79-
const isTerminalTool = isTerminalToolName(toolCall.toolName)
80-
const isLocalFileTool =
81-
toolCall.toolName === 'read_local_file' || toolCall.toolName === 'import_local_files'
82-
const authorized =
83-
isBrowserTool ||
84-
isTerminalTool ||
85-
isLocalFileTool ||
86-
isUserLocalVfsToolCall(toolCall.toolName, args)
87-
if (!authorized) {
79+
if (!isDesktopToolCall(toolCall.toolName, args)) {
8880
return NextResponse.json(
8981
{ error: 'Tool call is not authorized for desktop execution' },
9082
{ status: 403 }
@@ -116,7 +108,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
116108
{ status: 409 }
117109
)
118110
if (toolCall.status !== 'pending') return alreadyStarted()
119-
const { outcome } = await claimToolExecution({
111+
const { outcome } = await claimDesktopToolCall({
120112
toolCallId: toolCall.toolCallId,
121113
runId: toolCall.runId,
122114
userId,
@@ -136,7 +128,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
136128
if (parsed.data.body.claim && isLocalReadToolCall(toolCall.toolName, args)) {
137129
const notPending = () => createNotFoundResponse('Pending client tool call not found')
138130
if (toolCall.status === 'pending') {
139-
const { outcome } = await claimToolExecution({
131+
const { outcome } = await claimDesktopToolCall({
140132
toolCallId: toolCall.toolCallId,
141133
runId: toolCall.runId,
142134
userId,
@@ -151,16 +143,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
151143
// machine, so the pending call is claimed here, atomically, before crossing
152144
// the Electron boundary — a replayed renderer event must not run a command
153145
// or click a button twice.
154-
if (isBrowserTool || isTerminalTool) {
146+
const actionClaimOwner = getDesktopToolClaimOwner(toolCall.toolName)
147+
if (
148+
actionClaimOwner === DESKTOP_TOOL_CLAIM_OWNER.browser ||
149+
actionClaimOwner === DESKTOP_TOOL_CLAIM_OWNER.terminal
150+
) {
155151
const notPending = () => createNotFoundResponse('Pending client tool call not found')
156152
if (toolCall.status !== 'pending') return notPending()
157-
const { outcome } = await claimToolExecution({
153+
const { outcome } = await claimDesktopToolCall({
158154
toolCallId: toolCall.toolCallId,
159155
runId: toolCall.runId,
160156
userId,
161-
claimedBy: isBrowserTool
162-
? DESKTOP_TOOL_CLAIM_OWNER.browser
163-
: DESKTOP_TOOL_CLAIM_OWNER.terminal,
157+
claimedBy: actionClaimOwner,
164158
})
165159
if (outcome !== 'claimed') return refusedClaimResponse(outcome, notPending)
166160
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,15 @@ export function isWorkflowToolExecutionClaimable(
128128
)
129129
}
130130

131+
/** The confirmation status a settled call reports: its durable terminal status, as the wire names it. */
132+
export function getTerminalConfirmationStatus(
133+
status: AsyncTerminalStatus
134+
): AsyncConfirmationStatus {
135+
if (status === ASYNC_TOOL_STATUS.completed) return ASYNC_TOOL_CONFIRMATION_STATUS.success
136+
if (status === ASYNC_TOOL_STATUS.cancelled) return ASYNC_TOOL_CONFIRMATION_STATUS.cancelled
137+
return ASYNC_TOOL_CONFIRMATION_STATUS.error
138+
}
139+
131140
export function isTerminalAsyncStatus(
132141
status: CopilotAsyncToolStatus | AsyncLifecycleStatus | string | null | undefined
133142
): status is AsyncTerminalStatus {

0 commit comments

Comments
 (0)