Skip to content

Commit 2c923ff

Browse files
committed
refactor(mothership): one claim primitive, one desktop-tool classifier, sealed Stop results
The desktop claim is now an option of the run-locked tool execution claim (claimSimToolExecution becomes claimToolExecution) instead of a second copy of the admission check. Stop picks the open desktop calls with the shared TS classifier, which moves to lib/mothership/tools/desktop-tools.ts, instead of a SQL restatement of it, and seals each result the way the confirm route does, so a waiter restores what Stop did rather than failing to unseal it.
1 parent df74220 commit 2c923ff

16 files changed

Lines changed: 288 additions & 265 deletions

File tree

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

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { type NextRequest, NextResponse } from 'next/server'
88
import { copilotConfirmContract } from '@/lib/api/contracts/copilot'
99
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11-
import { getDesktopToolClaimOwner } from '@/lib/mothership/async-runs/desktop-tools'
1211
import {
1312
ASYNC_TOOL_CONFIRMATION_STATUS,
1413
ASYNC_TOOL_STATUS,
@@ -40,11 +39,9 @@ import {
4039
createUnauthorizedResponse,
4140
} from '@/lib/mothership/request/http'
4241
import { withIncomingGoSpan } from '@/lib/mothership/request/otel'
43-
import {
44-
retainSealedClientToolContext,
45-
sealClientToolCompletion,
46-
} from '@/lib/mothership/request/tools/client-completion-seal.server'
42+
import { sealClientToolSettlement } from '@/lib/mothership/request/tools/client-completion-seal.server'
4743
import { isWorkflowToolName } from '@/lib/mothership/tools/client-executed-tools'
44+
import { getDesktopToolClaimOwner } from '@/lib/mothership/tools/desktop-tools'
4845
import {
4946
createStructuralWorkflowToolCompletionData,
5047
getWorkflowToolCompletionExecutionId,
@@ -417,27 +414,24 @@ export const POST = withRouteHandler((req: NextRequest) => {
417414
}
418415
: {
419416
message: getClientToolCompletionMessage(status),
420-
data: {
421-
...retainSealedClientToolContext(existing.result),
422-
...(await sealClientToolCompletion({
423-
toolCallId,
424-
runId: existing.runId,
425-
userId: authenticatedUserId,
426-
...(isIndeterminateNativeExit
427-
? {
428-
message: NATIVE_HANDOFF_INTERRUPTED_MESSAGE,
429-
data: {
430-
error: NATIVE_HANDOFF_INTERRUPTED_MESSAGE,
431-
outcomeUnknown: true,
432-
doNotRetry: true,
433-
},
434-
}
435-
: {
436-
...(message !== undefined ? { message } : {}),
437-
...(data !== undefined ? { data } : {}),
438-
}),
439-
})),
440-
},
417+
data: await sealClientToolSettlement(existing.result, {
418+
toolCallId,
419+
runId: existing.runId,
420+
userId: authenticatedUserId,
421+
...(isIndeterminateNativeExit
422+
? {
423+
message: NATIVE_HANDOFF_INTERRUPTED_MESSAGE,
424+
data: {
425+
error: NATIVE_HANDOFF_INTERRUPTED_MESSAGE,
426+
outcomeUnknown: true,
427+
doNotRetry: true,
428+
},
429+
}
430+
: {
431+
...(message !== undefined ? { message } : {}),
432+
...(data !== undefined ? { data } : {}),
433+
}),
434+
}),
441435
}
442436

443437
const updateOutcome = await updateToolCallStatus(

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

Lines changed: 13 additions & 8 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 claimPendingAsyncToolCall = mothershipAsyncRunsMockFns.mockClaimPendingAsyncToolCall
20+
const claimToolExecution = 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-
claimPendingAsyncToolCall.mockResolvedValue('claimed')
53+
claimToolExecution.mockResolvedValue({ outcome: 'claimed' })
5454
})
5555

5656
it('never returns presentation activity as an executable browser argument', async () => {
@@ -84,7 +84,7 @@ describe('desktop tool authorization', () => {
8484
const response = await POST(request('retired-browser-tool'))
8585

8686
expect(response.status).toBe(403)
87-
expect(claimPendingAsyncToolCall).not.toHaveBeenCalled()
87+
expect(claimToolExecution).not.toHaveBeenCalled()
8888
})
8989

9090
it('rejects a replayed browser action after its pending row was claimed', async () => {
@@ -98,7 +98,7 @@ describe('desktop tool authorization', () => {
9898

9999
const response = await POST(request('browser-tool'))
100100
expect(response.status).toBe(404)
101-
expect(claimPendingAsyncToolCall).not.toHaveBeenCalled()
101+
expect(claimToolExecution).not.toHaveBeenCalled()
102102
})
103103

104104
it('rejects workspace VFS calls and mutating legacy local tools', async () => {
@@ -176,13 +176,18 @@ describe('desktop tool authorization', () => {
176176
{ userId: 'user-1', chatId: 'chat-1', organizationId: 'org-1', workspaceId: undefined },
177177
'target'
178178
)
179-
expect(claimPendingAsyncToolCall).toHaveBeenCalledExactlyOnceWith('import-1', 'desktop-files')
179+
expect(claimToolExecution).toHaveBeenCalledExactlyOnceWith({
180+
toolCallId: 'import-1',
181+
runId: 'run-1',
182+
userId: 'user-1',
183+
claimedBy: 'desktop-files',
184+
})
180185
getAsyncToolCall.mockResolvedValue({ ...tool, status: 'running', claimedBy: 'desktop-files' })
181186
expect((await POST(request('import-1', true))).status).toBe(409)
182187
expect((await POST(request('import-1'))).status).toBe(200)
183188
getAsyncToolCall.mockResolvedValue({ ...tool, status: 'running', claimedBy: 'sim-stream' })
184189
expect((await POST(request('import-1'))).status).toBe(404)
185-
expect(claimPendingAsyncToolCall).toHaveBeenCalledOnce()
190+
expect(claimToolExecution).toHaveBeenCalledOnce()
186191
})
187192

188193
it('rejects inaccessible destinations and lost import claims before exposing files', async () => {
@@ -197,8 +202,8 @@ describe('desktop tool authorization', () => {
197202
new OrchestrationError('not_found', 'Workspace not found')
198203
)
199204
expect((await POST(request('import-1', true))).status).toBe(404)
200-
expect(claimPendingAsyncToolCall).not.toHaveBeenCalled()
201-
claimPendingAsyncToolCall.mockResolvedValueOnce('not_pending')
205+
expect(claimToolExecution).not.toHaveBeenCalled()
206+
claimToolExecution.mockResolvedValueOnce({ outcome: 'existing' })
202207
expect((await POST(request('import-1', true))).status).toBe(409)
203208
})
204209

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

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import { resolveInvocationWorkspace } from '@/lib/mothership/application/workspace-target'
1010
import { DESKTOP_TOOL_CLAIM_OWNER } from '@/lib/mothership/async-runs/lifecycle'
1111
import {
12-
claimPendingAsyncToolCall,
12+
claimToolExecution,
1313
getAsyncToolCall,
1414
getRunSegment,
15-
type PendingToolCallClaim,
15+
type ToolExecutionClaim,
1616
} from '@/lib/mothership/async-runs/repository'
1717
import {
1818
authenticateCopilotRequestSessionOnly,
@@ -23,16 +23,16 @@ import { isUserLocalVfsToolCall } from '@/lib/mothership/tools/local-filesystem'
2323

2424
const admissionClosedResponse = () =>
2525
NextResponse.json(
26-
{ error: 'This chat was stopped, so the tool call can no longer run' },
26+
{ error: 'This chat turn ended or was stopped, so the tool call can no longer run' },
2727
{ status: 410 }
2828
)
2929

3030
/** A refused claim answers the same way for every tool, except how each reports a lost race. */
3131
function refusedClaimResponse(
32-
claim: Exclude<PendingToolCallClaim, 'claimed'>,
32+
claim: Exclude<ToolExecutionClaim['outcome'], 'claimed'>,
3333
notPending: () => NextResponse
3434
): NextResponse {
35-
if (claim === 'admission_closed') return admissionClosedResponse()
35+
if (claim === 'closed') return admissionClosedResponse()
3636
if (claim === 'awaiting_permission')
3737
return NextResponse.json({ error: 'The user has not approved this tool call' }, { status: 403 })
3838
return notPending()
@@ -115,11 +115,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
115115
{ status: 409 }
116116
)
117117
if (toolCall.status !== 'pending') return alreadyStarted()
118-
const claim = await claimPendingAsyncToolCall(
119-
toolCall.toolCallId,
120-
DESKTOP_TOOL_CLAIM_OWNER.files
121-
)
122-
if (claim !== 'claimed') return refusedClaimResponse(claim, alreadyStarted)
118+
const { outcome } = await claimToolExecution({
119+
toolCallId: toolCall.toolCallId,
120+
runId: toolCall.runId,
121+
userId,
122+
claimedBy: DESKTOP_TOOL_CLAIM_OWNER.files,
123+
})
124+
if (outcome !== 'claimed') return refusedClaimResponse(outcome, alreadyStarted)
123125
} else if (
124126
toolCall.status !== 'running' ||
125127
toolCall.claimedBy !== DESKTOP_TOOL_CLAIM_OWNER.files
@@ -134,11 +136,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
134136
if (isBrowserTool || isTerminalTool) {
135137
const notPending = () => createNotFoundResponse('Pending client tool call not found')
136138
if (toolCall.status !== 'pending') return notPending()
137-
const claim = await claimPendingAsyncToolCall(
138-
toolCall.toolCallId,
139-
isBrowserTool ? DESKTOP_TOOL_CLAIM_OWNER.browser : DESKTOP_TOOL_CLAIM_OWNER.terminal
140-
)
141-
if (claim !== 'claimed') return refusedClaimResponse(claim, notPending)
139+
const { outcome } = await claimToolExecution({
140+
toolCallId: toolCall.toolCallId,
141+
runId: toolCall.runId,
142+
userId,
143+
claimedBy: isBrowserTool
144+
? DESKTOP_TOOL_CLAIM_OWNER.browser
145+
: DESKTOP_TOOL_CLAIM_OWNER.terminal,
146+
})
147+
if (outcome !== 'claimed') return refusedClaimResponse(outcome, notPending)
142148
}
143149

144150
return NextResponse.json({

‎apps/sim/lib/mothership/agent-cli/saved-run-read.live.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ import { runCli } from '@/lib/mothership/agent-cli/run-cli'
9595
import { resolveCopilotWorkspaceFileReference } from '@/lib/mothership/application/execute-file-use-case'
9696
import {
9797
areStreamToolExecutionsSettled,
98-
claimSimToolExecution,
98+
claimToolExecution,
9999
claimWorkflowToolExecution,
100100
closeStreamToolAdmission,
101101
completeAsyncToolCall,
@@ -1750,7 +1750,7 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
17501750
status: 'running',
17511751
})
17521752
const owner = { runId, toolCallId, ownerToken, userId: 'run-reader' }
1753-
expect(await claimSimToolExecution(owner)).toEqual({ outcome: 'claimed' })
1753+
expect(await claimToolExecution(owner)).toEqual({ outcome: 'claimed' })
17541754
return { owner, chatId, streamId }
17551755
}
17561756

@@ -1811,7 +1811,7 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
18111811
).rejects.toThrow('outcome is unknown')
18121812
expect(await revokeExpiredSimToolExecutions(owner)).toHaveLength(1)
18131813
expect(await revokeExpiredSimToolExecutions(owner)).toEqual([])
1814-
expect(await claimSimToolExecution({ ...owner, ownerToken: generateId() })).toEqual({
1814+
expect(await claimToolExecution({ ...owner, ownerToken: generateId() })).toEqual({
18151815
outcome: 'existing',
18161816
})
18171817
const saved = await readTool(owner.toolCallId)
@@ -2058,7 +2058,7 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
20582058
})
20592059
expect(await claimWorkflowToolExecution(toolCallId, executionId, 'client')).not.toBeNull()
20602060
expect(
2061-
await claimSimToolExecution({
2061+
await claimToolExecution({
20622062
runId,
20632063
toolCallId,
20642064
userId: 'run-reader',
@@ -2132,7 +2132,7 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
21322132
])
21332133
const priorExecutionId = generateId()
21342134
await claimWorkflowToolExecution(priorToolId, priorExecutionId, 'client')
2135-
await claimSimToolExecution({
2135+
await claimToolExecution({
21362136
runId,
21372137
toolCallId,
21382138
userId: 'run-reader',
@@ -2175,7 +2175,7 @@ describe.skipIf(!process.env.MSHIP_TEST_DATABASE_URL)(
21752175
})
21762176
expect(await claimWorkflowToolExecution(toolCallId, generateId(), 'sim')).not.toBeNull()
21772177
expect(
2178-
await claimSimToolExecution({
2178+
await claimToolExecution({
21792179
runId,
21802180
toolCallId,
21812181
userId: 'run-reader',

‎apps/sim/lib/mothership/async-runs/orphaned-runs.integration.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import {
5252
sweepOrphanedRuns,
5353
} from '@/lib/mothership/async-runs/orphaned-runs'
5454
import {
55-
claimSimToolExecution,
55+
claimToolExecution,
5656
requestRunStop,
5757
updateRunStatus,
5858
} from '@/lib/mothership/async-runs/repository'
@@ -279,7 +279,7 @@ describe.runIf(Boolean(redisUrl))('Chat runs no controller owns', () => {
279279
/** A long tool call writes nothing to the run; only its execution heartbeat shows it is alive. */
280280
const orphan = await admittedRun({ idleMinutes: 90, status: 'paused_waiting_for_tool' })
281281
const tool = await dispatchedTool(orphan.runId)
282-
expect(await claimSimToolExecution(tool)).toEqual({ outcome: 'claimed' })
282+
expect(await claimToolExecution(tool)).toEqual({ outcome: 'claimed' })
283283

284284
expect((await sweepOrphanedRuns()).settledRunIds).not.toContain(orphan.runId)
285285
const live = await stored(orphan.runId)
@@ -309,7 +309,7 @@ describe.runIf(Boolean(redisUrl))('Chat runs no controller owns', () => {
309309
.where(eq(copilotRuns.id, orphan.runId))
310310
.for('update'),
311311
async (holder) => {
312-
const claim = claimSimToolExecution(tool)
312+
const claim = claimToolExecution(tool)
313313
const claimant = await lockWaiterBehind(holder)
314314
const sweep = sweepOrphanedRuns()
315315
await lockWaiterBehind(holder, claimant)
@@ -327,7 +327,7 @@ describe.runIf(Boolean(redisUrl))('Chat runs no controller owns', () => {
327327
it('never settles a run whose Sim tool lease a heartbeat renewed as the sweep settled it', async () => {
328328
const orphan = await admittedRun({ idleMinutes: 90, status: 'paused_waiting_for_tool' })
329329
const tool = await dispatchedTool(orphan.runId)
330-
expect(await claimSimToolExecution(tool)).toEqual({ outcome: 'claimed' })
330+
expect(await claimToolExecution(tool)).toEqual({ outcome: 'claimed' })
331331
await db
332332
.update(copilotAsyncToolCalls)
333333
.set({ executionLeaseExpiresAt: sql`clock_timestamp() - interval '1 second'` })

‎apps/sim/lib/mothership/async-runs/repository.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { dbChainMockFns, hasMockCondition, queueTableRows, resetDbChainMock } fr
99
import { beforeEach, describe, expect, it } from 'vitest'
1010
import {
1111
areStreamToolExecutionsSettled,
12-
claimSimToolExecution,
12+
claimToolExecution,
1313
claimWorkflowToolExecution,
1414
closeStreamToolAdmission,
1515
createRunSegment,
@@ -211,7 +211,7 @@ describe('durable Sim tool ownership', () => {
211211
dbChainMockFns.for.mockResolvedValueOnce([
212212
{ toolExecutionVersion: 2, toolAdmissionClosedAt: null, status },
213213
])
214-
expect(await claimSimToolExecution(input)).toEqual({ outcome: 'closed' })
214+
expect(await claimToolExecution(input)).toEqual({ outcome: 'closed' })
215215
dbChainMockFns.for.mockResolvedValueOnce([{ version: 2, closedAt: null, status }])
216216
await expect(
217217
recordSimSandboxProcess({
@@ -232,7 +232,7 @@ describe('durable Sim tool ownership', () => {
232232
{ id: input.runId, toolExecutionVersion: 2, toolAdmissionClosedAt: null },
233233
])
234234
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'row-1' }])
235-
expect(await claimSimToolExecution(input)).toEqual({ outcome: 'claimed' })
235+
expect(await claimToolExecution(input)).toEqual({ outcome: 'claimed' })
236236
expect(dbChainMockFns.for).toHaveBeenCalledWith('update')
237237
expect(
238238
hasMockCondition(
@@ -257,13 +257,13 @@ describe('durable Sim tool ownership', () => {
257257
dbChainMockFns.for.mockResolvedValueOnce([
258258
{ toolExecutionVersion: 2, toolAdmissionClosedAt: new Date() },
259259
])
260-
expect(await claimSimToolExecution(input)).toEqual({ outcome: 'closed' })
260+
expect(await claimToolExecution(input)).toEqual({ outcome: 'closed' })
261261
expect(dbChainMockFns.update).not.toHaveBeenCalled()
262262
})
263263

264264
it.each([0, 1])('does not certify or execute through tracking version %s', async (version) => {
265265
dbChainMockFns.for.mockResolvedValueOnce([{ toolExecutionVersion: version }])
266-
await expect(claimSimToolExecution(input)).rejects.toThrow('ownership is unavailable')
266+
await expect(claimToolExecution(input)).rejects.toThrow('ownership is unavailable')
267267
dbChainMockFns.returning.mockResolvedValueOnce([{ version }])
268268
expect(await closeStreamToolAdmission('stream-1', input.userId)).toBe(false)
269269
})
@@ -278,7 +278,7 @@ describe('durable Sim tool ownership', () => {
278278
executionSettledAt: null,
279279
}
280280
queueTableRows(copilotAsyncToolCalls, [record])
281-
expect(await claimSimToolExecution(input)).toEqual({ outcome: 'existing' })
281+
expect(await claimToolExecution(input)).toEqual({ outcome: 'existing' })
282282
})
283283

284284
it('keeps a terminal result distinct from actual execution settlement', async () => {

0 commit comments

Comments
 (0)