Skip to content

Commit 98fb34c

Browse files
committed
fix(mothership): hold desktop leases, the resume wait and first claims to the call deadline
1 parent f73b9db commit 98fb34c

6 files changed

Lines changed: 164 additions & 8 deletions

File tree

‎apps/sim/lib/desktop/application/executor.integration.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@ import {
5454
} from '@/lib/desktop/executor/errors'
5555
import { isDesktopPresent } from '@/lib/desktop/executor/presence'
5656
import { claimDesktopToolCall, requestRunStop } from '@/lib/mothership/async-runs/repository'
57+
import { TOOL_WATCHDOG_RESUME_GRACE_MS } from '@/lib/mothership/constants'
5758
import { waitForToolConfirmation } from '@/lib/mothership/persistence/tool-confirm'
5859
import { unsealClientToolCompletion } from '@/lib/mothership/request/tools/client-completion-seal.server'
60+
import { settleAbandonedDesktopToolCalls } from '@/lib/mothership/request/tools/desktop-wait'
5961

6062
const CAPABILITIES = { executor: 1, browser: true, terminal: true, localFiles: true }
6163

@@ -556,6 +558,7 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
556558
const toolCallId = await pendingCall(run.runId)
557559
const { executionToken } = await claim(desktop.principal, desktop.deviceId, toolCallId)
558560
const input = { deviceId: desktop.deviceId, toolCallId, executionToken }
561+
expect((await row(toolCallId)).executionDeadlineAt).toBeNull()
559562

560563
await db
561564
.update(copilotAsyncToolCalls)
@@ -583,6 +586,41 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
583586
).rejects.toBeInstanceOf(DesktopCallRevokedError)
584587
})
585588

589+
it('stops renewing a call once the resume gate gave up on its deadline, then settles it as outcome unknown', async () => {
590+
const desktop = await signedInDesktop()
591+
const run = await boundRun(desktop)
592+
const toolCallId = await pendingCall(run.runId)
593+
const setRow = (fields: Partial<typeof copilotAsyncToolCalls.$inferInsert>) =>
594+
db
595+
.update(copilotAsyncToolCalls)
596+
.set(fields)
597+
.where(eq(copilotAsyncToolCalls.toolCallId, toolCallId))
598+
const deadlinePassedBy = (seconds: number) =>
599+
setRow({ executionDeadlineAt: sql`clock_timestamp() - ${seconds} * interval '1 second'` })
600+
await setRow({ executionDeadlineAt: sql`clock_timestamp() + interval '10 minutes'` })
601+
const { executionToken } = await claim(desktop.principal, desktop.deviceId, toolCallId)
602+
const input = { deviceId: desktop.deviceId, toolCallId, executionToken }
603+
604+
// Inside the gate's grace past the deadline, the device keeps the call.
605+
await deadlinePassedBy(TOOL_WATCHDOG_RESUME_GRACE_MS / 1000 - 10)
606+
await expect(
607+
renewDesktopToolLease.execute({ principal: desktop.principal, input })
608+
).resolves.toEqual({ renewed: true })
609+
610+
await deadlinePassedBy(TOOL_WATCHDOG_RESUME_GRACE_MS / 1000 + 1)
611+
await expect(
612+
renewDesktopToolLease.execute({ principal: desktop.principal, input })
613+
).rejects.toBeInstanceOf(DesktopCallRevokedError)
614+
615+
// Unrenewed, the lease lapses, and the cleanup settles the call as it would any lapse.
616+
await setRow({ executionLeaseExpiresAt: sql`clock_timestamp() - interval '1 second'` })
617+
await settleAbandonedDesktopToolCalls(0)
618+
expect(await row(toolCallId)).toMatchObject({
619+
status: 'failed',
620+
error: expect.stringContaining('It may already have taken effect'),
621+
})
622+
})
623+
586624
it('after Stop, refuses the stopped calls, revokes the running one and lists it to cancel', async () => {
587625
const desktop = await signedInDesktop()
588626
const run = await boundRun(desktop)

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import {
5555
isAwaitingToolPermission,
5656
SIM_TOOL_EXECUTION_VERSION,
5757
} from '@/lib/mothership/async-runs/lifecycle'
58+
import { TOOL_WATCHDOG_RESUME_GRACE_MS } from '@/lib/mothership/constants'
5859
import { TraceAttr } from '@/lib/mothership/generated/trace-attributes-v1'
5960
import { TraceSpan } from '@/lib/mothership/generated/trace-spans-v1'
6061
import {
@@ -897,6 +898,12 @@ const unfinishedSimExecution = allOf(
897898
*/
898899
const tooLateToStart = sql`(${copilotAsyncToolCalls.executionDeadlineAt} IS NOT NULL AND ${copilotAsyncToolCalls.executionDeadlineAt} < clock_timestamp() + ${SIM_TOOL_MIN_START_BUDGET_MS} * interval '1 millisecond')`
899900

901+
/**
902+
* A call past its deadline plus the resume grace: the resume wait's ceiling for it (`run.ts`), past
903+
* which nothing still waits for its result. A call without a deadline never matches.
904+
*/
905+
const pastResumeGiveUp = sql`(${copilotAsyncToolCalls.executionDeadlineAt} IS NOT NULL AND ${copilotAsyncToolCalls.executionDeadlineAt} + ${TOOL_WATCHDOG_RESUME_GRACE_MS} * interval '1 millisecond' <= clock_timestamp())`
906+
900907
/**
901908
* A Sim execution whose owner died before recording a result: its lease lapsed, which a live
902909
* owner's heartbeat never lets happen. A takeover runs it again until it has started
@@ -1069,7 +1076,8 @@ export async function claimDesktopToolCall(
10691076
/**
10701077
* Expired ownership cannot be renewed, even before a follower has observed the expiry. With
10711078
* `desktop`, the call must still be running on a run bound to that device, so a call Stop settled
1072-
* cannot be renewed either.
1079+
* cannot be renewed either, nor one past its deadline plus the resume grace: its lease then lapses
1080+
* and the call settles as outcome unknown.
10731081
*/
10741082
export async function renewSimToolExecutionLease(
10751083
owner: SimToolExecutionOwner,
@@ -1092,6 +1100,7 @@ export async function renewSimToolExecutionLease(
10921100
desktop
10931101
? and(
10941102
eq(copilotAsyncToolCalls.status, ASYNC_TOOL_STATUS.running),
1103+
not(pastResumeGiveUp),
10951104
'deviceId' in desktop
10961105
? sql`EXISTS (SELECT 1 FROM ${copilotRuns} r WHERE r.id = ${copilotAsyncToolCalls.runId} AND r.desktop_device_id = ${desktop.deviceId})`
10971106
: and(

‎apps/sim/lib/mothership/request/lifecycle/run.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3672,6 +3672,41 @@ describe('runCopilotLifecycle', () => {
36723672
}
36733673
})
36743674

3675+
it('fails a call whose deadline already passed at that deadline plus grace, not a grace from now', async () => {
3676+
vi.useFakeTimers()
3677+
try {
3678+
mothershipAsyncRunsMockFns.mockGetChatViewDesktopLeaseRemainingMs.mockResolvedValue(50_000)
3679+
const turn = runImportTurn(Date.now() - 10_000)
3680+
await vi.advanceTimersByTimeAsync(19_000)
3681+
expect(turn.bodies).toHaveLength(1)
3682+
await vi.advanceTimersByTimeAsync(2_000)
3683+
expect((await turn.lifecycle).success).toBe(true)
3684+
expect(turn.resumedWithLostImport()).toBe(true)
3685+
expect(
3686+
mothershipAsyncRunsMockFns.mockGetChatViewDesktopLeaseRemainingMs
3687+
).not.toHaveBeenCalled()
3688+
} finally {
3689+
mothershipAsyncRunsMockFns.mockGetChatViewDesktopLeaseRemainingMs.mockReset()
3690+
vi.useRealTimers()
3691+
}
3692+
})
3693+
3694+
it('a call without a deadline keeps its full budget and grace', async () => {
3695+
vi.useFakeTimers()
3696+
try {
3697+
mothershipAsyncRunsMockFns.mockGetChatViewDesktopLeaseRemainingMs.mockResolvedValue(null)
3698+
const turn = runImportTurn()
3699+
await vi.advanceTimersByTimeAsync(89_000)
3700+
expect(turn.bodies).toHaveLength(1)
3701+
await vi.advanceTimersByTimeAsync(2_000)
3702+
expect((await turn.lifecycle).success).toBe(true)
3703+
expect(turn.resumedWithLostImport()).toBe(true)
3704+
} finally {
3705+
mothershipAsyncRunsMockFns.mockGetChatViewDesktopLeaseRemainingMs.mockReset()
3706+
vi.useRealTimers()
3707+
}
3708+
})
3709+
36753710
it('a lease lookup that fails is checked again instead of failing a renewed import', async () => {
36763711
vi.useFakeTimers()
36773712
try {

‎apps/sim/lib/mothership/request/lifecycle/run.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,21 +1450,22 @@ async function runCheckpointLoop(
14501450
const waitBudgetMs =
14511451
pendingToolWaitBudgetMs(toolCall, context.desktopDeviceId) +
14521452
TOOL_WATCHDOG_RESUME_GRACE_MS
1453-
const ceilingAt = now + Math.max(waitBudgetMs, CLIENT_TOOL_RESULT_TIMEOUT_MS)
1453+
const clientCapAt = now + Math.max(waitBudgetMs, CLIENT_TOOL_RESULT_TIMEOUT_MS)
1454+
// Neither the wait nor a lease carries a call past the deadline the worker named for it.
1455+
const ceilingAt =
1456+
toolCall?.deadlineAt === undefined
1457+
? clientCapAt
1458+
: Math.min(clientCapAt, toolCall.deadlineAt + TOOL_WATCHDOG_RESUME_GRACE_MS)
14541459
pendingWatchdogs.set(toolCallId, {
14551460
promise,
14561461
settlement: promise.then(
14571462
() => ({ toolCallId, promise }),
14581463
() => ({ toolCallId, promise })
14591464
),
1460-
deadlineAt: now + waitBudgetMs,
1465+
deadlineAt: Math.min(now + waitBudgetMs, ceilingAt),
14611466
waitBudgetMs,
14621467
startedAt: now,
1463-
// A lease never carries a call past the deadline the worker named for it.
1464-
ceilingAt:
1465-
toolCall?.deadlineAt === undefined
1466-
? ceilingAt
1467-
: Math.min(ceilingAt, toolCall.deadlineAt + TOOL_WATCHDOG_RESUME_GRACE_MS),
1468+
ceilingAt,
14681469
})
14691470
maximumWaitBudgetMs = Math.max(maximumWaitBudgetMs, waitBudgetMs)
14701471
}

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,67 @@ describe('executeToolAndReport provenance isolation', () => {
698698
expect(tool.error).toBe(TOOL_DEADLINE_BEFORE_START_MESSAGE)
699699
})
700700

701+
it('settles an unclaimed call past its start window as never run, without claiming it', async () => {
702+
completePendingAsyncToolCall.mockImplementationOnce(async (input) => ({ ...input }))
703+
const tool: ToolCallState = {
704+
...buildPendingToolCall(),
705+
name: 'generate_audio',
706+
deadlineAt: Date.now() + 30_000,
707+
}
708+
const completion = await executeToolAndReport(tool.id, buildStreamingContext(tool), {
709+
userId: 'user-1',
710+
workflowId: 'workflow-1',
711+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry(),
712+
})
713+
714+
expect(claimToolExecution).not.toHaveBeenCalled()
715+
expect(executeTool).not.toHaveBeenCalled()
716+
expect(completePendingAsyncToolCall).toHaveBeenCalledExactlyOnceWith(
717+
expect.objectContaining({
718+
toolCallId: tool.id,
719+
status: 'failed',
720+
error: TOOL_DEADLINE_BEFORE_START_MESSAGE,
721+
})
722+
)
723+
expect(completion.status).toBe('error')
724+
expect(tool.result?.output).toMatchObject({
725+
error: TOOL_DEADLINE_BEFORE_START_MESSAGE,
726+
notStarted: true,
727+
})
728+
})
729+
730+
it('leaves a call past its start window that is already claimed to its claim', async () => {
731+
completePendingAsyncToolCall.mockResolvedValueOnce(null)
732+
const tool: ToolCallState = {
733+
...buildPendingToolCall(),
734+
name: 'generate_audio',
735+
deadlineAt: Date.now() + 30_000,
736+
}
737+
await executeToolAndReport(tool.id, buildStreamingContext(tool), {
738+
userId: 'user-1',
739+
workflowId: 'workflow-1',
740+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry(),
741+
})
742+
743+
expect(claimToolExecution).toHaveBeenCalledOnce()
744+
expect(executeTool).not.toHaveBeenCalled()
745+
expect(tool.error).toBe(TOOL_DEADLINE_BEFORE_START_MESSAGE)
746+
})
747+
748+
it('claims a call without a deadline without trying to settle it first', async () => {
749+
executeTool.mockResolvedValue({ success: true, output: {} })
750+
const tool: ToolCallState = { ...buildPendingToolCall(), name: 'generate_audio' }
751+
await executeToolAndReport(tool.id, buildStreamingContext(tool), {
752+
userId: 'user-1',
753+
workflowId: 'workflow-1',
754+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry(),
755+
})
756+
757+
expect(completePendingAsyncToolCall).not.toHaveBeenCalled()
758+
expect(claimToolExecution).toHaveBeenCalledOnce()
759+
expect(executeTool).toHaveBeenCalledOnce()
760+
})
761+
701762
it('runs a call again only until the deadline recorded when it was first handed', async () => {
702763
vi.useFakeTimers()
703764
let signal: AbortSignal | undefined

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,18 @@ async function executeToolAndReportInner(
754754
})
755755
/** A re-run after a takeover gets what remains of the call's first deadline, never a fresh one. */
756756
if (record?.executionDeadlineAt) toolCall.deadlineAt = record.executionDeadlineAt.getTime()
757+
// Too late to start: an unclaimed call is settled as never run rather than claimed first, so a
758+
// crash before its handler refuses to start cannot leave it recovered as an unknown outcome.
759+
if (
760+
toolStartWindowClosed(toolCall.deadlineAt, Date.now()) &&
761+
(await failUnstartedToolCall(
762+
toolCall.id,
763+
context,
764+
execContext,
765+
TOOL_DEADLINE_BEFORE_START_MESSAGE
766+
))
767+
)
768+
return terminalCompletionFromToolCall(toolCall)
757769
if (context.runId) {
758770
const claim = await lifetime.claim(context.runId, execContext.userId)
759771
if (claim.outcome === 'closed') return settleCancelled('Run stopped before tool admission')

0 commit comments

Comments
 (0)