From 30313b0570d39d75f527996d7116ba63009765e4 Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Tue, 5 May 2026 09:19:35 +0900 Subject: [PATCH 1/2] [core] V2: skip inline step execution when suspension also has a wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inline `await executeStep(...)` blocks the V2 handler for the full step duration, but `wait_completed` events are only created on the *next* loop iteration's "complete elapsed waits" pass. This breaks `Promise.race(step, sleep)` semantics whenever the sleep is shorter than the step — replay still picks the step because the sleep's wait_completed event hasn't been written yet. Reproducer (`sleepWinsRaceWorkflow`): a 1s sleep raced against a 10s step. Expected `'sleep'` to win; the runtime returned `'step'`. On the failing run, `wait_completed` was created at t=11.86s — right after `step_completed` — instead of at t≈2.74s when its `resumeAt` elapsed. Fix: in `runtime.ts`, gate the inline-step pick on the absence of a wait timeout. When `suspensionResult.timeoutSeconds !== undefined`, queue every pending step instead and return with the wait timeout. This lets the wait timer drive a continuation in parallel, matching V1's behavior where each step ran in a separate function invocation. Pure step suspensions (without waits) still benefit from inline execution. Test plan: - New e2e tests `sleepWinsRaceWorkflow` and `stepWinsRaceWorkflow` exercising `Promise.race` between a step function and `sleep()`, in both directions. - Verified locally against `nextjs-turbopack` workbench: both pass. Event log confirms `wait_completed` is now created on time (t≈1s after `wait_created`) rather than after the inline step. Eager-processing changelog updated with a "Mixed Suspensions" section describing the carve-out and its rationale. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/fix-step-vs-wait-race.md | 5 ++++ .../docs/changelog/eager-processing.mdx | 10 ++++++- packages/core/e2e/e2e.test.ts | 16 ++++++++++ packages/core/src/runtime.ts | 19 +++++++++++- workbench/example/workflows/99_e2e.ts | 30 +++++++++++++++++++ 5 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-step-vs-wait-race.md diff --git a/.changeset/fix-step-vs-wait-race.md b/.changeset/fix-step-vs-wait-race.md new file mode 100644 index 0000000000..7cd87c6574 --- /dev/null +++ b/.changeset/fix-step-vs-wait-race.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Fix `Promise.race(step, sleep)` semantics in V2 mixed suspensions: when a workflow suspension contains both pending steps and at least one wait (sleep), the runtime now queues every step instead of executing one inline. Inline `await executeStep(...)` blocks the handler for the full step duration, so the wait timer never fires on time — a 1s sleep racing a 10s step would silently resolve to the step. Queueing the step in this case lets the wait timeout drive a continuation in parallel, restoring V1's race semantics. diff --git a/docs/content/docs/changelog/eager-processing.mdx b/docs/content/docs/changelog/eager-processing.mdx index 32218b33cc..d163d13c42 100644 --- a/docs/content/docs/changelog/eager-processing.mdx +++ b/docs/content/docs/changelog/eager-processing.mdx @@ -230,7 +230,15 @@ When an inline step fails with retries remaining: ### Mixed Suspensions -A suspension may contain steps, hooks, and waits simultaneously. The handler creates events for all, executes any pending step inline, and returns with the wait timeout if applicable. The workflow will re-suspend on next replay for the still-pending hooks/waits. +A suspension may contain steps, hooks, and waits simultaneously. The handler creates events for all, then chooses between inline execution and queue dispatch: + +- **Steps only** (no waits): one owned step is executed inline; the rest are queued. The loop continues after the inline step completes. +- **Steps + at least one wait**: every step is queued (no inline execution). The handler returns with the wait timeout. Whichever lands first — a step's continuation or the wait timer — drives the next replay. +- **Hooks / waits only**: handler returns with the wait timeout (or no timeout, for hook-only suspensions). The next continuation is driven by external resume or the wait timer. + +The "no inline when there's a wait" carve-out is necessary to preserve `Promise.race(step, sleep)` semantics. Inline `await executeStep(...)` blocks the handler for the full step duration, and `wait_completed` events are only created on the *next* loop iteration's "complete elapsed waits" pass — so a longer-running step would always swallow the shorter sleep and `Promise.race` would resolve incorrectly. Queueing the step in this case lets the wait timer drive a continuation in parallel, matching V1's behavior where each step ran in a separate function invocation. + +Pure step suspensions (without waits) still benefit from inline execution; the carve-out only costs an extra queue roundtrip when a step and a sleep coexist. ### Hook Conflicts diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts index 76d9fc88dd..e10b0f3ab5 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -581,6 +581,22 @@ describe('e2e', () => { expect(elapsed).toBeLessThan(25_000); }); + test('sleepWinsRaceWorkflow', { timeout: 60_000 }, async () => { + const run = await start(await e2e('sleepWinsRaceWorkflow'), []); + const returnValue = await run.returnValue; + expect(returnValue.winner).toBe('sleep'); + // Sleep is 1s; step would take 10s. Should resolve in ~1s, well under 5s. + expect(returnValue.durationMs).toBeLessThan(5_000); + }); + + test('stepWinsRaceWorkflow', { timeout: 60_000 }, async () => { + const run = await start(await e2e('stepWinsRaceWorkflow'), []); + const returnValue = await run.returnValue; + expect(returnValue.winner).toBe('step'); + // Step is 1s; sleep would take 10s. Should resolve in ~1s, well under 5s. + expect(returnValue.durationMs).toBeLessThan(5_000); + }); + test('nullByteWorkflow', { timeout: 60_000 }, async () => { const run = await start(await e2e('nullByteWorkflow'), []); const returnValue = await run.returnValue; diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 3c2bfb8f01..2407dca784 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -882,9 +882,26 @@ export function workflowEntrypoint( // Pick one owned step to execute inline (if any). // The rest of the pending steps are queued below. + // + // Skip inline execution entirely when the suspension + // also has a pending wait (sleep): an inline `await + // executeStep(...)` blocks the handler for the full + // step duration, so the wait timer never has a chance + // to fire on time. That defeats `Promise.race(step, + // sleep)` semantics — if the sleep is shorter than + // the step, replay still picks the step because + // wait_completed is only created on the *next* loop + // iteration, which doesn't run until the step + // finishes. Queueing every step in this case lets + // the wait timeout drive a continuation in parallel, + // matching V1's behavior where each step ran in a + // separate function invocation. const inlineStep: | (typeof pendingSteps)[number] - | undefined = ownedPendingSteps[0]; + | undefined = + suspensionResult.timeoutSeconds === undefined + ? ownedPendingSteps[0] + : undefined; // Queue every pending step except the one we're // executing inline. This mirrors V1's unconditional diff --git a/workbench/example/workflows/99_e2e.ts b/workbench/example/workflows/99_e2e.ts index 131535650e..54c46fe725 100644 --- a/workbench/example/workflows/99_e2e.ts +++ b/workbench/example/workflows/99_e2e.ts @@ -215,6 +215,36 @@ export async function parallelSleepWorkflow() { ////////////////////////////////////////////////////////// +async function delayMsStep(ms: number, label: string) { + 'use step'; + await new Promise((resolve) => setTimeout(resolve, ms)); + return label; +} + +export async function sleepWinsRaceWorkflow() { + 'use workflow'; + const startTime = Date.now(); + const winner = await Promise.race([ + delayMsStep(10_000, 'step'), + sleep('1s').then(() => 'sleep'), + ]); + const endTime = Date.now(); + return { winner, durationMs: endTime - startTime }; +} + +export async function stepWinsRaceWorkflow() { + 'use workflow'; + const startTime = Date.now(); + const winner = await Promise.race([ + delayMsStep(1_000, 'step'), + sleep('10s').then(() => 'sleep'), + ]); + const endTime = Date.now(); + return { winner, durationMs: endTime - startTime }; +} + +////////////////////////////////////////////////////////// + async function nullByteStep() { 'use step'; return 'null byte \0'; From ee02092f614c48f9de486c4168b364576b8de70d Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Tue, 5 May 2026 09:22:56 +0900 Subject: [PATCH 2/2] Apply suggestion from @VaguelySerious Signed-off-by: Peter Wielander --- .changeset/fix-step-vs-wait-race.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fix-step-vs-wait-race.md b/.changeset/fix-step-vs-wait-race.md index 7cd87c6574..0449254f40 100644 --- a/.changeset/fix-step-vs-wait-race.md +++ b/.changeset/fix-step-vs-wait-race.md @@ -2,4 +2,4 @@ "@workflow/core": patch --- -Fix `Promise.race(step, sleep)` semantics in V2 mixed suspensions: when a workflow suspension contains both pending steps and at least one wait (sleep), the runtime now queues every step instead of executing one inline. Inline `await executeStep(...)` blocks the handler for the full step duration, so the wait timer never fires on time — a 1s sleep racing a 10s step would silently resolve to the step. Queueing the step in this case lets the wait timeout drive a continuation in parallel, restoring V1's race semantics. +Fix `Promise.race(step, sleep)` always blocking until step completed