Skip to content

Commit 0ebbcc7

Browse files
fix(workflows): await execution log finalization (#6309)
1 parent 721b471 commit 0ebbcc7

5 files changed

Lines changed: 104 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: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const {
1313
loggingSessionConstructorMock,
1414
projectDiagnosticErrorMock,
1515
safeStartMock,
16+
waitForPostExecutionMock,
1617
setTrustedExecutionCorrelationMock,
1718
} = vi.hoisted(() => ({
1819
captureServerEventMock: vi.fn(),
@@ -21,6 +22,7 @@ const {
2122
loggingSessionConstructorMock: vi.fn(),
2223
projectDiagnosticErrorMock: vi.fn(),
2324
safeStartMock: vi.fn(),
25+
waitForPostExecutionMock: vi.fn(),
2426
setTrustedExecutionCorrelationMock: vi.fn(),
2527
}))
2628

@@ -32,6 +34,7 @@ vi.mock('@/lib/logs/execution/logging-session', () => ({
3234
LoggingSession: class {
3335
projectDiagnosticError = projectDiagnosticErrorMock
3436
safeStart = safeStartMock
37+
waitForPostExecution = waitForPostExecutionMock
3538
setTrustedExecutionCorrelation = setTrustedExecutionCorrelationMock
3639

3740
constructor(...args: unknown[]) {
@@ -89,10 +92,11 @@ const workflow = {
8992
variables: {},
9093
}
9194

92-
describe('executeWorkflow billing attribution', () => {
95+
describe('executeWorkflow', () => {
9396
beforeEach(() => {
9497
vi.clearAllMocks()
9598
safeStartMock.mockResolvedValue(true)
99+
waitForPostExecutionMock.mockResolvedValue(undefined)
96100
projectDiagnosticErrorMock.mockImplementation(
97101
(error: unknown, details: Record<string, unknown> = {}) => ({
98102
...details,
@@ -208,6 +212,92 @@ describe('executeWorkflow billing attribution', () => {
208212
)
209213
})
210214

215+
it('waits for post-execution persistence before resolving', async () => {
216+
let resolvePostExecution!: () => void
217+
waitForPostExecutionMock.mockReturnValueOnce(
218+
new Promise<void>((resolve) => {
219+
resolvePostExecution = resolve
220+
})
221+
)
222+
223+
let executionSettled = false
224+
const executionPromise = executeWorkflow(
225+
workflow,
226+
'request-1',
227+
{ prompt: 'hello' },
228+
'actor-1',
229+
{
230+
enabled: true,
231+
billingAttribution,
232+
}
233+
).then((result) => {
234+
executionSettled = true
235+
return result
236+
})
237+
238+
await vi.waitFor(() => expect(waitForPostExecutionMock).toHaveBeenCalledOnce())
239+
expect(executionSettled).toBe(false)
240+
241+
resolvePostExecution()
242+
await executionPromise
243+
244+
expect(executionSettled).toBe(true)
245+
})
246+
247+
it('waits for post-execution persistence before rejecting', async () => {
248+
const executionError = new Error('Request body size limit exceeded (10MB)')
249+
executeWorkflowCoreMock.mockRejectedValueOnce(executionError)
250+
251+
let resolvePostExecution!: () => void
252+
waitForPostExecutionMock.mockReturnValueOnce(
253+
new Promise<void>((resolve) => {
254+
resolvePostExecution = resolve
255+
})
256+
)
257+
258+
let executionSettled = false
259+
const executionPromise = executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
260+
enabled: true,
261+
billingAttribution,
262+
}).catch((error: unknown) => {
263+
executionSettled = true
264+
throw error
265+
})
266+
267+
await vi.waitFor(() => expect(waitForPostExecutionMock).toHaveBeenCalledOnce())
268+
expect(executionSettled).toBe(false)
269+
270+
resolvePostExecution()
271+
await expect(executionPromise).rejects.toBe(executionError)
272+
expect(executionSettled).toBe(true)
273+
})
274+
275+
it('transfers post-execution ownership with successful streaming metadata', async () => {
276+
const result = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
277+
enabled: true,
278+
skipLoggingComplete: true,
279+
billingAttribution,
280+
})
281+
282+
expect(waitForPostExecutionMock).not.toHaveBeenCalled()
283+
expect(result._streamingMetadata?.loggingSession).toBeDefined()
284+
})
285+
286+
it('retains post-execution ownership when streaming execution rejects', async () => {
287+
const executionError = new Error('Streaming execution failed')
288+
executeWorkflowCoreMock.mockRejectedValueOnce(executionError)
289+
290+
await expect(
291+
executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
292+
enabled: true,
293+
skipLoggingComplete: true,
294+
billingAttribution,
295+
})
296+
).rejects.toBe(executionError)
297+
298+
expect(waitForPostExecutionMock).toHaveBeenCalledOnce()
299+
})
300+
211301
it('persists server-issued workflow-group correlation in execution metadata', async () => {
212302
const correlation = {
213303
executionId: 'execution-1',

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
@@ -109,6 +110,7 @@ export async function executeWorkflow(
109110
if (streamConfig?.trustedExecutionCorrelation) {
110111
loggingSession.setTrustedExecutionCorrelation(streamConfig.trustedExecutionCorrelation)
111112
}
113+
let postExecutionOwnershipTransferred = false
112114

113115
try {
114116
const metadata: ExecutionMetadata = {
@@ -207,6 +209,7 @@ export async function executeWorkflow(
207209
await handlePostExecutionPauseState({ result, workflowId, executionId, loggingSession })
208210

209211
if (streamConfig?.skipLoggingComplete) {
212+
postExecutionOwnershipTransferred = true
210213
return {
211214
...result,
212215
_streamingMetadata: {
@@ -237,5 +240,9 @@ export async function executeWorkflow(
237240
)
238241

239242
throw error
243+
} finally {
244+
if (!postExecutionOwnershipTransferred) {
245+
await loggingSession.waitForPostExecution()
246+
}
240247
}
241248
}

0 commit comments

Comments
 (0)