world-local: converge redelivered hook_received re-ensure on its committed resumeId claim - #3297
Conversation
…itted resumeId claim A redelivered re-ensure of an already-committed resume (same runId + resumeId + digest) was rejected with HookNotFoundError when the hook had since been disposed by the workflow (dispose -> sleep releases the token while the run continues). The queue consumer treats HookNotFound as 'nothing left to resume' and acks the delivery — silently dropping whatever continuation the redelivered message carried and wedging the run. Check the (runId, resumeId) claim BEFORE the disposal/existence rejections: a committed claim whose pinned event is journaled proves this exact resume was accepted while the hook was alive, so return that event as success. Claims with a mismatched hookId or payload digest still fall through to full validation and are rejected as before, as are genuinely new resumes of a disposed hook.
🦋 Changeset detectedLatest commit: ee823c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 393223ms → this run 390119ms (Δ -3104ms, -1%) 1020 steps (queue-hop) Cumulative STSO time: main 7422ms → this run 8760ms (Δ +1338ms, +18%) ℹ️ Metric definitions & methodologyThe collapsed STSO distribution section above buckets every step gap of the sequential-steps run (not a sampled window), split by whether the step ending the gap ran inline — in the same warm process as the step before it, so the gap is pure framework overhead — or after a queue-hop — the first step of a fresh process, which pays queue dispatch, client reinit and event-log replay. Bars overlay the two runs: 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, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) 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 · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
🧪 E2E Test Results❌ Some tests failed ❌ Failed E2E Tests📦 Local Production (1 failed)hono-stable (1 failed):
E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
❌ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
There was a problem hiding this comment.
Pull request overview
Fixes a correctness edge case in @workflow/world-local’s lazy hook resumption dedup: when a hook_received re-ensure message is redelivered after the workflow has disposed the hook, the consumer should converge on the already-committed (runId, resumeId) claim (and return the already-journaled event) instead of rejecting with HookNotFoundError, which can otherwise cause the consumer to ack and drop a continuation.
Changes:
- Check the
(runId, resumeId)resume-claim convergence path before hook disposal/existence rejection forhook_received, returning the committed event when it’s already journaled. - Add a regression test covering “converge-after-dispose” redelivery plus the still-rejected “new resume after disposal” case.
- Add a patch changeset for
@workflow/world-local.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/world-local/src/storage/events-storage.ts | Reorders hook_received validation to converge redeliveries on an existing committed resume claim/event even if the hook has since been disposed. |
| packages/world-local/src/storage/hook-resume-dedup.test.ts | Adds regression coverage for redelivery convergence after hook disposal and confirms new resumes post-disposal still reject. |
| .changeset/world-local-resume-redelivery-converge.md | Publishes the fix as a patch release note for @workflow/world-local. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No backport to This is a correctness/wedge fix, but it is built entirely on the To override, re-run the Backport to stable workflow manually via |
Problem
world-local's
events.create('hook_received', …)runs its disposal-committed and hook-existence rejections before the(runId, resumeId)claim convergence added for lazy hook resumption (#3230). A queue redelivery of the consumer's re-ensure — after the workflow has disposed the hook (e.g. theusing hook = createHook(…)→ dispose →sleep()pattern, which releases the token while the run continues) — therefore getsHookNotFoundErroreven though this exact resume is already committed.The consumer contract (runtime.ts lazy-resume prologue) treats
HookNotFoundErroras "the run went terminal / nothing left to resume" and acks the delivery. Any continuation the redelivered message was carrying is silently dropped and the run wedges. Natural triggers include a consumer crash after the re-ensure but before ack, a queue retry racing the dispose, or an engine that reschedules via visibility-timeout redelivery of the resume message (where this was found: the QuickJS engine branch, #3048).Fix
Check the
(runId, resumeId)claim first: a committed claim whose pinned event is journaled proves this exact resume was accepted while the hook was alive — return that event as success (idempotent convergence, matching the documented server behavior). Everything else is unchanged:hookIdor payload digest still fall through to full validation and are rejected as beforeHookNotFoundErrorNotes
resumeIdcolumn) and stays on the sequential single-writer path, so the re-ensure redelivery shape never occurs there.hook-resume-dedup.test.tscovers converge-after-dispose plus the still-rejected new-resume case.