Repository navigation
Conversation
A `hook.getConflict()` awaiter emptied the suspension's inline batch, so every step the workflow started beside the awaiter was created eagerly and handed to the queue: a publish, a second invocation and a replay, to run a step the invocation was about to run itself. The awaiter's own continuation already resumes in-process (#3938); only the steps paid the hop. The reason the batch was emptied is real — an inline `await executeStep(...)` blocks the handler for the step's whole duration, so an awaiter settled after the batch is serialized behind a step body, defeating work the workflow expressed as parallel. It is the deferral, not the eager create, that buys the ordering: a deferred step has written nothing the invocation must answer for, so the hook-write continuation can simply run first and the batch can run on the pass after it. The QuickJS engine already works this way (it feeds the registration into the live VM before executing its inline candidates); this brings the node:vm engine in line. So the suspension now defers steps beside an awaited hook exactly as it does elsewhere, the replay loop takes the continuation ahead of the inline batch, and the batch runs on the pass the continuation drives — joined by whatever the settled awaiter started in the meantime, so `hook.getConflict().then(() => stepB())` racing `await stepA()` now runs both inline in one invocation instead of splitting them across two. - The hold is bounded to one pass per step (`heldForHookContinuationIds`), so a workflow awaiting one registration after another cannot park a step behind all of them; past the bound the bodies run and the awaiter settles on the pass after them, still in the same invocation. - Inline pairs are never pre-claimed beside an awaiter: a committed `step_started` whose body the hold defers would be recovered through owned recovery, the crash path, for a pass where nothing crashed. - A lone step beside an awaited hook now leaves the hook create as the suspension's only write, so its inline delta carries the log forward and the continuation reads nothing at all. - A conflicting token now reaches the workflow before the deferred step is created, so a run that dedupes on `getConflict()` no longer leaves an orphaned `step_created` behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com> Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 930a27a The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 97513ms → this run 135524ms (Δ +38011ms, +39%) 📜 Previous results (3)f142680Fri, 23 Oct 2026 02:00:58 GMT · run logs
8f48034Thu, 22 Oct 2026 21:54:40 GMT · run logs
1876c99Thu, 22 Oct 2026 21:26:33 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. |
🧪 E2E Test Results✅ All tests passed 🛠 Infra Events (absorbed by the harness)Platform anomalies the e2e harness detected and worked around (e.g. a run the queue never picked up, replaced by a fresh run). Clustered timestamps indicate a backend blip; a steady drip indicates a platform issue worth escalating.
E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 🌐 Cross-language Conformance
✅ dynamic-runs
✅ vercel-http-transport
✅ vercel-multi-region
✅ vercel-ws-transport
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Sim WorldSimulated world deterministic testing for races. Traces 🟢 world-sim scenario book — 0 fail of 44 total
Full trace: |
About these numbersSizes are gzip; parentheses show the change against
|
… before the hook continuation, serializing parallel branches and potentially deadlocking steps that depend on that continuation. This commit fixes the issue reported at packages/core/src/runtime.ts:4773-4781 ### Trigger and impact With `WORKFLOW_RETAINED_VM=0`, `continueOverHookWrite()` returns false because there is no retained session. Since the suspension now defers sibling steps, `inlineExecutions` is nonempty and this branch falls through into their bodies instead of advancing the workflow over `hook_created`. For `Promise.all([stepA(), hook.getConflict().then(() => stepB())])`, `stepB` cannot start until `stepA` finishes. If A waits for an action B performs, the invocation stalls until timeout. Previously A was queued and the orchestrator immediately re-invoked to settle the registration, allowing both branches to advance independently. Serialization-driven rejection of retention reaches the same fallback. ### Suggested fix and validation The patch performs a bounded cold replay before executing the held batch when no retained session is available, preserving the continuation-first ordering without a queue hop. A regression test with retention disabled and A waiting for B failed on the PR head: B did not start until the test manually released A. With the candidate fix, both focused suites (`retained-vm-loop.test.ts` and `suspension-handler.test.ts`) pass all 144 tests. Deployment-level tests were not run. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: VaguelySerious <mittgfu@gmail.com>
VaguelySerious
left a comment
VaguelySerious
left a comment
There was a problem hiding this comment.
(AI) Preliminary review: AI review: blocking issues found
| dispatchesSettled, | ||
| suspensionResult.deferredBatchWork, | ||
| ]); | ||
| const holdInlineStepsForHook = lazyInlineSteps.some( |
There was a problem hiding this comment.
AI Review: Blocking
The one-pass hold bound runs an earlier step's body ahead of a later awaiter's continuation, serializing (or wedging) work that main and the QuickJS engine both run in parallel.
Once every lazy step is already in heldForHookContinuationIds, holdInlineStepsForHook is false, so the next awaited registration skips continueOverHookWrite and falls through to the inline batch: the held body runs to completion before the second awaiter settles. Commit 1876c99 fixed this for the !retainedSession path because "a body may depend on work started by the registration's continuation", but the retained path still does it once the bound is used up.
Repro (workbench/vitest against world-local, core rebuilt for each side; timedStep sleeps for delayMs):
const longP = timedStep('W', 4000);
using h1 = createHook({ token: t1 }); await h1.getConflict();
using h2 = createHook({ token: t2 }); await h2.getConflict();
const r = await timedStep('R', 100);
await longP;| R starts after W starts | |
|---|---|
| merge-base (node:vm) | 6 ms |
| this PR (node:vm) | 4021 ms |
this PR, WORKFLOW_VM=quickjs |
0 ms |
If W's body waits for something R does (a signal, a row, a resume), the invocation blocks until W times out, then recovers through owned recovery on redelivery. On main W was queued and ran in its own invocation. The same shape in the retained-vm-loop.test.ts harness (W blocks until R releases it) passes on the merge-base and times out here.
QuickJS doesn't have this problem because it feeds new events before it runs inline candidates on every turn, with no per-step bound. Two options:
- Hold for as long as a fresh awaited hook exists.
continuedHookIdsalready guarantees progress per hook, and the replay budget and inline-duration guard bound the invocation (the same argument the description makes for termination). This brings node:vm in line with QuickJS. - Keep the bound, but when it runs out, hand the held steps to the queue (eager create + publish, the old behavior) and continue over the hook write instead of running them before it.
Either way, I'd ask for a regression test for the chained shape where the later step unblocks the earlier one. holds a step for one continuation only, across chained awaited hooks currently enforces the serializing behavior.
There was a problem hiding this comment.
Confirmed and fixed — in 8f48034 (before this reply), by taking option 1.
I reproduced the retention-off half of it first (reverting only the runtime half of 1876c99 kept its test red at 1s), which made the shape of the rule obvious: the hazard is a held body running while a registration the workflow is awaiting is unsettled, and a per-step bound re-admits exactly that one pass later. So the bound is gone. continuedHookIds already grants one continuation per awaited hook, which bounds the hold on its own — another pass may only hold again by presenting a hook the workflow newly created and awaited, and each of those costs the workflow its own write regardless. The rule is now one sentence, and it matches QuickJS.
1876c99's own fix is kept but folded into continueOverHookWrite as an explicit allowColdReplay, so the bookkeeping, the debug log and the span attribute all live in one place (see the reply on your nit about that).
Regression test added for the chained shape you asked for: does not run a step that the second registration unblocks before settling it — a step racing two separately-suspended registrations, parked until a step the second continuation starts releases it. It deadlocks under the reinstated per-step bound (fails at 1027 ms, release step never starts) and passes now. holds a step for one continuation only, across chained awaited hooks is gone; holds a step across every awaited registration, one continuation each replaces it and asserts 2 continuations, so it catches the bound as well (I checked both by reinstating it).
| // full step duration, so an awaiter settled after the | ||
| // batch would be serialized behind a step body — | ||
| // defeating work the workflow expressed as parallel | ||
| // (e.g. `hook.getConflict().then(() => stepB())` racing |
There was a problem hiding this comment.
AI Review: Note
The parallelism kept here only goes one step deep. Once a step is held for the continuation, the next pass runs it in the same inline batch as whatever the settled awaiter started, and the batch awaits every body (Promise.all(stepExecutionPromises)). So the awaiter's second step waits for the held step to finish:
const branch = hook.getConflict().then(async () => {
await timedStep('B', 100);
await timedStep('C', 100);
});
const a = await timedStep('A', 4000);C starts 115 ms after A starts on the merge-base and 4020 ms after it with this PR (world-local, node:vm). QuickJS gives 4016 ms too, and plain non-hook code (const p = a(); await b(); await c();) behaves the same, so this matches the existing inline model rather than being a bug. But it is a user-visible change from main, where the step beside the awaiter ran in its own invocation and the branch stayed fully parallel.
This comment, the one at ~4716, and the PR description all say the hold keeps hook.getConflict().then(…) racing await step() "genuinely parallel". I'd reword them and mention the change in the changeset. The e2e hookGetConflictThenStepParallelWorkflow only checks the branch's first step, so it can't catch this.
There was a problem hiding this comment.
You're right, and the "genuinely parallel" wording was mine overreaching. Reworded in both comments and the changeset.
What the hold actually buys is that the branch off the awaiter reaches the same batch as the step it was started beside — not that it becomes independent of the invocation. The batch awaits every body, so a third step the branch starts after its own first one waits for the held step too. As you say, that is the inline model everywhere else (and QuickJS gives the same number), so I've taken it as the trade-off rather than a bug, and said so where it will be read:
- the comment above
lazyInlineStepsnow ends with "One batch, not an unbounded fork: … Holding bounds the branch's reach to one batch; it does not make the branch independent of the invocation." - the continuation block's docstring says the same, naming the
Promise.all(stepExecutionPromises)join as the reason. - the changeset now states the change from
mainoutright: "Those steps now share one inline batch with whatever the settled awaiter started, so work the awaiter's branch begins after its own first step waits for them — where queueing put the branch in a separate invocation." - the PR description's parallelism claim is scoped the same way.
I deliberately did not add a test pinning "C waits for A". It would be a change detector that whoever improves the inline model has to delete, and the honest place for it is the comment next to the join. Happy to add one if you'd rather have it recorded.
|
|
||
| On a conflict, the resolved value is a `Run` handle for the run that owns the token, with durable step-backed accessors. The duplicate run can decide in code how to handle it: return or log `conflict.runId`, inspect `await conflict.status`, wait on `await conflict.returnValue`, or cancel the owner with `await conflict.cancel()` and continue in the current run. See [Run idempotency](/docs/foundations/idempotency#run-idempotency) for these strategies in context. | ||
|
|
||
| Work started *beside* the awaiter, rather than after it, is not slowed down by it. In `await Promise.all([hook.getConflict(), prepare()])` the registration and the step are scheduled together: the runtime commits the registration, resolves the awaiter in the same invocation, and then runs `prepare()` there too, so the step does not pay an extra round trip through the queue for having been started next to a `getConflict()`. The step still starts after the registration is recorded, which is what makes this shape interchangeable with awaiting the registration first. |
There was a problem hiding this comment.
AI Review: Note
"The step still starts after the registration is recorded, which is what makes this shape interchangeable with awaiting the registration first" only holds for the first WORKFLOW_MAX_INLINE_STEPS steps. Steps past the cap keep their eager create + publish, and those run concurrently with the hook create (the handleSuspension docstring says so itself), so they can start before the registration commits. With WORKFLOW_MAX_INLINE_STEPS=1 and Promise.all([hook.getConflict(), s1(), s2()]), s2 is created and queued in the same suspension as the hook.
The conflict case has the same limit: with overflow steps beside the awaiter, a run that dedupes on the conflict still leaves step_created rows behind (['hook_conflict', 'step_created', 'run_completed'] in the retained-VM harness). So the description's "no longer leaves an orphaned step_created behind" is also cap-bound.
This sentence reads as an ordering guarantee users will lean on (e.g. a step that hands the token to something that resumes it). I'd either qualify it with the cap, as runtime-tuning.mdx does, or keep recommending await hook.getConflict() before the step when ordering matters. The same applies to foundations/hooks.mdx:117.
There was a problem hiding this comment.
Agreed — it read as an ordering guarantee and it is not one. Qualified rather than dropped, on both pages, and pinned with a test.
create-hook.mdx now splits the claim in two: the scheduling win, then "This is a scheduling improvement, not an ordering guarantee. Only the steps the invocation runs inline — the first WORKFLOW_MAX_INLINE_STEPS of them — start after the registration is recorded. A wider fan-out beside the awaiter has its remaining steps created and queued alongside the hook, exactly as described above, so they can start before the registration commits and can start even if it conflicts. When the ordering is what you need, await hook.getConflict() before calling the step." foundations/hooks.mdx gets the short form and links here.
Your conflict prediction was exact, so it is now a test: still creates a step past the inline cap before the conflict settles drives Promise.all([hook.getConflict(), s1(), s2()]) against a taken token with WORKFLOW_MAX_INLINE_STEPS=1 and asserts ['hook_conflict', 'step_created', 'run_completed']. The PR description's "no longer leaves an orphaned step_created" is scoped to the cap too.
| * continuation. Steps in the same suspension ride along deferred rather than | ||
| * queued: an awaiter holds `lazyInlineSteps` back for the pass the | ||
| * continuation drives, and the conflict branch returns before any inline | ||
| * execution, so nothing this invocation does can order the continuation behind |
There was a problem hiding this comment.
AI Review: Nit
"nothing this invocation does can order the continuation behind a step body" no longer holds on the bounded fall-through (see the blocking comment at ~4752). There's a similar stale comment at ~4983: "the awaited-hook case emptied lazyInlineSteps; the checks below are defensive". lazyInlineSteps isn't emptied any more, and a batch can reach the turbo gate with hasAwaitedHookCreation true on the fall-through, so !suspensionResult.hasAwaitedHookCreation in forceOptimisticStart is now load-bearing rather than defensive.
There was a problem hiding this comment.
Both fixed.
The retention-policy sentence now says a step body is never ordered ahead of "a continuation this invocation is still going to take", with the refused-repeat fall-through called out in parentheses — that is the only way the batch gets to run first now that the per-step bound is gone.
You're right that !suspensionResult.hasAwaitedHookCreation in forceOptimisticStart is load-bearing rather than defensive. The turbo comment now says which of the three checks are defensive and which is not: "…making those two checks defensive; hasAwaitedHookCreation is not, since an awaiter whose continuation was refused falls through to this batch with the flag still set."
Also refreshed two more stale references the same change left behind: the chainedGetConflictWorkflow fixture comment (still described the removed bound) and handleSuspension's note pointing at heldForHookContinuationIds.
| }); | ||
|
|
||
| it('still runs the step inline under the kill switch, with no queue hop', async () => { | ||
| // No retained session to resume, so the awaiter settles on the cold |
There was a problem hiding this comment.
AI Review: Nit
After 1876c99 this comment is backwards, and so is the "Kill switch" bullet in the PR description. Under WORKFLOW_RETAINED_VM=0 the awaiter now settles on a cold replay before the step body runs, not on the replay that follows its terminal write. The test only checks result/queueSends/vmBuilds, so it can't tell which order happened. does not block the hook continuation behind a step when retention is off is the test that pins the order.
There was a problem hiding this comment.
Right on both counts. 8f48034 already flipped the comment to "the continuation is a cold replay over the reloaded log rather than a VM resume", and the PR description's kill-switch bullet with it; this commit adds the scope you're pointing at, since the test genuinely cannot tell the order: "This case only pins the delivery; does not block the hook continuation behind a step when retention is off is what pins the order the two happen in."
| (id) => !continuedHookIds.has(id) | ||
| ) | ||
| ) { | ||
| for (const id of suspensionResult.awaitedHookCorrelationIds) { |
There was a problem hiding this comment.
AI Review: Nit
This cold-replay hold adds to continuedHookIds but doesn't emit the 'Continuing over a hook write in-process' debug log or update the workflow.hook_write_continuations span attribute. A trace of a WORKFLOW_RETAINED_VM=0 (or serialization-blocked) run won't show why there was an extra full replay before the batch. Could it log and set the attribute the same way, perhaps with a retained: false / readBeforeResume: true field?
There was a problem hiding this comment.
Done in 8f48034, by routing the cold replay through continueOverHookWrite instead of open-coding it beside it — which was the right shape anyway, since the open-coded version also duplicated the continuedHookIds bookkeeping.
It now takes an allowColdReplay option, so one place does the fresh-id filter, the reload-vs-resume decision, the debug log and the span attribute. The log line carries coldReplay alongside the existing carriedForward / readBeforeResume fields (readBeforeResume is already true on this path, since a cold replay always reloads), and workflow.hook_write_continuations is updated the same way as the retained path. The awaited-creation branch passes allowColdReplay only when it has bodies to hold; callers with nothing to protect keep refusing, so the documented kill-switch behavior for a step-less awaiter and for every conflict is unchanged.
…teps Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
…get-conflict-inline-steps Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
…lay included Follow-up to the review finding on the fall-through path, with its fix folded into `continueOverHookWrite` and its bound replaced. The finding is real and I reproduced it: with `WORKFLOW_RETAINED_VM=0` there is no session to resume, so the continuation was refused and the held bodies ran first. For `Promise.all([stepA(), hook.getConflict().then(() => stepB())])` with A waiting on something B does, that stalls the invocation until its inline budget runs out — a deadlock the queued-step path could not have, because the step ran somewhere else entirely. `continueOverHookWrite` now takes `allowColdReplay`, and the awaited-creation branch passes it whenever it has bodies to hold: with no session, a cold replay over the reloaded log advances the workflow just as well, for a replay this boundary used to pay in the next delivery anyway. Callers with nothing to protect keep refusing, so the kill switch hands a step-less awaiter (and every conflict) back to the queue exactly as before. The per-step hold bound goes with it. `continuedHookIds` already grants one continuation per awaited hook, which bounds the hold on its own: another pass may only hold again by presenting a hook the workflow newly created and awaited, and each of those costs the workflow its own write regardless. The per-step bound bought a step one hook write of earliness in a chained- registration shape and, in exchange, allowed exactly the hazard above one pass later — a held body running while a second registration was still outstanding. Dropping it makes the rule one sentence: a held body never runs while a registration the workflow is awaiting is unsettled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com> Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
Six review comments, two of which the previous commit had already answered (the blocking one, by taking its option 1, and the missing continuation log, by folding the cold replay into `continueOverHookWrite`). The rest: - The chained shape the blocking comment asked for a test for: a step racing two separately-suspended registrations, released by a step the SECOND continuation starts. It deadlocks under the per-step bound and passes now, and `holds a step across every awaited registration` catches the bound too. - The parallelism the hold buys is one batch deep, not unbounded. The batch awaits every body, so work the awaiter's branch starts after its own first step waits for the held step — the inline model everywhere else, and what QuickJS does, but a change from queueing the step into another invocation. Said so in both comments that claimed "genuinely parallel", and in the changeset. - The ordering is bounded by `WORKFLOW_MAX_INLINE_STEPS`: steps past the cap keep their eager create beside the hook, so they can start before the registration commits, and a conflict can still leave their `step_created` behind. Qualified both docs pages rather than letting the sentence read as a guarantee, and pinned the conflict case with a test asserting exactly the `['hook_conflict', 'step_created', 'run_completed']` log it produces. - Refreshed the stale comments: the retention policy's "nothing this invocation does can order the continuation behind a step body" (the refused-repeat fall-through does), turbo's "the awaited-hook case emptied lazyInlineSteps" (it does not, and that check is now load-bearing), the chained fixture's reference to the removed bound, and the kill-switch test's scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com> Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
|
Review addressed — thanks, the blocking one was a real hole and the notes caught three claims I had overstated. Rundown, with per-comment detail in the threads: Blocking (the one-pass hold bound). Confirmed, and fixed in 8f48034 by taking your option 1: the per-step bound is gone and Parallelism is one batch deep. You're right and my "genuinely parallel" wording was wrong. Taken as the trade-off (it is the inline model everywhere else, and QuickJS already did it), and now stated in both comments, the changeset, and the PR description rather than glossed. The ordering is cap-bound. Qualified on both docs pages instead of reading as a guarantee, and your conflict prediction is now a test asserting exactly the Nits. Stale comments refreshed (retention policy, turbo's "defensive" checks —
|
|
Heads up: The deployment itself is fine: Everything else is green: 180 pass, 0 test failures, including every other Vercel Prod lane on both VMs. Could someone with write access re-run that job? I only have an app token here, which |
`E2E Vercel Prod Tests (nuxt - node)` failed on f142680 in `Resolve Vercel deployment`, before any test ran. The wait action found the deployment (`dpl_3SHU9... is BUILDING`) and then got `fetch failed` on every poll for 17 minutes until its 1000s timeout, so `VERCEL_DEPLOYMENT_ID` came out empty and the suite was skipped. Nothing about the change: the deployment reached Ready, and `nuxt - quickjs` tested against that same deployment successfully on the same commit, as did nuxt's local dev, prod and Postgres lanes. Re-running the one job needs `actions: write`, which this token does not have, so this empty commit is the re-trigger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com> Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
…inherits Node 24
`workbench/nest` is the only deployed workbench app that declares
`engines.node`, and it declares the open-ended `>=20.0.0` the `nest new`
scaffold emits. Vercel reads that range and resolves an open-ended one to the
newest major, so once Node 26 became available it overrode the project's 24.x
setting:
Warning: Due to "engines": { "node": ">=20.0.0" } ... the Node.js Version
defined in your Project Settings ("24.x") will not apply, Node.js Version
"26.x" will be used instead
Skipping build cache since Node.js version changed from "24.x" to "26.x"
[ERR_PNPM_UNSUPPORTED_ENGINE] Expected version: ^24.0.0 Got: v26.12.1
The root manifest pins `^24.0.0` (#5011), so `pnpm install` refuses and the
deployment fails before any build script runs — which fails
`E2E Vercel Prod Tests (nest - *)` in `Resolve Vercel deployment`, since there
is no deployment to resolve.
This is latent on `main`, not introduced here: main's last nest deployment
succeeded at 19:31 UTC, before the version flipped. Removing the block is what
the other 17 deployed workbench apps already do — none declares `engines`, so
each takes its project's configured version — and the published
`@workflow/nest` package declares none either, so nothing advertises a floor
this drops. The `npm` half goes with it: it is the same scaffold leftover, and
the repo installs with pnpm.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
|
The re-trigger turned up a second, unrelated red — this one real and latent on
Not introduced here: main's last nest deployment succeeded at 19:31 UTC, before the version flipped, and the next one will hit this. The earlier |
Summary
Awaiting
hook.getConflict()made every step started beside it go over the queue. The awaiter's own continuation has resumed in-process since #3938; only the steps still paid a publish, a second invocation and a replay to run work the invocation was about to run itself. vercel/eve#5240 hit this while parallelizing session boot —createSessionStep()ran next to the inbox hook's registration, so the first model step waited for a second invocation — and worked around it by confirming the registration only after the step it was started beside.The reason the batch was emptied is real, and the PR keeps it: an inline
await executeStep(...)blocks the handler for the step's whole duration, so an awaiter settled after the batch is serialized behind a step body, defeating work the workflow expressed as parallel. What was wrong is the mechanism. It is the deferral, not the eager create, that buys the ordering — a deferred step has written nothing the invocation must answer for, so the hook-write continuation can simply run first and the batch can run on the pass after it.The QuickJS engine already does exactly this: it writes the hook, feeds the registration into the live VM, and only then executes its inline candidates (whose creates it skipped). This brings the node:vm engine in line.
What changed
handleSuspensiondefers steps beside an awaited hook exactly as it does elsewhere, instead of emptyinglazyInlineSteps.step_startedthis invocation owns; if the hold defers its body, the step would come back through owned recovery — the crash path, for a pass where nothing crashed. The deferral still applies; only the fold is off.Net effect for
await Promise.all([hook.getConflict(), step()]): one invocation, one hook write, one lazy claim, zero queue sends and zero extra event-log reads (the hook create is now the suspension's only write, so its inline delta carries the log forward). Before: 1 publish + 1 step invocation + 1 wake + 1 replay, and a read on the continuation because the eagerstep_createdbrokeeventLogCarriedForward.hook.getConflict().then(() => stepB())racingawait stepA()improves too: holding stepA means stepB exists by the time the batch runs, so both run inline in one invocation rather than splitting across two.That parallelism is one batch deep, which is a change from
mainworth stating plainly (raised in review, and now in the changeset). The batch awaits every body, so a third step the branch starts after stepB waits for stepA as well — the same shape plainconst p = a(); await b(); await c();has, and what the QuickJS engine already did here, where queueing stepA put the branch in a separate invocation entirely. Holding bounds the branch's reach to one batch; it does not make it independent of the invocation.Correctness, ordering, retries
step_started, whose atomic create-claim is the same exactly-one-owner gate every inline step already relies on; a concurrent replay that re-derives the step loses the claim withEntityConflictError→skipped.suppressOptimisticStartis true for any suspension carrying a hook, so the body runs only after the claim is won.continueOverHookWritenow takesallowColdReplayand the awaited-creation branch passes it whenever it has bodies to hold, so the kill switch gets the same ordering through a cold replay instead. Callers with nothing to protect (a conflict, an awaiter whose suspension scheduled no inline work) keep refusing, so the documented kill-switch behavior for those is unchanged.continuedHookIdsgrants one continuation per awaited hook, which bounds the hold on its own: a further hold needs a hook the workflow newly created and awaited, and each of those costs the workflow its own write regardless. Once the continuation is refused — a repeat for the same hook, which cannot get anywhere — the bodies run and the registration is observed on the pass after them. The replay budget and the inline-duration guard bound the invocation either way. Both ends are tested (holds a step across every awaited registration…,stops holding and runs the step when a repeat pass cannot see the registration).hasAwaitedHookCreationis false on a conflict, so the existing conflict branch returns before any dispatch or inline execution — and now before the deferred steps are created at all. A run that dedupes ongetConflict()no longer leaves orphanedstep_createdrows for those (never creates a step held beside an awaiter the conflict settles, which fails onmainwith an extrastep_created). Steps pastWORKFLOW_MAX_INLINE_STEPSkeep their eager create beside the hook and can still leave a row behind — raised in review, and now pinned bystill creates a step past the inline cap before the conflict settles, which asserts exactly the['hook_conflict', 'step_created', 'run_completed']log it produces. Both docs pages say so rather than reading as an ordering guarantee.WORKFLOW_RETAINED_VM=0the continuation is a cold replay over the reloaded log rather than a VM resume. Same ordering, same delivery: the step still runs here and nothing is enqueued — better than the re-invocation this shape used to take.WORKFLOW_MAX_INLINE_STEPSkeep their eager create and their queue hop, which is the point of the cap. They also keepeventLogCarriedForwardfalse, so that suspension's continuation reads once, as before.Validation
packages/coreunit/integration suite on the merged tree: 3221 passed, 6 expected fail, 1 skipped.retained-vm-loop.test.ts, which drives the real handler over a stateful in-memory World. All seven fail without the fix, and the eight pre-existinghook write continuationtests still pass:listCalls === 1andqueueSends === 0;WORKFLOW_RETAINED_VM=0;allowColdReplayforced off while everything else stays green);holds a step across every awaited registration, one continuation eachcatches that bound too).suspension-handler.test.tsupdated: deferral still respectsgetMaxInlineSteps()beside an awaiter, nothing is pre-claimed, and the awaiter's lone-step suspension now carries the log forward (a new case keeps the "a step past the cap also wrote" accounting).workbench/nextjs-turbopack+world-local: the fulle2e.test.tssuite (151 passed / 1 skipped), and every hook test re-run on the final tree (42 passed). The fivehookGetConflict*tests — includinghookGetConflictWithParallelStepWorkflowandhookGetConflictThenStepParallelWorkflow, which asserts stepB starts before a 10s stepA ends — pass under both engines (WORKFLOW_VMunset and=quickjs, engine use confirmed in the server log).workbench/vitestsuite under both engines: 44 passed / 1 expected fail each.pnpm typecheck(43/43),biome ci,node scripts/check-changesets.mjs.On the benchmark comment
Read it as lane noise, not as this PR. Every path the change touches is gated on a
hook.getConflict()awaiter (hasConflictAwaiteron the queue item), and no benchmark scenario awaits one —97_bench.tscallscreateHook()but nevergetConflict(), so for all five scenarios the diff is provably inert:lazyInlineCorrelationIdsresolves to the same slice,inlinePairFoldEligiblegains&& true,continueOverHookWriteis never called, and the new branch is onefalseboolean read per pass.The lane itself swings far wider than this PR's deltas on unrelated changes: cumulative STSO came in at -6% on #4999, -6% on #5011 (a Node version bump), and -31% on #4990, while the baseline it compares against moved 105718 → 129233 → 105834 ms across consecutive main runs, and TTFS "best" ranges from -67% to +526% on those same PRs. The job is informational and green.
Simulation instead of TLA+
The repo already owns the tool this change needed, and it is not TLA+:
@workflow/world-simplus theworkbench/sim-worldscenario book runs the real runtime against a deterministic World where the script states the interleaving instead of racing for it, then re-reads the committed log, re-derives entity state, and cold-replays the run to check it agrees with itself. A TLA+ spec would model an abstraction of the runtime that I would have to write and keep in sync, against a repo with no.tlafiles, no TLC configuration and no CI lane to run one — and the property at issue here ("does deferring the step's rows behind the registration still replay?") is exactly what the simulator checks directly.Two scenarios added, the book's idiomatic pair — same workflow, same outcome, payload delivered one world call apart:
claim-hook-at-registration— the payload lands inside thehook_createdcommit, so the whole rest of the run, the deferred step included, is written behind an unclaimedhook_received.claim-hook-at-step-started— the payload lands inside the deferred step's own claim, the window that only exists because the step's rows now sort after the registration.Both pass with
replay=okand no consistency violations. Full book on the merged tree: 44 scenarios, 44 passed, 0 failed, 0 consistency violations.Docs
Updated to describe the shipped behavior.
foundations/hooks.mdxapi-reference/workflow/create-hook.mdxconfiguration/runtime-tuning.mdxWORKFLOW_MAX_INLINE_STEPS,WORKFLOW_RETAINED_VMThe preview sits behind deployment protection, so these need Vercel team access.
🤖 Generated with Claude Code