From 7fc96da084dc0c2d54080774c89114ee4a6bca44 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 17:29:25 -0700 Subject: [PATCH 1/6] fix(mothership): run same-workflow workflow tools in order instead of failing as busy When the agent fans out run_block/run_workflow/run_from_block/run_workflow_until_block calls on one workflow in a single step, the browser executor rejected every call after the first with WORKFLOW_EXECUTION_BUSY, because the editor keeps one visible execution per workflow. The agent then spent a round re-issuing them one by one. That was about 46% of run_block failures. Calls behind a run tool already running that workflow in this tab now wait in arrival order and launch when it releases. Stop cancels waiting calls without launching them, a chat recovery treats a waiting call as owned by this tab, and a run the agent did not start (the user's manual run) is still reported as busy. The wait needs no timeout of its own: a call nobody claims within the server's pickup grace is run by the server, and the late launch here gets the execute route's benign 409. --- .../tools/client/run-tool-execution.test.ts | 146 +++++++++++++++++- .../tools/client/run-tool-execution.ts | 120 +++++++++++--- 2 files changed, 247 insertions(+), 19 deletions(-) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts index 591231efdf5..5d798400e30 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts @@ -12,7 +12,7 @@ import { workflowRegistryStoreMock, workflowRegistryStoreMockFns, } from '@sim/testing/mocks/workflow-registry-store.mock' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest' import type { WorkflowExecutionOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils' import { useExecutionStore } from '@/stores/execution/store' @@ -668,4 +668,148 @@ describe('run tool execution cancellation', () => { expect(fetchMock).not.toHaveBeenCalled() expect(setIsExecuting).toHaveBeenCalledWith('wf-1', false) }) + + describe('same-workflow calls issued while an agent run is in flight', () => { + /** Holds every client execution open until the test settles it by tool call id. */ + function holdExecutions() { + const settle = new Map void>() + onTestFinished(() => { + executeWorkflowWithFullLogging.mockReset() + }) + executeWorkflowWithFullLogging.mockImplementation( + (options: WorkflowExecutionOptions) => + new Promise((resolve, reject) => { + settle.set(String(options.copilotToolCallId), resolve) + requireAbortSignal(options).addEventListener( + 'abort', + () => reject(new DOMException('Aborted', 'AbortError')), + { once: true } + ) + }) + ) + const launchedToolCallIds = () => + executeWorkflowWithFullLogging.mock.calls.map( + ([options]: [WorkflowExecutionOptions]) => options.copilotToolCallId + ) + const finish = (toolCallId: string) => { + const resolve = settle.get(toolCallId) + if (!resolve) throw new Error(`${toolCallId} was never launched`) + resolve({ success: true }) + } + return { finish, launchedToolCallIds } + } + + function confirmBodies(): Array> { + return vi + .mocked(fetch) + .mock.calls.filter(([url]) => url === '/api/copilot/confirm') + .map(([, init]) => JSON.parse(String(init?.body))) + } + + it('runs them one at a time in the order they were issued instead of rejecting them as busy', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + executeRunToolOnClient('tool-c', 'run_workflow', { workflowId: 'wf-1' }) + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a'])) + + finish('tool-a') + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + finish('tool-b') + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b', 'tool-c'])) + finish('tool-c') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + + expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + ['tool-a', 'success'], + ['tool-b', 'success'], + ['tool-c', 'success'], + ]) + }) + + it('keeps a call issued while the previous run is releasing behind the calls already waiting', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + let issuedLate = false + setIsExecuting.mockImplementation((workflowId: string, running: boolean) => { + if (workflowId === 'wf-1' && !running && !issuedLate) { + issuedLate = true + executeRunToolOnClient('tool-late', 'run_block', { workflowId: 'wf-1', blockId: 'b' }) + } + }) + onTestFinished(() => { + setIsExecuting.mockReset() + }) + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a'])) + + finish('tool-a') + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + finish('tool-b') + await vi.waitFor(() => + expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b', 'tool-late']) + ) + finish('tool-late') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + }) + + it('does not hold up a run of a different workflow', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + + executeRunToolOnClient('tool-a', 'run_workflow', { workflowId: 'wf-1' }) + executeRunToolOnClient('tool-other', 'run_workflow', { workflowId: 'wf-2' }) + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-other'])) + + finish('tool-a') + finish('tool-other') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-2')).toBe(false)) + }) + + it('cancels a stopped call that is still waiting without ever launching it', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + executeRunToolOnClient('tool-other-chat', 'run_block', { + workflowId: 'wf-1', + blockId: 'block-3', + }) + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a'])) + + stopRunToolExecutions(new Set(['tool-a', 'tool-b'])) + + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-other-chat'])) + finish('tool-other-chat') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + + const statusByToolCall = new Map( + confirmBodies().map((body) => [body.toolCallId, body.status]) + ) + expect(statusByToolCall.get('tool-a')).toBe('cancelled') + expect(statusByToolCall.get('tool-b')).toBe('cancelled') + expect(statusByToolCall.get('tool-other-chat')).toBe('success') + }) + + it('treats a waiting call as already owned by this tab when the chat recovers it', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a'])) + + await expect(bindRunToolToExecution('tool-b', 'wf-1')).resolves.toBe(true) + + finish('tool-a') + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + finish('tool-b') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + ['tool-a', 'success'], + ['tool-b', 'success'], + ]) + }) + }) }) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts index bf459388730..4c67feb209e 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts @@ -51,7 +51,14 @@ interface ActiveRunTool { execution?: { id: string; controller: AbortController } } +/** A run tool waiting for its workflow's in-flight run tool; null means it was stopped first. */ +interface WaitingRunTool { + toolCallId: string + admit: (run: ActiveRunTool | null) => void +} + const activeRunToolByWorkflowId = new Map() +const waitingRunToolsByWorkflowId = new Map() const manuallyStoppedToolCallIds = new Set() type RunToolReleaseListener = (workflowId: string) => void const runToolReleaseListeners = new Set() @@ -73,6 +80,72 @@ interface PendingCompletionReport { clearExecutionPointerAfterReport?: boolean } +/** + * Takes the workflow's run-tool slot, or waits in arrival order for the run tool + * holding it. + * + * The editor keeps one visible execution per workflow (running flag, current + * execution id, active blocks, terminal pointer), so two runs of one workflow in + * a tab would clobber each other. Calls the agent fans out in one step therefore + * run one after another rather than failing. The wait needs no timeout of its + * own: a tool call nobody claims within the server's pickup grace is run by the + * server, and the late launch here then gets the execute route's benign 409. + */ +function acquireRunToolSlot( + workflowId: string, + toolCallId: string +): ActiveRunTool | Promise { + if (!activeRunToolByWorkflowId.has(workflowId) && !waitingRunToolsByWorkflowId.has(workflowId)) { + const run: ActiveRunTool = { toolCallId } + activeRunToolByWorkflowId.set(workflowId, run) + return run + } + return new Promise((admit) => { + const waiting = waitingRunToolsByWorkflowId.get(workflowId) ?? [] + waiting.push({ toolCallId, admit }) + waitingRunToolsByWorkflowId.set(workflowId, waiting) + }) +} + +/** + * Gives a free slot to the oldest waiter. Called synchronously with the release + * that freed it, so a call issued later cannot take the slot first. + */ +function admitNextRunTool(workflowId: string): void { + if (activeRunToolByWorkflowId.has(workflowId)) return + const waiting = waitingRunToolsByWorkflowId.get(workflowId) + const next = waiting?.shift() + if (waiting?.length === 0) waitingRunToolsByWorkflowId.delete(workflowId) + if (!next) return + const nextRun: ActiveRunTool = { toolCallId: next.toolCallId } + activeRunToolByWorkflowId.set(workflowId, nextRun) + next.admit(nextRun) +} + +function releaseRunToolSlot(workflowId: string, run: ActiveRunTool): void { + if (activeRunToolByWorkflowId.get(workflowId) !== run) return + activeRunToolByWorkflowId.delete(workflowId) + admitNextRunTool(workflowId) +} + +function isRunToolWaiting(workflowId: string, toolCallId: string): boolean { + return ( + waitingRunToolsByWorkflowId.get(workflowId)?.some((w) => w.toolCallId === toolCallId) ?? false + ) +} + +function dropWaitingRunTools(toolCallIds: ReadonlySet): void { + for (const [workflowId, waiting] of waitingRunToolsByWorkflowId) { + const kept = waiting.filter((w) => !toolCallIds.has(w.toolCallId)) + if (kept.length === waiting.length) continue + if (kept.length === 0) waitingRunToolsByWorkflowId.delete(workflowId) + else waitingRunToolsByWorkflowId.set(workflowId, kept) + for (const dropped of waiting) { + if (toolCallIds.has(dropped.toolCallId)) dropped.admit(null) + } + } +} + function resolveWorkflowInput(params: Record): unknown { if (Object.hasOwn(params, 'workflow_input')) { return params.workflow_input @@ -276,7 +349,7 @@ export async function bindRunToolToExecution( workflowId: string ): Promise { const existingToolCallId = activeRunToolByWorkflowId.get(workflowId)?.toolCallId - if (existingToolCallId === toolCallId) { + if (existingToolCallId === toolCallId || isRunToolWaiting(workflowId, toolCallId)) { logger.info('[RunTool] Recovery skipped: run tool is already active in this tab', { workflowId, toolCallId, @@ -434,6 +507,7 @@ export function stopRunToolForExecution(workflowId: string, executionId: string /** Cancels exact tool-owned executions; a workflow's current UI pointer is not ownership. */ export function stopRunToolExecutions(toolCallIds: ReadonlySet): void { + dropWaitingRunTools(toolCallIds) for (const [workflowId, active] of activeRunToolByWorkflowId) { const { toolCallId, execution } = active if (!toolCallIds.has(toolCallId) || !execution || manuallyStoppedToolCallIds.has(toolCallId)) { @@ -501,32 +575,41 @@ async function doExecuteRunTool( return } - const existingToolCallId = activeRunToolByWorkflowId.get(targetWorkflowId) - if (existingToolCallId) { - logger.warn('[RunTool] Execution prevented: another run tool is already active', { + const slot = acquireRunToolSlot(targetWorkflowId, toolCallId) + let activeRun: ActiveRunTool + if (slot instanceof Promise) { + logger.info('[RunTool] Waiting for the run tool already running this workflow', { toolCallId, toolName, - existingToolCallId: existingToolCallId.toolCallId, + workflowId: targetWorkflowId, + activeToolCallId: activeRunToolByWorkflowId.get(targetWorkflowId)?.toolCallId, }) - await reportCompletion( - toolCallId, - MothershipStreamV1ToolOutcome.error, - WORKFLOW_EXECUTION_BUSY.message, - { code: WORKFLOW_EXECUTION_BUSY.code } - ) - return + const admitted = await slot + if (!admitted) { + logger.info('[RunTool] Stopped before its turn to run', { toolCallId, toolName }) + await reportCompletion( + toolCallId, + MothershipStreamV1ToolOutcome.cancelled, + getWorkflowToolCompletionMessage(MothershipStreamV1ToolOutcome.cancelled), + { reason: 'user_cancelled', cancelledByUser: true } + ) + return + } + activeRun = admitted + } else { + activeRun = slot } setActiveWorkflow(targetWorkflowId) - const activeRun: ActiveRunTool = { toolCallId } - activeRunToolByWorkflowId.set(targetWorkflowId, activeRun) const { getWorkflowExecution, setIsExecuting } = useExecutionStore.getState() const { isExecuting } = getWorkflowExecution(targetWorkflowId) + // The previous run tool has already released the workflow, so a run here is + // one no run tool drives: the user's, or a dropped stream being re-attached. if (isExecuting) { logger.warn('[RunTool] Execution prevented: already executing', { toolCallId, toolName }) - activeRunToolByWorkflowId.delete(targetWorkflowId) + releaseRunToolSlot(targetWorkflowId, activeRun) await reportCompletion( toolCallId, MothershipStreamV1ToolOutcome.error, @@ -551,9 +634,7 @@ async function doExecuteRunTool( triggerBlockId ) } finally { - if (activeRunToolByWorkflowId.get(targetWorkflowId) === activeRun) { - activeRunToolByWorkflowId.delete(targetWorkflowId) - } + releaseRunToolSlot(targetWorkflowId, activeRun) } return } @@ -801,5 +882,8 @@ async function doExecuteRunTool( if (streamInterrupted && ownsRegistration) { notifyRunToolReleased(targetWorkflowId) } + if (ownsRegistration) { + admitNextRunTool(targetWorkflowId) + } } } From c037c034ae94767a774ee614b717b1647e0afae0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 18:20:59 -0700 Subject: [PATCH 2/6] fix(mothership): keep a workflow for a run whose stream dropped until it settles A run tool whose execute stream is interrupted leaves the run executing server-side and keeps its terminal pointer so the editor can re-attach. Admitting the next waiting call right away overwrote that pointer and claimed the workflow, so reconnect skipped it and the still-running execution lost its live output and the editor's Stop. The interrupted execution now holds the workflow until the server reports it settled and any reconnect has drained it. The hold is not run-tool ownership, so reconnect can still claim the pointer. Waiting calls start after it; one still waiting past the server's pickup grace is run by the server, and its late start here gets the benign 409. --- .../tools/client/run-tool-execution.test.ts | 36 ++++++++++ .../tools/client/run-tool-execution.ts | 69 +++++++++++++++++-- 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts index 5d798400e30..54306c8a149 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts @@ -13,6 +13,7 @@ import { workflowRegistryStoreMockFns, } from '@sim/testing/mocks/workflow-registry-store.mock' import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest' +import { getWorkflowExecutionContract } from '@/lib/api/contracts/workflows' import type { WorkflowExecutionOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils' import { useExecutionStore } from '@/stores/execution/store' @@ -793,6 +794,41 @@ describe('run tool execution cancellation', () => { expect(statusByToolCall.get('tool-other-chat')).toBe('success') }) + it('keeps the workflow for a run whose stream dropped until that run settles on the server', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + let serverStatus = 'running' + mockRequestJson.mockImplementation(async (contract: unknown) => + contract === getWorkflowExecutionContract ? { status: serverStatus } : { success: true } + ) + executeWorkflowWithFullLogging.mockRejectedValueOnce( + new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-interrupted') + ) + const released = vi.fn() + onTestFinished(subscribeToRunToolRelease(released)) + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + await vi.waitFor(() => expect(released).toHaveBeenCalledWith('wf-1')) + await vi.waitFor(() => + expect( + mockRequestJson.mock.calls.filter(([c]) => c === getWorkflowExecutionContract).length + ).toBeGreaterThan(2) + ) + + expect(launchedToolCallIds()).toEqual(['tool-a']) + expect(saveExecutionPointer).toHaveBeenCalledTimes(1) + expect(isRunToolActiveForWorkflow('wf-1')).toBe(false) + + serverStatus = 'completed' + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + finish('tool-b') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + ['tool-a', 'background'], + ['tool-b', 'success'], + ]) + }) + it('treats a waiting call as already owned by this tab when the chat recovers it', async () => { const { finish, launchedToolCallIds } = holdExecutions() diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts index 4c67feb209e..008aa30a51e 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts @@ -1,9 +1,15 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' +import { sleep } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' import { isPlainRecord } from '@sim/utils/object' +import { backoffWithJitter } from '@sim/utils/retry' +import { ApiClientError } from '@/lib/api/client/errors' import { requestJson } from '@/lib/api/client/request' -import { cancelWorkflowExecutionContract } from '@/lib/api/contracts/workflows' +import { + cancelWorkflowExecutionContract, + getWorkflowExecutionContract, +} from '@/lib/api/contracts/workflows' import { ASYNC_TOOL_CONFIRMATION_STATUS, type AsyncConfirmationStatus, @@ -59,6 +65,9 @@ interface WaitingRunTool { const activeRunToolByWorkflowId = new Map() const waitingRunToolsByWorkflowId = new Map() +/** Executions whose run tool lost its stream while the server kept running them. */ +const interruptedExecutionIdByWorkflowId = new Map() +const INTERRUPTED_RUN_POLL_MAX_MS = 15_000 const manuallyStoppedToolCallIds = new Set() type RunToolReleaseListener = (workflowId: string) => void const runToolReleaseListeners = new Set() @@ -95,7 +104,11 @@ function acquireRunToolSlot( workflowId: string, toolCallId: string ): ActiveRunTool | Promise { - if (!activeRunToolByWorkflowId.has(workflowId) && !waitingRunToolsByWorkflowId.has(workflowId)) { + if ( + !activeRunToolByWorkflowId.has(workflowId) && + !waitingRunToolsByWorkflowId.has(workflowId) && + !interruptedExecutionIdByWorkflowId.has(workflowId) + ) { const run: ActiveRunTool = { toolCallId } activeRunToolByWorkflowId.set(workflowId, run) return run @@ -113,6 +126,7 @@ function acquireRunToolSlot( */ function admitNextRunTool(workflowId: string): void { if (activeRunToolByWorkflowId.has(workflowId)) return + if (interruptedExecutionIdByWorkflowId.has(workflowId)) return const waiting = waitingRunToolsByWorkflowId.get(workflowId) const next = waiting?.shift() if (waiting?.length === 0) waitingRunToolsByWorkflowId.delete(workflowId) @@ -128,6 +142,50 @@ function releaseRunToolSlot(workflowId: string, run: ActiveRunTool): void { admitNextRunTool(workflowId) } +/** + * Holds the workflow for an execution whose run tool lost its stream while the + * server kept running it, until that execution settles. + * + * The interrupted run keeps its terminal pointer so the editor's reconnect can + * re-attach to it. A waiting call admitted now would overwrite that pointer and + * claim the workflow, leaving the still-running execution without live output or + * the editor's Stop. The hold is not run-tool ownership, so reconnect may claim it. + */ +async function holdForInterruptedExecution(workflowId: string, executionId: string) { + interruptedExecutionIdByWorkflowId.set(workflowId, executionId) + for (let attempt = 1; ; attempt++) { + await sleep(backoffWithJitter(attempt, null, { maxMs: INTERRUPTED_RUN_POLL_MAX_MS })) + if (interruptedExecutionIdByWorkflowId.get(workflowId) !== executionId) return + if (await isExecutionStillRunning(workflowId, executionId)) continue + // A reconnect that re-attached clears this once it has drained the run's last events. + if (useExecutionStore.getState().getCurrentExecutionId(workflowId) === executionId) continue + break + } + interruptedExecutionIdByWorkflowId.delete(workflowId) + admitNextRunTool(workflowId) +} + +async function isExecutionStillRunning(workflowId: string, executionId: string) { + try { + const { status } = await requestJson(getWorkflowExecutionContract, { + params: { id: workflowId, executionId }, + query: {}, + }) + return status === 'queued' || status === 'pending' || status === 'running' + } catch (error) { + // Gone or not ours: nothing left to wait for. Anything else may be transient. + if (error instanceof ApiClientError && (error.status === 403 || error.status === 404)) { + return false + } + logger.warn('[RunTool] Could not check an interrupted execution; still holding', { + workflowId, + executionId, + error: toError(error).message, + }) + return true + } +} + function isRunToolWaiting(workflowId: string, toolCallId: string): boolean { return ( waitingRunToolsByWorkflowId.get(workflowId)?.some((w) => w.toolCallId === toolCallId) ?? false @@ -729,7 +787,7 @@ async function doExecuteRunTool( }) let leaveExecutionRecoverable = false - let streamInterrupted = false + let interruptedExecutionId: string | undefined try { const result = await executeWorkflowWithFullLogging({ @@ -810,7 +868,7 @@ async function doExecuteRunTool( const msg = toError(err).message if (err instanceof SSEEventHandlerError || err instanceof SSEStreamInterruptedError) { leaveExecutionRecoverable = true - streamInterrupted = true + interruptedExecutionId = err.executionId ?? executionId logger.warn( '[RunTool] Execution stream interrupted; leaving workflow execution in background', { @@ -879,7 +937,8 @@ async function doExecuteRunTool( setIsExecuting(targetWorkflowId, false) setActiveBlocks(targetWorkflowId, new Set()) } - if (streamInterrupted && ownsRegistration) { + if (interruptedExecutionId && ownsRegistration) { + void holdForInterruptedExecution(targetWorkflowId, interruptedExecutionId) notifyRunToolReleased(targetWorkflowId) } if (ownsRegistration) { From fd8ffd0673dd63b507b1c1f3561b13c0b194a880 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 18:29:24 -0700 Subject: [PATCH 3/6] fix(mothership): release a dropped run's hold once it settles, even after an abandoned reconnect The hold waited for the execution store to stop showing the interrupted execution as current. Navigating away cancels the editor's reconnect without clearing that, so a settled run kept later calls waiting until the server's pickup fallback ran them. The hold now waits for the server to settle the execution and for no reconnect stream to be open for it, then clears the stale visible state it left behind before admitting the next call. --- apps/sim/hooks/use-execution-stream.ts | 5 +++ .../tools/client/run-tool-execution.test.ts | 39 +++++++++++++++++++ .../tools/client/run-tool-execution.ts | 13 ++++++- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/apps/sim/hooks/use-execution-stream.ts b/apps/sim/hooks/use-execution-stream.ts index 024b4174a84..89efdd7100e 100644 --- a/apps/sim/hooks/use-execution-stream.ts +++ b/apps/sim/hooks/use-execution-stream.ts @@ -285,6 +285,11 @@ function reconnectStreamKey(workflowId: string, executionId: string): string { return `${workflowId}:reconnect:${executionId}` } +/** Whether a reconnect stream for this execution is currently open in this tab. */ +export function isReconnectStreamOpen(workflowId: string, executionId: string): boolean { + return sharedAbortControllers.has(reconnectStreamKey(workflowId, executionId)) +} + function abortStream(key: string): void { const controller = sharedAbortControllers.get(key) if (!controller) return diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts index 54306c8a149..e97e942bbc5 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts @@ -19,11 +19,13 @@ import { useExecutionStore } from '@/stores/execution/store' const { executeWorkflowWithFullLogging, + isReconnectStreamOpen, MockExecutionStreamHttpError, MockSSEEventHandlerError, MockSSEStreamInterruptedError, } = vi.hoisted(() => ({ executeWorkflowWithFullLogging: vi.fn(), + isReconnectStreamOpen: vi.fn(() => false), MockExecutionStreamHttpError: class ExecutionStreamHttpError extends Error { constructor( message: string, @@ -79,6 +81,7 @@ function requireAbortSignal(options: WorkflowExecutionOptions): AbortSignal { vi.mock('@/hooks/use-execution-stream', () => ({ ExecutionStreamHttpError: MockExecutionStreamHttpError, isExecutionStreamHttpError: (error: unknown) => error instanceof MockExecutionStreamHttpError, + isReconnectStreamOpen, SSEEventHandlerError: MockSSEEventHandlerError, SSEStreamInterruptedError: MockSSEStreamInterruptedError, })) @@ -829,6 +832,42 @@ describe('run tool execution cancellation', () => { ]) }) + it('lets waiting calls run once the dropped run settles, even if its reconnect was abandoned', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + let serverStatus = 'running' + mockRequestJson.mockImplementation(async (contract: unknown) => + contract === getWorkflowExecutionContract ? { status: serverStatus } : { success: true } + ) + let reconnectOpen = true + isReconnectStreamOpen.mockImplementation(() => reconnectOpen) + executeWorkflowWithFullLogging.mockRejectedValueOnce( + new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-abandoned') + ) + const released = vi.fn() + onTestFinished(subscribeToRunToolRelease(released)) + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + await vi.waitFor(() => expect(released).toHaveBeenCalledWith('wf-1')) + // The editor's reconnect re-attaches, then the user navigates away and it is + // cancelled, leaving the execution marked current in the store. + setCurrentExecutionId('wf-1', 'exec-abandoned') + serverStatus = 'completed' + await vi.waitFor(() => + expect(isReconnectStreamOpen).toHaveBeenCalledWith('wf-1', 'exec-abandoned') + ) + expect(launchedToolCallIds()).toEqual(['tool-a']) + + reconnectOpen = false + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + finish('tool-b') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + ['tool-a', 'background'], + ['tool-b', 'success'], + ]) + }) + it('treats a waiting call as already owned by this tab when the chat recovers it', async () => { const { finish, launchedToolCallIds } = holdExecutions() diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts index 008aa30a51e..91ee9699453 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts @@ -38,6 +38,7 @@ import { import { executeWorkflowWithFullLogging } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils' import { isExecutionStreamHttpError, + isReconnectStreamOpen, SSEEventHandlerError, SSEStreamInterruptedError, } from '@/hooks/use-execution-stream' @@ -157,11 +158,19 @@ async function holdForInterruptedExecution(workflowId: string, executionId: stri await sleep(backoffWithJitter(attempt, null, { maxMs: INTERRUPTED_RUN_POLL_MAX_MS })) if (interruptedExecutionIdByWorkflowId.get(workflowId) !== executionId) return if (await isExecutionStillRunning(workflowId, executionId)) continue - // A reconnect that re-attached clears this once it has drained the run's last events. - if (useExecutionStore.getState().getCurrentExecutionId(workflowId) === executionId) continue + // A live reconnect is still draining the run's last events into the editor. + if (isReconnectStreamOpen(workflowId, executionId)) continue break } interruptedExecutionIdByWorkflowId.delete(workflowId) + // A reconnect cancelled by navigating away leaves the settled run looking current; + // left as is, the next run would read it as a run someone else is driving. + const executionState = useExecutionStore.getState() + if (executionState.getCurrentExecutionId(workflowId) === executionId) { + executionState.setCurrentExecutionId(workflowId, null) + executionState.setIsExecuting(workflowId, false) + executionState.setActiveBlocks(workflowId, new Set()) + } admitNextRunTool(workflowId) } From 607b58c1c6266eb019fe7f30e32389c785a48eb2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 18:37:34 -0700 Subject: [PATCH 4/6] fix(mothership): re-send an undelivered workflow tool completion while another run owns the workflow A run whose completion report failed keeps it in session storage for recovery to re-send. Recovery refused to touch the call whenever another run tool owned the workflow, which the queue now makes common, so the stored report was never delivered. Re-sending a stored report needs nothing from the workflow; recovery now does it first, and only skips the pointer lookup and cleanup that belong to the run that owns the workflow. Also makes the abandoned-reconnect test track the running flag, so it fails without the stale-state clearing it covers. --- .../tools/client/run-tool-execution.test.ts | 43 +++++++++++++++++++ .../tools/client/run-tool-execution.ts | 28 +++++++----- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts index e97e942bbc5..d81ac72b23c 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts @@ -840,6 +840,18 @@ describe('run tool execution cancellation', () => { ) let reconnectOpen = true isReconnectStreamOpen.mockImplementation(() => reconnectOpen) + const running = new Map() + setIsExecuting.mockImplementation((workflowId: string, value: boolean) => { + running.set(workflowId, value) + }) + getWorkflowExecution.mockImplementation((workflowId: string) => ({ + isExecuting: running.get(workflowId) ?? false, + })) + onTestFinished(() => { + setIsExecuting.mockReset() + getWorkflowExecution.mockReset() + getWorkflowExecution.mockImplementation(() => ({ isExecuting: false })) + }) executeWorkflowWithFullLogging.mockRejectedValueOnce( new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-abandoned') ) @@ -852,6 +864,7 @@ describe('run tool execution cancellation', () => { // The editor's reconnect re-attaches, then the user navigates away and it is // cancelled, leaving the execution marked current in the store. setCurrentExecutionId('wf-1', 'exec-abandoned') + setIsExecuting('wf-1', true) serverStatus = 'completed' await vi.waitFor(() => expect(isReconnectStreamOpen).toHaveBeenCalledWith('wf-1', 'exec-abandoned') @@ -868,6 +881,36 @@ describe('run tool execution cancellation', () => { ]) }) + it('re-sends an undelivered completion while a later call owns the workflow', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + let confirmsFailFor: string | null = 'tool-a' + vi.stubGlobal( + 'fetch', + vi.fn(async (_url: string, init?: RequestInit) => + confirmsFailFor && String(init?.body).includes(`"toolCallId":"${confirmsFailFor}"`) + ? { ok: false, status: 500 } + : { ok: true } + ) + ) + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a'])) + const firstExecutionId = executeWorkflowWithFullLogging.mock.calls[0][0].executionId + finish('tool-a') + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + + confirmsFailFor = null + await expect(bindRunToolToExecution('tool-a', 'wf-1')).resolves.toBe(true) + + const delivered = confirmBodies() + .filter((body) => body.toolCallId === 'tool-a') + .at(-1) + expect(delivered).toMatchObject({ status: 'success', executionId: firstExecutionId }) + finish('tool-b') + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + }) + it('treats a waiting call as already owned by this tab when the chat recovers it', async () => { const { finish, launchedToolCallIds } = holdExecutions() diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts index 91ee9699453..ba143e44ba0 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts @@ -423,18 +423,14 @@ export async function bindRunToolToExecution( }) return true } - if (existingToolCallId && existingToolCallId !== toolCallId) { - logger.warn('[RunTool] Recovery skipped: another run tool is already active', { - workflowId, - toolCallId, - existingToolCallId, - }) - return false - } - - const pointer = await loadExecutionPointer(workflowId).catch(() => null) + // Another run owns the workflow's pointer, so it says nothing about this call. + const otherRunOwnsWorkflow = + existingToolCallId !== undefined || interruptedExecutionIdByWorkflowId.has(workflowId) const pendingCompletion = loadPendingCompletionReport(toolCallId) if (pendingCompletion) { + const pointer = otherRunOwnsWorkflow + ? null + : await loadExecutionPointer(workflowId).catch(() => null) const executionId = pendingCompletion.executionId ?? pointer?.executionId logger.info('[RunTool] Recovery re-sending pending completion report', { workflowId, @@ -453,7 +449,7 @@ export async function bindRunToolToExecution( executionId ) clearPendingCompletionReport(toolCallId) - if (pendingCompletion.clearExecutionPointerAfterReport) { + if (pendingCompletion.clearExecutionPointerAfterReport && !otherRunOwnsWorkflow) { await clearExecutionPointer(workflowId) } } catch (error) { @@ -467,6 +463,16 @@ export async function bindRunToolToExecution( return true } + if (otherRunOwnsWorkflow) { + logger.warn('[RunTool] Recovery skipped: another run owns this workflow', { + workflowId, + toolCallId, + existingToolCallId, + }) + return false + } + + const pointer = await loadExecutionPointer(workflowId).catch(() => null) if (!pointer?.executionId) { logger.info('[RunTool] Recovery skipped: no tab-local execution pointer', { workflowId, From 9e1037d55f097f859c934d59554f939070a81324 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 18:52:38 -0700 Subject: [PATCH 5/6] improvement(mothership): keep each workflow's run-tool slot in one record and poll a dropped run only when needed Replaces the three parallel maps (owning run tool, waiters, interrupted execution) with one per-workflow record whose owner is a run tool or an interrupted execution. Acquire, admit and release are its only mutators, and every run tool releases through one path. A dropped run is now checked only while a call is actually waiting behind it, never while the editor's reconnect stream is following it or the tab is hidden, stops fetching once the server reports it settled, and gives up the hold after repeated status failures. Also extracts the visible-execution reset, makes waiter removal a single pass and moves the design notes to the PR description. --- .../tools/client/run-tool-execution.test.ts | 177 +++++---- .../tools/client/run-tool-execution.ts | 340 ++++++++++-------- 2 files changed, 303 insertions(+), 214 deletions(-) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts index d81ac72b23c..9661fc03312 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts @@ -12,6 +12,7 @@ import { workflowRegistryStoreMock, workflowRegistryStoreMockFns, } from '@sim/testing/mocks/workflow-registry-store.mock' +import { sleep } from '@sim/utils/helpers' import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest' import { getWorkflowExecutionContract } from '@/lib/api/contracts/workflows' import type { WorkflowExecutionOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils' @@ -78,6 +79,12 @@ function requireAbortSignal(options: WorkflowExecutionOptions): AbortSignal { return options.abortSignal } +/** Waits for the run tool to launch its client execution and returns the execution id. */ +async function launchedExecutionId(): Promise { + await vi.waitFor(() => expect(executeWorkflowWithFullLogging).toHaveBeenCalled()) + return executeWorkflowWithFullLogging.mock.calls[0][0].executionId +} + vi.mock('@/hooks/use-execution-stream', () => ({ ExecutionStreamHttpError: MockExecutionStreamHttpError, isExecutionStreamHttpError: (error: unknown) => error instanceof MockExecutionStreamHttpError, @@ -184,22 +191,24 @@ describe('run tool execution cancellation', () => { ['transport', new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-1')], ])( 'releases a run whose stream was cut by a %s failure only after giving up ownership', - async (_kind, interruption) => { + async (kind, interruption) => { + // A dropped run keeps its workflow until it settles, so each case gets its own. + const workflowId = `wf-stream-${kind}` const ownedAtRelease: boolean[] = [] - const listener = vi.fn((workflowId: string) => { - ownedAtRelease.push(isRunToolActiveForWorkflow(workflowId)) + const listener = vi.fn((releasedWorkflowId: string) => { + ownedAtRelease.push(isRunToolActiveForWorkflow(releasedWorkflowId)) }) const unsubscribe = subscribeToRunToolRelease(listener) executeWorkflowWithFullLogging.mockRejectedValueOnce(interruption) try { - executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' }) - await vi.waitFor(() => expect(listener).toHaveBeenCalledWith('wf-1')) + executeRunToolOnClient('tool-1', 'run_workflow', { workflowId }) + await vi.waitFor(() => expect(listener).toHaveBeenCalledWith(workflowId)) expect(listener).toHaveBeenCalledTimes(1) expect(ownedAtRelease).toEqual([false]) - expect(setIsExecuting).toHaveBeenCalledWith('wf-1', false) - expect(setCurrentExecutionId).toHaveBeenCalledWith('wf-1', null) + expect(setIsExecuting).toHaveBeenCalledWith(workflowId, false) + expect(setCurrentExecutionId).toHaveBeenCalledWith(workflowId, null) expect(setIsExecuting.mock.invocationCallOrder.at(-1)).toBeLessThan( listener.mock.invocationCallOrder[0] ) @@ -245,7 +254,7 @@ describe('run tool execution cancellation', () => { }) ) executeRunToolOnClient('tool-override', 'run_workflow', { workflowId: 'wf-1' }) - const executionId = executeWorkflowWithFullLogging.mock.calls[0][0].executionId + const executionId = await launchedExecutionId() expect(stopRunToolForExecution('wf-1', 'newer-manual-execution')).toBe(false) expect(mockRequestJson).not.toHaveBeenCalled() getCurrentExecutionId.mockReturnValue('newer-manual-execution') @@ -287,9 +296,7 @@ describe('run tool execution cancellation', () => { useDeployedState: true, }) - await Promise.resolve() - await Promise.resolve() - + await launchedExecutionId() expect(executeWorkflowWithFullLogging).toHaveBeenCalledWith( expect.objectContaining({ workflowId: 'wf-1', @@ -299,7 +306,7 @@ describe('run tool execution cancellation', () => { useDraftState: false, }) ) - const executionId = executeWorkflowWithFullLogging.mock.calls[0][0].executionId + const executionId = await launchedExecutionId() await vi.waitFor(() => { expect(fetch).toHaveBeenCalledWith( '/api/copilot/confirm', @@ -324,8 +331,8 @@ describe('run tool execution cancellation', () => { }) ) const toolCallId = `owned-${outcome}` - executeRunToolOnClient(toolCallId, 'run_workflow', { workflowId: 'wf-1' }) - const executionId = executeWorkflowWithFullLogging.mock.calls[0][0].executionId + executeRunToolOnClient(toolCallId, 'run_workflow', { workflowId: `wf-owned-${outcome}` }) + const executionId = await launchedExecutionId() getCurrentExecutionId.mockReturnValue('new-manual-execution') setCurrentExecutionId.mockClear() setIsExecuting.mockClear() @@ -517,7 +524,7 @@ describe('run tool execution cancellation', () => { }) ) }) - const executionId = executeWorkflowWithFullLogging.mock.calls[0][0].executionId + const executionId = await launchedExecutionId() expect(fetchMock.mock.calls[0][1]?.body).toContain(`"executionId":"${executionId}"`) expect(fetchMock.mock.calls[0][1]?.body).not.toContain('workflow failed') }) @@ -710,6 +717,54 @@ describe('run tool execution cancellation', () => { .map(([, init]) => JSON.parse(String(init?.body))) } + function confirmStatuses(): Array<[unknown, unknown]> { + return confirmBodies().map((body) => [body.toolCallId, body.status]) + } + + function statusFetches(): number { + return mockRequestJson.mock.calls.filter( + ([contract]) => contract === getWorkflowExecutionContract + ).length + } + + async function drain(workflowId = 'wf-1') { + await vi.waitFor(() => expect(isRunToolActiveForWorkflow(workflowId)).toBe(false)) + } + + /** Makes the next launched run lose its stream while the server keeps running it. */ + function interruptFirstRun(executionId: string) { + let serverStatus = 'running' + mockRequestJson.mockImplementation(async (contract: unknown) => + contract === getWorkflowExecutionContract ? { status: serverStatus } : { success: true } + ) + executeWorkflowWithFullLogging.mockRejectedValueOnce( + new MockSSEStreamInterruptedError('Execution stream interrupted', executionId) + ) + const released = vi.fn() + onTestFinished(subscribeToRunToolRelease(released)) + return { + released, + setServerStatus: (status: string) => { + serverStatus = status + }, + } + } + + /** Controls whether the editor holds a reconnect stream open for the interrupted run. */ + function reconnectStream(open: boolean) { + let isOpen = open + isReconnectStreamOpen.mockImplementation(() => isOpen) + onTestFinished(() => { + isReconnectStreamOpen.mockReset() + isReconnectStreamOpen.mockReturnValue(false) + }) + return { + close: () => { + isOpen = false + }, + } + } + it('runs them one at a time in the order they were issued instead of rejecting them as busy', async () => { const { finish, launchedToolCallIds } = holdExecutions() @@ -723,9 +778,9 @@ describe('run tool execution cancellation', () => { finish('tool-b') await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b', 'tool-c'])) finish('tool-c') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + await drain() - expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + expect(confirmStatuses()).toEqual([ ['tool-a', 'success'], ['tool-b', 'success'], ['tool-c', 'success'], @@ -756,7 +811,7 @@ describe('run tool execution cancellation', () => { expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b', 'tool-late']) ) finish('tool-late') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + await drain() }) it('does not hold up a run of a different workflow', async () => { @@ -768,8 +823,8 @@ describe('run tool execution cancellation', () => { finish('tool-a') finish('tool-other') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-2')).toBe(false)) + await drain('wf-1') + await drain('wf-2') }) it('cancels a stopped call that is still waiting without ever launching it', async () => { @@ -787,11 +842,9 @@ describe('run tool execution cancellation', () => { await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-other-chat'])) finish('tool-other-chat') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + await drain() - const statusByToolCall = new Map( - confirmBodies().map((body) => [body.toolCallId, body.status]) - ) + const statusByToolCall = new Map(confirmStatuses()) expect(statusByToolCall.get('tool-a')).toBe('cancelled') expect(statusByToolCall.get('tool-b')).toBe('cancelled') expect(statusByToolCall.get('tool-other-chat')).toBe('success') @@ -799,47 +852,54 @@ describe('run tool execution cancellation', () => { it('keeps the workflow for a run whose stream dropped until that run settles on the server', async () => { const { finish, launchedToolCallIds } = holdExecutions() - let serverStatus = 'running' - mockRequestJson.mockImplementation(async (contract: unknown) => - contract === getWorkflowExecutionContract ? { status: serverStatus } : { success: true } - ) - executeWorkflowWithFullLogging.mockRejectedValueOnce( - new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-interrupted') - ) - const released = vi.fn() - onTestFinished(subscribeToRunToolRelease(released)) + const { released, setServerStatus } = interruptFirstRun('exec-interrupted') executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) await vi.waitFor(() => expect(released).toHaveBeenCalledWith('wf-1')) - await vi.waitFor(() => - expect( - mockRequestJson.mock.calls.filter(([c]) => c === getWorkflowExecutionContract).length - ).toBeGreaterThan(2) - ) + await vi.waitFor(() => expect(statusFetches()).toBeGreaterThan(2)) expect(launchedToolCallIds()).toEqual(['tool-a']) expect(saveExecutionPointer).toHaveBeenCalledTimes(1) expect(isRunToolActiveForWorkflow('wf-1')).toBe(false) - serverStatus = 'completed' + setServerStatus('completed') await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) finish('tool-b') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) - expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + await drain() + expect(confirmStatuses()).toEqual([ ['tool-a', 'background'], ['tool-b', 'success'], ]) }) + it('checks a dropped run only while a call waits on it and no reconnect is following it', async () => { + const { finish, launchedToolCallIds } = holdExecutions() + const { released, setServerStatus } = interruptFirstRun('exec-watched') + const reconnect = reconnectStream(true) + + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) + await vi.waitFor(() => expect(released).toHaveBeenCalledWith('wf-1')) + await sleep(20) + expect(isReconnectStreamOpen).not.toHaveBeenCalled() + + executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) + await vi.waitFor(() => expect(isReconnectStreamOpen.mock.calls.length).toBeGreaterThan(2)) + expect(statusFetches()).toBe(0) + expect(launchedToolCallIds()).toEqual(['tool-a']) + + setServerStatus('completed') + reconnect.close() + await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + expect(statusFetches()).toBe(1) + finish('tool-b') + await drain() + }) + it('lets waiting calls run once the dropped run settles, even if its reconnect was abandoned', async () => { const { finish, launchedToolCallIds } = holdExecutions() - let serverStatus = 'running' - mockRequestJson.mockImplementation(async (contract: unknown) => - contract === getWorkflowExecutionContract ? { status: serverStatus } : { success: true } - ) - let reconnectOpen = true - isReconnectStreamOpen.mockImplementation(() => reconnectOpen) + const { released, setServerStatus } = interruptFirstRun('exec-abandoned') + const reconnect = reconnectStream(true) const running = new Map() setIsExecuting.mockImplementation((workflowId: string, value: boolean) => { running.set(workflowId, value) @@ -852,30 +912,25 @@ describe('run tool execution cancellation', () => { getWorkflowExecution.mockReset() getWorkflowExecution.mockImplementation(() => ({ isExecuting: false })) }) - executeWorkflowWithFullLogging.mockRejectedValueOnce( - new MockSSEStreamInterruptedError('Execution stream interrupted', 'exec-abandoned') - ) - const released = vi.fn() - onTestFinished(subscribeToRunToolRelease(released)) executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) await vi.waitFor(() => expect(released).toHaveBeenCalledWith('wf-1')) // The editor's reconnect re-attaches, then the user navigates away and it is - // cancelled, leaving the execution marked current in the store. + // cancelled, leaving the execution marked current and running in the store. setCurrentExecutionId('wf-1', 'exec-abandoned') setIsExecuting('wf-1', true) - serverStatus = 'completed' + setServerStatus('completed') await vi.waitFor(() => expect(isReconnectStreamOpen).toHaveBeenCalledWith('wf-1', 'exec-abandoned') ) expect(launchedToolCallIds()).toEqual(['tool-a']) - reconnectOpen = false + reconnect.close() await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) finish('tool-b') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) - expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + await drain() + expect(confirmStatuses()).toEqual([ ['tool-a', 'background'], ['tool-b', 'success'], ]) @@ -896,7 +951,7 @@ describe('run tool execution cancellation', () => { executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a'])) - const firstExecutionId = executeWorkflowWithFullLogging.mock.calls[0][0].executionId + const firstExecutionId = await launchedExecutionId() finish('tool-a') await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) @@ -908,7 +963,7 @@ describe('run tool execution cancellation', () => { .at(-1) expect(delivered).toMatchObject({ status: 'success', executionId: firstExecutionId }) finish('tool-b') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + await drain() }) it('treats a waiting call as already owned by this tab when the chat recovers it', async () => { @@ -923,8 +978,8 @@ describe('run tool execution cancellation', () => { finish('tool-a') await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) finish('tool-b') - await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) - expect(confirmBodies().map((body) => [body.toolCallId, body.status])).toEqual([ + await drain() + expect(confirmStatuses()).toEqual([ ['tool-a', 'success'], ['tool-b', 'success'], ]) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts index ba143e44ba0..81e630beb59 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts @@ -58,17 +58,29 @@ interface ActiveRunTool { execution?: { id: string; controller: AbortController } } -/** A run tool waiting for its workflow's in-flight run tool; null means it was stopped first. */ +/** A run tool queued behind the workflow's owner; admitted with null when stopped first. */ interface WaitingRunTool { toolCallId: string admit: (run: ActiveRunTool | null) => void } -const activeRunToolByWorkflowId = new Map() -const waitingRunToolsByWorkflowId = new Map() -/** Executions whose run tool lost its stream while the server kept running them. */ -const interruptedExecutionIdByWorkflowId = new Map() +/** + * Who holds a workflow's single visible run in this tab: a run tool, or an execution + * whose run tool lost its stream while the server kept running it. + */ +type RunSlotOwner = + | { kind: 'runTool'; run: ActiveRunTool } + | { kind: 'interrupted'; executionId: string } + +interface WorkflowRunSlot { + owner: RunSlotOwner | null + waiters: WaitingRunTool[] + watchingInterrupted: boolean +} + +const runSlotsByWorkflowId = new Map() const INTERRUPTED_RUN_POLL_MAX_MS = 15_000 +const INTERRUPTED_RUN_MAX_STATUS_FAILURES = 8 const manuallyStoppedToolCallIds = new Set() type RunToolReleaseListener = (workflowId: string) => void const runToolReleaseListeners = new Set() @@ -90,129 +102,187 @@ interface PendingCompletionReport { clearExecutionPointerAfterReport?: boolean } +function slotFor(workflowId: string): WorkflowRunSlot { + let slot = runSlotsByWorkflowId.get(workflowId) + if (!slot) { + slot = { owner: null, waiters: [], watchingInterrupted: false } + runSlotsByWorkflowId.set(workflowId, slot) + } + return slot +} + +function pruneSlot(workflowId: string, slot: WorkflowRunSlot): void { + if (!slot.owner && slot.waiters.length === 0) runSlotsByWorkflowId.delete(workflowId) +} + +/** Whether a new run tool for this workflow has to queue. */ +function isSlotHeld(workflowId: string): boolean { + return runSlotsByWorkflowId.has(workflowId) +} + +function runToolOwner(workflowId: string): ActiveRunTool | undefined { + const owner = runSlotsByWorkflowId.get(workflowId)?.owner + return owner?.kind === 'runTool' ? owner.run : undefined +} + +function* runToolOwners(): Generator<[string, ActiveRunTool]> { + for (const [workflowId, slot] of runSlotsByWorkflowId) { + if (slot.owner?.kind === 'runTool') yield [workflowId, slot.owner.run] + } +} + /** - * Takes the workflow's run-tool slot, or waits in arrival order for the run tool - * holding it. - * - * The editor keeps one visible execution per workflow (running flag, current - * execution id, active blocks, terminal pointer), so two runs of one workflow in - * a tab would clobber each other. Calls the agent fans out in one step therefore - * run one after another rather than failing. The wait needs no timeout of its - * own: a tool call nobody claims within the server's pickup grace is run by the - * server, and the late launch here then gets the execute route's benign 409. + * Takes the workflow's run slot, or waits in arrival order behind its owner. + * Resolves null when the call is stopped while waiting. */ -function acquireRunToolSlot( - workflowId: string, - toolCallId: string -): ActiveRunTool | Promise { - if ( - !activeRunToolByWorkflowId.has(workflowId) && - !waitingRunToolsByWorkflowId.has(workflowId) && - !interruptedExecutionIdByWorkflowId.has(workflowId) - ) { +function acquireRunToolSlot(workflowId: string, toolCallId: string): Promise { + if (!isSlotHeld(workflowId)) { const run: ActiveRunTool = { toolCallId } - activeRunToolByWorkflowId.set(workflowId, run) - return run + slotFor(workflowId).owner = { kind: 'runTool', run } + return Promise.resolve(run) } + const slot = slotFor(workflowId) + logger.info("[RunTool] Waiting for this workflow's current run", { + toolCallId, + workflowId, + owner: slot.owner?.kind === 'runTool' ? slot.owner.run.toolCallId : slot.owner?.kind, + }) return new Promise((admit) => { - const waiting = waitingRunToolsByWorkflowId.get(workflowId) ?? [] - waiting.push({ toolCallId, admit }) - waitingRunToolsByWorkflowId.set(workflowId, waiting) + slot.waiters.push({ toolCallId, admit }) + if (slot.owner?.kind === 'interrupted') watchInterruptedExecution(workflowId, slot) }) } +/** Runs synchronously with the release that freed the slot, so a later call cannot jump ahead. */ +function admitNextRunTool(workflowId: string, slot: WorkflowRunSlot): void { + if (!slot.owner) { + const next = slot.waiters.shift() + if (next) { + const run: ActiveRunTool = { toolCallId: next.toolCallId } + slot.owner = { kind: 'runTool', run } + next.admit(run) + } + } + pruneSlot(workflowId, slot) +} + /** - * Gives a free slot to the oldest waiter. Called synchronously with the release - * that freed it, so a call issued later cannot take the slot first. + * Gives up a run tool's slot. A run whose stream dropped while the server kept + * executing it hands the slot to that execution instead, until it settles. */ -function admitNextRunTool(workflowId: string): void { - if (activeRunToolByWorkflowId.has(workflowId)) return - if (interruptedExecutionIdByWorkflowId.has(workflowId)) return - const waiting = waitingRunToolsByWorkflowId.get(workflowId) - const next = waiting?.shift() - if (waiting?.length === 0) waitingRunToolsByWorkflowId.delete(workflowId) - if (!next) return - const nextRun: ActiveRunTool = { toolCallId: next.toolCallId } - activeRunToolByWorkflowId.set(workflowId, nextRun) - next.admit(nextRun) +function releaseRunToolSlot( + workflowId: string, + run: ActiveRunTool, + interruptedExecutionId?: string +): void { + const slot = runSlotsByWorkflowId.get(workflowId) + if (!slot || runToolOwner(workflowId) !== run) return + slot.owner = interruptedExecutionId + ? { kind: 'interrupted', executionId: interruptedExecutionId } + : null + admitNextRunTool(workflowId, slot) + if (interruptedExecutionId && slot.waiters.length > 0) { + watchInterruptedExecution(workflowId, slot) + } } -function releaseRunToolSlot(workflowId: string, run: ActiveRunTool): void { - if (activeRunToolByWorkflowId.get(workflowId) !== run) return - activeRunToolByWorkflowId.delete(workflowId) - admitNextRunTool(workflowId) +function releaseInterruptedHold(workflowId: string, slot: WorkflowRunSlot, executionId: string) { + if (slot.owner?.kind !== 'interrupted' || slot.owner.executionId !== executionId) return + // A reconnect cancelled by navigating away leaves the settled run marked current. + clearVisibleExecution(workflowId, executionId) + slot.owner = null + admitNextRunTool(workflowId, slot) } /** - * Holds the workflow for an execution whose run tool lost its stream while the - * server kept running it, until that execution settles. - * - * The interrupted run keeps its terminal pointer so the editor's reconnect can - * re-attach to it. A waiting call admitted now would overwrite that pointer and - * claim the workflow, leaving the still-running execution without live output or - * the editor's Stop. The hold is not run-tool ownership, so reconnect may claim it. + * Polls an interrupted execution while calls wait behind it, releasing the hold once + * the server has settled it and no reconnect stream is still draining it. */ -async function holdForInterruptedExecution(workflowId: string, executionId: string) { - interruptedExecutionIdByWorkflowId.set(workflowId, executionId) - for (let attempt = 1; ; attempt++) { - await sleep(backoffWithJitter(attempt, null, { maxMs: INTERRUPTED_RUN_POLL_MAX_MS })) - if (interruptedExecutionIdByWorkflowId.get(workflowId) !== executionId) return - if (await isExecutionStillRunning(workflowId, executionId)) continue - // A live reconnect is still draining the run's last events into the editor. - if (isReconnectStreamOpen(workflowId, executionId)) continue - break - } - interruptedExecutionIdByWorkflowId.delete(workflowId) - // A reconnect cancelled by navigating away leaves the settled run looking current; - // left as is, the next run would read it as a run someone else is driving. - const executionState = useExecutionStore.getState() - if (executionState.getCurrentExecutionId(workflowId) === executionId) { - executionState.setCurrentExecutionId(workflowId, null) - executionState.setIsExecuting(workflowId, false) - executionState.setActiveBlocks(workflowId, new Set()) - } - admitNextRunTool(workflowId) -} - -async function isExecutionStillRunning(workflowId: string, executionId: string) { +function watchInterruptedExecution(workflowId: string, slot: WorkflowRunSlot): void { + if (slot.watchingInterrupted || slot.owner?.kind !== 'interrupted') return + slot.watchingInterrupted = true + const { executionId } = slot.owner + const isHeldByThisExecution = () => + slot.owner?.kind === 'interrupted' && slot.owner.executionId === executionId + void (async () => { + let serverSettled = false + let statusFailures = 0 + for (let attempt = 1; ; attempt++) { + await sleep(backoffWithJitter(attempt, null, { maxMs: INTERRUPTED_RUN_POLL_MAX_MS })) + if (!isHeldByThisExecution() || slot.waiters.length === 0) break + if (isDocumentHidden() || isReconnectStreamOpen(workflowId, executionId)) continue + if (!serverSettled) { + const status = await readExecutionStatus(workflowId, executionId) + if (status === 'running') continue + if (status === 'unknown' && ++statusFailures < INTERRUPTED_RUN_MAX_STATUS_FAILURES) continue + serverSettled = true + if (isReconnectStreamOpen(workflowId, executionId)) continue + } + releaseInterruptedHold(workflowId, slot, executionId) + break + } + slot.watchingInterrupted = false + })() +} + +function isDocumentHidden(): boolean { + return typeof document !== 'undefined' && document.hidden +} + +/** 'settled' also covers an execution that is gone or not ours (403/404). */ +async function readExecutionStatus( + workflowId: string, + executionId: string +): Promise<'running' | 'settled' | 'unknown'> { try { const { status } = await requestJson(getWorkflowExecutionContract, { params: { id: workflowId, executionId }, query: {}, }) return status === 'queued' || status === 'pending' || status === 'running' + ? 'running' + : 'settled' } catch (error) { - // Gone or not ours: nothing left to wait for. Anything else may be transient. if (error instanceof ApiClientError && (error.status === 403 || error.status === 404)) { - return false + return 'settled' } - logger.warn('[RunTool] Could not check an interrupted execution; still holding', { + logger.warn("[RunTool] Could not read an interrupted execution's status", { workflowId, executionId, error: toError(error).message, }) - return true + return 'unknown' } } function isRunToolWaiting(workflowId: string, toolCallId: string): boolean { return ( - waitingRunToolsByWorkflowId.get(workflowId)?.some((w) => w.toolCallId === toolCallId) ?? false + runSlotsByWorkflowId.get(workflowId)?.waiters.some((w) => w.toolCallId === toolCallId) ?? false ) } function dropWaitingRunTools(toolCallIds: ReadonlySet): void { - for (const [workflowId, waiting] of waitingRunToolsByWorkflowId) { - const kept = waiting.filter((w) => !toolCallIds.has(w.toolCallId)) - if (kept.length === waiting.length) continue - if (kept.length === 0) waitingRunToolsByWorkflowId.delete(workflowId) - else waitingRunToolsByWorkflowId.set(workflowId, kept) - for (const dropped of waiting) { - if (toolCallIds.has(dropped.toolCallId)) dropped.admit(null) + for (const [workflowId, slot] of runSlotsByWorkflowId) { + const kept: WaitingRunTool[] = [] + for (const waiter of slot.waiters) { + if (toolCallIds.has(waiter.toolCallId)) waiter.admit(null) + else kept.push(waiter) } + slot.waiters = kept + pruneSlot(workflowId, slot) } } +/** Clears the editor's visible run when it still shows `executionId`; returns whether it did. */ +function clearVisibleExecution(workflowId: string, executionId: string): boolean { + const state = useExecutionStore.getState() + if (state.getCurrentExecutionId(workflowId) !== executionId) return false + state.setCurrentExecutionId(workflowId, null) + state.setIsExecuting(workflowId, false) + state.setActiveBlocks(workflowId, new Set()) + return true +} + function resolveWorkflowInput(params: Record): unknown { if (Object.hasOwn(params, 'workflow_input')) { return params.workflow_input @@ -415,22 +485,22 @@ export async function bindRunToolToExecution( toolCallId: string, workflowId: string ): Promise { - const existingToolCallId = activeRunToolByWorkflowId.get(workflowId)?.toolCallId - if (existingToolCallId === toolCallId || isRunToolWaiting(workflowId, toolCallId)) { + const owner = runSlotsByWorkflowId.get(workflowId)?.owner + const ownerToolCallId = owner?.kind === 'runTool' ? owner.run.toolCallId : undefined + if (ownerToolCallId === toolCallId || isRunToolWaiting(workflowId, toolCallId)) { logger.info('[RunTool] Recovery skipped: run tool is already active in this tab', { workflowId, toolCallId, }) return true } - // Another run owns the workflow's pointer, so it says nothing about this call. - const otherRunOwnsWorkflow = - existingToolCallId !== undefined || interruptedExecutionIdByWorkflowId.has(workflowId) + // Another run's pointer says nothing about this call. + const otherRunOwnsWorkflow = owner != null + const pointer = otherRunOwnsWorkflow + ? null + : await loadExecutionPointer(workflowId).catch(() => null) const pendingCompletion = loadPendingCompletionReport(toolCallId) if (pendingCompletion) { - const pointer = otherRunOwnsWorkflow - ? null - : await loadExecutionPointer(workflowId).catch(() => null) const executionId = pendingCompletion.executionId ?? pointer?.executionId logger.info('[RunTool] Recovery re-sending pending completion report', { workflowId, @@ -467,12 +537,11 @@ export async function bindRunToolToExecution( logger.warn('[RunTool] Recovery skipped: another run owns this workflow', { workflowId, toolCallId, - existingToolCallId, + owner: ownerToolCallId ?? owner.kind, }) return false } - const pointer = await loadExecutionPointer(workflowId).catch(() => null) if (!pointer?.executionId) { logger.info('[RunTool] Recovery skipped: no tab-local execution pointer', { workflowId, @@ -532,8 +601,8 @@ export function executeRunToolOnClient( } export function isRunToolActiveForId(toolCallId: string): boolean { - for (const active of activeRunToolByWorkflowId.values()) { - if (active.toolCallId === toolCallId) return true + for (const [, run] of runToolOwners()) { + if (run.toolCallId === toolCallId) return true } return false } @@ -547,7 +616,7 @@ export function isRunToolActiveForId(toolCallId: string): boolean { * acknowledged the run. */ export function isRunToolActiveForWorkflow(workflowId: string): boolean { - return activeRunToolByWorkflowId.has(workflowId) + return runToolOwner(workflowId) !== undefined } /** @@ -572,7 +641,7 @@ function notifyRunToolReleased(workflowId: string): void { /** The resource's Stop button refers to its displayed execution, which may be a manual run. */ export function stopRunToolForExecution(workflowId: string, executionId: string | null): boolean { - const active = activeRunToolByWorkflowId.get(workflowId) + const active = runToolOwner(workflowId) if (!executionId || active?.execution?.id !== executionId) return false stopRunToolExecutions(new Set([active.toolCallId])) return true @@ -581,7 +650,7 @@ export function stopRunToolForExecution(workflowId: string, executionId: string /** Cancels exact tool-owned executions; a workflow's current UI pointer is not ownership. */ export function stopRunToolExecutions(toolCallIds: ReadonlySet): void { dropWaitingRunTools(toolCallIds) - for (const [workflowId, active] of activeRunToolByWorkflowId) { + for (const [workflowId, active] of runToolOwners()) { const { toolCallId, execution } = active if (!toolCallIds.has(toolCallId) || !execution || manuallyStoppedToolCallIds.has(toolCallId)) { continue @@ -617,13 +686,7 @@ export function stopRunToolExecutions(toolCallIds: ReadonlySet): void { blockName: 'Run Cancelled', blockType: 'cancelled', }) - const state = useExecutionStore.getState() - if (state.getCurrentExecutionId(workflowId) === execution.id) { - clearExecutionPointer(workflowId) - state.setCurrentExecutionId(workflowId, null) - state.setIsExecuting(workflowId, false) - state.setActiveBlocks(workflowId, new Set()) - } + if (clearVisibleExecution(workflowId, execution.id)) clearExecutionPointer(workflowId) } } @@ -648,29 +711,16 @@ async function doExecuteRunTool( return } - const slot = acquireRunToolSlot(targetWorkflowId, toolCallId) - let activeRun: ActiveRunTool - if (slot instanceof Promise) { - logger.info('[RunTool] Waiting for the run tool already running this workflow', { + const activeRun = await acquireRunToolSlot(targetWorkflowId, toolCallId) + if (!activeRun) { + logger.info('[RunTool] Stopped before its turn to run', { toolCallId, toolName }) + await reportCompletion( toolCallId, - toolName, - workflowId: targetWorkflowId, - activeToolCallId: activeRunToolByWorkflowId.get(targetWorkflowId)?.toolCallId, - }) - const admitted = await slot - if (!admitted) { - logger.info('[RunTool] Stopped before its turn to run', { toolCallId, toolName }) - await reportCompletion( - toolCallId, - MothershipStreamV1ToolOutcome.cancelled, - getWorkflowToolCompletionMessage(MothershipStreamV1ToolOutcome.cancelled), - { reason: 'user_cancelled', cancelledByUser: true } - ) - return - } - activeRun = admitted - } else { - activeRun = slot + MothershipStreamV1ToolOutcome.cancelled, + getWorkflowToolCompletionMessage(MothershipStreamV1ToolOutcome.cancelled), + { reason: 'user_cancelled', cancelledByUser: true } + ) + return } setActiveWorkflow(targetWorkflowId) @@ -756,15 +806,11 @@ async function doExecuteRunTool( setCurrentExecutionId(targetWorkflowId, executionId) saveExecutionPointer({ workflowId: targetWorkflowId, executionId, lastEventId: 0 }) const releaseVisibleExecutionForBackground = () => { - const { setCurrentExecutionId: clearExecId, setActiveBlocks } = useExecutionStore.getState() if ( - activeRunToolByWorkflowId.get(targetWorkflowId) === activeRun && - useExecutionStore.getState().getCurrentExecutionId(targetWorkflowId) === executionId + runToolOwner(targetWorkflowId) === activeRun && + clearVisibleExecution(targetWorkflowId, executionId) ) { - clearExecId(targetWorkflowId, null) consolePersistence.executionEnded(persistenceExecution) - setIsExecuting(targetWorkflowId, false) - setActiveBlocks(targetWorkflowId, new Set()) } } @@ -936,28 +982,16 @@ async function doExecuteRunTool( window.removeEventListener('pagehide', onPageHide) } manuallyStoppedToolCallIds.delete(toolCallId) - const ownsRegistration = activeRunToolByWorkflowId.get(targetWorkflowId) === activeRun - if (ownsRegistration) { - activeRunToolByWorkflowId.delete(targetWorkflowId) - } + const ownsRegistration = runToolOwner(targetWorkflowId) === activeRun consolePersistence.executionEnded(persistenceExecution) - const { setCurrentExecutionId: clearExecId, setActiveBlocks } = useExecutionStore.getState() if ( - !leaveExecutionRecoverable && ownsRegistration && - useExecutionStore.getState().getCurrentExecutionId(targetWorkflowId) === executionId + !leaveExecutionRecoverable && + clearVisibleExecution(targetWorkflowId, executionId) ) { - clearExecId(targetWorkflowId, null) clearExecutionPointer(targetWorkflowId) - setIsExecuting(targetWorkflowId, false) - setActiveBlocks(targetWorkflowId, new Set()) - } - if (interruptedExecutionId && ownsRegistration) { - void holdForInterruptedExecution(targetWorkflowId, interruptedExecutionId) - notifyRunToolReleased(targetWorkflowId) - } - if (ownsRegistration) { - admitNextRunTool(targetWorkflowId) } + releaseRunToolSlot(targetWorkflowId, activeRun, interruptedExecutionId) + if (ownsRegistration && interruptedExecutionId) notifyRunToolReleased(targetWorkflowId) } } From e65f6c7580cce89eeee299907d4a423b4b83d92f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 18:59:06 -0700 Subject: [PATCH 6/6] fix(mothership): clear a dropped run's saved pointer when its hold is released A reconnect cancelled by navigating away leaves the settled run's execution pointer saved. Releasing the hold now clears it when it still names that execution, before the next run is admitted, so recovery cannot later report the finished run as a live one. --- .../tools/client/run-tool-execution.test.ts | 11 +++++++++++ .../mothership/tools/client/run-tool-execution.ts | 13 ++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts index 9661fc03312..aa43f04fb1f 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.test.ts @@ -913,6 +913,12 @@ describe('run tool execution cancellation', () => { getWorkflowExecution.mockImplementation(() => ({ isExecuting: false })) }) + loadExecutionPointer.mockResolvedValue({ + workflowId: 'wf-1', + executionId: 'exec-abandoned', + lastEventId: 3, + }) + executeRunToolOnClient('tool-a', 'run_block', { workflowId: 'wf-1', blockId: 'block-1' }) executeRunToolOnClient('tool-b', 'run_block', { workflowId: 'wf-1', blockId: 'block-2' }) await vi.waitFor(() => expect(released).toHaveBeenCalledWith('wf-1')) @@ -928,6 +934,11 @@ describe('run tool execution cancellation', () => { reconnect.close() await vi.waitFor(() => expect(launchedToolCallIds()).toEqual(['tool-a', 'tool-b'])) + // The abandoned run's pointer is gone before the next run saves its own. + expect(clearExecutionPointer).toHaveBeenCalledWith('wf-1') + expect(clearExecutionPointer.mock.invocationCallOrder[0]).toBeLessThan( + saveExecutionPointer.mock.invocationCallOrder.at(-1) ?? 0 + ) finish('tool-b') await drain() expect(confirmStatuses()).toEqual([ diff --git a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts index 81e630beb59..5b29355dc99 100644 --- a/apps/sim/lib/mothership/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/mothership/tools/client/run-tool-execution.ts @@ -186,10 +186,17 @@ function releaseRunToolSlot( } } -function releaseInterruptedHold(workflowId: string, slot: WorkflowRunSlot, executionId: string) { +async function releaseInterruptedHold( + workflowId: string, + slot: WorkflowRunSlot, + executionId: string +): Promise { if (slot.owner?.kind !== 'interrupted' || slot.owner.executionId !== executionId) return - // A reconnect cancelled by navigating away leaves the settled run marked current. + // A reconnect cancelled by navigating away leaves the settled run marked current and its + // pointer saved; both go before the next run can write its own. clearVisibleExecution(workflowId, executionId) + const pointer = await loadExecutionPointer(workflowId).catch(() => null) + if (pointer?.executionId === executionId) await clearExecutionPointer(workflowId) slot.owner = null admitNextRunTool(workflowId, slot) } @@ -218,7 +225,7 @@ function watchInterruptedExecution(workflowId: string, slot: WorkflowRunSlot): v serverSettled = true if (isReconnectStreamOpen(workflowId, executionId)) continue } - releaseInterruptedHold(workflowId, slot, executionId) + await releaseInterruptedHold(workflowId, slot, executionId) break } slot.watchingInterrupted = false