Skip to content

Commit 455c2bd

Browse files
authored
fix(mothership): start no tool call with less than a minute of its deadline left (#9092)
* fix(mothership): start no tool call with less than a minute of its deadline left * fix(mothership): refuse a late claim at the database and settle an unclaimed call as not run
1 parent c41323b commit 455c2bd

13 files changed

Lines changed: 218 additions & 36 deletions

‎apps/sim/lib/mothership/async-runs/execution-lease.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ export const INTERRUPTED_SIM_TOOL_MESSAGE =
2020
'Tool execution lost its owner before a result was recorded. Its outcome is unknown; inspect current state before retrying a mutation.'
2121

2222
/**
23-
* A takeover re-runs an interrupted call only with at least this much of its deadline left;
24-
* otherwise the call fails with {@link DEADLINE_SIM_TOOL_MESSAGE} instead of starting work
25-
* that could not finish in time.
23+
* A call starts, the first time or again after a takeover, only with at least this much of its
24+
* deadline left; otherwise it fails as not run instead of starting work that would likely be cut
25+
* off with its outcome unknown. About 99% of non-workflow tool calls finish within a minute.
2626
*/
27-
export const SIM_TOOL_RERUN_MIN_REMAINING_MS = 60_000
27+
export const SIM_TOOL_MIN_START_BUDGET_MS = 60_000
2828

2929
export const DEADLINE_SIM_TOOL_MESSAGE =
3030
'Tool execution was interrupted because the Sim server running it stopped, and too little of its time remained to run it again. Its outcome is unknown; inspect current state before retrying a mutation.'

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import {
4242
INTERRUPTED_SIM_TOOL_MESSAGE,
4343
SIM_TOOL_EXECUTION_LEASE_SECONDS,
4444
SIM_TOOL_EXECUTION_MAX_ATTEMPTS,
45-
SIM_TOOL_RERUN_MIN_REMAINING_MS,
45+
SIM_TOOL_MIN_START_BUDGET_MS,
4646
SimToolExecutionLeaseLostError,
4747
type SimToolExecutionOwner,
4848
} from '@/lib/mothership/async-runs/execution-lease'
@@ -891,8 +891,11 @@ const unfinishedSimExecution = allOf(
891891
isNull(copilotAsyncToolCalls.clientWorkflowExecutionId)
892892
)
893893

894-
/** A call whose deadline leaves less than a re-run needs; a call without one never matches. */
895-
const tooLateToRerun = sql`(${copilotAsyncToolCalls.executionDeadlineAt} IS NOT NULL AND ${copilotAsyncToolCalls.executionDeadlineAt} < clock_timestamp() + ${SIM_TOOL_RERUN_MIN_REMAINING_MS} * interval '1 millisecond')`
894+
/**
895+
* A call whose deadline leaves less than the minimum start budget, so nothing may start it: no
896+
* re-run and no claim. A call without a deadline never matches.
897+
*/
898+
const tooLateToStart = sql`(${copilotAsyncToolCalls.executionDeadlineAt} IS NOT NULL AND ${copilotAsyncToolCalls.executionDeadlineAt} < clock_timestamp() + ${SIM_TOOL_MIN_START_BUDGET_MS} * interval '1 millisecond')`
896899

897900
/**
898901
* A Sim execution whose owner died before recording a result: its lease lapsed, which a live
@@ -903,7 +906,7 @@ const interruptedSimExecution = allOf(
903906
unfinishedSimExecution,
904907
sql`${copilotAsyncToolCalls.executionLeaseExpiresAt} <= clock_timestamp()`,
905908
lt(copilotAsyncToolCalls.executionAttempts, SIM_TOOL_EXECUTION_MAX_ATTEMPTS),
906-
not(tooLateToRerun)
909+
not(tooLateToStart)
907910
)
908911

909912
/**
@@ -1030,6 +1033,7 @@ export async function claimDesktopToolCall(
10301033
and(
10311034
thisCall,
10321035
eq(copilotAsyncToolCalls.status, ASYNC_TOOL_STATUS.pending),
1036+
not(tooLateToStart),
10331037
// The executor takes only a call Sim offered it, inside its pickup window; once the
10341038
// window closed the call belongs to its not-started settlement.
10351039
executor
@@ -1135,7 +1139,7 @@ export async function getChatViewDesktopLeaseRemainingMs(
11351139
* Why a lapsed execution ended without a result: once, after every re-run it was allowed, or
11361140
* with too little of its deadline left to run again.
11371141
*/
1138-
const interruptionMessage = sql<string>`CASE WHEN ${copilotAsyncToolCalls.executionAttempts} >= ${SIM_TOOL_EXECUTION_MAX_ATTEMPTS} THEN ${EXHAUSTED_SIM_TOOL_MESSAGE}::text WHEN ${tooLateToRerun} THEN ${DEADLINE_SIM_TOOL_MESSAGE}::text ELSE ${INTERRUPTED_SIM_TOOL_MESSAGE}::text END`
1142+
const interruptionMessage = sql<string>`CASE WHEN ${copilotAsyncToolCalls.executionAttempts} >= ${SIM_TOOL_EXECUTION_MAX_ATTEMPTS} THEN ${EXHAUSTED_SIM_TOOL_MESSAGE}::text WHEN ${tooLateToStart} THEN ${DEADLINE_SIM_TOOL_MESSAGE}::text ELSE ${INTERRUPTED_SIM_TOOL_MESSAGE}::text END`
11391143

11401144
/**
11411145
* Whether one of the run's Sim handlers is still running with its execution lease held. Its
@@ -1192,7 +1196,7 @@ export async function revokeExhaustedSimToolExecutions(runId: string) {
11921196
eq(copilotAsyncToolCalls.runId, runId),
11931197
or(
11941198
gte(copilotAsyncToolCalls.executionAttempts, SIM_TOOL_EXECUTION_MAX_ATTEMPTS),
1195-
tooLateToRerun
1199+
tooLateToStart
11961200
)
11971201
)
11981202
)
@@ -1776,6 +1780,7 @@ export async function claimWorkflowToolExecution(
17761780
.where(
17771781
and(
17781782
eq(copilotAsyncToolCalls.toolCallId, toolCallId),
1783+
not(tooLateToStart),
17791784
or(
17801785
and(
17811786
isNull(copilotAsyncToolCalls.claimedBy),

‎apps/sim/lib/mothership/request/application/abandoned-runs.integration.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,32 @@ describe.runIf(Boolean(redisUrl))('taking over a run whose controller died', ()
549549
}
550550
})
551551

552+
it.each([
553+
[30, false],
554+
[600, true],
555+
])(
556+
'a browser claims a workflow call with %is of its deadline left: %s',
557+
async (seconds, claimable) => {
558+
const { runId, toolCallId } = await runAbandonedMidTool({ toolName: 'run_workflow' })
559+
/** Offered to a browser and not yet picked up, with `seconds` of its deadline left. */
560+
await db
561+
.update(copilotAsyncToolCalls)
562+
.set({
563+
claimedBy: null,
564+
executionStartedAt: null,
565+
executionOwnerToken: null,
566+
executionDeadlineAt: sql`clock_timestamp() + ${seconds} * interval '1 second'`,
567+
})
568+
.where(eq(copilotAsyncToolCalls.toolCallId, toolCallId))
569+
try {
570+
const claim = await claimWorkflowToolExecution(toolCallId, generateId(), 'client')
571+
expect(claim !== null).toBe(claimable)
572+
} finally {
573+
await db.update(copilotRuns).set({ status: 'cancelled' }).where(eq(copilotRuns.id, runId))
574+
}
575+
}
576+
)
577+
552578
it('fails an exhausted workflow run before a takeover offers it to anyone', async () => {
553579
const { runId, toolCallId } = await runAbandonedMidTool({
554580
toolName: 'run_workflow',

‎apps/sim/lib/mothership/request/handlers/handlers.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,37 @@ describe('sse-handlers tool lifecycle', () => {
775775
expectFailedWith(onEvent, TOOL_DEADLINE_BEFORE_START_MESSAGE)
776776
})
777777

778+
it('does not offer an unclaimed client call with less than a minute of its deadline left', async () => {
779+
mothershipAsyncRunsMockFns.mockCompletePendingAsyncToolCall.mockImplementationOnce(
780+
async (input) => ({ ...input })
781+
)
782+
const onEvent = await runLocalRead(TOOL_DEADLINE_SETTLE_MARGIN_MS + 30_000)
783+
await Promise.allSettled(context.pendingToolPromises.values())
784+
785+
expect(waitForClientToolCompletion).not.toHaveBeenCalled()
786+
expectFailedWith(onEvent, TOOL_DEADLINE_BEFORE_START_MESSAGE)
787+
})
788+
789+
it('settles a client call nothing claimed by its deadline as not run', async () => {
790+
vi.useFakeTimers()
791+
try {
792+
waitForClientToolCompletion.mockImplementationOnce(
793+
() => new Promise((resolve) => setTimeout(() => resolve(null), 65_000))
794+
)
795+
mothershipAsyncRunsMockFns.mockCompletePendingAsyncToolCall.mockImplementationOnce(
796+
async (input) => ({ ...input })
797+
)
798+
const onEvent = await runLocalRead(TOOL_DEADLINE_SETTLE_MARGIN_MS + 65_000)
799+
await vi.advanceTimersByTimeAsync(65_000)
800+
await Promise.allSettled(context.pendingToolPromises.values())
801+
802+
expect(waitForClientToolCompletion).toHaveBeenCalledOnce()
803+
expectFailedWith(onEvent, TOOL_DEADLINE_BEFORE_START_MESSAGE)
804+
} finally {
805+
vi.useRealTimers()
806+
}
807+
})
808+
778809
it('reads a claimed call past its deadline and reports its outcome as unknown', async () => {
779810
const onEvent = await runLocalRead(TOOL_DEADLINE_SETTLE_MARGIN_MS)
780811
await Promise.allSettled(context.pendingToolPromises.values())

‎apps/sim/lib/mothership/request/handlers/tool.ts‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
TOOL_DEADLINE_EXCEEDED_MESSAGE,
4242
toDeadlineDate,
4343
toolDeadlinePassed,
44+
toolStartWindowClosed,
4445
} from '@/lib/mothership/request/tools/deadline'
4546
import { waitForDesktopToolCall } from '@/lib/mothership/request/tools/desktop-wait'
4647
import {
@@ -1000,7 +1001,7 @@ async function dispatchToolExecution(
10001001
// provably never ran; a claimed one (a takeover re-hands calls) is read, then settled
10011002
// below as outcome unknown.
10021003
if (
1003-
toolDeadlinePassed(toolCall.deadlineAt, Date.now()) &&
1004+
toolStartWindowClosed(toolCall.deadlineAt, Date.now()) &&
10041005
(await failUnstartedToolCall(
10051006
toolCallId,
10061007
context,
@@ -1079,13 +1080,22 @@ async function dispatchToolExecution(
10791080
}
10801081
span.setAttribute(TraceAttr.ToolExecutor, MothershipStreamV1ToolExecutor.client)
10811082
if (completion === null && toolDeadlinePassed(toolCall.deadlineAt, Date.now())) {
1082-
// Settled durably, so a desktop that claimed the call stops renewing it.
1083-
await failPendingToolCall(
1084-
toolCallId,
1085-
context,
1086-
execContext,
1087-
TOOL_DEADLINE_EXCEEDED_MESSAGE
1083+
// A call nothing claimed never ran. A claimed one is settled durably as outcome
1084+
// unknown, so a desktop that claimed it stops renewing it.
1085+
if (
1086+
!(await failUnstartedToolCall(
1087+
toolCallId,
1088+
context,
1089+
execContext,
1090+
TOOL_DEADLINE_BEFORE_START_MESSAGE
1091+
))
10881092
)
1093+
await failPendingToolCall(
1094+
toolCallId,
1095+
context,
1096+
execContext,
1097+
TOOL_DEADLINE_EXCEEDED_MESSAGE
1098+
)
10891099
completion = settledClientCompletion(toolCall)
10901100
}
10911101
// Both waiters resolve `T | null`, never undefined — comparing against

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { SIM_TOOL_MIN_START_BUDGET_MS } from '@/lib/mothership/async-runs/execution-lease'
12
import { TOOL_DEADLINE_SETTLE_MARGIN_MS } from '@/lib/mothership/generated/protocol'
23

34
/** The longest worker wait a frame may name; a larger or malformed value is ignored. */
@@ -27,14 +28,28 @@ export const TOOL_DEADLINE_BEFORE_START_MESSAGE =
2728
export const TOOL_DEADLINE_EXCEEDED_MESSAGE =
2829
'This tool call did not finish within its time limit and was abandoned. Its outcome is unknown: it may already have taken effect, so inspect the current state before repeating it.'
2930

30-
/**
31-
* Whether the call's deadline has passed, so nothing more may start for it: no handler, approval
32-
* wait, server fallback or allowed decision. A call without a deadline never passes one.
33-
*/
31+
/** Whether the call's deadline has passed. A call without a deadline never passes one. */
3432
export function toolDeadlinePassed(deadlineAt: number | undefined, now: number): boolean {
3533
return deadlineAt !== undefined && now >= deadlineAt
3634
}
3735

36+
/**
37+
* The last moment anything may start for a call: its deadline less the minimum start budget,
38+
* since work started later would likely be abandoned mid-flight, its effects unknown.
39+
*/
40+
export function lastToolStartAt(deadlineAt: number | undefined): number | undefined {
41+
return deadlineAt === undefined ? undefined : deadlineAt - SIM_TOOL_MIN_START_BUDGET_MS
42+
}
43+
44+
/**
45+
* Whether it is too late to start anything for the call: no handler, approval prompt, allowed
46+
* decision, server fallback or client pickup. Exactly the minimum budget left still starts.
47+
*/
48+
export function toolStartWindowClosed(deadlineAt: number | undefined, now: number): boolean {
49+
const lastStartAt = lastToolStartAt(deadlineAt)
50+
return lastStartAt !== undefined && now > lastStartAt
51+
}
52+
3853
/** The column value for a call's deadline. */
3954
export function toDeadlineDate(deadlineAt: number | undefined): Date | undefined {
4055
return deadlineAt === undefined ? undefined : new Date(deadlineAt)

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ describe('executeToolAndReport provenance isolation', () => {
706706
return new Promise(() => {})
707707
})
708708
upsertAsyncToolCall.mockResolvedValueOnce({
709-
executionDeadlineAt: new Date(Date.now() + 30_000),
709+
executionDeadlineAt: new Date(Date.now() + 90_000),
710710
})
711711
const tool: ToolCallState = {
712712
...buildPendingToolCall(),
@@ -718,12 +718,47 @@ describe('executeToolAndReport provenance isolation', () => {
718718
workflowId: 'workflow-1',
719719
})
720720

721-
await vi.advanceTimersByTimeAsync(30_000)
721+
await vi.advanceTimersByTimeAsync(90_000)
722722

723723
expect(signal?.aborted).toBe(true)
724724
expect((await pending).status).toBe('error')
725725
})
726726

727+
it('starts a handler with exactly the minimum budget of its deadline left', async () => {
728+
vi.useFakeTimers()
729+
vi.setSystemTime(new Date('2026-10-15T12:00:00Z'))
730+
executeTool.mockResolvedValue({ success: true, output: {} })
731+
const tool: ToolCallState = {
732+
...buildPendingToolCall(),
733+
name: 'generate_audio',
734+
deadlineAt: Date.now() + 60_000,
735+
}
736+
await executeToolAndReport(tool.id, buildStreamingContext(tool), {
737+
userId: 'user-1',
738+
workflowId: 'workflow-1',
739+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry(),
740+
})
741+
742+
expect(executeTool).toHaveBeenCalledOnce()
743+
})
744+
745+
it('does not start a handler with less than a minute of its deadline left', async () => {
746+
const tool: ToolCallState = {
747+
...buildPendingToolCall(),
748+
name: 'generate_audio',
749+
deadlineAt: Date.now() + 30_000,
750+
}
751+
const completion = await executeToolAndReport(tool.id, buildStreamingContext(tool), {
752+
userId: 'user-1',
753+
workflowId: 'workflow-1',
754+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry(),
755+
})
756+
757+
expect(executeTool).not.toHaveBeenCalled()
758+
expect(completion.status).toBe('error')
759+
expect(tool.error).toBe(TOOL_DEADLINE_BEFORE_START_MESSAGE)
760+
})
761+
727762
it('aborts a timed-out handler without cancelling a parallel tool', async () => {
728763
vi.useFakeTimers()
729764
const parent = new AbortController()

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ import {
6666
capToToolDeadline,
6767
TOOL_DEADLINE_BEFORE_START_MESSAGE,
6868
toDeadlineDate,
69-
toolDeadlinePassed,
69+
toolStartWindowClosed,
7070
} from '@/lib/mothership/request/tools/deadline'
7171
import { desktopToolNotStarted } from '@/lib/mothership/request/tools/desktop-wait'
7272
import {
@@ -376,8 +376,8 @@ async function executeToolWithWatchdog(
376376
// execution always dispatches on the model's real tool name.
377377
const executableName = toolCall.execName ?? toolCall.name
378378
const now = Date.now()
379-
// Admission or approval can spend the whole deadline; the handler then never starts.
380-
if (toolDeadlinePassed(toolCall.deadlineAt, now))
379+
// Admission or approval can leave too little time to finish; the handler then never starts.
380+
if (toolStartWindowClosed(toolCall.deadlineAt, now))
381381
throw new Error(TOOL_DEADLINE_BEFORE_START_MESSAGE)
382382
const timeoutMs = capToToolDeadline(
383383
toolWatchdogTimeoutMs(executableName),

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ vi.mock('@/lib/mothership/persistence/tool-permission', async (importOriginal) =
2222

2323
import { MothershipStreamV1ToolExecutor } from '@/lib/mothership/generated/mothership-stream-v1'
2424
import { createStreamingContext } from '@/lib/mothership/request/context/request-context'
25+
import { TOOL_DEADLINE_BEFORE_START_MESSAGE } from '@/lib/mothership/request/tools/deadline'
2526
import {
2627
runGatedToolExecution,
2728
toolCallNeedsApproval,
@@ -236,6 +237,28 @@ describe('runGatedToolExecution', () => {
236237
}
237238
})
238239

240+
it('does not honor an approval that leaves less than a minute to run the call', async () => {
241+
vi.useFakeTimers()
242+
try {
243+
waitForToolPermissionDecision.mockImplementation(
244+
() =>
245+
new Promise((resolve) =>
246+
setTimeout(() => resolve({ toolCallId: 'call-1', decision: 'allow' }), 40_000)
247+
)
248+
)
249+
const toolCall = { ...makeToolCall(), deadlineAt: Date.now() + 90_000 }
250+
const execute = vi.fn()
251+
const pending = gate(makeContext(), toolCall, execute, [])
252+
253+
await vi.advanceTimersByTimeAsync(40_000)
254+
255+
expect(await pending).toMatchObject({ status: 'error' })
256+
expect(execute).not.toHaveBeenCalled()
257+
} finally {
258+
vi.useRealTimers()
259+
}
260+
})
261+
239262
it('stops waiting for an answer at the deadline the worker named for the call', async () => {
240263
vi.useFakeTimers()
241264
try {
@@ -254,7 +277,7 @@ describe('runGatedToolExecution', () => {
254277

255278
expect(settled).toBe(true)
256279
expect(await pending).toMatchObject({ status: 'error' })
257-
expect(toolCall.error).toBe('Timed out waiting for the user to approve this tool.')
280+
expect(toolCall.error).toBe(TOOL_DEADLINE_BEFORE_START_MESSAGE)
258281
expect(execute).not.toHaveBeenCalled()
259282
} finally {
260283
vi.useRealTimers()

0 commit comments

Comments
 (0)