diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx index 110e77c5ac3..3677c7b57f1 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx @@ -19,12 +19,15 @@ const { mockFetch, mockHandleExecutionCancelledConsole, mockHandleExecutionErrorConsole, + mockIsExecutionStreamHttpError, + mockIsRunToolActiveForWorkflow, mockLoadExecutionPointer, mockReconnect, mockRequestJson, mockResolveStartCandidates, mockSelectBestTrigger, mockUploadInternalFileSession, + runToolReleaseListeners, terminalStoreState, workflowBlocks, workflowStoreState, @@ -101,12 +104,15 @@ const { mockFetch: vi.fn(), mockHandleExecutionCancelledConsole: vi.fn(), mockHandleExecutionErrorConsole: vi.fn(), + mockIsExecutionStreamHttpError: vi.fn(() => false), + mockIsRunToolActiveForWorkflow: vi.fn(() => false), mockLoadExecutionPointer: vi.fn(), mockReconnect: vi.fn(), mockRequestJson: vi.fn(), mockResolveStartCandidates: vi.fn(), mockSelectBestTrigger: vi.fn(), mockUploadInternalFileSession: vi.fn(), + runToolReleaseListeners: new Set<(workflowId: string) => void>(), terminalStoreState, workflowBlocks, workflowStoreState, @@ -125,6 +131,16 @@ vi.mock('@/lib/api/client/request', () => ({ requestJson: mockRequestJson, })) +vi.mock('@/lib/copilot/tools/client/run-tool-execution', () => ({ + isRunToolActiveForWorkflow: mockIsRunToolActiveForWorkflow, + subscribeToRunToolRelease: (listener: (workflowId: string) => void) => { + runToolReleaseListeners.add(listener) + return () => { + runToolReleaseListeners.delete(listener) + } + }, +})) + vi.mock('@/lib/api/contracts/workflows', () => ({ cancelWorkflowExecutionContract: {}, workflowLogContract: {}, @@ -214,7 +230,7 @@ vi.mock('@/hooks/use-execution-stream', () => { class SSEStreamInterruptedError extends Error {} return { - isExecutionStreamHttpError: () => false, + isExecutionStreamHttpError: mockIsExecutionStreamHttpError, SSEEventHandlerError, SSEStreamInterruptedError, useExecutionStream: () => ({ @@ -419,6 +435,8 @@ function resetWorkflowExecutionTestState() { mockBeginScopedExecution.mockReset().mockReturnValue({}) mockAdoptScopedExecution.mockReset().mockReturnValue(undefined) mockEndScopedExecution.mockReset().mockReturnValue(true) + mockIsExecutionStreamHttpError.mockReset().mockReturnValue(false) + mockIsRunToolActiveForWorkflow.mockReset().mockReturnValue(false) mockLoadExecutionPointer.mockReset().mockResolvedValue(null) mockReconnect.mockReset().mockResolvedValue(undefined) mockResolveStartCandidates.mockReset().mockReturnValue([]) @@ -430,6 +448,36 @@ function resetWorkflowExecutionTestState() { executionStoreState.getWorkflowExecution.mockReturnValue(idleExecution) executionStoreState.getCurrentExecutionId.mockReturnValue(null) workflowStoreState.edges.length = 0 + runToolReleaseListeners.clear() +} + +/** + * The store and pointer state a Sim run tool leaves behind the moment it starts + * a run, before the server has acknowledged it: this is what the reconnect + * flow reads as an orphaned run. + */ +function primeRunToolOwnedExecution() { + terminalStoreState._hasHydrated = true + executionStoreState.getWorkflowExecution.mockReturnValue({ + ...executionStoreState.getWorkflowExecution(), + status: 'running', + isExecuting: true, + currentExecutionId: 'execution-1', + }) + executionStoreState.getCurrentExecutionId.mockReturnValue('execution-1') + mockLoadExecutionPointer.mockResolvedValue({ + workflowId: 'workflow-1', + executionId: 'execution-1', + lastEventId: 0, + }) +} + +/** The reconnect endpoint's answer while the run's buffer does not exist yet. */ +function rejectReconnectWithMissingRunBuffer() { + mockIsExecutionStreamHttpError.mockReturnValue(true) + mockReconnect.mockRejectedValue( + Object.assign(new Error('Reconnect failed (404)'), { httpStatus: 404 }) + ) } describe('useWorkflowExecution lifecycle ownership', () => { @@ -594,6 +642,118 @@ describe('useWorkflowExecution lifecycle ownership', () => { unmount() }) + it('logs a Run Error when a reconnect for an unowned pointer finds no run buffer', async () => { + primeRunToolOwnedExecution() + rejectReconnectWithMissingRunBuffer() + + const { unmount } = renderWorkflowExecutionHook() + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(mockReconnect).toHaveBeenCalledTimes(1) + expect(mockHandleExecutionErrorConsole.mock.calls[0]).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + executionId: 'execution-1', + error: 'Execution state is no longer available after reconnect', + }), + ]) + ) + expect(executionStoreState.setCurrentExecutionId).toHaveBeenCalledWith('workflow-1', null) + expect(executionStoreState.setIsExecuting).toHaveBeenCalledWith('workflow-1', false) + expect(mockClearExecutionPointer).toHaveBeenCalledWith('workflow-1') + + unmount() + }) + + it('leaves a run owned by a client run tool to its live stream instead of reconnecting', async () => { + /* + * Same state as above, but a Sim run tool in this tab still owns the run. + * Its live stream is the source of truth, so reconnecting here would race + * the run's own start (the 404 above, logged as a Run Error mid-run), tear + * down the live run's store state, and clear the pointer the tool keeps + * for reload recovery. + */ + primeRunToolOwnedExecution() + rejectReconnectWithMissingRunBuffer() + mockIsRunToolActiveForWorkflow.mockReturnValue(true) + + const { unmount } = renderWorkflowExecutionHook() + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(mockIsRunToolActiveForWorkflow).toHaveBeenCalledWith('workflow-1') + expect(mockReconnect).not.toHaveBeenCalled() + expect(mockHandleExecutionErrorConsole).not.toHaveBeenCalled() + expect(mockClearExecutionPointer).not.toHaveBeenCalled() + expect(executionStoreState.setCurrentExecutionId).not.toHaveBeenCalled() + expect(executionStoreState.setIsExecuting).not.toHaveBeenCalled() + expect(executionStoreState.setActiveBlocks).not.toHaveBeenCalled() + + unmount() + }) + + it('reconnects once the client run tool releases a run whose stream dropped', async () => { + primeRunToolOwnedExecution() + mockIsRunToolActiveForWorkflow.mockReturnValue(true) + + const { unmount } = renderWorkflowExecutionHook() + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + expect(mockReconnect).not.toHaveBeenCalled() + expect(runToolReleaseListeners.size).toBeGreaterThan(0) + + /* + * What the run tool leaves behind when it gives the run up: no current + * execution, not executing, ownership released, and the pointer still + * carrying the last event it persisted. + */ + executionStoreState.getWorkflowExecution.mockReturnValue({ + ...executionStoreState.getWorkflowExecution(), + status: 'idle', + isExecuting: false, + currentExecutionId: null, + }) + executionStoreState.getCurrentExecutionId.mockReturnValue(null) + mockLoadExecutionPointer.mockResolvedValue({ + workflowId: 'workflow-1', + executionId: 'execution-1', + lastEventId: 5, + }) + mockIsRunToolActiveForWorkflow.mockReturnValue(false) + await act(async () => { + for (const listener of runToolReleaseListeners) listener('workflow-2') + await Promise.resolve() + await Promise.resolve() + }) + expect(mockReconnect).not.toHaveBeenCalled() + + await act(async () => { + for (const listener of runToolReleaseListeners) listener('workflow-1') + await Promise.resolve() + await Promise.resolve() + }) + + expect(mockReconnect).toHaveBeenCalledTimes(1) + expect(mockReconnect).toHaveBeenCalledWith( + expect.objectContaining({ + workflowId: 'workflow-1', + executionId: 'execution-1', + fromEventId: 5, + }) + ) + expect(mockClearExecutionPointer).not.toHaveBeenCalled() + + unmount() + expect(runToolReleaseListeners.size).toBe(0) + }) + it('does not let delayed debug completion reset a replacement execution', async () => { const debugPersistenceExecution = {} const replacementPersistenceExecution = {} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts index 6fd7b15084b..4b25eb8ac05 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts @@ -21,6 +21,10 @@ import { workflowLogContract, workflowStateSchema, } from '@/lib/api/contracts/workflows' +import { + isRunToolActiveForWorkflow, + subscribeToRunToolRelease, +} from '@/lib/copilot/tools/client/run-tool-execution' import type { SecretSafeBlockLog } from '@/lib/logs/execution/display-types' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' import { processStreamingBlockLogs } from '@/lib/tokenization' @@ -2386,6 +2390,14 @@ export function useWorkflowExecution() { [activeWorkflowId, setExecutionResult, tryStartExecution] ) + useEffect(() => { + if (!activeWorkflowId) return + return subscribeToRunToolRelease((workflowId) => { + if (workflowId !== activeWorkflowId) return + setReconnectAttemptNonce((nonce) => nonce + 1) + }) + }, [activeWorkflowId]) + useEffect(() => { if (!activeWorkflowId || !hasHydrated) return if (activeReconnections.has(activeWorkflowId)) return @@ -2404,6 +2416,13 @@ export function useWorkflowExecution() { } const runReconnect = async () => { + if (isRunToolActiveForWorkflow(reconnectWorkflowId)) { + logger.info('Reconnection skipped; a client run tool owns this workflow run', { + workflowId: reconnectWorkflowId, + }) + return + } + let executionId: string | undefined let fromEventId = 0 diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts index 10db82c7614..5e4e2dbb241 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts @@ -13,7 +13,11 @@ import { reconcileFinalBlockLogs, } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils' import type { BlockLog } from '@/executor/types' -import type { ExecutionStreamHttpError } from '@/hooks/use-execution-stream' +import { + ExecutionStreamHttpError, + SSEEventHandlerError, + SSEStreamInterruptedError, +} from '@/hooks/use-execution-stream' import { useExecutionStore } from '@/stores/execution' describe('workflow-execution-utils', () => { @@ -61,6 +65,111 @@ describe('workflow-execution-utils', () => { expect(terminalConsoleMockFns.mockAddConsole).not.toHaveBeenCalled() }) + describe('executeWorkflowWithFullLogging stream interruption', () => { + /** A response whose server acknowledged the run and whose body then fails with `readError`. */ + function stubAcknowledgedStream(readError: unknown) { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: { get: (name: string) => (name === 'X-Execution-Id' ? 'exec-server' : null) }, + body: { + getReader: () => ({ + read: vi.fn().mockRejectedValue(readError), + releaseLock: vi.fn(), + }), + }, + }) + ) + } + + function stubExecutionStore() { + const store = { + getCurrentExecutionId: vi.fn(() => 'exec-server'), + setActiveBlocks: vi.fn(), + setBlockRunStatus: vi.fn(), + setCurrentExecutionId: vi.fn(), + setEdgeRunStatus: vi.fn(), + setIsExecuting: vi.fn(), + } + vi.mocked(useExecutionStore.getState).mockReturnValue(store as any) + return store + } + + it.each([ + ['Chrome', 'network error'], + ['Chrome before headers', 'Failed to fetch'], + ['Firefox', 'NetworkError when attempting to fetch resource.'], + ['Safari', 'Load failed'], + ])( + 'classifies a %s transport drop after the server acknowledged the run as an interruption', + async (_browser, message) => { + /* + * The Chat run tool only preserves a run for reconnect when it sees + * SSEStreamInterruptedError; a raw TypeError from the body reader used to + * fall through as a plain failure, reporting an error to Sim and tearing + * the run down while the server kept executing it. + */ + const store = stubExecutionStore() + stubAcknowledgedStream(new TypeError(message)) + + const promise = executeWorkflowWithFullLogging({ + workflowId: 'wf-1', + executionId: 'exec-1', + copilotToolCallId: 'tool-1', + preserveExecutionOnTerminal: true, + }) + + await expect(promise).rejects.toBeInstanceOf(SSEStreamInterruptedError) + await expect(promise).rejects.toMatchObject({ executionId: 'exec-server' }) + expect(store.setCurrentExecutionId).toHaveBeenCalledWith('wf-1', 'exec-server') + expect(store.setCurrentExecutionId).not.toHaveBeenCalledWith('wf-1', null) + expect(store.setIsExecuting).not.toHaveBeenCalled() + } + ) + + it.each([ + ['a nullish rejection', null], + ['a client abort', new DOMException('Aborted', 'AbortError')], + [ + 'an HTTP rejection whose message mentions a transport phrase', + new ExecutionStreamHttpError('Failed to fetch workflow state', 500), + ], + [ + 'a handler failure whose message mentions a transport phrase', + new SSEEventHandlerError( + 'network error while persisting console rows', + 'block:completed', + 3, + 'exec-server', + new Error('persist failed') + ), + ], + [ + 'the run tool stop reason, which aborts with a plain string', + 'user_stop:cancelRunToolExecution', + ], + ['a non-transport failure', new Error('Unexpected token in JSON')], + ])('rethrows %s unclassified', async (_label, readError) => { + stubExecutionStore() + stubAcknowledgedStream(readError) + + const rejection = await executeWorkflowWithFullLogging({ + workflowId: 'wf-1', + executionId: 'exec-1', + preserveExecutionOnTerminal: true, + }).then( + () => { + throw new Error('expected the stream failure to reject') + }, + (error: unknown) => error + ) + + expect(rejection).toBe(readError) + }) + }) + describe('createBlockEventHandlers', () => { it('skips duplicate block start rows during reconnect replay', () => { terminalConsoleMockFns.mockAddConsole({ diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts index 70bb9fbde9f..3c67bd35c6a 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts @@ -1,5 +1,5 @@ import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' +import { getErrorMessage, toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { isPlainRecord } from '@sim/utils/object' import { normalizeWorkflowEdgeSourceHandle } from '@sim/workflow-types/workflow' @@ -19,6 +19,7 @@ import { processSSEStream, SSEEventHandlerError, SSEStreamInterruptedError, + toStreamInterruptedError, } from '@/hooks/use-execution-stream' import { useExecutionStore } from '@/stores/execution' import type { ConsoleEntry, ConsoleUpdate } from '@/stores/terminal' @@ -946,7 +947,7 @@ export function handleExecutionCancelledConsole( addCancelledConsoleEntry(deps.addConsole, params) } -interface WorkflowExecutionOptions { +export interface WorkflowExecutionOptions { workflowId?: string workflowInput?: any onStream?: (se: StreamingExecution) => Promise @@ -1221,6 +1222,20 @@ export async function executeWorkflowWithFullLogging( 'CopilotExecution' ) } catch (error) { + const interrupted = toStreamInterruptedError( + error, + executionIdRef.current, + 'Execution stream interrupted before a terminal event was received' + ) + if (interrupted) { + logger.warn('Execution stream interrupted; preserving execution for reconnect', { + workflowId: wfId, + executionId: executionIdRef.current, + error: getErrorMessage(error), + }) + preserveExecutionForRecovery = true + throw interrupted + } if (error instanceof SSEEventHandlerError || error instanceof SSEStreamInterruptedError) { preserveExecutionForRecovery = true } diff --git a/apps/sim/hooks/use-execution-stream.ts b/apps/sim/hooks/use-execution-stream.ts index fb156950e88..b4bcc38d3b4 100644 --- a/apps/sim/hooks/use-execution-stream.ts +++ b/apps/sim/hooks/use-execution-stream.ts @@ -1,6 +1,7 @@ import { useCallback } from 'react' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' +import { isRecordLike } from '@sim/utils/object' import type { WorkflowStateContractInput } from '@/lib/api/contracts/workflows' import { readSSEEvents } from '@/lib/core/utils/sse' import type { @@ -67,18 +68,58 @@ export class SSEStreamInterruptedError extends Error { * Detects errors caused by the browser killing a fetch (page refresh, navigation, tab close). * These should be treated as clean disconnects, not execution errors. */ -function isClientDisconnectError(error: any): boolean { - return error.name === 'AbortError' +function isClientDisconnectError(error: unknown): boolean { + return isRecordLike(error) && error.name === 'AbortError' } -function isRecoverableStreamError(error: any): boolean { - if (isClientDisconnectError(error)) return false - const msg = (error.message ?? '').toLowerCase() +/** + * Messages browsers put on the TypeError a fetch or body read rejects with when + * the connection drops: Chrome's "network error" and "Failed to fetch", + * Firefox's "NetworkError when attempting to fetch resource.", and Safari's + * "Load failed". + */ +const TRANSPORT_FAILURE_MESSAGE_PATTERNS = [ + /network\s?error/, + /failed to fetch/, + /load failed/, +] as const + +/** + * Errors the stream layer raises itself carry their own meaning (an HTTP + * rejection, a handler failure, an already classified drop), so their message + * text must never be mistaken for a transport failure. + */ +function isStreamLayerError(error: unknown): boolean { return ( - msg.includes('network error') || msg.includes('failed to fetch') || msg.includes('load failed') + error instanceof ExecutionStreamHttpError || + error instanceof SSEEventHandlerError || + error instanceof SSEStreamInterruptedError ) } +function isRecoverableStreamError(error: unknown): boolean { + if (!isRecordLike(error) || isClientDisconnectError(error) || isStreamLayerError(error)) { + return false + } + const msg = typeof error.message === 'string' ? error.message.toLowerCase() : '' + return TRANSPORT_FAILURE_MESSAGE_PATTERNS.some((pattern) => pattern.test(msg)) +} + +/** + * Wraps a transport failure that cut a live execution stream before its + * terminal event, so every consumer of a live stream classifies interruptions + * the same way and recovery code can rely on one error type. Returns null for + * client aborts and for anything that is not a transport failure. + */ +export function toStreamInterruptedError( + error: unknown, + executionId: string | undefined, + message: string +): SSEStreamInterruptedError | null { + if (!isRecoverableStreamError(error)) return null + return new SSEStreamInterruptedError(message, executionId, error) +} + /** * Processes SSE events from a response body and invokes appropriate callbacks. * Exported for use by standalone (non-hook) execution paths like executeWorkflowWithFullLogging. @@ -318,16 +359,17 @@ export function useExecutionStream() { logger.info('Execution stream disconnected (page unload or abort)') return } - if (isRecoverableStreamError(error)) { + const interrupted = toStreamInterruptedError( + error, + serverExecutionId, + 'Execution stream interrupted before a terminal event was received' + ) + if (interrupted) { logger.warn('Execution stream interrupted; preserving execution for reconnect', { executionId: serverExecutionId, error: error.message, }) - throw new SSEStreamInterruptedError( - 'Execution stream interrupted before a terminal event was received', - serverExecutionId, - error - ) + throw interrupted } logger.error('Execution stream error:', error) if (!(error instanceof SSEEventHandlerError)) { @@ -423,16 +465,17 @@ export function useExecutionStream() { logger.info('Run-from-block stream disconnected (page unload or abort)') return } - if (isRecoverableStreamError(error)) { + const interrupted = toStreamInterruptedError( + error, + serverExecutionId, + 'Run-from-block stream interrupted before a terminal event was received' + ) + if (interrupted) { logger.warn('Run-from-block stream interrupted; preserving execution for reconnect', { executionId: serverExecutionId, error: error.message, }) - throw new SSEStreamInterruptedError( - 'Run-from-block stream interrupted before a terminal event was received', - serverExecutionId, - error - ) + throw interrupted } logger.error('Run-from-block execution error:', error) if (!(error instanceof SSEEventHandlerError)) { diff --git a/apps/sim/lib/copilot/tools/client/run-tool-execution.test.ts b/apps/sim/lib/copilot/tools/client/run-tool-execution.test.ts index 12fc1b480a0..4df007c34cd 100644 --- a/apps/sim/lib/copilot/tools/client/run-tool-execution.test.ts +++ b/apps/sim/lib/copilot/tools/client/run-tool-execution.test.ts @@ -3,6 +3,7 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { WorkflowExecutionOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils' const { clearExecutionPointer, @@ -66,6 +67,12 @@ vi.mock('@/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-u executeWorkflowWithFullLogging, })) +/** The abort signal the run tool wires into every client-side execution. */ +function requireAbortSignal(options: WorkflowExecutionOptions): AbortSignal { + if (!options.abortSignal) throw new Error('run tool did not pass an abort signal') + return options.abortSignal +} + vi.mock('@/stores/execution/store', () => ({ useExecutionStore: { getState: () => ({ @@ -115,7 +122,9 @@ import { cancelRunToolExecution, executeRunToolOnClient, isRunToolActiveForId, + isRunToolActiveForWorkflow, reportManualRunToolStop, + subscribeToRunToolRelease, } from './run-tool-execution' describe('run tool execution cancellation', () => { @@ -130,16 +139,18 @@ describe('run tool execution cancellation', () => { it('passes an abort signal into executeWorkflowWithFullLogging and aborts it', async () => { let capturedSignal: AbortSignal | undefined - executeWorkflowWithFullLogging.mockImplementationOnce(async (options: any) => { - capturedSignal = options.abortSignal - await new Promise((_, reject) => { - options.abortSignal.addEventListener( - 'abort', - () => reject(new DOMException('Aborted', 'AbortError')), - { once: true } - ) - }) - }) + executeWorkflowWithFullLogging.mockImplementationOnce( + async (options: WorkflowExecutionOptions) => { + capturedSignal = requireAbortSignal(options) + await new Promise((_, reject) => { + capturedSignal?.addEventListener( + 'abort', + () => reject(new DOMException('Aborted', 'AbortError')), + { once: true } + ) + }) + } + ) executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' }) await Promise.resolve() @@ -150,6 +161,96 @@ describe('run tool execution cancellation', () => { expect(capturedSignal?.aborted).toBe(true) }) + it('owns the workflow for exactly as long as the client run is in flight', async () => { + executeWorkflowWithFullLogging.mockImplementationOnce( + async (options: WorkflowExecutionOptions) => { + await new Promise((_, reject) => { + requireAbortSignal(options).addEventListener( + 'abort', + () => reject(new DOMException('Aborted', 'AbortError')), + { once: true } + ) + }) + } + ) + let ownedWhenPointerSaved: boolean | undefined + saveExecutionPointer.mockImplementationOnce(() => { + ownedWhenPointerSaved = isRunToolActiveForWorkflow('wf-1') + }) + expect(isRunToolActiveForWorkflow('wf-1')).toBe(false) + + executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' }) + await Promise.resolve() + const ownedWhileInFlight = isRunToolActiveForWorkflow('wf-1') + const otherWorkflowOwnedWhileInFlight = isRunToolActiveForWorkflow('wf-2') + + cancelRunToolExecution('wf-1') + await vi.waitFor(() => expect(clearExecutionPointer).toHaveBeenCalledWith('wf-1')) + + expect(ownedWhenPointerSaved).toBe(true) + expect(ownedWhileInFlight).toBe(true) + expect(otherWorkflowOwnedWhileInFlight).toBe(false) + expect(saveExecutionPointer).toHaveBeenCalledWith( + expect.objectContaining({ workflowId: 'wf-1', lastEventId: 0 }) + ) + expect(isRunToolActiveForWorkflow('wf-1')).toBe(false) + }) + + it.each([ + ['handler', new MockSSEEventHandlerError('Block handler failed on event 7', 'exec-1')], + ['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) => { + const ownedAtRelease: boolean[] = [] + const listener = vi.fn((workflowId: string) => { + ownedAtRelease.push(isRunToolActiveForWorkflow(workflowId)) + }) + const unsubscribe = subscribeToRunToolRelease(listener) + executeWorkflowWithFullLogging.mockRejectedValueOnce(interruption) + + try { + executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' }) + await vi.waitFor(() => expect(listener).toHaveBeenCalledWith('wf-1')) + + expect(listener).toHaveBeenCalledTimes(1) + expect(ownedAtRelease).toEqual([false]) + expect(setIsExecuting).toHaveBeenCalledWith('wf-1', false) + expect(setCurrentExecutionId).toHaveBeenCalledWith('wf-1', null) + expect(setIsExecuting.mock.invocationCallOrder.at(-1)).toBeLessThan( + listener.mock.invocationCallOrder[0] + ) + expect(clearExecutionPointer).not.toHaveBeenCalled() + expect(fetch).toHaveBeenCalledWith( + '/api/copilot/confirm', + expect.objectContaining({ + body: expect.stringContaining('"status":"background"'), + }) + ) + expect(vi.mocked(fetch).mock.calls[0][1]?.body).toContain('"executionId":"exec-1"') + } finally { + unsubscribe() + } + } + ) + + it('does not release a run it observed to completion, even when the report fails', async () => { + const listener = vi.fn() + const unsubscribe = subscribeToRunToolRelease(listener) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 500 })) + executeWorkflowWithFullLogging.mockResolvedValueOnce({ success: true }) + + try { + executeRunToolOnClient('tool-1', 'run_workflow', { workflowId: 'wf-1' }) + await vi.waitFor(() => expect(isRunToolActiveForWorkflow('wf-1')).toBe(false)) + + expect(listener).not.toHaveBeenCalled() + expect(clearExecutionPointer).not.toHaveBeenCalled() + } finally { + unsubscribe() + } + }) + it('can report a manual stop using the explicit toolCallId override', async () => { const fetchMock = vi.fn().mockResolvedValue({ ok: true }) vi.stubGlobal('fetch', fetchMock) @@ -252,12 +353,11 @@ describe('run tool execution cancellation', () => { expect(fetchMock.mock.calls[1][0]).toBe('/api/copilot/confirm') expect(fetchMock.mock.calls[1][1]?.body).toContain('"status":"background"') expect(fetchMock.mock.calls[1][1]?.body).toContain('"executionId":"exec-async"') - expect(saveExecutionPointer).toHaveBeenCalledWith({ - workflowId: 'wf-1', - executionId: 'exec-async', - lastEventId: 0, - }) - expect(clearExecutionPointer).toHaveBeenCalledWith('wf-1') + // An async run has no reconnectable stream, so it must never leave the + // terminal a pointer that a reconnect would 404 against. + expect(saveExecutionPointer).not.toHaveBeenCalled() + expect(clearExecutionPointer).not.toHaveBeenCalled() + expect(window.sessionStorage.getItem('sim:copilot:run-tool-completion:tool-async')).toBeNull() }) it('recovers a queued async launch by re-reporting it without enqueueing again', async () => { @@ -283,11 +383,10 @@ describe('run tool execution cancellation', () => { await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(6)) await vi.waitFor(() => expect(isRunToolActiveForId('tool-recover-async')).toBe(false)) - loadExecutionPointer.mockResolvedValueOnce({ - workflowId: 'wf-1', - executionId: 'exec-recover-async', - lastEventId: 0, - }) + expect(saveExecutionPointer).not.toHaveBeenCalled() + expect( + window.sessionStorage.getItem('sim:copilot:run-tool-completion:tool-recover-async') + ).toContain('"executionId":"exec-recover-async"') await expect(bindRunToolToExecution('tool-recover-async', 'wf-1')).resolves.toBe(true) @@ -298,6 +397,34 @@ describe('run tool execution cancellation', () => { expect( fetchMock.mock.calls.filter(([url]) => url === '/api/workflows/wf-1/execute') ).toHaveLength(1) + expect(clearExecutionPointer).not.toHaveBeenCalled() + expect( + window.sessionStorage.getItem('sim:copilot:run-tool-completion:tool-recover-async') + ).toBeNull() + }) + + it('cleans up the terminal pointer an earlier client left for an async launch', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true }) + vi.stubGlobal('fetch', fetchMock) + loadExecutionPointer.mockResolvedValueOnce({ + workflowId: 'wf-1', + executionId: 'exec-legacy-async', + lastEventId: 0, + }) + window.sessionStorage.setItem( + 'sim:copilot:run-tool-completion:tool-legacy-async', + JSON.stringify({ + status: 'background', + executionId: 'exec-legacy-async', + clearExecutionPointerAfterReport: true, + }) + ) + + await expect(bindRunToolToExecution('tool-legacy-async', 'wf-1')).resolves.toBe(true) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(fetchMock.mock.calls[0][1]?.body).toContain('"status":"background"') + expect(fetchMock.mock.calls[0][1]?.body).toContain('"executionId":"exec-legacy-async"') expect(clearExecutionPointer).toHaveBeenCalledWith('wf-1') }) diff --git a/apps/sim/lib/copilot/tools/client/run-tool-execution.ts b/apps/sim/lib/copilot/tools/client/run-tool-execution.ts index ec341f1491a..99ca035e6c2 100644 --- a/apps/sim/lib/copilot/tools/client/run-tool-execution.ts +++ b/apps/sim/lib/copilot/tools/client/run-tool-execution.ts @@ -45,11 +45,23 @@ const logger = createLogger('CopilotRunToolExecution') const activeRunToolByWorkflowId = new Map() const activeRunAbortByWorkflowId = new Map() const manuallyStoppedToolCallIds = new Set() +type RunToolReleaseListener = (workflowId: string) => void +const runToolReleaseListeners = new Set() const PENDING_COMPLETION_STORAGE_PREFIX = 'sim:copilot:run-tool-completion:' +/** + * Tab-local record of a completion this tab still owes Sim for a tool call, + * written just before the report is sent and cleared once it lands, so a reload + * mid-report can re-send it instead of re-running the tool. + */ interface PendingCompletionReport { status: AsyncConfirmationStatus executionId?: string + /** + * Written by earlier clients for async launches, which also wrote a terminal + * execution pointer for a run that has no reconnectable stream. Honoured so + * that pointer is cleaned up once the pending report is delivered. + */ clearExecutionPointerAfterReport?: boolean } @@ -165,13 +177,7 @@ async function enqueueAsyncWorkflowRun( const pendingCompletion: PendingCompletionReport = { status: ASYNC_TOOL_CONFIRMATION_STATUS.background, executionId: responseExecutionId, - clearExecutionPointerAfterReport: true, } - await saveExecutionPointer({ - workflowId, - executionId: responseExecutionId, - lastEventId: 0, - }) savePendingCompletionReport(toolCallId, pendingCompletion) try { @@ -183,7 +189,6 @@ async function enqueueAsyncWorkflowRun( pendingCompletion.executionId ) clearPendingCompletionReport(toolCallId) - await clearExecutionPointer(workflowId) } catch (error) { logger.error( '[RunTool] Async workflow was queued but background status could not be reported', @@ -249,6 +254,15 @@ function clearPendingCompletionReport(toolCallId: string): void { } } +/** + * Re-binds a tool call that the server still shows as executing to whatever + * this tab already knows about it, instead of running the tool again. + * + * Two tab-local records can answer: a pending completion report (a report this + * tab owed Sim and never delivered) is re-sent as is, and otherwise a terminal + * execution pointer (a live run this tab was observing) is reported as + * continuing in the background. With neither, the caller runs the tool. + */ export async function bindRunToolToExecution( toolCallId: string, workflowId: string @@ -271,21 +285,15 @@ export async function bindRunToolToExecution( } const pointer = await loadExecutionPointer(workflowId).catch(() => null) - if (!pointer?.executionId) { - logger.info('[RunTool] Recovery skipped: no tab-local execution pointer', { + const pendingCompletion = loadPendingCompletionReport(toolCallId) + if (pendingCompletion) { + const executionId = pendingCompletion.executionId ?? pointer?.executionId + logger.info('[RunTool] Recovery re-sending pending completion report', { workflowId, toolCallId, + executionId, + status: pendingCompletion.status, }) - return false - } - - logger.info('[RunTool] Recovery moved to background for existing execution pointer', { - workflowId, - toolCallId, - executionId: pointer.executionId, - }) - const pendingCompletion = loadPendingCompletionReport(toolCallId) - if (pendingCompletion) { try { await reportCompletion( toolCallId, @@ -294,7 +302,7 @@ export async function bindRunToolToExecution( pendingCompletion.status === MothershipStreamV1ToolOutcome.cancelled ? { reason: 'user_cancelled', cancelledByUser: true } : undefined, - pendingCompletion.executionId ?? pointer.executionId + executionId ) clearPendingCompletionReport(toolCallId) if (pendingCompletion.clearExecutionPointerAfterReport) { @@ -304,13 +312,27 @@ export async function bindRunToolToExecution( logger.warn('[RunTool] Failed to report recovered terminal completion', { workflowId, toolCallId, - executionId: pointer.executionId, + executionId, error: toError(error).message, }) } return true } + if (!pointer?.executionId) { + logger.info('[RunTool] Recovery skipped: no tab-local execution pointer', { + workflowId, + toolCallId, + }) + return false + } + + logger.info('[RunTool] Recovery moved to background for existing execution pointer', { + workflowId, + toolCallId, + executionId: pointer.executionId, + }) + try { await reportCompletion( toolCallId, @@ -375,6 +397,38 @@ export function isRunToolActiveForId(toolCallId: string): boolean { return false } +/** + * Whether a client run tool in this tab currently owns the workflow's run. + * + * While it does, its live execute stream is the source of truth for the run and + * for the completion it reports to Sim, so the terminal's reconnect flow must + * not claim the execution pointer the tool writes before the server has + * acknowledged the run. + */ +export function isRunToolActiveForWorkflow(workflowId: string): boolean { + return activeRunToolByWorkflowId.has(workflowId) +} + +/** + * Subscribes to a client run tool releasing a workflow run whose stream dropped + * before the run finished. The run keeps executing server-side and its + * execution pointer is retained, so a subscriber that can re-attach to the + * execution stream should do so once this fires. It does not fire for runs the + * tool observed to completion, even when reporting that completion failed. + */ +export function subscribeToRunToolRelease(listener: RunToolReleaseListener): () => void { + runToolReleaseListeners.add(listener) + return () => { + runToolReleaseListeners.delete(listener) + } +} + +function notifyRunToolReleased(workflowId: string): void { + for (const listener of runToolReleaseListeners) { + listener(workflowId) + } +} + export function cancelRunToolExecution(workflowId: string): void { const controller = activeRunAbortByWorkflowId.get(workflowId) if (!controller) return @@ -566,6 +620,7 @@ async function doExecuteRunTool( }) let leaveExecutionRecoverable = false + let streamInterrupted = false try { const result = await executeWorkflowWithFullLogging({ @@ -649,6 +704,7 @@ async function doExecuteRunTool( const msg = toError(err).message if (err instanceof SSEEventHandlerError || err instanceof SSEStreamInterruptedError) { leaveExecutionRecoverable = true + streamInterrupted = true logger.warn( '[RunTool] Execution stream interrupted; leaving workflow execution in background', { @@ -719,5 +775,8 @@ async function doExecuteRunTool( setIsExecuting(targetWorkflowId, false) setActiveBlocks(targetWorkflowId, new Set()) } + if (streamInterrupted && activeToolCallId === toolCallId) { + notifyRunToolReleased(targetWorkflowId) + } } }