Repository navigation
Add start({ hook }) atomic admission for idempotent starts - #4996
Conversation
🦋 Changeset detectedLatest commit: 9181a8f The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 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 |
Sim WorldSimulated world deterministic testing for races. Traces 🟢 world-sim scenario book — 0 fail of 45 total
Full trace: |
🧪 E2E Test Results✅ All tests passed
|
| Passed | Failed | Skipped | Total | |
|---|---|---|---|---|
| ✅ ▲ Vercel Production | 4008 | 0 | 1392 | 5400 |
| ✅ 💻 Local Development | 5249 | 0 | 551 | 5800 |
| ✅ 📦 Local Production | 5249 | 0 | 551 | 5800 |
| ✅ 🐘 Local Postgres | 5249 | 0 | 551 | 5800 |
| ✅ 🪟 Windows | 388 | 0 | 12 | 400 |
| ✅ 🌐 Cross-language Conformance | 68 | 0 | 107 | 175 |
| ✅ dynamic-runs | 0 | 0 | 0 | 0 |
| ✅ vercel-http-transport | 903 | 0 | 297 | 1200 |
| ✅ vercel-multi-region | 27 | 0 | 0 | 27 |
| ✅ vercel-spec9 | 344 | 0 | 56 | 400 |
| ✅ vercel-ws-transport | 611 | 0 | 189 | 800 |
| Total | 22096 | 0 | 3706 | 25802 |
Details by Category
✅ ▲ Vercel Production
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-node | 146 | 0 | 54 |
| ✅ astro-quickjs | 146 | 0 | 54 |
| ✅ example-node | 146 | 0 | 54 |
| ✅ example-quickjs | 146 | 0 | 54 |
| ✅ express-node | 146 | 0 | 54 |
| ✅ express-quickjs | 146 | 0 | 54 |
| ✅ fastify-node | 146 | 0 | 54 |
| ✅ fastify-quickjs | 146 | 0 | 54 |
| ✅ hono-node | 146 | 0 | 54 |
| ✅ hono-quickjs | 146 | 0 | 54 |
| ✅ nest-node | 146 | 0 | 54 |
| ✅ nest-quickjs | 146 | 0 | 54 |
| ✅ nextjs-turbopack-node | 173 | 0 | 27 |
| ✅ nextjs-turbopack-quickjs | 173 | 0 | 27 |
| ✅ nextjs-webpack-node | 173 | 0 | 27 |
| ✅ nextjs-webpack-quickjs | 173 | 0 | 27 |
| ✅ nitro-node | 146 | 0 | 54 |
| ✅ nitro-quickjs | 146 | 0 | 54 |
| ✅ nuxt-node | 146 | 0 | 54 |
| ✅ nuxt-quickjs | 146 | 0 | 54 |
| ✅ python-node | 66 | 0 | 134 |
| ✅ sveltekit-node | 165 | 0 | 35 |
| ✅ sveltekit-quickjs | 165 | 0 | 35 |
| ✅ tanstack-start-node | 146 | 0 | 54 |
| ✅ tanstack-start-quickjs | 146 | 0 | 54 |
| ✅ vite-node | 146 | 0 | 54 |
| ✅ vite-quickjs | 146 | 0 | 54 |
✅ 💻 Local Development
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 171 | 0 | 29 |
| ✅ astro-stable-quickjs | 171 | 0 | 29 |
| ✅ express-stable-node | 171 | 0 | 29 |
| ✅ express-stable-quickjs | 171 | 0 | 29 |
| ✅ fastify-stable-node | 171 | 0 | 29 |
| ✅ fastify-stable-quickjs | 171 | 0 | 29 |
| ✅ hono-stable-node | 171 | 0 | 29 |
| ✅ hono-stable-quickjs | 171 | 0 | 29 |
| ✅ nest-stable-node | 171 | 0 | 29 |
| ✅ nest-stable-quickjs | 171 | 0 | 29 |
| ✅ nextjs-turbopack-canary-node | 199 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 199 | 0 | 1 |
| ✅ nextjs-turbopack-quickjs-snapshot | 199 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 199 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 199 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 199 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 199 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 199 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 199 | 0 | 1 |
| ✅ nitro-stable-node | 171 | 0 | 29 |
| ✅ nitro-stable-quickjs | 171 | 0 | 29 |
| ✅ nuxt-stable-node | 171 | 0 | 29 |
| ✅ nuxt-stable-quickjs | 171 | 0 | 29 |
| ✅ sveltekit-stable-node | 190 | 0 | 10 |
| ✅ sveltekit-stable-quickjs | 190 | 0 | 10 |
| ✅ tanstack-start-node | 171 | 0 | 29 |
| ✅ tanstack-start-quickjs | 171 | 0 | 29 |
| ✅ vite-stable-node | 171 | 0 | 29 |
| ✅ vite-stable-quickjs | 171 | 0 | 29 |
✅ 📦 Local Production
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 171 | 0 | 29 |
| ✅ astro-stable-quickjs | 171 | 0 | 29 |
| ✅ express-stable-node | 171 | 0 | 29 |
| ✅ express-stable-quickjs | 171 | 0 | 29 |
| ✅ fastify-stable-node | 171 | 0 | 29 |
| ✅ fastify-stable-quickjs | 171 | 0 | 29 |
| ✅ hono-stable-node | 171 | 0 | 29 |
| ✅ hono-stable-quickjs | 171 | 0 | 29 |
| ✅ nest-stable-node | 171 | 0 | 29 |
| ✅ nest-stable-quickjs | 171 | 0 | 29 |
| ✅ nextjs-turbopack-canary-node | 199 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 199 | 0 | 1 |
| ✅ nextjs-turbopack-quickjs-snapshot | 199 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 199 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 199 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 199 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 199 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 199 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 199 | 0 | 1 |
| ✅ nitro-stable-node | 171 | 0 | 29 |
| ✅ nitro-stable-quickjs | 171 | 0 | 29 |
| ✅ nuxt-stable-node | 171 | 0 | 29 |
| ✅ nuxt-stable-quickjs | 171 | 0 | 29 |
| ✅ sveltekit-stable-node | 190 | 0 | 10 |
| ✅ sveltekit-stable-quickjs | 190 | 0 | 10 |
| ✅ tanstack-start-node | 171 | 0 | 29 |
| ✅ tanstack-start-quickjs | 171 | 0 | 29 |
| ✅ vite-stable-node | 171 | 0 | 29 |
| ✅ vite-stable-quickjs | 171 | 0 | 29 |
✅ 🐘 Local Postgres
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 171 | 0 | 29 |
| ✅ astro-stable-quickjs | 171 | 0 | 29 |
| ✅ express-stable-node | 171 | 0 | 29 |
| ✅ express-stable-quickjs | 171 | 0 | 29 |
| ✅ fastify-stable-node | 171 | 0 | 29 |
| ✅ fastify-stable-quickjs | 171 | 0 | 29 |
| ✅ hono-stable-node | 171 | 0 | 29 |
| ✅ hono-stable-quickjs | 171 | 0 | 29 |
| ✅ nest-stable-node | 171 | 0 | 29 |
| ✅ nest-stable-quickjs | 171 | 0 | 29 |
| ✅ nextjs-turbopack-canary-node | 199 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 199 | 0 | 1 |
| ✅ nextjs-turbopack-quickjs-snapshot | 199 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 199 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 199 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 199 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 199 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 199 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 199 | 0 | 1 |
| ✅ nitro-stable-node | 171 | 0 | 29 |
| ✅ nitro-stable-quickjs | 171 | 0 | 29 |
| ✅ nuxt-stable-node | 171 | 0 | 29 |
| ✅ nuxt-stable-quickjs | 171 | 0 | 29 |
| ✅ sveltekit-stable-node | 190 | 0 | 10 |
| ✅ sveltekit-stable-quickjs | 190 | 0 | 10 |
| ✅ tanstack-start-node | 171 | 0 | 29 |
| ✅ tanstack-start-quickjs | 171 | 0 | 29 |
| ✅ vite-stable-node | 171 | 0 | 29 |
| ✅ vite-stable-quickjs | 171 | 0 | 29 |
✅ 🪟 Windows
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack-node | 194 | 0 | 6 |
| ✅ nextjs-turbopack-quickjs | 194 | 0 | 6 |
✅ 🌐 Cross-language Conformance
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ python | 68 | 0 | 107 |
✅ dynamic-runs
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-vercel | 0 | 0 | 0 |
| ✅ example-vercel | 0 | 0 | 0 |
| ✅ express-vercel | 0 | 0 | 0 |
| ✅ fastify-vercel | 0 | 0 | 0 |
| ✅ hono-vercel | 0 | 0 | 0 |
| ✅ nest-vercel | 0 | 0 | 0 |
| ✅ nextjs-turbopack-vercel | 0 | 0 | 0 |
| ✅ nextjs-webpack-vercel | 0 | 0 | 0 |
| ✅ nitro-vercel | 0 | 0 | 0 |
| ✅ nuxt-vercel | 0 | 0 | 0 |
| ✅ sveltekit-vercel | 0 | 0 | 0 |
| ✅ tanstack-start-vercel | 0 | 0 | 0 |
| ✅ vite-vercel | 0 | 0 | 0 |
✅ vercel-http-transport
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ example | 146 | 0 | 54 |
| ✅ express | 146 | 0 | 54 |
| ✅ hono | 146 | 0 | 54 |
| ✅ nextjs-turbopack | 173 | 0 | 27 |
| ✅ nitro | 146 | 0 | 54 |
| ✅ vite | 146 | 0 | 54 |
✅ vercel-multi-region
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack | 27 | 0 | 0 |
✅ vercel-spec9
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack-node | 172 | 0 | 28 |
| ✅ nextjs-turbopack-quickjs | 172 | 0 | 28 |
✅ vercel-ws-transport
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ example | 146 | 0 | 54 |
| ✅ express | 146 | 0 | 54 |
| ✅ nextjs-turbopack | 173 | 0 | 27 |
| ✅ vite | 146 | 0 | 54 |
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 184058ms → this run 103183ms (Δ -80875ms, -44%) 📜 Previous results (9)e22ecb2Fri, 30 Oct 2026 17:40:25 GMT · run logs
b99a1b6Thu, 29 Oct 2026 19:13:02 GMT · run logs
80ddf9bThu, 29 Oct 2026 18:01:40 GMT · run logs
6457a0bThu, 29 Oct 2026 17:02:40 GMT · run logs
cba1d09Thu, 29 Oct 2026 16:50:45 GMT · run logs
89875b6Thu, 29 Oct 2026 16:11:58 GMT · run logs
f7d93e9Wed, 28 Oct 2026 21:59:27 GMT · run logs
1ead5fdTue, 27 Oct 2026 19:11:40 GMT · run logs
b37d7fdTue, 27 Oct 2026 19:01:02 GMT · run logs
ℹ️ Metric definitions & methodologyThe collapsed STSO distribution section above buckets every step gap, split inline (same warm process — pure framework overhead) vs queue-hop (fresh process — dispatch, reinit, replay). Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body) · Fan-out TTFS: fan-out time to first step (in-deployment start() → first of the parallel step bodies to complete) · Fan-out TTLS: fan-out time to last step (in-deployment start() → last of the parallel step bodies to complete, i.e. when the Promise.all resolves) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · Promise.all(100 steps): 100 trivial no-op steps started together in a single Promise.all; Fan-out TTFS is the first of them to complete and Fan-out TTLS the last, both from the in-deployment clientStart, so their gap is the spread the runtime adds across the fan-out 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 All timestamps are deployment-side; runs are triggered in-deployment, so the CI runner and api.vercel.com sit outside every measured window. TTFS = Cold starts stay in the numbers (real bursty-workload latency, inflates P75+); Best is the warm floor. |
About these numbersSizes are gzip; parentheses show the change against
|
c59fc83 to
cc849a0
Compare
…rator branch Squash of #4996 at 8ea0c2b (its own commits only), resolved against this branch: the spec-8 engine's admission handling moves to runtime/spec8/delivery.ts with a shared start-hook-delivery.ts, capability and queue-schema edits sit beside the single-orchestrator ones, and the start() unit tests declare the in-band fence spec 9 needs. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
…wStartError a WorkflowError Brings #4996's recut onto the single-orchestrator engine: setup on both engines treats exactly a run_started response that failed its schema at `run` as a redelivery (warned, span attribute workflow.run_started.runless_response) instead of a fatal contract error; WorkflowStartError extends WorkflowError and drops its inert `retryable` field; and the conformance suite covers an adoption without its own retention keeping the start's. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
cc849a0 to
e133836
Compare
…rator branch Squash of #4996 at 8ea0c2b (its own commits only), resolved against this branch: the spec-8 engine's admission handling moves to runtime/spec8/delivery.ts with a shared start-hook-delivery.ts, capability and queue-schema edits sit beside the single-orchestrator ones, and the start() unit tests declare the in-band fence spec 9 needs. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
…wStartError a WorkflowError Brings #4996's recut onto the single-orchestrator engine: setup on both engines treats exactly a run_started response that failed its schema at `run` as a redelivery (warned, span attribute workflow.run_started.runless_response) instead of a fatal contract error; WorkflowStartError extends WorkflowError and drops its inert `retryable` field; and the conformance suite covers an adoption without its own retention keeping the start's. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
a973c32 to
b37d7fd
Compare
b37d7fd to
1ead5fd
Compare
|
Addressed as of Where it differs from your suggestion:
F7, F8, F9, F10, F12 and F13 are in a separate commit, "[core][world] Trace start Hook admission outcomes". F6 is deferred to its own PR, because it changes serialization for every World error. Local agent review (`anthropic/claude-opus-5.5`) |
…rator branch Squash of #4996 at 8ea0c2b (its own commits only), resolved against this branch: the spec-8 engine's admission handling moves to runtime/spec8/delivery.ts with a shared start-hook-delivery.ts, capability and queue-schema edits sit beside the single-orchestrator ones, and the start() unit tests declare the in-band fence spec 9 needs. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
…wStartError a WorkflowError Brings #4996's recut onto the single-orchestrator engine: setup on both engines treats exactly a run_started response that failed its schema at `run` as a redelivery (warned, span attribute workflow.run_started.runless_response) instead of a fatal contract error; WorkflowStartError extends WorkflowError and drops its inert `retryable` field; and the conformance suite covers an adoption without its own retention keeping the start's. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
pranaygp
left a comment
There was a problem hiding this comment.
Re-review at 1ead5fd. The first round is largely fixed:
- the kill-switch refusal is no longer final;
- world-local uses the 24h bound;
- the holder wake runs in the background;
- the confirm loop covers non-streamed starts;
- forced claims get a refusal reason;
- admission rows are cleaned up;
- the tracing commit covers F7–F10, F12 and F13;
- error messages are reworded;
- world-postgres start Hooks are opt-in.
start() without hook still makes the same World calls, in the same order, with the same awaits as main.
Two fixes, though, introduced regressions for apps that never use start({ hook }), on world-postgres and on the Local World.
What I ran:
- core start-hook, resume-hook and serialization tests: 585 pass;
- world: 280 pass;
- world-vercel: 1019 pass;
- world-local: 747 pass. The 3 failures in
vercel-deployment-warning.test.tsare outside this PR; they come from theVERCEL*env on my machine.
Status of the first round
- Fixed: C1, C3–C9, F1, F3–F5, F7–F10, F12, F13. F2 is fixed by making start Hooks opt-in.
- F6 is deferred to its own PR, which is fine. But document now that
code,statusandreasononWorkflowWorldErrorareundefinedinside workflow code (workflow-world-error.mdx). - C2: the flag-off path is close to main, but see N1 and the migration note below.
Correctness
N1. world-postgres, callers who don't use the feature: a concurrent replay of the same createHook can write a hook_conflict naming the run's own hook.
- Where:
packages/world-postgres/src/storage.ts:2767. - The fresh-hook insert now uses
onConflictDoNothing({ target: Schema.hooks.hookId })and setsclaims_token = true. - When two writes of the same (runId, hookId, token) both pass the ON CONFLICT pre-check, the second raises 23505 on
workflow_hooks_token_claim_unique. That isn't the arbiter, so it's caught as "another writer claimed the token" (:2679), and the re-read finds the run's own hook as the owner. The run then gets both ahook_createdand ahook_conflictfor one correlationId. - On main, the untargeted
ON CONFLICT DO NOTHINGmade the second write a no-op, and the event insert deduplicated it. - Reproduced with the flag unset: 7 of 400 concurrent rounds.
- Suggested fix: after a 23505 on the claim index, re-read the token. If the owner is this (runId, hookId), treat it as the idempotent duplicate (
EntityConflictError). Add a looped regression test.
N2. Local World: cancelling a missing run now blocks any later creation of that run ID, ordinary runs included.
- Where:
packages/world-local/src/storage/events-storage.ts:1737,packages/core/src/runtime/runs.ts:194. cancelRunnow sendsrun_cancelledfor a missing run whenever the World advertisesatomicStartHook, and the Local World always does.- The Local World then writes a token-less fence at the run path for any ID younger than 24h. Ordinary creation doesn't recognise the fence:
run_createdfails with "already exists", andrun_startedwith not-found, whileruns.getstill says not found. - Example: an ordinary resilient start that is cancelled before its
run_startedlands can never be created. - Suggested fix: make only start Hook admission honour the fence. For instance, ordinary creation could treat a token-less fence as absent, or create the run already cancelled. Alternatively, fence only when a reservation claim names the ID. Add a test that cancels an ordinary resilient run before its
run_started.
Risks
N3. world-postgres, mixed versions: placeholder reservation rows are returned as real Hooks by the previous release's lookup.
- Where:
packages/world-postgres/src/drizzle/schema.ts:362,storage.ts:3820-3844. - With the flag on, a reservation is an ordinary
workflow_hooksrow (shr_<runId>), and it stays in place after adoption. - The previous release's
getByTokeniswhere token = ? and available limit 1, with no ordering and no filter, so it can return the placeholder.resumeHookthen appendshook_receivedunder a correlationId nothing awaits, and resolves successfully: the payload is silently lost.hooks.listshows the placeholder too. postgres.mdxonly warns about older queue workers. Extend it to every process that reads the database (resume and webhook handlers, the CLI, the dashboard), or keep the placeholder out of the rows an older query matches.
N4. world-postgres: the two copies of one candidate can fail with a raw workflow_hooks_pkey violation.
- Where:
packages/world-postgres/src/start-hook.ts:198. - The placeholder insert arbitrates on the token's partial index only, but both copies share
shr_<runId>. - Correctness holds: the direct copy retries and replays, and the queued copy is redelivered. The cost is a confirm sleep, a burned queue retry and an error log, in a race every
start()creates on purpose (3 + 3 of 300 rounds). - Suggested fix: use an untargeted
onConflictDoNothing(), or translate the pkey 23505 into the race path.
N5. world-postgres: deleteExpiredDecisions makes concurrent admissions queue behind each other.
- Where:
packages/world-postgres/src/start-hook.ts:317. - Every admission deletes the same oldest expired rows inside its own transaction, so admissions for unrelated tokens wait on each other's commit.
- Suggested fix: add
FOR UPDATE SKIP LOCKED, or move the cleanup off the admission transaction.
N6. The in-step reserved-token wait also runs when the token is reserved by the step's own run.
- Where:
packages/core/src/runtime/resume-hook.ts:337. - The owner can't register its hook while its own step is running, so the 30s wait can't succeed. It costs about 30s per attempt, roughly 2 minutes with the default retries.
- Suggested fix: skip the wait when
error.runIdis the currentworkflowRunId.
N7. The raw token is in world-postgres's new 503 messages.
- Where:
packages/world-postgres/src/start-hook.ts:212,storage.ts:3091. - These messages flow into
WorkflowStartError's message, theworkflow.startspan status and the newruntimeLogger.warn. Everything else in the PR keeps the token out. - Name the run instead.
N8. The reworded HookConflictError doesn't survive the step boundary.
- Where:
packages/core/src/serialization/reducers/common.ts:559,common-vm.ts:472. - The revivers rebuild the error through its constructor and discard the message. Inside a workflow, a refused
start({ hook })therefore shows thecreateHookwording, raw token included, and if it's uncaught that becomes the run's failure message. - Rewriting
messageafter construction also leaves the stack stale and drops the docs link (start.ts:597). - Suggested fix: carry a
kind: 'start'field through the reducer and build the message from it.
Compatibility notes
- Migration 0028. It's now required even with the flag off, because every hook insert writes
claims_token, and it takes table locks with nolock_timeout. Say so inpostgres.mdxand the changeset, and set alock_timeout. - Older viewers. Dashboards and CLIs released before this one can't read the new
HookReservedError/WorkflowStartErrortags ("Unknown type …"), and these now routinely reachstep_failed/run_failed. Either encode them through a tag older revivers understand (the named-Error encoding), or say in the changesets that viewers must be upgraded first.
Nits
- world-local answers
WorkflowRunNotFoundErrorafter fencing a cancelled candidate (events-storage.ts:1771), unlikeSTART_HOOK_CANDIDATE_CANCELLEDelsewhere. TheWorkflowStartErrordocs (workflow-start-error.mdx:50) describe the Local World behaviour wrongly as a result. hook-reserved-error.mdx:17doesn't mention the automatic in-step wait, and its example retry loop stacks on top of it.- An unsupported or switched-off World gets the same error code as a World that broke the contract (
start.ts:898). The refusal doesn't name the env var. - A stale JSDoc block now sits on the wrong declaration (
start.ts:433). - world-vercel sends
x-workflow-hook-conflict-reasonson every request (utils.ts:282), including callers who don't use the feature. - Still open from the first round, all small:
- world-local parses every run read as a fence-or-run union (
helpers.ts:517); - a lookup of a disposed adopted reservation does the full
hooks/scan (hooks-storage.ts:309-365); - health checks always parse capabilities (
helpers.ts:244,:410).
- world-local parses every run read as a fence-or-run union (
- The holder-wake result is now only in logs. The span attribute was removed with no replacement (
start-hook-holder.ts).
Requesting changes for N1 and N2. Both are regressions for apps that don't use start({ hook }). N3–N8 can follow, but N3 should be fixed or documented before anyone turns on the Postgres flag.
|
Pushed f7d93e9 with e2e cases for how a run uses its reserved token after admission. They run in the
One World gap the investigation turned up (world-postgres, raw World API only):
|
|
New requirement: a start Hook can be resumed from the moment its run is admitted. This removes
I prototyped this on top of this PR so it can be validated:
Merge them into this PR, or rework them however you like. The PR description lists the open decisions and one known gap: world-postgres's existing 🤖 Generated with Claude Code |
|
Replying to #4996 (comment) Thanks. We're adopting the resumable-from-admission design. #4996 will absorb #5119, #5120 and f7d93e9 as one commit, and #1292 gets recut the same way against your change list. We changed two things and kept your choices everywhere else. Change either if you see it differently:
Kept as you have them:
Also folded in: a direct copy reads the constraint before its transaction, which saves the cancelled 5-item write on duplicates. Accepted decisions expire at window + 1h. Open:
I'll re-request review once both are green. |
pranaygp
left a comment
There was a problem hiding this comment.
Final pass at 6457a0b. I confirmed this absorbed #5119, #5120 and f7d93e9, and closed those two:
- all 19
start({ hook })e2e tests are here; - the start Hook binding is in core on both engines, with eager parking;
HookReservedErrorand the 24-hour default are gone;- the Idempotency rewrite is in.
The two deviations are good calls:
- Disposal keeps the token until the run ends, so a key never has two live runs.
- world-postgres's
createHookrace is closed by the unique index.
Of the earlier findings (C1–C9, F1–F13, N1–N8), 29 are fixed or obsolete. CF6 (WorkflowWorldError.code at the step boundary) stays deferred, as agreed.
What I ran locally
-
Unit tests:
Package Result core 3438 passed world 280 passed world-local 763 passed world-vercel 1027 passed world-postgres 472 passed errors, web-shared, cli all pass world-local's 3 failures are in
vercel-deployment-warning.test.ts, which this PR doesn't touch; they come from theVERCEL*env on my machine. -
e2e:
- the
start({ hook })block passes 19/19 on the Local World and on Postgres (withWORKFLOW_POSTGRES_EXPERIMENTAL_START_HOOK=1), each under Node and QuickJS; - the full suite passes on the Local World (173 passed, 1 skipped).
- the
-
CI: the only red check is Windows unit, from
world-local/src/storage/fence-lock.test.ts, which this PR doesn't touch; main's test runs are failing intermittently too.
Still blocking
1. world-postgres: every hook_disposed now runs an extra SELECT on workflow_runs (packages/world-postgres/src/storage.ts:3115).
- It runs whatever the flag says, so every Postgres app pays it on every dispose,
usingexit and webhook, even if it never usesstart({ hook }). - The same event already reads the run in
getRunForValidation. - Suggested fix: add
start_hook_cbor(or only its token) to that read and reuse it, or decide from the hook row itself (hookId === startHookId(runId), or a column set when a Hook takes the start token). Either way, apps that don't use start Hooks are back to main's statements.
Non-blocking
- Migration 0028's lock ordering (
0028_add_start_hook_admissions.sql:44).- It is now required for every Postgres user, and drizzle runs it in one transaction.
ALTER TABLE workflow_runs ADD COLUMNtakes ACCESS EXCLUSIVE, and then theworkflow_hooksunique index builds while that lock is held.- Build the index before the
workflow_runsALTER, or split it out. At least say inpostgres.mdxthat the build scansworkflow_hookswhileworkflow_runsis locked.
- Unread start Hook payloads are re-offered before every event (
packages/core/src/events-consumer.ts:472).- In the idempotency-key-only use, eager-parked start Hook events stay in
parked, anddrainParked()offers each one to every callback before every event. That costs O(K × callbacks) per event on every replay, and the list grows for the run's whole life. - QuickJS also hydrates each payload into the heap and its snapshots.
- Offer them only once the start Hook is bound, and in QuickJS keep raw bytes until a payload is taken.
- In the idempotency-key-only use, eager-parked start Hook events stay in
- Test gap: no concurrent test covers two different hook ids of one run on one token (
world-postgres/test/atomic-start-hook.test.ts:626). That case must give ahook_conflictnaming the run, neverEntityConflictError. - Docs and small fixes:
- The start-or-route example throws when the owner never got a run (an orphan candidate) (
idempotency.mdx:117). upgrading-to-v5.mdx:154still sayshook_disposedalways releases the token.- The World support table doesn't mention the Postgres migration prerequisite.
workflow-world-error.mdx:49:retryAfterdoesn't mention the run-pending 503.start-hook-binding.ts:98puts raw start tokens in aWorkflowRuntimeErrormessage.- world-local's
hook_disposedre-reads the run record it already loaded.
- The start-or-route example throws when the owner never got a run (an orphan candidate) (
Not approving yet, because of item 1. It's a small change. The cross-region resume on the Vercel World also depends on a backend fix being tracked separately, so the Vercel opt-in should stay off until that ships.
🤖 Generated with Claude Code
pranaygp
left a comment
There was a problem hiding this comment.
Approving at b99a1b6. The blocker from my last pass is fixed:
- world-postgres
hook_disposedis back to main's singleDELETE … RETURNING(identical to main), so apps that don't use start Hooks pay nothing extra. The only added read is the admission's, insideif (startHook).
Also addressed:
- Disposal releases the token, as any Hook does (4750f87), consistently with the backend.
- Unread start Hook payloads are no longer offered to every callback until the Hook is opened (
events-consumer.ts). - Migration 0028 builds the
workflow_hooksindex before it locksworkflow_runs, with a 10slock_timeout. - Docs and error text: the
retryAfterdoc, the postgres prerequisite, and no raw tokens in the binding's error. - A concurrent test for two Hooks of one run on one token (one Hook, plus a conflict naming the run).
What I ran locally at this head:
- unit tests: core 3419, world 280, world-vercel 1027, world-postgres 470, world-local 762. world-local's 3
vercel-deployment-warningfailures come from theVERCEL*env on my machine. - e2e: the
start({ hook })block passes 19/19 on the Local World and on Postgres (withWORKFLOW_POSTGRES_EXPERIMENTAL_START_HOOK=1), each under Node and QuickJS. - CI: green.
Follow-ups:
WorkflowWorldError.code/status/reasonare dropped at the step boundary (CF6). These are deferred to their own PR, as agreed. Until then, a workflow can't tellSTART_HOOK_ADMISSION_REJECTEDfromWORLD_CONTRACT_ERROR.- The Vercel lanes skip the
start({ hook })e2e block untilWORKFLOW_VERCEL_EXPERIMENTAL_START_HOOKis set for them. Turn it on there once the backend is deployed, so cross-region resumes get e2e coverage before any caller opts in.
🤖 Generated with Claude Code
start(workflow, args, { hook: { token, metadata?, experimental_minRetention? } })
admits the run and creates its start Hook in one atomic World step. A duplicate
start throws HookConflictError (kind 'start') naming the owner and never
becomes a run. The start Hook (startHookId(runId)) can be resumed from
admission, before the run starts; the workflow's createHook({ token }) binds to
it without a write, and payloads resumed earlier are delivered in order.
The token is the run's idempotency key: the run holds it until it ends, even
after disposing the start Hook, then any experimental_minRetention applies.
Implemented for world-local, world-postgres (behind
WORKFLOW_POSTGRES_EXPERIMENTAL_START_HOOK, migration 0028; every Hook row now
claims its token through a unique index) and world-vercel (behind
WORKFLOW_VERCEL_EXPERIMENTAL_START_HOOK), with a shared conformance suite,
tracing of admission outcomes, docs and e2e coverage.
Co-authored-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
…-inert scan The terminal-inert check in TERMINAL_STATE_JS reads a drawn hook id with no __hooks entry as a closed Hook. The start Hook's id is derived from the run's id rather than drawn, so it sorts before every id the VM draws: once the VM had drawn one, a payload resumed into the start Hook before the body opened it was dropped as a closed Hook's straggler. reflected() now answers false for the start Hook until it is bound. Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
… it is wired
The spec 9 consumer does not yet skip turbo for a start Hook candidate, carry
the request on run_started, check the echo, or acknowledge a refusal. A
refused candidate's queued copy could therefore create its run unkeyed and run
it next to the token's owner, and Node would not bind createHook({ token }) to
the start Hook.
start() now throws a contract error, before it publishes or admits anything,
when the effective spec version is 9 or later (an explicit specVersion and a
probed target included). The spec 9 consumer acknowledges a keyed first
delivery, from another SDK version, before any write or execution. The
start({ hook }) e2e block skips on the spec 9 lane.
Signed-off-by: Alex Langenfeld <alex.langenfeld@vercel.com>
|
No backport to This is new feature work: it adds the To override, re-run the Backport to stable workflow manually via |
Summary & Motivation
start()used to create a new run on every call, so a workflow could only spot a duplicate after the duplicate run already existed. Now a start can carry a key: the World admits the run and creates its hook in the same atomic step. A second start with the same key is refused, gets the owner's run ID back, and never becomes a run.Workflows need to start at most once per key, and their hook needs to accept payloads from the moment the run is admitted.
start(workflow, args, { hook: { token, metadata?, experimental_minRetention? } })admits the run together with its start Hook (startHookId(runId)). A duplicate throwsHookConflictErrorwithkind: 'start'andconflictingRunId, and none of its code runs.resumeHook()failed until the workflow body calledcreateHook(), so callers had to retry. Now the start Hook accepts resumes as soon asstart()returns. In the workflow,createHook({ token })binds to the start Hook without a write, and any earlier payloads are delivered in order.experimental_minRetention. There is no default retention. Disposing the start Hook releases the token at once.start()can't confirm that its run was queued or admitted, it asks the World again with the same candidate. If it still can't confirm, it throws the newWorkflowStartErrorwith the candidate'srunIdandstage, and retrying with the same token is safe.cancel()of a candidate that never became a run stops it from ever being created.createHook()could both win the same token. Now every Hook row claims its token through a unique index (migration0028_add_start_hook_admissions), so only one can. The migration is required even with the flag off.start({ hook })throws a contract error before it publishes anything, because that engine isn't wired for start Hooks yet.WorkflowStartError. CLI re-runs don't forward the source run's start Hook. Admission outcomes and rejection reasons are traced.This addresses #2376 and #85 and carries forward #3425, #3426, #3449 and #3440. #5156 wires start Hooks into the single-orchestrator engine and lifts the spec 9 refusal.
Rollout: always on for the Local World. On the Postgres World it's behind
WORKFLOW_POSTGRES_EXPERIMENTAL_START_HOOK=1, which the CI Postgres lane sets. On the Vercel World it's behindWORKFLOW_VERCEL_EXPERIMENTAL_START_HOOK=1, which needs a backend that supports start Hooks.Test Plan
Added unit tests, a shared World conformance suite and a 19-test
start({ hook })e2e block. Locally, typecheck and the unit suites pass for the affected packages. The only failure is a Postgres listen-reconnect test that needs Docker-managed Postgres. The e2e block passes on nextjs-turbopack against the Local and Postgres Worlds, on both the Node and QuickJS VMs. A run that forced the Vercel capability on passed it on the prod, WS and HTTP lanes against the production backend.