Skip to content

workflow: deliver a hook resume carried on the queue - #300

Open
fantix wants to merge 1 commit into
mainfrom
fantix/workflow-lazy-resume
Open

workflow: deliver a hook resume carried on the queue#300
fantix wants to merge 1 commit into
mainfrom
fantix/workflow-lazy-resume

Conversation

@fantix

@fantix fantix commented Aug 14, 2026

Copy link
Copy Markdown
Member

This is similar to the resilient start, and mirrors vercel/workflow#1834 with a few following fixes.

In short, a vqs message to resume a hook can now also carry the actual payload, in addition to the hook_received workflow server event. And these 2 things may arrive without a particular order for performance, so the SDK must be able to handle all cases properly.

This would unblock the hook-based e2e test fixtures. This PR does not introduce any public API.


The bug

Resuming a hook takes two writes: the hook_received event, and the queue message
that wakes the run. resumeHook() can do both at once and put the payload on
the message — so a delivery can arrive before the event exists.

We never read that payload. The replay found nothing new, suspended, and answered
200:

run_created, run_started, hook_created, wait_created     ← log at replay time
hook_received                                            ← producer's write, landed after

Nothing was left to wake the run: exactly one message is published per resume, and
this delivery consumed it. The resume was lost, not delayed.

In the TypeScript e2e suite hookWithSleepWorkflow stalled on its first payload
every time; hookWithSleepFinalStepWorkflow escaped only because its driver sends
a second payload, whose delivery replays the whole log and repairs the miss.
Setting WORKFLOW_DISABLE_LAZY_HOOK_RESUME=1 on the driver made both pass, which
is what pinned the cause.

The fix

Layer Change
world.py hookInput on the invoke payload, token on hook_received, resumeId on the stored event, HOOK_RESUME_INPUT_VERSION
runtime.py write the event before replay when the log lacks it; advertise the version from start() and the health-check answer
worlds/vercel.py forward the resume identity on the request; the backend enforces it
worlds/local.py no server to defer to, so a (runId, resumeId) claim file, byte-shaped like world-local's
ulid.py decode_time(), so the event is dated when the resume happened rather than when we got to it

Two writers of one resume must collapse into one event, or the workflow body
gets the payload twice — worse than the stall. That is what the identity is for.

One divergence from world-local — worth a look

A claim whose event has not landed yet: upstream takes over the position,
reasoning that only a crashed writer leaves one behind. That holds while both
writers share the process its per-hook lock serializes. Across processes it does
not — the other writer numbers events by position, so finding ours where it meant
to publish it bumps to the next free slot instead of failing. Two events, in
three of eight suite runs.

We raise instead, costing one redelivery. What that gives up is a writer that
really did crash between its two writes; that resume now exhausts a retry budget
rather than being quietly taken over. Same reasoning tightens _is_resume_event:
upstream also accepts an event with no resume id at the claimed position, which
across processes is an unrelated payload for the same hook, and adopting it would
drop this one silently.

Verification

  • 29 passed | 108 skipped against the TypeScript e2e driver, up from 27, with
    both hookWithSleep* fixtures off unsupported and zero duplicated resumes in
    the logs of six runs.
  • New integration test drives a real LocalWorld with a producer that publishes
    before writing. Reverting the handler hunk fails all three of its tests with the
    original signature (log is ['run_created', 'run_started', 'hook_created']).
  • Unit coverage split by layer, matching the resilient-start tests: models,
    handler, local world.

Not in scope

  • hookInput.deploymentId is parsed and unused; upstream re-routes a misrouted
    delivery with it, which this SDK does for no message type.
  • resume_hook() still writes hook_received without token. Harmless while it
    is the only writer of its event, but it is the same lossy-parsing family as the
    other hook gaps.
  • Found on the way: a log containing a created-event the body never issues makes
    resume() spin forever instead of failing. Pre-existing, filed separately.

Resuming a hook takes two writes: the `hook_received` event, and the queue
message that wakes the run. `resumeHook()` can do both at once and put the
payload on the message, which means a delivery can arrive before the event
exists. Nothing here read that payload, so the replay found nothing new,
suspended, and answered 200 -- with nothing left to wake the run, because
exactly one message is published per resume. The resume was lost, not
delayed. `hookWithSleepWorkflow` in the TypeScript e2e suite stalled on its
first payload every time.

The payload is now written before replay when the log does not already have
it, dated by the time inside the resume id rather than by now.

One resume then has two writers, and they have to collapse into one event or
the workflow body gets the payload twice. Both pass a `HookResume` identity
to `events_create`: the Vercel backend keys its constraint on it, and
`LocalWorld`, having no server, keeps a claim file. Unlike `world-local` it
refuses a claim whose event has not landed instead of taking it over -- that
shortcut is only safe while both writers share a process, and taking over
across processes appended a second event in three of eight suite runs.

Runs this SDK creates, and its health-check answers, now advertise
`hookResumeInputVersion`, which is what lets a producer choose the fast path
against us at all.

Conformance against the TypeScript driver: 27 -> 29 passing, with both
`hookWithSleep*` fixtures off the `unsupported` list.
@fantix
fantix force-pushed the fantix/workflow-lazy-resume branch from 6a10dd4 to 8ad9d2e Compare August 14, 2026 21:12
@fantix
fantix requested a review from a team August 15, 2026 01:31
fantix added a commit that referenced this pull request Aug 15, 2026
Integration-branch only. #300 gave `World.events_create` its `resume`
parameter and updated every test double in the tree; #302's double was
written against `main`, where neither the parameter nor `HookResume` exists
yet. Stacking the two is what makes them disagree, so the fix belongs here
rather than in either PR, and it disappears once both land.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant