Skip to content

Commit ec26ca8

Browse files
fix(workflows): await execution log finalization (#6309)
1 parent 1bbad84 commit ec26ca8

5 files changed

Lines changed: 103 additions & 4 deletions

File tree

.agents/skills/ship/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ When the user runs `/ship`:
6969
for s in check:boundaries check:api-validation:strict check:desktop-bridge check:desktop-ipc \
7070
check:utils check:zustand-v5 \
7171
check:react-query check:client-boundary check:bare-icons check:icon-paths \
72-
check:realtime-prune check:tool-registry-boundary tool-metadata:check \
72+
check:realtime-prune check:tool-registry-boundary check:tool-request-boundary \
73+
tool-metadata:check \
7374
integration-catalog:check skills:check agent-stream-docs:check; do
7475
( bun run "$s" >"/tmp/ship-audit-${s//:/-}.log" 2>&1; echo "$? $s" >>/tmp/ship-audit-results ) &
7576
done

.claude/commands/ship.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ When the user runs `/ship`:
6868
for s in check:boundaries check:api-validation:strict check:desktop-bridge check:desktop-ipc \
6969
check:utils check:zustand-v5 \
7070
check:react-query check:client-boundary check:bare-icons check:icon-paths \
71-
check:realtime-prune check:tool-registry-boundary tool-metadata:check \
71+
check:realtime-prune check:tool-registry-boundary check:tool-request-boundary \
72+
tool-metadata:check \
7273
integration-catalog:check skills:check agent-stream-docs:check; do
7374
( bun run "$s" >"/tmp/ship-audit-${s//:/-}.log" 2>&1; echo "$? $s" >>/tmp/ship-audit-results ) &
7475
done

.cursor/commands/ship.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ When the user runs `/ship`:
6363
for s in check:boundaries check:api-validation:strict check:desktop-bridge check:desktop-ipc \
6464
check:utils check:zustand-v5 \
6565
check:react-query check:client-boundary check:bare-icons check:icon-paths \
66-
check:realtime-prune check:tool-registry-boundary tool-metadata:check \
66+
check:realtime-prune check:tool-registry-boundary check:tool-request-boundary \
67+
tool-metadata:check \
6768
integration-catalog:check skills:check agent-stream-docs:check; do
6869
( bun run "$s" >"/tmp/ship-audit-${s//:/-}.log" 2>&1; echo "$? $s" >>/tmp/ship-audit-results ) &
6970
done

apps/sim/lib/workflows/executor/execute-workflow.test.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ const {
1111
handlePostExecutionPauseStateMock,
1212
loggingSessionConstructorMock,
1313
safeStartMock,
14+
waitForPostExecutionMock,
1415
} = vi.hoisted(() => ({
1516
captureServerEventMock: vi.fn(),
1617
executeWorkflowCoreMock: vi.fn(),
1718
handlePostExecutionPauseStateMock: vi.fn(),
1819
loggingSessionConstructorMock: vi.fn(),
1920
safeStartMock: vi.fn(),
21+
waitForPostExecutionMock: vi.fn(),
2022
}))
2123

2224
vi.mock('@sim/utils/id', () => ({
@@ -26,6 +28,7 @@ vi.mock('@sim/utils/id', () => ({
2628
vi.mock('@/lib/logs/execution/logging-session', () => ({
2729
LoggingSession: class {
2830
safeStart = safeStartMock
31+
waitForPostExecution = waitForPostExecutionMock
2932

3033
constructor(...args: unknown[]) {
3134
loggingSessionConstructorMock(...args)
@@ -75,10 +78,11 @@ const workflow = {
7578
variables: {},
7679
}
7780

78-
describe('executeWorkflow billing attribution', () => {
81+
describe('executeWorkflow', () => {
7982
beforeEach(() => {
8083
vi.clearAllMocks()
8184
safeStartMock.mockResolvedValue(true)
85+
waitForPostExecutionMock.mockResolvedValue(undefined)
8286
handlePostExecutionPauseStateMock.mockResolvedValue(undefined)
8387
executeWorkflowCoreMock.mockImplementation(
8488
async (params: {
@@ -186,4 +190,89 @@ describe('executeWorkflow billing attribution', () => {
186190
expect.objectContaining({ trustedInitialResolvedSecretTraceProvenance: provenance })
187191
)
188192
})
193+
it('waits for post-execution persistence before resolving', async () => {
194+
let resolvePostExecution!: () => void
195+
waitForPostExecutionMock.mockReturnValueOnce(
196+
new Promise<void>((resolve) => {
197+
resolvePostExecution = resolve
198+
})
199+
)
200+
201+
let executionSettled = false
202+
const executionPromise = executeWorkflow(
203+
workflow,
204+
'request-1',
205+
{ prompt: 'hello' },
206+
'actor-1',
207+
{
208+
enabled: true,
209+
billingAttribution,
210+
}
211+
).then((result) => {
212+
executionSettled = true
213+
return result
214+
})
215+
216+
await vi.waitFor(() => expect(waitForPostExecutionMock).toHaveBeenCalledOnce())
217+
expect(executionSettled).toBe(false)
218+
219+
resolvePostExecution()
220+
await executionPromise
221+
222+
expect(executionSettled).toBe(true)
223+
})
224+
225+
it('waits for post-execution persistence before rejecting', async () => {
226+
const executionError = new Error('Request body size limit exceeded (10MB)')
227+
executeWorkflowCoreMock.mockRejectedValueOnce(executionError)
228+
229+
let resolvePostExecution!: () => void
230+
waitForPostExecutionMock.mockReturnValueOnce(
231+
new Promise<void>((resolve) => {
232+
resolvePostExecution = resolve
233+
})
234+
)
235+
236+
let executionSettled = false
237+
const executionPromise = executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
238+
enabled: true,
239+
billingAttribution,
240+
}).catch((error: unknown) => {
241+
executionSettled = true
242+
throw error
243+
})
244+
245+
await vi.waitFor(() => expect(waitForPostExecutionMock).toHaveBeenCalledOnce())
246+
expect(executionSettled).toBe(false)
247+
248+
resolvePostExecution()
249+
await expect(executionPromise).rejects.toBe(executionError)
250+
expect(executionSettled).toBe(true)
251+
})
252+
253+
it('transfers post-execution ownership with successful streaming metadata', async () => {
254+
const result = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
255+
enabled: true,
256+
skipLoggingComplete: true,
257+
billingAttribution,
258+
})
259+
260+
expect(waitForPostExecutionMock).not.toHaveBeenCalled()
261+
expect(result._streamingMetadata?.loggingSession).toBeDefined()
262+
})
263+
264+
it('retains post-execution ownership when streaming execution rejects', async () => {
265+
const executionError = new Error('Streaming execution failed')
266+
executeWorkflowCoreMock.mockRejectedValueOnce(executionError)
267+
268+
await expect(
269+
executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
270+
enabled: true,
271+
skipLoggingComplete: true,
272+
billingAttribution,
273+
})
274+
).rejects.toBe(executionError)
275+
276+
expect(waitForPostExecutionMock).toHaveBeenCalledOnce()
277+
})
189278
})

apps/sim/lib/workflows/executor/execute-workflow.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export interface ExecuteWorkflowOptions {
3636
executionOrder: number
3737
) => Promise<void>
3838
onBlockComplete?: (blockId: string, output: unknown) => Promise<void>
39+
/** Transfers post-execution logging ownership to the streaming caller after execution succeeds. */
3940
skipLoggingComplete?: boolean
4041
includeFileBase64?: boolean
4142
base64MaxBytes?: number
@@ -104,6 +105,7 @@ export async function executeWorkflow(
104105
const executionId = providedExecutionId || generateId()
105106
const triggerType = streamConfig?.workflowTriggerType || 'api'
106107
const loggingSession = new LoggingSession(workflowId, executionId, triggerType, requestId)
108+
let postExecutionOwnershipTransferred = false
107109

108110
try {
109111
const metadata: ExecutionMetadata = {
@@ -201,6 +203,7 @@ export async function executeWorkflow(
201203
await handlePostExecutionPauseState({ result, workflowId, executionId, loggingSession })
202204

203205
if (streamConfig?.skipLoggingComplete) {
206+
postExecutionOwnershipTransferred = true
204207
return {
205208
...result,
206209
_streamingMetadata: {
@@ -227,5 +230,9 @@ export async function executeWorkflow(
227230
)
228231

229232
throw error
233+
} finally {
234+
if (!postExecutionOwnershipTransferred) {
235+
await loggingSession.waitForPostExecution()
236+
}
230237
}
231238
}

0 commit comments

Comments
 (0)