[core] Settle a hook's awaiter in-process instead of re-invoking, on creation and on conflict - #3938
Conversation
🦋 Changeset detectedLatest commit: f17930b The changes in this PR will be included in the next version bump. 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 |
There was a problem hiding this comment.
🟡 Changes recommended
The newly added hook tests rely on a fixed real-time sleep (setTimeout(..., 20)), which can introduce avoidable flakiness and should be made deterministic.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR optimizes the hook.getConflict() awaiter path in @workflow/core by continuing execution in-process (via retained VM) after a hook_created write, instead of forcing a queue re-invocation and cold replay to read back the just-written event. It extends the existing sinceCursor “inline delta” contract so hook creation can carry the event log forward when it is the sole continuation for the suspension.
Changes:
- Request and (when safe) fold an inline event-log delta (
CreateEventParams.sinceCursor) fromhook_createdwrites so the caller can continue without anevents.listround-trip. - Relax VM retention for the specific “awaited hook creation” boundary and resume the retained session in-process; keep a fallback to
reinvoke(0)for repeats / kill switch / missing delta. - Add generation-guarded hook suspension signaling and new tests covering the in-process continuation and delta-folding behavior.
File summaries
| File | Description |
|---|---|
| packages/world/src/events.ts | Documents the expanded sinceCursor delta use for hook_created and updates EventResult producer docs. |
| packages/world-sim/src/store.ts | Mirrors backend behavior by returning sinceCursor deltas for hook_created (in addition to step-terminal events). |
| packages/core/src/workflow/hook.ts | Adds a suspension-generation guard to hook idle signaling to avoid stale suspensions after retained-session resume. |
| packages/core/src/workflow/hook.test.ts | Adds tests verifying stale hook suspension signals are dropped and live ones still fire. |
| packages/core/src/runtime/suspension-handler.ts | Requests/absorbs sinceCursor deltas for single-hook awaited creations and reports eventLogCarriedForward + awaited hook IDs. |
| packages/core/src/runtime/suspension-handler.test.ts | Adds coverage for delta folding, cursor movement, and “carry-forward” gating conditions. |
| packages/core/src/runtime/helpers.ts | Extracts appendEventLog helper shared by runtime loop and suspension handler. |
| packages/core/src/runtime.ts | Extends retention predicate for awaited hook creation and resumes retained VM in-process for hook.getConflict() awaiters. |
| packages/core/src/retained-vm-loop.test.ts | Adds an integration-style harness test asserting the awaiter resolves within one delivery (and fallback cases). |
| packages/core/src/private.ts | Updates docs to reflect hook suspension signals now use the generation guard. |
| .changeset/hook-conflict-in-process.md | Changeset describing the optimization and fallback behavior. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /** Let every queued timer and microtask settle. */ | ||
| function settleTimers(): Promise<void> { | ||
| return new Promise((resolve) => setTimeout(resolve, 20)); | ||
| } |
There was a problem hiding this comment.
Done — settleTimers now awaits a handful of explicit setTimeout(0) turns (the unit scheduleWhenIdle polls in) instead of a fixed 20ms sleep.
| '@workflow/world': patch | ||
| --- | ||
|
|
||
| Resolve a `hook.getConflict()` awaiter in the invocation that created the hook instead of re-invoking through the queue. The `hook_created` write now asks for the event-log delta since the replay's cursor (`CreateEventParams.sinceCursor`), and the runtime resumes the retained VM over the returned event — removing a delivery round-trip and a cold replay per awaited hook. Worlds that return no delta fall back to an incremental read, and `WORKFLOW_RETAINED_VM=0` restores the re-invocation. |
There was a problem hiding this comment.
| Resolve a `hook.getConflict()` awaiter in the invocation that created the hook instead of re-invoking through the queue. The `hook_created` write now asks for the event-log delta since the replay's cursor (`CreateEventParams.sinceCursor`), and the runtime resumes the retained VM over the returned event — removing a delivery round-trip and a cold replay per awaited hook. Worlds that return no delta fall back to an incremental read, and `WORKFLOW_RETAINED_VM=0` restores the re-invocation. | |
| Resolve `hook.getConflict()` in the same invocation after creating the hook, avoiding a queue hop and replay. Fall back to an incremental read or the existing re-invocation path when needed. |
There was a problem hiding this comment.
Took the terser shape, and split it per package so each entry says only what changed in it: @workflow/core gets the in-process settle (now covering both hook_created and hook_conflict, since the conflict path landed in the same PR), and the three World packages get a one-liner about answering the sinceCursor delta on hook_conflict.
karthikscale3
left a comment
There was a problem hiding this comment.
out of scope for this PR, but wondering separately if there is any harm in optimizing the hook_conflict scenario also to return the event log and continue in process instead of re enqueuing by default?
karthikscale3
left a comment
There was a problem hiding this comment.
Reviewed the retained-VM continuation path and its fallbacks. The hook_created delta is only consumed when complete; missing or partial deltas fall back safely, and conflicts retain the existing re-invocation behavior. Approved.
Sim WorldSimulated world deterministic testing for races. Traces 🟠 world-sim scenario book — 1 fail of 41 total
Full trace: |
About these numbersSizes are gzip; parentheses show the change against
|
|
No backport to This is a performance optimization, not a stability fix: it settles a hook's awaiter on the retained VM in the writing invocation instead of paying a queue hop plus cold replay, and it adds new surface ( To override, re-run the Backport to stable workflow manually via |
…Release job `changeset version` assembles a release plan from every pending changeset before it bumps anything, and throws on a changeset it cannot place there: one naming a package outside the workspace, or one mixing a package from the `ignore` list with published ones. #3938 shipped the latter and every push to main since has failed to publish (#3963). Nothing at PR time ran that step. scripts/check-changesets.mjs runs the same assembly on the same inputs, resolving the libraries from @changesets/cli's own install so the check uses exactly the versions the Release job does, and stops before the network-bound changelog generation. lint.yml runs it on every PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…Release job (#3964) `changeset version` assembles a release plan from every pending changeset before it bumps anything, and throws on a changeset it cannot place there: one naming a package outside the workspace, or one mixing a package from the `ignore` list with published ones. #3938 shipped the latter and every push to main since has failed to publish (#3963). Nothing at PR time ran that step. scripts/check-changesets.mjs runs the same assembly on the same inputs, resolving the libraries from @changesets/cli's own install so the check uses exactly the versions the Release job does, and stops before the network-bound changelog generation. lint.yml runs it on every PR. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…c-workflow-source * origin/main: (22 commits) feat(streams): add writer session seam (#3832) docs: document WORKFLOW_NODE_HTTP in the v4 World docs (#4050) [world-vercel] Honor WORKFLOW_NODE_HTTP on the queue transport (#4044) feat(streams): add WebSocket capability gate (#3764) fix(world-local): retry JSON reads on Windows (#4051) [core] Add the wake-loop scenario to the event log race repro (#4017) [ci] Cap concurrent Vercel E2E action repo-wide (10 by default) (#4039) Add `Run#getWritable()` for appending to another run's stream (#3972) feat(streams): add WebSocket v1 client protocol contract (#3763) [swc-playground] Update to Next.js v16.3.4 (#4018) [core] Add a retention option to start() (#3787) fix(swc-plugin): register class expressions via an IIFE instead of by name (#3971) [docs] Fix prose typos across v4/v5 docs and the SWC plugin README (#3948) Validate pending changesets in CI so a bad one fails the PR, not the Release job (#3964) Version Packages (beta) (#3919) Classify Workflow stream failures (#3850) Drop the ignored @workflow/world-sim package from the hook_conflict delta changeset (#3963) Add attribute inspection to the CLI (#3950) [core] Settle a hook's awaiter in-process instead of re-invoking, on creation and on conflict (#3938) Use the storage APIs for run detail views (#3944) ...
The problem
A suspension whose only pending continuation is a hook's own awaiter is resolved by the event that hook's create commits, and by nothing else. The runtime answered both of those events with
reinvoke(0):So the invocation wrote the event, threw away the VM that was one
awaitfrom consuming it, handed the run to the queue, and paid a full replay to read back what it had just written. The parked VM was discarded because the boundary was unretainable:getRetentionDecisionrefuses a suspension with no step or attribute write asno_replay_driver, on the grounds that nothing in the current delivery can advance it — which is true of a hook-only boundary in general, and false of this one.A create takes one of two outcomes and both settle that awaiter:
hook_createdfor a clean registration, andhook_conflictwhen another run already holds the token — rejecting a payload await, or resolvinghook.getConflict()with the conflicting run. Same write, same event slot, same continuation.The change
Carry the log forward on the write.
handleSuspensionnow setsCreateEventParams.sinceCursoron the hook create (the same contract the step-terminal write already uses), folds a complete delta into the caller's loaded log, and reportseventLogCarriedForward— true only when the delta accounted for every event the suspension committed. Asked for on the single-hook suspension only: two creates issued from one snapshot diff against the same cursor and only the first delta can be folded in, so the log would end up short of the other's event with nothing to say so. The delta is indifferent to which event was committed — it is the slice of the log after the caller's cursor either way.Server side: vercel/workflow-server#913.
Continue on the retained VM, on either outcome. Both branches resume the parked session in the same invocation instead of re-invoking, through one
continueOverHookWritehelper. When the delta carried the log forward the next iteration reads nothing at all; otherwise it does one incremental read from the cursor — still cheaper than the delivery round-trip and full replay it replaces. The conflict branch keeps its position ahead of the attribute detour and all step dispatch: aPromise.racebetween the hook and a step must still let the durable conflict win without executing the losing step, so what changes is only how the workflow gets to observe it.Retain across exactly this boundary.
getRetentionDecisiongrows ahookContinuationarm.mainalready retains across open hooks and waits when a step or attribute write drives the next iteration, and refuses a hook- or wait-only suspension asno_replay_driver. Here the runtime itself is the driver: the continuation resumes the session in-process, so the boundary is retained on its own terms. Steps in the same suspension ride along queued (an awaiter emptieslazyInlineSteps, and the conflict branch returns before any inline execution). An open wait in the log does not block retention but does force the incremental read before resuming, for the reason the inline-delta gate already gives: await_completedis a resolution the replay is waiting on rather than an event it can observe an iteration late.Hook signaler guard. The hook consumer's suspension signal must carry the generation guard for this boundary to be retainable at all: without it, a signal armed for the boundary the resume moved past would raise a suspension the workflow never reached.
mainnow provides that through the sharedscheduleWorkflowSuspensionhelper (#3604), so this PR no longer adds its own; it keeps the twohook.test.tscases that pin the behavior (stale signal dropped, live signal still lands).Worlds.
world-simandworld-localboth return ahook_conflictearly, ahead of their sharedsinceCursorblock, so they answered no delta on exactly the write that asked for one. Both now compute it there.CreateEventParams.sinceCursordocuments that a World answering the delta onhook_createdmust answer it onhook_conflict.Docs. The
WORKFLOW_RETAINED_VMentry in runtime tuning said hook-only suspensions always park the invocation. It now names this one exception.What is deliberately unchanged
lazyInlineSteps, so every step the suspension schedules is queued and the continuation races them rather than waiting behind a step body. The continuation happens before any inline execution, so the ordering the empty batch protects is preserved. (The conflict branch gets this for free — it returns before step dispatch entirely.)WORKFLOW_RETAINED_VM=0restores the re-invocation exactly. So does a World that returns no delta and a read that still does not show the write: continuations are tracked by hook id, so a repeat for the same hook falls back toreinvoke(0)rather than spinning, while a workflow creating one awaited hook after another keeps continuing in-process for each.Also lifts
appendEventLogintoruntime/helpers.ts— the suspension handler and the replay loop now share the one log-append primitive instead of keeping two copies that have to agree.Testing
Rebased onto
mainafter #3604 / #3609 / #3892 landed.packages/core: 2351 passed (+ 3 expected fail, 1 skipped).world-simandworld-localsuites pass.tscclean on every changed package.New coverage:
runtime/suspension-handler.test.ts— the created hook event is folded into the caller's log and the cursor moves with it; the same for a committedhook_conflict, which is reported by hook id withhasAwaitedHookCreation: false;eventLogCarriedForwardis false when a step also wrote, when a wait also wrote alongside a conflict, when two hooks are created, when the delta is truncated, when the World returns none, and when the log has no cursor (turbo); an awaiter-less conflict defers its step and so still carries the log forward.retained-vm-loop.test.ts— agetConflictworkflow completes inside a single delivery with oneevents.listand zero queue sends. The same for a conflicting create, both with agetConflict()branch and with a plain payload await (the shape that reports onlyhasHookConflict). Both still complete, with an extra read, when the World withholds the delta; both fall back to the re-invocation underWORKFLOW_RETAINED_VM=0; and a repeat pass that still cannot see the conflict re-invokes after exactly one retry instead of spinning. The harness now modelssinceCursor, token conflicts, and a lagging read the way a World does.workflow/hook.test.ts— a signal armed for a boundary the run has moved past is dropped, with a control proving one that is still wanted lands. These pin the guardmainalready provides (the boundary here depends on it); they are driven by explicit macrotask turns rather than a wall-clock sleep.world-sim/store.test.ts,world-local/storage.test.ts— a conflicting create answers thesinceCursordelta with thehook_conflicton it, byte-identical to a list from the same cursor, and omits it when the caller did not ask.Every new test except the two
hook.test.tspins was verified to fail without the corresponding production change.Docs Preview
WORKFLOW_RETAINED_VM🤖 Generated with Claude Code