feat(core): resume hooks from stored resumeContext and seal to the run key - #3125
Conversation
🦋 Changeset detectedLatest commit: d52d089 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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:
📜 Previous results (5)abb34ddTue, 28 Jul 2026 05:03:46 GMT · run logs
fd5b918Tue, 28 Jul 2026 02:01:10 GMT · run logs
677cd6dMon, 27 Jul 2026 23:49:23 GMT · run logs
d85d03cMon, 27 Jul 2026 20:14:26 GMT · run logs
906e26cMon, 27 Jul 2026 19:41:37 GMT · run logs
ℹ️ Metric definitions & methodologyBest/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 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 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✅ All tests passed E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
The optimization is well-shaped — resumeContext is a good abstraction (immutable slice only, explicitly excluding mutable status/secrets), the fallback is transparent, and the refactor cleanly separates context resolution from key resolution so key work stays behind the terminal check. But the terminal-run error contract does not hold on the fast path, and I think it's worth fixing before this lands.
Blocking: the fast path's error re-keying targets the wrong error. The code assumes a terminal run makes hook_received fail with a 404 that the World maps to HookNotFoundError:
if (HookNotFoundError.is(err)) throw new HookNotFoundError(hook.token);
throw err;That 404 mapping is real, but it means hook does not exist — world-vercel's translation is gated on isHookEventRequiringExistence(...) && err.status === 404. A terminal run is a different rejection: the backend refuses non-allowed events on an ended run with a conflict (409), which http-core.ts maps to EntityConflictError — so it falls straight through the HookNotFoundError check and escapes as EntityConflictError. The self-hosted worlds are also not covered: both world-local (events-storage.ts, the hook_received terminal guard) and world-postgres (storage.ts, the FOR UPDATE status check) throw RunExpiredError for exactly this case.
Why it usually looks fine — and where it doesn't:
- Hooks are deleted when a run finishes, so a token-based resume against a long-ended run 404s at
getByTokenbefore reaching either path. That's the common case and it keeps working. - It breaks where no
getByTokenhappens or the hook still exists:resumeWebhookcallsresumeHook(hook, request, encryptionKey)with the Hook object (so does any caller holding a retained hook), and the run can terminate between the lookup and thehook_receivedwrite. Pre-PR those all producedHookNotFoundError(token)from the local terminal check; on the fast path they now surfaceEntityConflictError(Vercel) orRunExpiredError(local/postgres).
The fix is small — widen the catch to the errors that actually represent "run already ended":
if (HookNotFoundError.is(err) || EntityConflictError.is(err) || RunExpiredError.is(err)) {
throw new HookNotFoundError(hook.token);
}(or invert it: keep a terminal-status check on the fast path by including status in resumeContext — though that would break its nice immutability property, so I'd prefer widening the catch.) Please also correct the two code comments asserting the 404-on-terminal-run premise, and extend resume-hook.test.ts — the current fast-path test mocks HookNotFoundError from events.create, which bakes in the assumption; cases rejecting with EntityConflictError/RunExpiredError would have caught this.
Also worth resolving before merge: the world-postgres half-wiring. Migration 0017 adds resume_context and schema.ts types it, but nothing in world-postgres ever writes or reads the column — so postgres can't take the fast path today. That's fine as sequencing, but it's exactly the trap in the item above: the moment a follow-up populates it, postgres resumes go fast-path and terminal-run resumes start throwing RunExpiredError. Fixing the catch now makes that follow-up safe. Worth a line in the PR description saying the column is intentionally inert for now (the journal numbering is clean — 0017 follows 0016 with no gap).
Everything else checks out:
- The
getEncryptionKeyForRun(runId, { deploymentId })overload already exists on the World interface, so the fast-path key resolution is contract-compliant, andencryptionKeyOverridestill short-circuits it (the??ordering is right — no wasted lookup when a key is passed in, whichresumeWebhookdoes). - Capability/compression selection now reads
runSpecVersion/workflowCoreVersionfrom the context — same values the run entity provided, andresumeContextFromRuntype-guards both when synthesizing from a run. - Naming
runSpecVersionto avoid collision with the hook's ownspecVersionis a good call, as is routingworld-vercelthrough the sharedHookSchemaso the new field flows without a world-side change. - The
attribute-panel.tsxnull renderer correctly keeps this internal plumbing out of the UI. - 7/7
resume-hooktests pass locally, and the fast-path assertions (runsGetnot called, key resolved once) do pin the round-trip savings this PR is for.
Happy to re-review quickly once the catch is widened — the rest of this is ready.
|
Thanks for the thorough review — all four items addressed in
Ready for another look when you have a moment. |
|
Follow-up |
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
Re-reviewed fd5b918e4 — the blocking item is fully addressed. Approving.
- The catch now re-keys all three real rejection shapes (
HookNotFoundErrorfor a genuinely missing hook,EntityConflictErrorfor the Vercel 409,RunExpiredErrorfor world-local / world-postgres), soresumeHookon an ended run throwsHookNotFoundError(hook.token)on every world — including theresumeWebhookpath that passes a Hook object and therefore has nogetByToken404 to fall back on. - Both misleading comments are corrected, and the new inline comment enumerating which World produces which error is exactly the context a future reader needs to not re-narrow this.
- The two new tests pin the 409 and expired cases against the token contract and assert the queue isn't re-triggered — which is the part that would silently regress if someone later reshuffled the catch.
Verified locally on the updated head: 9/9 resume-hook tests and the full core suite at 1584 (+3 pre-existing expected-fails). CI is green (21 pass, 0 fail).
Two notes for later, neither blocking:
- Widening to
EntityConflictErroris the right trade today, but it's the loosest of the three — onworld-localanevents.createconflict can also mean a crashed-retry event-ID collision (the staging-path collision inevents-storage.ts), which would now report asHookNotFoundError. That case is rare and its old surface (EntityConflictError) wasn't especially actionable either, so I'd leave it — but the clean long-term fix is for the worlds to agree on one error class for "terminal run rejected this event", after which the catch could drop the conflict arm. - The
world-postgresresume_contextcolumn is still inert (written/read by nothing). Now that the error mapping is correct, activating it later is safe — just worth a line in the description or a tracking issue so the follow-up that populates it isn't mistaken for dead-code cleanup.
…n key Hooks can carry an optional `resumeContext` mirrored from the run at creation time. When present, `resumeHook`/`resumeWebhook` resume directly from it instead of fetching the full run, saving a round trip per resume. When the context also carries the run's `encryptionPublicKey`, the resume seals its payload (`encp`) directly to that key. Combined with the sealed envelope work (#3093-#3096), a default webhook resume then needs neither a run read nor a cross-deployment run-key lookup: the key is resolved only when the hook actually stores metadata that must be hydrated symmetrically. Everything falls back transparently to the full run fetch and symmetric key when the context (or the public key within it) is absent, so new clients interoperate with old servers and vice versa. - world: optional `encryptionPublicKey` on `HookResumeContext` - world-postgres: `resume_context` column migration - core: combined fast-path + seal in resume-hook; fast-path control-flow suite split from the real-serialization crypto suite - world-vercel: cover the `getEncryptionKeyForRun(runId, { deploymentId })` overload the fast path relies on - web-shared: render `resumeContext` in the attribute panel Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| "@workflow/web-shared": patch | ||
| --- | ||
|
|
||
| Hooks can carry an optional `resumeContext`; when present, `resumeHook`/`resumeWebhook` resume from it directly instead of fetching the full run, saving a round trip per resume. When the context also carries the run's `encryptionPublicKey`, the resume seals its payload (`encp`) directly to that key, so a default webhook resume needs neither a run read nor a run-key lookup. These two optimizations fall back independently: it fetches the full run when the context is absent, and uses symmetric encryption when no usable public key is available. |
|
No backport to This is a performance optimization and new API surface, not a defect fix: it adds an optional To override, re-run the Backport to stable workflow manually via |
2 participants
What
Hooks can carry an optional
resumeContext— the immutable slice of the owning run needed to route and decrypt a resume: deployment ID, workflow name, run spec version, core version, trace carrier, and (when the run publishes one) its X25519encryptionPublicKey.resumeHook/resumeWebhookuse it to resume a hook directly.Why
Resuming a hook previously always fetched the full run to recover its routing and encryption context, adding a run round trip to every resume — including on the hot webhook path. Sealed payloads made this worse: a resume would additionally fetch the run's symmetric key (~350ms cross-deployment
run-keyhop) to write its payload.How
resumeContextis present, route the resume from the stored context and skipruns.getentirely.encryptionPublicKey, seal the payload (encp) directly to that key — no symmetric-key material required. Building on the sealed-envelope work (feat(core): addencpsealed-box encryption primitive #3093–feat(core): seal hook payloads to the target run's public key #3096), a default webhook resume then needs neither a run read nor a run-key lookup.metadatathat must be hydrated — the one case that genuinely needs it. Webhooks with stored metadata keep a single key lookup; the common metadata-less webhook pays none.runs.get+ symmetric-key path — no behavior change for existing hooks.HookNotFoundErrorcontract (error keyed to the hook token). On the fast path the terminal check happens on the receiving side and the backend-specific rejection is re-keyed toHookNotFoundError: current Vercel returns 404 (world-vercel maps it toHookNotFoundError), world-local / world-postgres reject withRunExpiredError, andEntityConflictError(HTTP 409) is caught for compatibility with older / conflict-shaped behavior.Backwards compatible:
resumeContextand itsencryptionPublicKeyare optional on both the schema and the resume path, so new clients interoperate with older records and vice versa.Tests
encp-sealed payload end-to-end and the default webhook is asserted to make zero key lookups while a metadata-bearing webhook makes exactly one.world-vercelcovers thegetEncryptionKeyForRun(runId, { deploymentId })overload the fast path relies on (derive locally for the current deployment, fetch for a different one).Note
The
world-postgresmigration (0017_add_hook_resume_context) adds aresume_contextcolumn. Reads are safe (nullable); population of the column is a separate data-only change requiring no migration coordination. The journal numbering is clean —0017follows0016with no gap.The web attribute panel gains a small renderer so a hook's
resumeContextis inspectable in the UI.