From 9aa2eefdd57882a493bc700eeae2e9cc70f5b53b Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 5 Aug 2026 20:35:58 -0700 Subject: [PATCH] fix(workflows): await execution log finalization --- .../executor/execute-workflow.test.ts | 92 ++++++++++++++++++- .../workflows/executor/execute-workflow.ts | 7 ++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/workflows/executor/execute-workflow.test.ts b/apps/sim/lib/workflows/executor/execute-workflow.test.ts index 24cb260f3fd..5f147f2a2e6 100644 --- a/apps/sim/lib/workflows/executor/execute-workflow.test.ts +++ b/apps/sim/lib/workflows/executor/execute-workflow.test.ts @@ -11,12 +11,14 @@ const { handlePostExecutionPauseStateMock, loggingSessionConstructorMock, safeStartMock, + waitForPostExecutionMock, } = vi.hoisted(() => ({ captureServerEventMock: vi.fn(), executeWorkflowCoreMock: vi.fn(), handlePostExecutionPauseStateMock: vi.fn(), loggingSessionConstructorMock: vi.fn(), safeStartMock: vi.fn(), + waitForPostExecutionMock: vi.fn(), })) vi.mock('@sim/utils/id', () => ({ @@ -26,6 +28,7 @@ vi.mock('@sim/utils/id', () => ({ vi.mock('@/lib/logs/execution/logging-session', () => ({ LoggingSession: class { safeStart = safeStartMock + waitForPostExecution = waitForPostExecutionMock constructor(...args: unknown[]) { loggingSessionConstructorMock(...args) @@ -75,10 +78,11 @@ const workflow = { variables: {}, } -describe('executeWorkflow billing attribution', () => { +describe('executeWorkflow', () => { beforeEach(() => { vi.clearAllMocks() safeStartMock.mockResolvedValue(true) + waitForPostExecutionMock.mockResolvedValue(undefined) handlePostExecutionPauseStateMock.mockResolvedValue(undefined) executeWorkflowCoreMock.mockImplementation( async (params: { @@ -186,4 +190,90 @@ describe('executeWorkflow billing attribution', () => { expect.objectContaining({ trustedInitialResolvedSecretTraceProvenance: provenance }) ) }) + + it('waits for post-execution persistence before resolving', async () => { + let resolvePostExecution!: () => void + waitForPostExecutionMock.mockReturnValueOnce( + new Promise((resolve) => { + resolvePostExecution = resolve + }) + ) + + let executionSettled = false + const executionPromise = executeWorkflow( + workflow, + 'request-1', + { prompt: 'hello' }, + 'actor-1', + { + enabled: true, + billingAttribution, + } + ).then((result) => { + executionSettled = true + return result + }) + + await vi.waitFor(() => expect(waitForPostExecutionMock).toHaveBeenCalledOnce()) + expect(executionSettled).toBe(false) + + resolvePostExecution() + await executionPromise + + expect(executionSettled).toBe(true) + }) + + it('waits for post-execution persistence before rejecting', async () => { + const executionError = new Error('Request body size limit exceeded (10MB)') + executeWorkflowCoreMock.mockRejectedValueOnce(executionError) + + let resolvePostExecution!: () => void + waitForPostExecutionMock.mockReturnValueOnce( + new Promise((resolve) => { + resolvePostExecution = resolve + }) + ) + + let executionSettled = false + const executionPromise = executeWorkflow(workflow, 'request-1', undefined, 'actor-1', { + enabled: true, + billingAttribution, + }).catch((error: unknown) => { + executionSettled = true + throw error + }) + + await vi.waitFor(() => expect(waitForPostExecutionMock).toHaveBeenCalledOnce()) + expect(executionSettled).toBe(false) + + resolvePostExecution() + await expect(executionPromise).rejects.toBe(executionError) + expect(executionSettled).toBe(true) + }) + + it('transfers post-execution ownership with successful streaming metadata', async () => { + const result = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', { + enabled: true, + skipLoggingComplete: true, + billingAttribution, + }) + + expect(waitForPostExecutionMock).not.toHaveBeenCalled() + expect(result._streamingMetadata?.loggingSession).toBeDefined() + }) + + it('retains post-execution ownership when streaming execution rejects', async () => { + const executionError = new Error('Streaming execution failed') + executeWorkflowCoreMock.mockRejectedValueOnce(executionError) + + await expect( + executeWorkflow(workflow, 'request-1', undefined, 'actor-1', { + enabled: true, + skipLoggingComplete: true, + billingAttribution, + }) + ).rejects.toBe(executionError) + + expect(waitForPostExecutionMock).toHaveBeenCalledOnce() + }) }) diff --git a/apps/sim/lib/workflows/executor/execute-workflow.ts b/apps/sim/lib/workflows/executor/execute-workflow.ts index 98bdadb0546..ddba558d048 100644 --- a/apps/sim/lib/workflows/executor/execute-workflow.ts +++ b/apps/sim/lib/workflows/executor/execute-workflow.ts @@ -36,6 +36,7 @@ export interface ExecuteWorkflowOptions { executionOrder: number ) => Promise onBlockComplete?: (blockId: string, output: unknown) => Promise + /** Transfers post-execution logging ownership to the streaming caller after execution succeeds. */ skipLoggingComplete?: boolean includeFileBase64?: boolean base64MaxBytes?: number @@ -104,6 +105,7 @@ export async function executeWorkflow( const executionId = providedExecutionId || generateId() const triggerType = streamConfig?.workflowTriggerType || 'api' const loggingSession = new LoggingSession(workflowId, executionId, triggerType, requestId) + let postExecutionOwnershipTransferred = false try { const metadata: ExecutionMetadata = { @@ -201,6 +203,7 @@ export async function executeWorkflow( await handlePostExecutionPauseState({ result, workflowId, executionId, loggingSession }) if (streamConfig?.skipLoggingComplete) { + postExecutionOwnershipTransferred = true return { ...result, _streamingMetadata: { @@ -227,5 +230,9 @@ export async function executeWorkflow( ) throw error + } finally { + if (!postExecutionOwnershipTransferred) { + await loggingSession.waitForPostExecution() + } } }