Uh oh!
There was an error while loading. Please reload this page.
[core] Remove duplicate waitUntil for suspension handler async operations - #2345
Merged
Conversation
…lete The suspension handler registered the step_created / wait_created event writes on a detached waitUntil(Promise.all(ops)) in addition to awaiting them. The detached copy framed those progress-critical writes as droppable background work and re-threw send failures into a promise nothing consumes (unhandled rejection -> process exit 128). It now strictly awaits the writes only, preserving the ack-ordering invariant: the orchestrator message is not acked until the writes (and the dispatch sends the caller makes afterward) complete, so a crash before then leaves the message un-acked for VQS to redeliver instead of orphaning the step. Adds runtime.test.ts ack-ordering tests: the dispatch send must complete before ack, a hanging send must not ack, and a failing send must reject the handler (no ack) without rejecting any waitUntil promise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 07fd5f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enforces a crash-safety invariant in the core runtime: the orchestrator’s queue message must not be acked until all progress-critical work is durably complete (notably step_created / wait_created event writes), so redelivery/replay can safely recover from crashes without orphaning steps.
Changes:
handleSuspension()no longer registers progress-critical event writes on a detachedwaitUntil(Promise.all(...)); it strictly awaits them.- Adds a new
workflowEntrypoint step-dispatch ack orderingtest suite to ensure step-dispatch sends complete (or fail/hang) before the handler resolves (ack). - Adds a changeset documenting the behavioral fix for
@workflow/core.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/runtime/suspension-handler.ts | Removes detached waitUntil usage for progress-gating event writes; keeps strict await to preserve ack ordering. |
| packages/core/src/runtime.test.ts | Adds ack-ordering regression tests around step dispatch and waitUntil behavior. |
| .changeset/ack-after-step-dispatch.md | Documents the patch-level behavioral fix and rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+636
to
+638
| }) { | ||
| const workflowRun = await makeRunningRun(opts.runId); | ||
| const order: string[] = []; |
ContributorAuthor
There was a problem hiding this comment.
Done — waitUntilPromises.length = 0 now runs at the start of driveHandler(), so the rejection check only ever observes promises that invocation registered (robust against reordering/.only, not just the per-test afterEach). Fixed in 8d28013.
Comment on lines
21
to
25
| vi.mock('@vercel/functions', () => ({ | ||
| waitUntil: vi.fn(), | ||
| waitUntil: vi.fn((p: Promise<unknown>) => { | ||
| waitUntilPromises.push(p); | ||
| }), | ||
| })); |
ContributorAuthor
There was a problem hiding this comment.
Good catch. The mock now attaches a no-op .catch() to each promise the instant it's recorded, while still pushing the original promise into waitUntilPromises for later allSettled inspection — so a rejection can never surface as a real unhandled rejection in the test process before anyWaitUntilPromiseRejected() runs. Fixed in 8d28013.
pranaygp
commented
Jun 11, 2026
| '@workflow/core': patch | ||
| --- | ||
| Never ack the orchestrator's queue message before the events that make the run progress are durably written. The suspension handler used to register the `step_created` / `wait_created` writes on a detached `waitUntil(Promise.all(ops))` in addition to awaiting them; the detached copy framed those progress-critical writes as droppable background work and re-threw send failures into a promise nothing consumes (unhandled rejection → process exit 128). It now strictly awaits the writes only, so the orchestrator message is never acked until they complete — a crash before then leaves the message un-acked and VQS redelivers within the lease, re-creating the (idempotent) entities and re-dispatching instead of orphaning the step. Complements the crash fix in #2336. |
ContributorAuthor
There was a problem hiding this comment.
too verbose. match the format/terseness of other changesets
ContributorAuthor
There was a problem hiding this comment.
Trimmed to match the sibling changesets (now ~4 lines, in line with abort-e2e-flakes.md etc). Fixed in 8d28013.
- Trim the changeset to match the terseness of sibling changesets. - Attach a no-op .catch() to each promise the waitUntil mock records so a rejecting promise can never surface as a real unhandled rejection in the test process; keep the original promise for allSettled inspection. - Reset waitUntilPromises at the start of driveHandler() so the rejection check only observes this invocation's promises (robust to reordering/.only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
VaguelySerious
approved these changes
Jun 11, 2026
step_created/wait_created calls fail in waitUntilstep_created/wait_created calls fail in waitUntilwaitUntil for suspension handler async operationsUh oh!
There was an error while loading. Please reload this page.
github-actionsBot
added a commit
that referenced
this pull request
Jun 11, 2026
…ations (#2345) Co-authored-by: Peter Wielander <mittgfu@gmail.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
Contributor
Backport PR opened against |
Merged
This was referenced Jun 11, 2026
pranaygp added a commit
that referenced
this pull request
Jun 11, 2026
…hook-ready * origin-https/main: (119 commits) fix: settle aborted parallel steps before completing abortParallelWorkflow (#2244) Add native v4 workflow attribute events (#2226) Version Packages (beta) (#2326) [ci] Fix flaky windows unit tests (#2359) Capture Vercel runtime logs when e2e Vercel Prod lanes fail (#2356) Fix e2e failure reporting under vitest 4 and preserve fetch error causes (#2355) [core] Fix process crash from rejected waitUntil promises (#2336) [core] Remove duplicate `waitUntil` for suspension handler async operations (#2345) Prevent local tests from hanging (#2338) feat(core): add optional namespace for queue topic prefix (#2305) [web-shared] Show precise durations in the new trace viewer (#2335) Validate unique workflow step IDs at build time (#2018) Move run attributes into their own detail card (#2327) [core] Forward-port stream reconnect to getReadable level (#2318) [docs] Add "Step executed multiple times" error page (#2310) Fix flickering on the detail panel when navigating the trace viewer (#2325) Version Packages (beta) (#2254) Patch vulnerable package dependencies (#2301) Deprecate DurableAgent and update it with WorkflowAgent in v5 docs (#2285) [codex] Fix partial world-local exclusive writes (#2296) ... # Conflicts: # packages/core/src/runtime.ts # packages/core/src/runtime/suspension-handler.ts
pranaygp added a commit
that referenced
this pull request
Jun 11, 2026
…t-race-eager-wait * origin/main: docs(observability): remove MVP implementation detail bullet (#2367) fix: settle aborted parallel steps before completing abortParallelWorkflow (#2244) Add native v4 workflow attribute events (#2226) Version Packages (beta) (#2326) [ci] Fix flaky windows unit tests (#2359) Capture Vercel runtime logs when e2e Vercel Prod lanes fail (#2356) Fix e2e failure reporting under vitest 4 and preserve fetch error causes (#2355) [core] Fix process crash from rejected waitUntil promises (#2336) [core] Remove duplicate `waitUntil` for suspension handler async operations (#2345)
pranaygp added a commit
that referenced
this pull request
Jun 11, 2026
…docs-run-idempotency * origin-https/main: (115 commits) [core] V2: unify wait+step queue dispatch in suspension processing (#1925) fix(world-local,world-postgres): make duplicate hook_created idempotent (#2295) docs(observability): remove MVP implementation detail bullet (#2367) fix: settle aborted parallel steps before completing abortParallelWorkflow (#2244) Add native v4 workflow attribute events (#2226) Version Packages (beta) (#2326) [ci] Fix flaky windows unit tests (#2359) Capture Vercel runtime logs when e2e Vercel Prod lanes fail (#2356) Fix e2e failure reporting under vitest 4 and preserve fetch error causes (#2355) [core] Fix process crash from rejected waitUntil promises (#2336) [core] Remove duplicate `waitUntil` for suspension handler async operations (#2345) Prevent local tests from hanging (#2338) feat(core): add optional namespace for queue topic prefix (#2305) [web-shared] Show precise durations in the new trace viewer (#2335) Validate unique workflow step IDs at build time (#2018) Move run attributes into their own detail card (#2327) [core] Forward-port stream reconnect to getReadable level (#2318) [docs] Add "Step executed multiple times" error page (#2310) Fix flickering on the detail panel when navigating the trace viewer (#2325) Version Packages (beta) (#2254) ... # Conflicts: # docs/content/docs/v4/cookbook/advanced/child-workflows.mdx # docs/content/docs/v5/cookbook/advanced/child-workflows.mdx # docs/content/docs/v5/cookbook/agent-patterns/durable-agent.mdx
pranaygp added a commit
that referenced
this pull request
Jun 11, 2026
* origin/main: [core] V2: unify wait+step queue dispatch in suspension processing (#1925) fix(world-local,world-postgres): make duplicate hook_created idempotent (#2295) docs(observability): remove MVP implementation detail bullet (#2367) fix: settle aborted parallel steps before completing abortParallelWorkflow (#2244) Add native v4 workflow attribute events (#2226) Version Packages (beta) (#2326) [ci] Fix flaky windows unit tests (#2359) Capture Vercel runtime logs when e2e Vercel Prod lanes fail (#2356) Fix e2e failure reporting under vitest 4 and preserve fetch error causes (#2355) [core] Fix process crash from rejected waitUntil promises (#2336) [core] Remove duplicate `waitUntil` for suspension handler async operations (#2345) Prevent local tests from hanging (#2338) feat(core): add optional namespace for queue topic prefix (#2305) [web-shared] Show precise durations in the new trace viewer (#2335) Validate unique workflow step IDs at build time (#2018) Move run attributes into their own detail card (#2327) [core] Forward-port stream reconnect to getReadable level (#2318) [docs] Add "Step executed multiple times" error page (#2310) Fix flickering on the detail panel when navigating the trace viewer (#2325)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Peter: Ignore all the below, this PR is smaller in scope than it leads on. Read the code, it's a few lines.
The invariant
Never ack the orchestrator's queue message until all work that makes the run progress is durably complete — including creating AND enqueueing subsequent steps. Queue sends are idempotent (idempotency keys), so a crash before ack yields a redelivery → replay → safe re-send, recovering within the VQS ~300s lease instead of orphaning the step. The codebase already states this principle for timeout re-enqueues (
packages/world-vercel/src/queue.ts: "Send new message BEFORE acknowledging current message. This ensures crash safety…"); this PR makes it hold for the suspension-handler's progress-critical event writes.Production evidence
Run
wrun_01KTSMXMKD7C544YFHX0GNPEG1(prod,codeReviewWorkflowV2, dpldpl_F4kcruzPhU3EY6Mbo3vqvEPAvRYM, 2026-06-10): the deployment was crash-looping with exit-128 (unhandled-rejection; fix in open PR #2336).step_createdcommitted at 21:03:32.611; VQS shows zero step-queue[SEND]s for the run for 4h35m (the dispatch never happened); no lease redeliveries fired in the gap — the orchestrator message chain had concluded (= acked). A PR webhook at 01:38:40 finally re-drove the orchestrator, which rediscovered and dispatched the step in ~1s. ~130 warnings/day across 89 runs on one team share the signature, with orphan windows up to ~24h.What changed
packages/core/src/runtime/suspension-handler.tspreviously registered thestep_created/wait_createdevent writes (ops) on a detachedwaitUntil(Promise.all(ops))in addition to awaiting them on the next line. The detached copy:The fix removes the detached
waitUntilregistration and keeps only the strictawait Promise.all(ops). On this branch the step-dispatchqueueMessagesends already live inruntime.ts(workflowEntrypoint) and are awaited there before the handler returns; this change ensures thestep_createdevents those dispatches depend on are likewise strictly awaited, so the whole progress-critical chain completes before the queue handler acks the orchestrator message.What stayed background (and why)
The
waitUntil(Promise.all(ops))registrations instep-handler.ts,start.ts, andresume-hook.tswere not touched: theiropsare stream-flush writes (not progress-gating), have no awaited copy, and legitimately needwaitUntilto keep the function alive. The orchestrator re-trigger / step-continuationqueueMessagesends in those files are already awaited. PR #2336 hardens those background sites with asafeWaitUntilhelper.Test coverage
Adds an
ack orderingsuite topackages/core/src/runtime.test.tsdriving the realworkflowEntrypointagainst a workflow that suspends on a step + sleep (so the step is queued, not run inline):queue()send completes before the ack sentinel (andstep_createdprecedes the dispatch);waitUntil.Relationship to #2336
Orthogonal and complementary. #2336 stops the crash class (rejected
waitUntilpromises → exit 128) so the queue can re-drive normally. This PR guarantees crash recovery via redelivery regardless of why the process crashed by enforcing the ack-ordering invariant. Both editsuspension-handler.ts; #2336 also drops the samewaitUntilregistration there, so expect a trivial rebase overlap (the resolution is identical — drop the registration, keep the await).