feat(recovery): start the World at server boot to recover in-flight runs#2544
feat(recovery): start the World at server boot to recover in-flight runs#2544pranaygp wants to merge 17 commits into
Conversation
Self-hosted worlds (local, postgres) run boot-time recovery (`reenqueueActiveRuns`) inside `world.start()`, but nothing called it at server startup — so a process that restarted mid-flight never resumed its in-flight runs without a workflow operation. - core: add idempotent `ensureWorldStarted()` (once-per-process), exported from `@workflow/core/runtime` and `workflow/runtime`. - world-vercel: add a no-op `start()` for interface compliance (push-based; VQS redelivers, no boot recovery needed). Document the `start()` contract. - framework startup wiring (un-gated; no-op on Vercel): Next workbench `instrumentation.ts`, a Nitro server plugin (covers express/hono/fastify/ nuxt), un-gate SvelteKit `init` + Nest `bootstrap`, Astro middleware. - test: kill/restart e2e proving an in-flight sleeping run resumes after a hard restart with no workflow op; fails if startup wiring is removed. Covers local + postgres. New `e2e-restart-recovery` CI job. - docs: deploying/recovering-in-flight-runs (v4 + v5). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 34ea8fb The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) 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 | Express | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 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: Express | Nitro | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | 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: Nitro | Express | Next.js (Turbopack) 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: Express | Nitro | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 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: Nitro | Express | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
|
||
| You can call this regardless of which World you target. On the [Vercel World](/docs/deploying/world/vercel-world) it is a no-op — delivery is push-based and the queue redelivers in-flight messages on its own, so there is no long-lived process to recover. | ||
|
|
||
| ## Wiring it per framework |
There was a problem hiding this comment.
we should also have these ben an optional accordion/compressed setup step mentioned in each of the framework's getting started guides. the step should state this this is not required for vercel deployments (serverless/push based queue worlds) but required for pull based/worker based workflow sdk deployments. and it can link to this docs pages for details
There was a problem hiding this comment.
same for v4 and v5 docs
There was a problem hiding this comment.
Done in 6116307 — added an optional, collapsed accordion ("Recover in-flight runs after a restart") to each framework's getting-started guide (Next/Nitro/Express/Hono/Fastify/Nuxt/Vite/TanStack Start/SvelteKit/Nest/Astro, v4 + v5). Each shows the framework's startup snippet, notes it is not required for Vercel deployments, and links to the full Recovering in-flight runs page.
There was a problem hiding this comment.
Superseded by your later comment: rather than accordions in each getting-started guide, the startup step now lives in the local-world and postgres-world docs (the worlds that actually need it), with this page as the framework-wide reference. Done.
| { | ||
| "title": "Deploying", | ||
| "pages": ["...deploying", "building-a-world"] | ||
| "pages": ["...deploying", "building-a-world", "recovering-in-flight-runs"] |
There was a problem hiding this comment.
put this before "building a world"
There was a problem hiding this comment.
same for v4 and v5
There was a problem hiding this comment.
Done in 6116307 — moved recovering-in-flight-runs before building-a-world in the Deploying nav (v4 + v5).
There was a problem hiding this comment.
Done — "Recovering in-flight runs" now sorts before "Building a World" in both the v4 and v5 meta.json.
CI surfaced two failures from un-gating `world.start()` at server boot: - world-local: `initDataDir` ran `parseVersion()` on the `bundled` sentinel that `getPackageInfo()` returns in framework server bundles, throwing `Invalid version string: "bundled"` and crashing startup (500s on sveltekit/astro/vite/etc.). Skip version-compat enforcement when the version is `bundled`. - nitro: the startup plugin's bare static `import "@workflow/core/runtime"` couldn't be resolved by Rollup/Vite, breaking every Nitro(+Vite) build (503s on nitro/express/hono/fastify/tanstack-start). Resolve the runtime to a `file://` URL at build time and dynamic-import it (vite-/webpack-ignored), mirroring the dashboard handler; gate the plugin to non-Vercel builds (the Vercel World's start() is a no-op and the file:// path wouldn't resolve in a serverless function). Verified: express (Nitro, bundled build) boots and serves the manifest 200 with no version/world-start errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… order
- Move "Recovering in-flight runs" before "Building a World" in the Deploying
nav (v4 + v5).
- Add an optional, collapsed accordion ("Recover in-flight runs after a
restart") to each framework's getting-started guide (Next/Nitro/Express/
Hono/Fastify/Nuxt/Vite/TanStack Start/SvelteKit/Nest/Astro, v4 + v5) with the
framework-specific startup snippet, noting it is NOT required for Vercel
deployments and linking to the full Recovering in-flight runs page.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
||
| ## Deploying to Production | ||
|
|
||
| <Accordion type="single" collapsible> |
There was a problem hiding this comment.
This isn't the right place. Actually let's not add anything to each individual framework getting started (the "recovering runs section" is good enough imo. instead let's just ensure that the postgres world and local world, world docs, mention that they need the world to be started at instantiation with the right framework-wide docs (just like the we did in recovering in-flight runs) along with the code snippet since only those worlds need it
There was a problem hiding this comment.
Done in 54b5b2e. Reverted the per-framework getting-started accordions, and instead added a "Starting the World" note to the local-world and postgres-world docs (only the self-hosted worlds need it): each states the World must be started at server boot, shows the ensureWorldStarted() snippet, and links to Recovering in-flight runs for per-framework wiring. Also consolidated postgres-world's prior per-framework world.start() tabs into that single snippet + link (now using the idempotent helper).
There was a problem hiding this comment.
Done — reverted the per-framework getting-started accordions and instead added a "Starting the World" section (with the ensureWorldStarted() snippet + a link to this guide) to the local-world and postgres-world docs, since only those worlds need it.
Per review: revert the per-framework getting-started accordions (the Recovering in-flight runs page is enough) and instead document the startup requirement on the worlds that actually need it. - local-world / postgres-world: add a "Starting the World" note that the World must be started at server boot, with the `ensureWorldStarted()` snippet and a link to Recovering in-flight runs for per-framework wiring. - postgres-world: consolidate the prior per-framework `world.start()` tabs into that single snippet + link (DRY; uses the idempotent helper). - Remove the getting-started accordions added in the previous commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Re-introduce the dropped auto startup plugin so self-hosted Nitro apps (Nitro v2/v3, Nuxt, Express/Hono/Fastify on Nitro) recover in-flight runs after a restart with no manual wiring. The previously-reverted version imported the runtime via a build-time `file://` URL, which collided with the bundled flow handler's copy of the same file (CJS/ESM dual-load -> ERR_INTERNAL_ASSERTION, 500ing the flow route). This version instead emits a real plugin file in the build dir that imports `workflow/runtime` via a *bare* dynamic import — mirroring a hand-written Nitro plugin — so the bundler resolves and dedupes it with the flow handler's runtime. `ensureWorldStarted()` caches its start promise on `globalThis`, so the World starts exactly once. Gated off Vercel deploys (the Vercel World's start() is a no-op). Removes the manual `start-pg-world.ts` workbench workaround and updates the recovering-in-flight-runs docs to note Nitro starts the World automatically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-sleep The existing case kills during a workflow `sleep()` — a delayed, unlocked queue job. This adds a case that kills while a STEP is executing, so the step's queue job is held/locked at crash time (the postgres graphile-worker case from #679). Adds `longStepWorkflow` (one ~12s step) and a second restart-recovery test that kills mid-step and asserts the run resumes after a restart with no workflow op. Verified locally on both the local and postgres worlds: the run recovers (reenqueueActiveRuns re-drives the flow; the replayed step is re-dispatched and graphile schedules a fresh run rather than stalling on the locked job). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The postgres-world "Starting the World" section listed "a Nitro server plugin" as a place to call ensureWorldStarted(), which is now misleading — @workflow/nitro starts the World automatically. Clarify that Nitro/Nuxt apps need no manual call (consistent with the recovering-in-flight-runs page and local-world docs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s it) `@workflow/nuxt` registers `@workflow/nitro`, which now auto-starts the World at boot. The pre-existing `server/plugins/start-pg-world.ts` called `world.start()` directly (bypassing the `ensureWorldStarted` once-guard), so with the auto-plugin nuxt+postgres started the World twice (double `queue.start()`). Remove it — boot recovery is handled automatically. Verified: nuxt dev (local world) starts the World at boot via the auto-plugin with no manual plugin present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… auto-start) Removing the workbench `start-pg-world.ts` plugins broke CI: `nitro-v3/plugins` is the canonical source that express/hono/vite/tanstack-start/fastify symlink to (`plugins -> ../nitro-v3/plugins`), so deleting it left dangling symlinks and `resolve-symlinks.sh`'s `cp` failed for the whole e2e matrix. The removal was also unnecessary: `world.start()` is idempotent — postgres `queue.start()` is guarded by a cached `startPromise` (the worker starts once) and `reenqueueActiveRuns` is safe to call twice — so the manual plugins and the new auto-start plugin coexist without double-starting. Restore them as CI scaffolding; end users still get auto-start via @workflow/nitro (docs unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… Nuxt prod)
Two-part fix for the auto-start plugin's production failures:
1. The original generated a real file in `buildDir` and registered its absolute
path. Pure-Nitro apps bundle that, but Nuxt's standalone `.output` doesn't
copy it → prod boot crashed with ERR_MODULE_NOT_FOUND → ECONNREFUSED. Fixed
by registering a *virtual* module (inlined into the server bundle by every
framework, like the flow/webhook handlers).
2. The virtual module must import `workflow/runtime` via a PLAIN bare dynamic
import so the bundler inlines it — sharing the one runtime instance the flow
handler uses. Marking it `@vite-ignore` (my first attempt) left it external,
so Node loaded a *second* runtime copy at runtime that collided with the
bundled one: ERR_INTERNAL_ASSERTION ("imported again after being required")
in createWorld → crashed world start AND 500'd the flow handler → postgres
e2e hung and was cancelled.
Verified locally against the exact failing scenario: nitro-v3 prod + Postgres
runs real queued workflows through the flow handler with zero ERR_INTERNAL_
ASSERTION (sleep/hook/loop e2e tests pass), and nuxt prod builds + boots +
runs boot recovery with no module error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-worker) Adds a deterministic postgres repro for the concern that boot-time recovery (`reenqueueActiveRuns`) might re-run steps that are healthily in-flight on another worker: worker B runs a long step, worker A boots while it's running (its instrumentation re-enqueues ALL active runs), and the test counts how many times the step body actually executed (via a shared side-effect log). Result: exactly once. The re-enqueue replays the workflow, but the step's `correlationId` idempotency key means graphile-worker won't run a concurrent duplicate of the locked job, and by the time any re-dispatch could run the original step has completed and the terminal-state guard skips it. So the re-enqueue does NOT override step idempotency. Adds `sideEffectStepWorkflow` (records each execution before sleeping). Gated behind MULTIWORKER_RECOVERY_TEST + the postgres world; not in the default CI gate. Verified locally: VITEST EXIT 0, step executed 1×. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| * redelivery, not from a long-lived process re-scanning storage — so they | ||
| * implement an empty `start()` purely for interface compliance. | ||
| */ | ||
| start?(): Promise<void>; |
There was a problem hiding this comment.
If we're adding this to world-vercel for compliance, should it be made required on the type too? Breaking, but only released to beta and all current worlds support it, except community worlds which already don't support 5.x beta, AFAIK
There was a problem hiding this comment.
I left it optional, with rationale — open to flipping if you'd rather. The wrinkle is community worlds: main just added Platformatic to worlds-manifest.json, and those worlds implement the interface externally, so promoting start() to required is a breaking interface change for them. Meanwhile all first-party worlds (local/postgres/vercel) already implement it, and ensureWorldStarted() calls world.start?.() so an absent start is handled gracefully; the interface doc already documents that start() MAY be a no-op. If you'd prefer the harder contract for beta, I'm happy to make it required and drop the optional chaining — your call.
| '@workflow/nitro': minor | ||
| --- | ||
|
|
||
| Start the workflow World automatically at server boot via a generated Nitro plugin, so self-hosted Nitro apps (Nitro v2/v3, Nuxt, Express/Hono/Fastify on Nitro) recover in-flight runs after a restart with no manual wiring. Skipped on Vercel deploys. |
There was a problem hiding this comment.
So just for Nitro apps in particular, local world now also calls start by default and picks up previous runs?
There was a problem hiding this comment.
Yes — the Nitro startup plugin is un-gated (it runs for any non-Vercel deploy), so the local world on Nitro also calls start() at boot and re-enqueues its previously-in-flight runs, same as postgres. I clarified the changeset in 39d5469 to state this applies to both the local and Postgres worlds (not just postgres).
VaguelySerious
left a comment
VaguelySerious
left a comment
There was a problem hiding this comment.
AI review: blocking issues found
|
|
||
| // Multi-worker duplicate-execution repro: only meaningful on the postgres world | ||
| // (a shared DB + a graphile-worker pool). Gated behind its own flag so it stays | ||
| // out of the default CI gate until the owner-aware recovery fix lands. |
There was a problem hiding this comment.
AI Review: Blocking
Boot-time reenqueueActiveRuns() now runs on every instance start. Before this PR world.start() was effectively never called for self-hosted users, so on a horizontally-scaled / rolling-deploy Postgres setup this is genuinely new behavior: each booting worker re-enqueues all pending/running runs cluster-wide, including runs that are healthily in-flight on other workers.
This multi-worker test asserts exactly-once but is gated out of the default CI gate "until the owner-aware recovery fix lands" — so the concurrency guarantee is not enforced by CI, and that fix isn't in this PR. The exactly-once argument rests on graphile's locked-job dedup. Please confirm it also holds for a sleeping (unlocked) run that a second worker re-enqueues, where re-execution of a non-idempotent 'use step' body depends purely on replay idempotency.
Recommendation: either confirm exactly-once for the concurrent-sleeping case, or add a clear "single long-lived instance" caveat to recovering-in-flight-runs.mdx until owner-aware recovery lands.
There was a problem hiding this comment.
Addressed in 39d5469 — and since you offered either, I did both (confirm + caveat).
Exactly-once holds for the sleeping/unlocked case too. It does not rest on graphile's locked-job dedup — that only prevents redundant run-level reprocessing. The non-idempotent 'use step' body is protected one level down by the step's create-claim: the step row is inserted via onConflictDoNothing().returning() keyed on correlationId, so a second delivery (locked or not) loses the claim → EntityConflictError → skipped; step_started is additionally guarded by notInArray(status, terminalStepStatuses). So a re-enqueued sleeping run replays, but the step body runs once.
Caveat added anyway, since owner-aware recovery isn't in this PR: recovering-in-flight-runs.mdx (v4 + v5) now has a Multi-instance deployments section spelling out that boot recovery re-enqueues all active runs cluster-wide, that step execution stays exactly-once via the step-level claim (independent of any queue lock), and that a re-enqueued sleeping run may wake early but re-honors its deadline on replay (the strengthened test now asserts exactly this). It also notes single-instance recovery as the mitigation until owner-aware recovery lands.
| // restart without needing a workflow operation. No-op on the Vercel World; | ||
| // runs recovery for the local/postgres worlds. | ||
| const { ensureWorldStarted } = await import('workflow/runtime'); | ||
| await ensureWorldStarted(); |
There was a problem hiding this comment.
AI Review: Note
Error handling is inconsistent across the startup wiring. The Nitro plugin catches and logs failures (.catch(...)), so a recovery error never breaks boot. Here (and in SvelteKit/Next) ensureWorldStarted() is awaited unguarded — a throw (e.g. DB unreachable at boot) propagates and, in Nest's bootstrap(), prevents the server from listening at all. Decide whether fail-fast or fail-open is the intended contract and make it uniform across integrations (the docs samples are unguarded too).
There was a problem hiding this comment.
Good catch — made it uniform and fail-open, centralized inside ensureWorldStarted() itself (39d5469): it now catches, logs, and clears its cached promise (so a later call retries) instead of throwing. Boot-time recovery is best-effort — the runs are durable and recover on a later start() / the next enqueue — so a DB blip at boot must not stop the server from listening. With the guard inside the helper, every caller (Next/SvelteKit/Nest/Nitro/Astro) and the docs samples can await ensureWorldStarted() unguarded and behave identically. The Nitro plugin keeps its own .catch only to guard the dynamic import() itself failing, which is outside the helper.
| // sleeping when we detect it and kill the server, short enough that by the time | ||
| // the restarted server recovers it, the sleep deadline has typically already | ||
| // passed (so it completes promptly on replay). | ||
| const SLEEP_MS = 8_000; |
There was a problem hiding this comment.
AI Review: Note
The sleeping-run recovery test never exercises a deadline still in the future — as the comment notes, by the time recovery runs the 8s deadline has "typically already passed." So if recovery ever woke a run before its deadline, this test would still pass. Consider asserting the run does not complete before the original sleep deadline, to actually cover "remaining sleep is honored on replay."
There was a problem hiding this comment.
Fixed in 39d5469. SLEEP_MS is now 60s (deadline comfortably in the future after the kill/restart cycle), and the test captures a deadlineLowerBound (= startedAt + SLEEP_MS, which is strictly before the true deadline since the server calls sleep() after we record startedAt) and calls a new assertNotCompletedBefore(runId, deadlineLowerBound) after the restart — it polls until the deadline and fails if the run completes early. An early-wake regression (recovery not honoring the remaining sleep) now fails the test.
| # the other e2e jobs, this one does NOT pre-start the server; the test owns | ||
| # the server lifecycle (spawn, SIGKILL, respawn). Covers local + postgres on | ||
| # nextjs-turbopack. | ||
| e2e-restart-recovery: |
There was a problem hiding this comment.
AI Review: Note
This job is added directly to the required E2E Required Check gate, and it's a process-spawn / SIGKILL / process-group / lsof port-reclaim test — the class most prone to CI flake. The kill/port-reclaim logic looks careful, but consider soaking it as non-required for a few days before gating every merge on it; a flake here would block all merges.
There was a problem hiding this comment.
Agreed. De-gated in 39d5469: in e2e-required-check the restart-recovery status is now a non-blocking ::warning:: rather than contributing to FAILED_JOBS, so the job still runs and reports its own status for signal but a flake won't block merges. Left a comment to promote it back into FAILED_JOBS once it's run green for a sustained period.
|
|
||
| ## Wiring it per framework | ||
|
|
||
| ### Next.js |
There was a problem hiding this comment.
AI Review: Note
Worth making the asymmetry explicit: only Nitro auto-wires recovery; Next.js/SvelteKit/Nest/Astro require manual wiring (reasonable given the documented Turbopack interop issue, but a user might assume the framework package handles it). Also note that Astro's middleware approach defers recovery to first request rather than true server boot.
There was a problem hiding this comment.
Made explicit in 39d5469 (v4 + v5). The "Wiring it per framework" section now opens with: only @workflow/nitro auto-wires this; every other framework needs the one-time manual setup below, and a framework package handling your workflow routes does not imply it starts the World for you. It also calls out that Astro's middleware approach runs recovery on the first request after boot rather than at boot itself.
| // runs recover after a restart without needing a workflow operation. | ||
| // Covers self-hosted Nitro apps (Nitro v2/v3, Nuxt). Skipped on Vercel: | ||
| // the Vercel World's start() is a no-op (push-based — VQS redelivers). | ||
| if (!isVercelDeploy) { |
There was a problem hiding this comment.
AI Review: Nit
The !isVercelDeploy gate means a (rare) Postgres-world-on-Vercel-via-Nitro deployment would get no boot recovery. Almost certainly out of scope, but a one-line doc note wouldn't hurt.
There was a problem hiding this comment.
Added a note in 39d5469 right at the !isVercelDeploy gate: it gates on the deploy target, not the configured World, so a (rare, non-default) Postgres-World-on-Vercel-via-Nitro deployment gets no boot recovery here and relies on queue redelivery / the next enqueue instead.
* origin/main: (46 commits) [web] Fix HTTP/2 in bundled server build so observability reads work (#2632) [core] Fix turbo-mode step-body writes racing run_started (#2629) fix(world-postgres): rename setup command (#2644) chore: ignore workflow swc caches (#2640) fix(ai): ignore intentional stream aborts (#2635) fix(nitro): reload steps during Vite HMR (#2572) [world] Make @workflow/world own its Zod dependency (#2622) feat(web-shared): extend cn with custom tailwind-merge class groups (#2619) [world-vercel] Enable HTTP/2 for the events API and stream writes (#2573) Remove shadow from off-screen marker indicator button (#2614) Fix discovery of dotted JS workflow imports (#2594) Version Packages (beta) (#2597) Thread occurredAt into web-shared entities (#2608) Propagate trace context to vercel-workflow.com in workbench instrumentation (#2601) Reduce workflow build log noise (#2565) Version Packages (beta) (#2596) Use text-gray-900 for trace shortcut helper text (#2595) Trace /flow route initialization (#2592) Version Packages (beta) (#2591) feat(web): add trace step shortcut helper (#2582) ... # Conflicts: # workbench/nextjs-turbopack/instrumentation.ts # workbench/nextjs-webpack/instrumentation.ts
…k CI, docs caveats - ensureWorldStarted() is now fail-open: it logs and retries on failure instead of throwing, so a boot-time recovery error never prevents the server from coming up. This makes error handling uniform across Next/SvelteKit/Nest/Nitro/ Astro (and the docs samples), which all `await ensureWorldStarted()` unguarded. - Strengthen the sleeping-run recovery test: 60s sleep + assert the recovered run does NOT complete before its deadline, proving remaining sleep is honored on replay (an early-wake regression would now fail the test). - De-gate e2e-restart-recovery from the E2E Required Check (soak as non-required): it is a process-spawn / SIGKILL / port-reclaim test prone to CI flake, so a failure must not block every merge while we build confidence. Still runs/reports. - Docs (v4+v5): add a "Multi-instance deployments" caveat (exactly-once rests on the step-level idempotency claim, not a queue lock, so it holds for sleeping runs too; a re-enqueued sleeping run may wake early but re-honors its deadline on replay; owner-aware recovery is a planned refinement) and make the Nitro-auto-wires vs. manual-wiring asymmetry explicit. - Nitro: note the !isVercelDeploy gate skips a Postgres-on-Vercel-via-Nitro setup. - Changeset: clarify both the local and Postgres worlds recover on Nitro. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VaguelySerious
left a comment
VaguelySerious
left a comment
There was a problem hiding this comment.
AI review: no blocking issues
|
|
||
| `start()` re-enqueues **every** `pending`/`running` run it finds in storage — it does not filter by which instance owns a run. On a horizontally-scaled or rolling deployment, each booting instance therefore re-enqueues all in-flight runs cluster-wide, including runs that are healthily in progress on another instance. This is safe, but worth understanding: | ||
|
|
||
| - **Steps still execute exactly once.** A re-enqueued run replays from its event log, and each `"use step"` is guarded by a step-level idempotency claim — a booting instance that races a step already running elsewhere loses the claim and skips execution. This does not depend on a queue-level lock, so it holds whether the run was mid-step or sleeping when it was re-enqueued. |
There was a problem hiding this comment.
AI Review: Note
Thanks for adding this caveat — the conclusion (exactly-once holds for the local + Postgres worlds) is right, but the stated mechanism is inaccurate and would mislead a custom-World author.
step_started (world.events.create) only returns a conflict (409 / EntityConflictError) when the step is already in a terminal state (see packages/core/src/runtime/step-handler.ts ~L255 / L308). A step merely running on another instance does not reject a concurrent step_started — it increments the attempt and proceeds. So there is no queue-lock-independent "claim" that a racing instance "loses" mid-step.
For a run re-enqueued mid-step, what actually prevents concurrent double-execution is the queue's job-key (correlationId) lock — exactly what this PR's own multi-worker test comment credits ("graphile-worker does not run a concurrent duplicate of a locked job"), and what step.ts notes for the step_started case ("We rely on the queue's idempotency"). The terminal-state guard then covers the after-completion re-dispatch window.
The sleeping case is genuinely safe, but for a different reason than stated: a sleeping run has no step executing, so replay just re-evaluates the wait — there's no concurrency to dedup at all.
Suggest rewording to (a) credit the queue-level job-key dedup for the concurrent mid-step window, and (b) note the contract implication — a custom World whose queue lacks job-key locking would not get exactly-once for that window. The identical text is duplicated in the v4 page (L36).
| echo "windows=$WINDOWS_STATUS" >> $GITHUB_OUTPUT | ||
|
|
||
| if [[ "$VERCEL_STATUS" == "failure" || "$LOCAL_DEV_STATUS" == "failure" || "$LOCAL_PROD_STATUS" == "failure" || "$POSTGRES_STATUS" == "failure" || "$WINDOWS_STATUS" == "failure" ]]; then | ||
| if [[ "$VERCEL_STATUS" == "failure" || "$LOCAL_DEV_STATUS" == "failure" || "$LOCAL_PROD_STATUS" == "failure" || "$POSTGRES_STATUS" == "failure" || "$RESTART_RECOVERY_STATUS" == "failure" || "$WINDOWS_STATUS" == "failure" ]]; then |
There was a problem hiding this comment.
AI Review: Nit
The soak de-gate is complete for the required check (L1178), but this has_failures computation in the E2E Summary job still includes RESTART_RECOVERY_STATUS == "failure". A flake in the soaking job will therefore still append a "❌ Some E2E test jobs failed" notice to the PR comment — non-blocking, but noisy in a way that partly defeats the soak intent. Consider dropping it here too (and from the failure-notice list just below) for consistent soak signal.
…ering them
In development the workflow code changes constantly, and recovery replays the
event log against the *current* code — so re-enqueuing a run that started on
now-changed code would diverge. On a dev server restart we now CANCEL those
previous-session runs (with an explanatory reason + info log) instead, and only
recover (re-enqueue) in production.
- @workflow/world: add `cancelActiveRuns()` and a tri-state `StartOptions`
(`onRestart: 'recover' | 'cancel' | 'ignore'`) to `start()`; add an optional
`reason` to the `run_cancelled` event.
- world-local / world-postgres: `start()` honors `onRestart`; postgres cancels
before booting the worker so it can't drain a run we're about to cancel.
- core: `ensureWorldStarted({ dev? })` maps dev→cancel / prod→recover. `dev`
defaults to `NODE_ENV === 'development'` (recover unless confidently dev);
`WORKFLOW_RECOVER_IN_DEV=1` forces recovery in dev.
- Framework-native dev detection (fallback to NODE_ENV): Nitro bakes
`nitro.options.dev` into the generated plugin (zero config); SvelteKit uses
`$app/environment` `dev`; Astro uses `import.meta.env.DEV`; Nest derives from
NODE_ENV; Next relies on NODE_ENV (set reliably by next dev/build).
- Tests: deterministic cancel unit tests (world-local + world-postgres),
ensureWorldStarted dev-detection unit test, and a dev-restart e2e
(pnpm dev → kill → restart → run is cancelled, not recovered).
- Docs (v4+v5): "Development vs production" section with the per-framework dev
signal table; document the HMR gap (cancellation only fires on process
restart, not live module reload) in local-world and the replay-divergence
error page.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VaguelySerious
left a comment
VaguelySerious
left a comment
There was a problem hiding this comment.
Needs to be conflict-resolved with #2888 once that lands
Closes #679
Related: #1531 (closed; same root cause)
Context
Self-hosted worlds (
local,postgres) run boot-time recovery —reenqueueActiveRuns()re-enqueuespending/runningruns — but it lives only insideworld.start(), and nothing calledworld.start()at server startup. So a self-hosted server that restarted while a run was in flight (sleeping, waiting on a hook, between steps) never resumed that run without a subsequent workflow operation. The Vercel World didn't even implementstart().This wires
world.start()to run once at server boot across the framework integrations, makes the Vercel World'sstart()an explicit no-op (push-based — VQS redelivers, no boot recovery needed), and adds an e2e test that proves recovery happens on startup with no workflow operation — and fails if the wiring is removed.Changes
@workflow/core: add idempotentensureWorldStarted()(once-per-process guard;getWorld()→world.start?.()), exported from@workflow/core/runtimeandworkflow/runtime.@workflow/world-vercel: add a no-opasync start()for interface compliance; expand theWorld.start()contract doc (must be idempotent; may be a no-op for push-based worlds).instrumentation.tscallsensureWorldStarted()fromworkflow/runtime, guarded byNEXT_RUNTIME === 'nodejs'.@workflow/nitroauto-registers a Nitro server plugin that runs at app boot — no manual wiring required. (See the "Nitro auto-start" update below for the implementation detail.)world.start()calls (were@workflow/world-postgres-only, so local never recovered) and route throughensureWorldStarted().src/middleware.tsonce-guard (Astro has no all-adapter startup hook).packages/core/e2e/restart-recovery.test.ts— starts a sleeping run server-side, hard-kills the server mid-sleep, restarts it, and asserts the run completes with no workflow op. Pure-reader test process; robust process-group kill. Gated byRESTART_RECOVERY_TEST=1; covers local + postgres on nextjs-turbopack.e2e-restart-recoveryjob (matrix local + postgres; owns the server lifecycle, so it does not pre-start the server) added to the summary +e2e-required-checkgates.deploying/recovering-in-flight-runs.mdx(v4 + v5).Verification
Ran the restart e2e locally, all four combinations:
running, 120s timeoutrunning, 120s timeoutThe run is killed mid-sleep (~0.5s into an 8s sleep) and only recovers after restart via the startup
ensureWorldStarted(). Postgres fails-without-fix because the graphile worker only auto-boots on the next enqueue, and the test issues no post-restart op.pnpm build/typecheckpass for all changed packages.Note
The plan called for a
@workflow/nextregisterhelper, but consuming it viaworkflow/next/instrumentationbroke Turbopack (a CJS re-export double-hop hit@workflow/core/dist/runtimeexports-encapsulation and dropped the named export through interop). UsingensureWorldStarted()fromworkflow/runtimedirectly is robust and consistent with how SvelteKit/Nest/Astro are wired.Docs Preview
(Preview sits behind deployment protection — requires Vercel team access.)
Update (CI fixes)
Initial CI surfaced two issues from un-gating
world.start(), both fixed:world-local:initDataDirthrewInvalid version string: "bundled"in bundled server builds — now skips version-compat when the version is thebundledsentinel.ERR_INTERNAL_ASSERTION. It was briefly removed in favor of manual wiring, then restored with a fix — see below.Update (Nitro auto-start, restored & fixed)
Nitro now starts the World automatically again — no manual server plugin needed.
The original auto-plugin imported the runtime via a build-time
file://URL, which collided with the bundled flow handler'srequire()of the same file (CJS/ESM dual-load →ERR_INTERNAL_ASSERTION, 500ing the flow route). The fix:@workflow/nitronow emits a real plugin file in the build dir that importsworkflow/runtimevia a bare dynamic import — mirroring a hand-written Nitro plugin — so the bundler resolves and dedupes it with the flow handler's runtime (no second physical module, no dual-load).ensureWorldStarted()caches its start promise onglobalThis, so the World starts exactly once. Registration is gated off Vercel deploys (the Vercel World'sstart()is a no-op).The Nitro/Nuxt section of the recovery docs is updated to "no action required." The workbench
start-pg-world.tsplugins are kept as CI test scaffolding —world.start()is idempotent (postgresqueue.start()is guarded by a cachedstartPromise;reenqueueActiveRunsis safe to repeat), so they coexist with the auto-plugin without double-starting. (Removing them was tried and reverted:nitro-v3/pluginsis the symlink source for several other workbenches, so deleting it broke CI symlink resolution.)Verified on
workbench/nitro-v3(local world), dev and production build:world.start()runs at true boot before any request (creates the data dir with no request made), and the flow handler serves HTTP 200 with zeroERR_INTERNAL_ASSERTIONafter the boot plugin has loaded the runtime — the exact regression that caused the original removal. The nitro+postgres path should be confirmed green in CI.🤖 Generated with Claude Code