Skip to content

Commit e4ec6c3

Browse files
committed
fix(mothership): Stop only cancels detached runs still running, and a held workflow run never blocks the next turn's workbench
1 parent 3e23d39 commit e4ec6c3

4 files changed

Lines changed: 234 additions & 37 deletions

File tree

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

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
copilotRequestStops,
1212
copilotRuns,
1313
user,
14+
workflowExecutionLogs,
1415
} from '@sim/db/schema'
1516
import { createLogger } from '@sim/logger'
1617
import { filterUndefined, toRecordOrNull } from '@sim/utils/object'
@@ -79,6 +80,10 @@ import {
7980
NO_SECRET_AUTHORITY,
8081
type TurnSecretAuthority,
8182
} from '@/lib/mothership/secret-mount-policy'
83+
import {
84+
isWorkflowToolName,
85+
WORKFLOW_TOOL_NAMES,
86+
} from '@/lib/mothership/tools/client-executed-tools'
8287
import {
8388
type DesktopToolClaimOwner,
8489
isDesktopToolCall,
@@ -1753,6 +1758,7 @@ export async function prepareWorkbenchAccess(
17531758
settledAt: copilotAsyncToolCalls.executionSettledAt,
17541759
revokedAt: copilotAsyncToolCalls.executionRevokedAt,
17551760
toolCallId: copilotAsyncToolCalls.toolCallId,
1761+
executableName: copilotAsyncToolCalls.executableName,
17561762
processes: copilotAsyncToolCalls.sandboxProcesses,
17571763
})
17581764
.from(copilotAsyncToolCalls)
@@ -1820,7 +1826,16 @@ export async function prepareWorkbenchAccess(
18201826
if (processes.some((process) => process.sessionKey !== input.sessionKey))
18211827
throw new Error('Recorded workbench ownership does not match this chat')
18221828
return {
1823-
handlersPending: previous.some((row) => !row.settledAt && !row.revokedAt),
1829+
/**
1830+
* A workflow tool's handler runs a workflow, which never touches the workbench. One answered
1831+
* still running holds its run past its turn, so it must not hold the workbench too.
1832+
*/
1833+
handlersPending: previous.some(
1834+
(row) =>
1835+
!row.settledAt &&
1836+
!row.revokedAt &&
1837+
!(row.executableName && isWorkflowToolName(row.executableName))
1838+
),
18241839
processes,
18251840
}
18261841
})
@@ -1925,13 +1940,6 @@ export async function getUnsettledClientWorkflowExecutions(
19251940
)
19261941
}
19271942

1928-
const WORKFLOW_RUN_TOOL_NAMES = [
1929-
'run_workflow',
1930-
'run_workflow_until_block',
1931-
'run_from_block',
1932-
'run_block',
1933-
] as const
1934-
19351943
/**
19361944
* The workflow execution a settled call started and left running: a `workflows run` the CLI
19371945
* answered still running (exit 4 and its run record), or a workflow tool answered still running
@@ -1969,8 +1977,11 @@ export function detachedWorkflowExecutionId(call: {
19691977
}
19701978

19711979
/**
1972-
* Workflow executions this run's calls started and left running past the call: what a Stop
1973-
* cancels beyond the calls still open, since a detached run takes no call's abort signal.
1980+
* Workflow executions this run's calls started and left running past the call, and that are still
1981+
* running: what a Stop cancels beyond the calls still open, since a detached run takes no call's
1982+
* abort signal. A run whose log has settled or paused is left alone: a paused run waits on a
1983+
* person, not on this turn, and the durable cancel flag would fail its resume under the same
1984+
* execution id. A run with no log yet may still be starting, so it is cancelled.
19741985
*/
19751986
export async function getDetachedWorkflowExecutions(
19761987
streamId: string,
@@ -1983,23 +1994,45 @@ export async function getDetachedWorkflowExecutions(
19831994
'copilot_async_tool_calls',
19841995
{ [TraceAttr.StreamId]: streamId, [TraceAttr.UserId]: userId },
19851996
async () => {
1997+
const { args, result } = copilotAsyncToolCalls
1998+
/** Only the fields `detachedWorkflowExecutionId` reads, so no call's payload is loaded. */
19861999
const rows = await executor
19872000
.select({
19882001
executableName: copilotAsyncToolCalls.executableName,
1989-
args: copilotAsyncToolCalls.args,
1990-
result: copilotAsyncToolCalls.result,
2002+
args: sql<unknown>`jsonb_build_object('request', jsonb_build_object('invocation', jsonb_build_object('argv', ${args} #> '{request,invocation,argv}')))`,
2003+
result: sql<unknown>`jsonb_build_object('exitCode', ${result} -> 'exitCode', 'stdout', ${result} -> 'stdout', 'status', ${result} -> 'status', 'executionId', ${result} -> 'executionId')`,
19912004
})
19922005
.from(copilotAsyncToolCalls)
19932006
.innerJoin(copilotRuns, eq(copilotRuns.id, copilotAsyncToolCalls.runId))
19942007
.where(
19952008
and(
19962009
eq(copilotRuns.streamId, streamId),
19972010
eq(copilotRuns.userId, userId),
1998-
inArray(copilotAsyncToolCalls.executableName, ['sim_cli', ...WORKFLOW_RUN_TOOL_NAMES]),
1999-
isNotNull(copilotAsyncToolCalls.result)
2011+
or(
2012+
and(
2013+
eq(copilotAsyncToolCalls.executableName, 'sim_cli'),
2014+
sql`${result} ->> 'exitCode' = '4'`
2015+
),
2016+
and(
2017+
inArray(copilotAsyncToolCalls.executableName, [...WORKFLOW_TOOL_NAMES]),
2018+
sql`${result} ->> 'status' = 'running'`
2019+
)
2020+
)
2021+
)
2022+
)
2023+
const detached = [...new Set(rows.flatMap((row) => detachedWorkflowExecutionId(row) ?? []))]
2024+
if (detached.length === 0) return []
2025+
const settled = await executor
2026+
.select({ executionId: workflowExecutionLogs.executionId })
2027+
.from(workflowExecutionLogs)
2028+
.where(
2029+
and(
2030+
inArray(workflowExecutionLogs.executionId, detached),
2031+
ne(workflowExecutionLogs.status, 'running')
20002032
)
20012033
)
2002-
return [...new Set(rows.flatMap((row) => detachedWorkflowExecutionId(row) ?? []))]
2034+
const notRunning = new Set(settled.map((row) => row.executionId))
2035+
return detached.filter((executionId) => !notRunning.has(executionId))
20032036
}
20042037
)
20052038
}

‎apps/sim/lib/mothership/request/tools/workflow-client-settlement.integration.ts‎

Lines changed: 151 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,14 @@ import { eq, inArray } from 'drizzle-orm'
3333
import { closeRedisConnection, getRedisClient } from '@/lib/core/config/redis'
3434
import { SIM_TOOL_EXECUTION_VERSION } from '@/lib/mothership/async-runs/lifecycle'
3535
import {
36+
claimToolExecution,
3637
claimWorkflowToolExecution,
3738
completeAsyncToolCall,
3839
detachAsyncToolCall,
3940
getDetachedWorkflowExecutions,
41+
prepareWorkbenchAccess,
4042
settleClientWorkflowToolExecution,
43+
upsertAsyncToolCall,
4144
} from '@/lib/mothership/async-runs/repository'
4245
import { getToolConfirmation } from '@/lib/mothership/persistence/tool-confirm'
4346
import { waitForWorkflowToolCompletion } from '@/lib/mothership/request/tools/client'
@@ -46,6 +49,7 @@ import {
4649
reportQueuedClientWorkflowTool,
4750
reportSettledClientWorkflowTool,
4851
} from '@/lib/mothership/request/tools/workflow-client-settlement'
52+
import { chatSandboxSessionKey } from '@/lib/mothership/tools/sandbox-session-key'
4953
import { PauseResumeManager } from '@/lib/workflows/executor/human-in-the-loop-manager'
5054

5155
/** Longer than the waiter's durable poll, far shorter than the hour it used to park for. */
@@ -532,40 +536,175 @@ describe.runIf(Boolean(redisUrl))('settled client-claimed workflow tools', () =>
532536
})
533537

534538
it("names the runs this run's calls started and left running, for a Stop to cancel", async () => {
535-
const insertCall = async (executableName: string, args: unknown, result: unknown) =>
536-
db.insert(copilotAsyncToolCalls).values({
539+
const insertCall = async (executableName: string, args: unknown, result: unknown) => {
540+
const toolCallId = generateId()
541+
await db.insert(copilotAsyncToolCalls).values({
537542
runId,
538-
toolCallId: generateId(),
543+
toolCallId,
539544
toolName: executableName,
540545
executableName,
541546
args: args as Record<string, unknown>,
542547
result: result as Record<string, unknown>,
543548
status: 'completed',
544549
})
545-
const detachedCli = generateId()
546-
const detachedTool = generateId()
550+
return toolCallId
551+
}
547552
const record = (runIdValue: string, status: string) =>
548553
JSON.stringify({ runId: runIdValue, workflowId, status })
549-
await insertCall(
550-
'sim_cli',
551-
{ request: { invocation: { kind: 'cli', argv: ['workflows', 'run', workflowId] } } },
552-
{ exitCode: 4, stdout: record(detachedCli, 'running'), stderr: '' }
554+
const detachedCli = generateId()
555+
await executionLog(
556+
await insertCall(
557+
'sim_cli',
558+
{ request: { invocation: { kind: 'cli', argv: ['workflows', 'run', workflowId] } } },
559+
{ exitCode: 4, stdout: record(detachedCli, 'running'), stderr: '' }
560+
),
561+
detachedCli,
562+
'running'
553563
)
554564
// A wait on a run proves nothing about who started it.
555565
await insertCall(
556566
'sim_cli',
557567
{ request: { invocation: { kind: 'cli', argv: ['workflows', 'runs', 'wait', 'other'] } } },
558568
{ exitCode: 4, stdout: record(generateId(), 'running'), stderr: '' }
559569
)
570+
const detachedTool = generateId()
571+
await executionLog(
572+
await insertCall(
573+
'run_workflow',
574+
{ workflowId },
575+
{ status: 'running', executionId: detachedTool, workflowId }
576+
),
577+
detachedTool,
578+
'running'
579+
)
580+
// Answered running but not yet logged: it may still be starting, and a Stop must reach it.
581+
const startingTool = generateId()
560582
await insertCall(
561-
'run_workflow',
583+
'run_from_block',
562584
{ workflowId },
563-
{ status: 'running', executionId: detachedTool, workflowId }
585+
{ status: 'running', executionId: startingTool, workflowId }
564586
)
565587
await insertCall('run_block', { workflowId }, { success: true, executionId: generateId() })
566588

567589
expect((await getDetachedWorkflowExecutions(streamId, userId)).sort()).toEqual(
568-
[detachedCli, detachedTool].sort()
590+
[detachedCli, detachedTool, startingTool].sort()
569591
)
570592
})
593+
594+
it.each(['pending', 'completed', 'failed', 'cancelled'] as const)(
595+
'leaves a detached run whose log is %s for a Stop to skip',
596+
async (logStatus) => {
597+
const detachedCli = generateId()
598+
const detachedTool = generateId()
599+
const cliCall = generateId()
600+
const toolCall = generateId()
601+
await db.insert(copilotAsyncToolCalls).values([
602+
{
603+
runId,
604+
toolCallId: cliCall,
605+
toolName: 'sim_cli',
606+
executableName: 'sim_cli',
607+
args: {
608+
request: { invocation: { kind: 'cli', argv: ['workflows', 'run', workflowId] } },
609+
},
610+
result: {
611+
exitCode: 4,
612+
stdout: JSON.stringify({ runId: detachedCli, workflowId, status: 'running' }),
613+
stderr: '',
614+
},
615+
status: 'completed',
616+
},
617+
{
618+
runId,
619+
toolCallId: toolCall,
620+
toolName: 'run_workflow',
621+
executableName: 'run_workflow',
622+
args: { workflowId },
623+
result: { status: 'running', executionId: detachedTool, workflowId },
624+
status: 'completed',
625+
},
626+
])
627+
// The run went on past its call and then settled, or paused at a human approval: a paused
628+
// run's resume subscribes under the same id, so a cancel flag would fail it.
629+
await executionLog(cliCall, detachedCli, logStatus)
630+
await executionLog(toolCall, detachedTool, logStatus)
631+
632+
const cancellable = await getDetachedWorkflowExecutions(streamId, userId)
633+
expect(cancellable).not.toContain(detachedCli)
634+
expect(cancellable).not.toContain(detachedTool)
635+
}
636+
)
637+
638+
/**
639+
* An earlier turn of a new chat whose `earlierTool` call is still executing under a live lease,
640+
* as a workflow tool answered still running is while its run goes on, and a later turn whose
641+
* own Sim call asks for the chat's workbench.
642+
*/
643+
async function laterTurnWorkbenchAccess(earlierTool: string) {
644+
const turnChatId = generateId()
645+
await db.insert(copilotChats).values({
646+
id: turnChatId,
647+
userId,
648+
workspaceId,
649+
type: 'mothership',
650+
conversationId: generateId(),
651+
})
652+
try {
653+
const turn = async (startedAt: Date, toolName: string) => {
654+
const turnRunId = generateId()
655+
await db.insert(copilotRuns).values({
656+
id: turnRunId,
657+
executionId: generateId(),
658+
chatId: turnChatId,
659+
userId,
660+
workspaceId,
661+
streamId: generateId(),
662+
toolExecutionVersion: SIM_TOOL_EXECUTION_VERSION,
663+
status: 'active',
664+
startedAt,
665+
requestContext: { source: 'headless_lifecycle' },
666+
})
667+
const owner = {
668+
toolCallId: generateId(),
669+
runId: turnRunId,
670+
userId,
671+
ownerToken: generateId(),
672+
}
673+
await upsertAsyncToolCall({ runId: turnRunId, toolCallId: owner.toolCallId, toolName })
674+
expect(await claimToolExecution(owner)).toMatchObject({ outcome: 'claimed' })
675+
return owner
676+
}
677+
const earlier = await turn(new Date(Date.now() - 60_000), earlierTool)
678+
await completeAsyncToolCall({
679+
toolCallId: earlier.toolCallId,
680+
status: 'completed',
681+
result: { status: 'running', executionId: generateId(), workflowId },
682+
})
683+
// The earlier turn answered and ended; its call's execution still holds the run.
684+
await db
685+
.update(copilotRuns)
686+
.set({ status: 'complete', toolAdmissionClosedAt: new Date() })
687+
.where(eq(copilotRuns.id, earlier.runId))
688+
const later = await turn(new Date(), 'function_execute')
689+
return await prepareWorkbenchAccess({
690+
...later,
691+
sessionKey: chatSandboxSessionKey(turnChatId),
692+
})
693+
} finally {
694+
await db.delete(copilotChats).where(eq(copilotChats.id, turnChatId))
695+
}
696+
}
697+
698+
it("opens the workbench to a later turn while an earlier turn's workflow tool run goes on", async () => {
699+
expect(await laterTurnWorkbenchAccess('run_workflow')).toEqual({
700+
handlersPending: false,
701+
processes: [],
702+
})
703+
})
704+
705+
it("keeps the workbench closed while an earlier turn's other tool is still executing", async () => {
706+
expect(await laterTurnWorkbenchAccess('function_execute')).toMatchObject({
707+
handlersPending: true,
708+
})
709+
})
571710
})

‎apps/sim/lib/mothership/tools/client-executed-tools.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { isDesktopToolCall } from '@/lib/mothership/tools/desktop-tools'
22

3-
const WORKFLOW_TOOL_NAMES = new Set<string>([
3+
/** The tools that run a workflow, in the browser or on the server. */
4+
export const WORKFLOW_TOOL_NAMES = [
45
'run_workflow',
56
'run_workflow_until_block',
67
'run_block',
78
'run_from_block',
8-
])
9+
] as const
910

1011
export function isWorkflowToolName(name: string): boolean {
11-
return WORKFLOW_TOOL_NAMES.has(name)
12+
return (WORKFLOW_TOOL_NAMES as readonly string[]).includes(name)
1213
}
1314

1415
/**

0 commit comments

Comments
 (0)