Skip to content

Commit 60a4d18

Browse files
committed
fix(mothership): settle an unclaimed desktop call whose wait ends before the grace
A wait shorter than the pickup grace ended with no result and left the call claimable; it now settles it as never started like any unclaimed call. The desktop E2E fixture models claims per call, the way the server accepts a local read's repeat claim and refuses a second import claim. The admission probe follows the claim rename.
1 parent d9b5bc2 commit 60a4d18

4 files changed

Lines changed: 31 additions & 18 deletions

File tree

‎apps/desktop/e2e/local-files.spec.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ test('native file tools read and import through the installed preload without Si
1616
const png =
1717
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII='
1818
writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64'))
19-
let claimed = false
19+
/** Calls the server saw claimed; like the server, only an import refuses a second claim. */
20+
const claimed = new Set<string>()
2021
const calls: Record<string, { toolName: string; args: Record<string, unknown> }> = {
2122
text: { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } },
2223
image: { toolName: 'read_local_file', args: { path: join(source, 'image.png') } },
@@ -44,11 +45,14 @@ test('native file tools read and import through the installed preload without Si
4445
for await (const chunk of request) body += chunk.toString()
4546
const input = JSON.parse(body)
4647
const call = calls[input.toolCallId]
47-
if (!call || (input.claim && claimed)) {
48+
if (
49+
!call ||
50+
(input.claim && call.toolName === 'import_local_files' && claimed.has(input.toolCallId))
51+
) {
4852
response.writeHead(call ? 409 : 403, { 'Content-Type': 'application/json' }).end('{}')
4953
return
5054
}
51-
if (input.claim) claimed = true
55+
if (input.claim) claimed.add(input.toolCallId)
5256
response
5357
.writeHead(200, { 'Content-Type': 'application/json' })
5458
.end(JSON.stringify({ ...call, chatId: 'org-chat' }))
@@ -92,6 +96,7 @@ test('native file tools read and import through the installed preload without Si
9296
ok: true,
9397
data: { observations: [{ mediaType: 'image/png', data: png }] },
9498
})
99+
expect([...claimed]).toEqual(['text', 'image'])
95100
const result = await invoke({ operation: 'manifest', toolCallId: 'import' })
96101
if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result))
97102
expect(result.data.targetWorkspaceId).toBe('target-workspace')

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,19 @@ describe('waitForDesktopToolCall', () => {
4545
abortSignal.addEventListener('abort', () => resolve(desktopResult), { once: true })
4646
})
4747
)
48-
completePendingAsyncToolCall.mockResolvedValueOnce(null)
4948

5049
const answer = waitForDesktopToolCall(params)
5150
await vi.advanceTimersByTimeAsync(GRACE_MS)
5251

5352
expect(await answer).toEqual(desktopResult)
5453
})
54+
55+
it('settles an unclaimed call as never started when its wait ends before the grace', async () => {
56+
waitForClientToolCompletion.mockResolvedValueOnce(null)
57+
completePendingAsyncToolCall.mockImplementationOnce(async (input) => ({ ...input }))
58+
59+
const answer = await waitForDesktopToolCall({ ...params, timeoutMs: 1_000 })
60+
61+
expect(answer).toMatchObject({ status: 'error', data: { notStarted: true } })
62+
})
5563
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export async function waitForDesktopToolCall(
6262
})),
6363
])
6464
graceOver.abort()
65-
if (first.kind === 'reported') return first.completion
65+
if (first.kind === 'reported' && first.completion) return first.completion
6666
if (abortSignal?.aborted) return pickupWait
6767

6868
// A result that landed as the grace ran out is already being restored: it is the answer.

‎scripts/probes/mothership-request-admission.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ try {
124124
await repo.upsertAsyncToolCall({ runId: run.id, toolCallId, toolName: 'run_code' })
125125
await repo.updateRunStatus(run.id, status, { completedAt: new Date() })
126126
assert.equal(
127-
(await repo.claimSimToolExecution({ runId: run.id, toolCallId, userId: 'user-1' })).outcome,
127+
(await repo.claimToolExecution({ runId: run.id, toolCallId, userId: 'user-1' })).outcome,
128128
'closed'
129129
)
130130
await repo.updateRunStatus(run.id, 'paused_waiting_for_tool')
@@ -142,7 +142,7 @@ try {
142142
toolCallId: ownerTool.toolCallId,
143143
toolName: 'run_code',
144144
})
145-
assert.equal((await repo.claimSimToolExecution(ownerTool)).outcome, 'claimed')
145+
assert.equal((await repo.claimToolExecution(ownerTool)).outcome, 'claimed')
146146
const commandIdentity = {
147147
id: generateId(),
148148
sandboxId: 'recorded-sandbox',
@@ -180,7 +180,7 @@ try {
180180
toolName: 'run_code',
181181
})
182182
const [claim] = await Promise.all([
183-
repo.claimSimToolExecution(tool),
183+
repo.claimToolExecution(tool),
184184
repo.updateRunStatus(run.id, 'complete'),
185185
])
186186
assert.equal(
@@ -207,12 +207,12 @@ try {
207207
const workbenchChatId = await createWorkbenchChat()
208208
const sessionKey = `mothership-chat:${workbenchChatId}`
209209
const prior = await createWorkbenchTool(workbenchChatId, 'workbench-prior')
210-
assert.equal((await repo.claimSimToolExecution(prior)).outcome, 'claimed')
210+
assert.equal((await repo.claimToolExecution(prior)).outcome, 'claimed')
211211
const priorCommand = { id: generateId(), sandboxId: 'workbench-vm', sessionKey }
212212
await repo.recordSimSandboxProcess({ ...prior, process: priorCommand })
213213
await repo.updateRunStatus(prior.runId, 'complete')
214214
const current = await createWorkbenchTool(workbenchChatId, 'workbench-current')
215-
assert.equal((await repo.claimSimToolExecution(current)).outcome, 'claimed')
215+
assert.equal((await repo.claimToolExecution(current)).outcome, 'claimed')
216216
const access = { ...current, sessionKey }
217217
assert.deepEqual(await repo.prepareWorkbenchAccess(access), {
218218
handlersPending: true,
@@ -229,7 +229,7 @@ try {
229229
await assert.rejects(repo.prepareWorkbenchAccess({ ...prior, sessionKey }))
230230
const sibling = { ...current, toolCallId: 'workbench-sibling' }
231231
await repo.upsertAsyncToolCall({ ...sibling, toolName: 'run_code' })
232-
assert.equal((await repo.claimSimToolExecution(sibling)).outcome, 'claimed')
232+
assert.equal((await repo.claimToolExecution(sibling)).outcome, 'claimed')
233233
const siblingCommand = { ...priorCommand, id: generateId() }
234234
await repo.recordSimSandboxProcess({ ...sibling, process: siblingCommand })
235235
assert.deepEqual(await repo.prepareWorkbenchAccess(access), ready)
@@ -252,7 +252,7 @@ try {
252252
await repo.settleSimToolExecution(sibling.toolCallId)
253253
const nextTool = { runId: emptyNext.id, userId: 'user-1', toolCallId: 'workbench-next-tool' }
254254
await repo.upsertAsyncToolCall({ ...nextTool, toolName: 'run_code' })
255-
assert.equal((await repo.claimSimToolExecution(nextTool)).outcome, 'claimed')
255+
assert.equal((await repo.claimToolExecution(nextTool)).outcome, 'claimed')
256256
assert.deepEqual(await repo.prepareWorkbenchAccess({ ...nextTool, sessionKey }), {
257257
handlersPending: false,
258258
processes: [{ ...siblingCommand, toolCallId: sibling.toolCallId }],
@@ -266,14 +266,14 @@ try {
266266
const raceChatId = await createWorkbenchChat()
267267
const older = await createWorkbenchTool(raceChatId, `workbench-race-old-${i}`)
268268
const newer = await createWorkbenchTool(raceChatId, `workbench-race-new-${i}`)
269-
assert.equal((await repo.claimSimToolExecution(newer)).outcome, 'claimed')
269+
assert.equal((await repo.claimToolExecution(newer)).outcome, 'claimed')
270270
const [claim, state] = await Promise.all([
271-
repo.claimSimToolExecution(older),
271+
repo.claimToolExecution(older),
272272
repo.prepareWorkbenchAccess({ ...newer, sessionKey: `mothership-chat:${raceChatId}` }),
273273
])
274274
assert.deepEqual(state, { handlersPending: claim.outcome === 'claimed', processes: [] })
275275
assert.equal(
276-
(await repo.claimSimToolExecution({ ...older, toolCallId: 'late' })).outcome,
276+
(await repo.claimToolExecution({ ...older, toolCallId: 'late' })).outcome,
277277
'closed'
278278
)
279279
await assert.rejects(
@@ -283,11 +283,11 @@ try {
283283
logger.info('PASS thirty predecessor-claim/workbench-takeover races without false readiness')
284284
const corruptChatId = await createWorkbenchChat()
285285
const corruptOwner = await createWorkbenchTool(corruptChatId, 'workbench-corrupt-owner')
286-
assert.equal((await repo.claimSimToolExecution(corruptOwner)).outcome, 'claimed')
286+
assert.equal((await repo.claimToolExecution(corruptOwner)).outcome, 'claimed')
287287
const corruptCommand = { id: generateId(), sandboxId: 'foreign', sessionKey: 'another-chat' }
288288
await repo.recordSimSandboxProcess({ ...corruptOwner, process: corruptCommand })
289289
const successor = await createWorkbenchTool(corruptChatId, 'workbench-corrupt-successor')
290-
assert.equal((await repo.claimSimToolExecution(successor)).outcome, 'claimed')
290+
assert.equal((await repo.claimToolExecution(successor)).outcome, 'claimed')
291291
const successorAccess = { ...successor, sessionKey: `mothership-chat:${corruptChatId}` }
292292
await assert.rejects(repo.prepareWorkbenchAccess(successorAccess), /does not match this chat/)
293293
assert.equal(
@@ -334,7 +334,7 @@ try {
334334
})
335335
assert.equal(
336336
(
337-
await repo.claimSimToolExecution({
337+
await repo.claimToolExecution({
338338
runId: delayed.id,
339339
toolCallId: 'late-tool',
340340
userId: 'user-1',

0 commit comments

Comments
 (0)