Skip to content

fix(core): preserve Hook retention in QuickJS - #3319

Merged
VaguelySerious merged 4 commits into
mainfrom
codex/fix-quickjs-hook-retention
Aug 4, 2026
Merged

fix(core): preserve Hook retention in QuickJS#3319
VaguelySerious merged 4 commits into
mainfrom
codex/fix-quickjs-hook-retention

Conversation

@NathanColosimo

Copy link
Copy Markdown
Contributor

Summary

  • carry a Hook's minimum-retention deadline across the QuickJS VM boundary
  • include that deadline in the resulting hook_created event
  • add a focused QuickJS regression test

Root cause

The QuickJS hook shim created its own pending-operation object but omitted experimental_minRetention. The host therefore persisted the Hook without tokenRetentionUntil, and terminal-run cleanup made it unavailable immediately. The node VM path already forwarded the deadline correctly.

Testing

  • pnpm turbo run build --filter='!./workbench/*'
  • pnpm --filter @workflow/core test (1,905 passed; 3 expected failures)
  • targeted QuickJS E2E: hookMinRetentionWorkflow - terminal Hook cannot resume and its token stays unavailable

@NathanColosimo
NathanColosimo requested a review from a team as a code ownerAugust 4, 2026 04:40
@vercel

vercelBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@changeset-bot

changeset-botBot commented Aug 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8adf51f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
NameType
@workflow/corePatch
@workflow/buildersPatch
@workflow/cliPatch
@workflow/nextPatch
@workflow/nitroPatch
@workflow/vitestPatch
@workflow/web-sharedPatch
@workflow/webPatch
workflowPatch
@workflow/world-testingPatch
@workflow/astroPatch
@workflow/nestPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/nuxtPatch

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

@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

Some tests failed

❌ Failed E2E Tests

📋 Other (1 failed)

e2e-vercel-prod-tanstack-start-quickjs (1 failed):

  • health check (CLI) - workflow health command reports healthy endpoints

E2E Test Summary

Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production278004963276
✅ 💻 Local Development315304353588
✅ 📦 Local Production329004543744
✅ 🐘 Local Postgres329004543744
✅ 🪟 Windows31200312
❌ 📋 Other206714282496
✅ vercel-multi-region270027
Total149191226717187
Details by Category

✅ ▲ Vercel Production

AppPassedFailedSkipped
✅ astro-node127029
✅ astro-quickjs127029
✅ example-node127029
✅ example-quickjs127029
✅ express-node127029
✅ express-quickjs127029
✅ fastify-node127029
✅ fastify-quickjs127029
✅ hono-node127029
✅ hono-quickjs127029
✅ nextjs-turbopack-node15204
✅ nextjs-webpack-node15204
✅ nextjs-webpack-quickjs15204
✅ nitro-node127029
✅ nitro-quickjs127029
✅ nuxt-node127029
✅ nuxt-quickjs127029
✅ sveltekit-node146010
✅ sveltekit-quickjs146010
✅ vite-node127029
✅ vite-quickjs127029

✅ 💻 Local Development

AppPassedFailedSkipped
✅ astro-stable-node130026
✅ astro-stable-quickjs130026
✅ express-stable-node130026
✅ express-stable-quickjs130026
✅ fastify-stable-node130026
✅ fastify-stable-quickjs130026
✅ hono-stable-node130026
✅ hono-stable-quickjs130026
✅ nextjs-turbopack-canary-node137019
✅ nextjs-turbopack-canary-quickjs137019
✅ nextjs-turbopack-stable-node15600
✅ nextjs-turbopack-stable-quickjs15600
✅ nextjs-webpack-canary-node137019
✅ nextjs-webpack-stable-node15600
✅ nextjs-webpack-stable-quickjs15600
✅ nitro-stable-node130026
✅ nitro-stable-quickjs130026
✅ nuxt-stable-node130026
✅ nuxt-stable-quickjs130026
✅ sveltekit-stable-node14907
✅ sveltekit-stable-quickjs14907
✅ vite-stable-node130026
✅ vite-stable-quickjs130026

✅ 📦 Local Production

AppPassedFailedSkipped
✅ astro-stable-node130026
✅ astro-stable-quickjs130026
✅ express-stable-node130026
✅ express-stable-quickjs130026
✅ fastify-stable-node130026
✅ fastify-stable-quickjs130026
✅ hono-stable-node130026
✅ hono-stable-quickjs130026
✅ nextjs-turbopack-canary-node137019
✅ nextjs-turbopack-canary-quickjs137019
✅ nextjs-turbopack-stable-node15600
✅ nextjs-turbopack-stable-quickjs15600
✅ nextjs-webpack-canary-node137019
✅ nextjs-webpack-canary-quickjs137019
✅ nextjs-webpack-stable-node15600
✅ nextjs-webpack-stable-quickjs15600
✅ nitro-stable-node130026
✅ nitro-stable-quickjs130026
✅ nuxt-stable-node130026
✅ nuxt-stable-quickjs130026
✅ sveltekit-stable-node14907
✅ sveltekit-stable-quickjs14907
✅ vite-stable-node130026
✅ vite-stable-quickjs130026

✅ 🐘 Local Postgres

AppPassedFailedSkipped
✅ astro-stable-node130026
✅ astro-stable-quickjs130026
✅ express-stable-node130026
✅ express-stable-quickjs130026
✅ fastify-stable-node130026
✅ fastify-stable-quickjs130026
✅ hono-stable-node130026
✅ hono-stable-quickjs130026
✅ nextjs-turbopack-canary-node137019
✅ nextjs-turbopack-canary-quickjs137019
✅ nextjs-turbopack-stable-node15600
✅ nextjs-turbopack-stable-quickjs15600
✅ nextjs-webpack-canary-node137019
✅ nextjs-webpack-canary-quickjs137019
✅ nextjs-webpack-stable-node15600
✅ nextjs-webpack-stable-quickjs15600
✅ nitro-stable-node130026
✅ nitro-stable-quickjs130026
✅ nuxt-stable-node130026
✅ nuxt-stable-quickjs130026
✅ sveltekit-stable-node14907
✅ sveltekit-stable-quickjs14907
✅ vite-stable-node130026
✅ vite-stable-quickjs130026

✅ 🪟 Windows

AppPassedFailedSkipped
✅ nextjs-turbopack-node15600
✅ nextjs-turbopack-quickjs15600

❌ 📋 Other

AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable-node130026
✅ e2e-local-dev-nest-stable-quickjs130026
✅ e2e-local-dev-tanstack-start-node130026
✅ e2e-local-dev-tanstack-start-quickjs130026
✅ e2e-local-postgres-nest-stable-node130026
✅ e2e-local-postgres-nest-stable-quickjs130026
✅ e2e-local-postgres-tanstack-start-node130026
✅ e2e-local-postgres-tanstack-start-quickjs130026
✅ e2e-local-prod-nest-stable-node130026
✅ e2e-local-prod-nest-stable-quickjs130026
✅ e2e-local-prod-tanstack-start-node130026
✅ e2e-local-prod-tanstack-start-quickjs130026
✅ e2e-vercel-prod-nest-node127029
✅ e2e-vercel-prod-nest-quickjs127029
✅ e2e-vercel-prod-tanstack-start-node127029
❌ e2e-vercel-prod-tanstack-start-quickjs126129

✅ vercel-multi-region

AppPassedFailedSkipped
✅ nextjs-turbopack2700

📋 View full workflow run

@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📊 Workflow Benchmarks

commit 8adf51f · Tue, 04 Aug 2026 18:32:21 GMT · run logs

Backend: vercel · app: nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep500 (+95%) 🔻1353 🔴 (+21%) 🔻1421 🔴 (+22%) 🔻1474 🔴 (+23%) 🔻30
TTFSstream1276 (+419%) 🔻1337 🔴 (+19%) 🔻1346 🔴 (+16%) 🔻1402 🔴 (+15%)30
TTFShook + stream1404 (+261%) 🔻1648 🔴 (+20%) 🔻1694 🔴 (+13%)1794 🔴 (-34%) 💚30
STSO1020 steps (inline)62 (-37%) 💚123 (-20%) 💚142 (-22%) 💚204 (-39%) 💚1018
STSO1020 steps (queue-hop)2667 (-10.0%)2667 (-10.0%)2667 (-10.0%)2667 (-10.0%)1
WO1020 steps125336 (-18%) 💚125336 (-18%) 💚125336 (-18%) 💚125336 (-18%) 💚1
SLstream latency85 (-12%)180 🔴 (+31%) 🔻203 🔴 (+31%) 🔻243 🔴 (-63%) 💚30
SOstream overhead (text)101 (-19%) 💚146 (-65%) 💚158 (-81%) 💚228 (-92%) 💚30
SOstream overhead (structured)97 (-9.3%)137 (-54%) 💚151 (-73%) 💚305 (-67%) 💚30
📈 STSO distribution vs main (inline / queue-hop histograms)

1020 steps (inline)

Cumulative STSO time: main 149536ms → this run 121240ms (Δ -28296ms, -19%)

 50-100 ms ┃ main 1 this 39 +38
100-150 ms ███████████████████░░░░┃ main 721 this 903 +182
150-200 ms █┃████ main 227 this 61 -166
200-250 ms ┃ main 40 this 9 -31
250-300 ms ┃ main 12 this 5 -7
300-350 ms ┃ main 8 this 1 -7
350-400 ms ┃ main 5 this 0 -5
400-450 ms ┃ main 2 this 0 -2
550-600 ms ┃ main 1 this 0 -1
1100-1150 ms ┃ main 1 this 0 -1

1020 steps (queue-hop)

Cumulative STSO time: main 2962ms → this run 2667ms (Δ -295ms, -10%)

2500-3000 ms ███████████████████████┃ main 1 this 1 +0
📜 Previous results (2)

342c64c

Tue, 04 Aug 2026 06:21:51 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep1286 (+509%) 🔻1358 🔴 (+23%) 🔻1396 🔴 (+25%) 🔻1442 🔴 (+1.8%)30
TTFSstream1290 (+486%) 🔻1337 🔴 (+19%) 🔻1379 🔴 (+17%) 🔻1416 🔴 (+13%)30
TTFShook + stream1553 (+310%) 🔻1650 🔴 (+28%) 🔻1664 🔴 (+19%) 🔻1784 🔴 (+21%) 🔻30
STSO1020 steps (inline)115 (+24%) 🔻145 (-5.8%)162 (-13%)248 (-24%) 💚1018
STSO1020 steps (queue-hop)3223 (+7.0%)3223 (+7.0%)3223 (+7.0%)3223 (+7.0%)1
WO1020 steps151079 (-2.8%)151079 (-2.8%)151079 (-2.8%)151079 (-2.8%)1
SLstream latency131 (+51%) 🔻162 🔴 (+17%) 🔻172 🔴 (±0%)211 🔴 (-68%) 💚30
SOstream overhead (text)132 (-0.8%)196 (-47%) 💚253 (-63%) 💚362 (-79%) 💚30
SOstream overhead (structured)140 (+17%) 🔻220 (-36%) 💚287 (-63%) 💚8112 🔴 (+783%) 🔻30

98692e4

Tue, 04 Aug 2026 04:57:12 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep340 (+61%) 🔻1385 🔴 (+26%) 🔻1458 🔴 (+31%) 🔻1545 🔴 (+9.1%)30
TTFSstream1241 (+464%) 🔻1284 🔴 (+14%)1308 🔴 (+11%)1339 🔴 (+6.4%)30
TTFShook + stream1482 (+291%) 🔻1584 🔴 (+22%) 🔻1613 🔴 (+16%) 🔻1659 🔴 (+13%)30
STSO1020 steps (inline)80 (-14%)117 (-24%) 💚140 (-25%) 💚218 (-33%) 💚1019
WO1020 steps118156 (-24%) 💚118156 (-24%) 💚118156 (-24%) 💚118156 (-24%) 💚1
SLstream latency99 (+14%)137 🔴 (-1.4%)146 🔴 (-15%) 💚188 🔴 (-72%) 💚30
SOstream overhead (text)106 (-20%) 💚135 (-64%) 💚156 (-77%) 💚178 (-90%) 💚30
SOstream overhead (structured)101 (-16%) 💚170 (-50%) 💚234 (-70%) 💚2600 🔴 (+183%) 🔻30
ℹ️ Metric definitions & methodology

The 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: is main, marks where this run lands, bridges the gap when this run has more samples in a bucket.

Best/P75/P90/P99 deltas compare against the most recent benchmark run on main at the time of this run. 🔻 flags a delta worse than +15%, 💚 one better than −15%.

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 (clientStart) right before start(), so the CI runner’s request and its path through api.vercel.com sit outside every measured window. TTFS = in-deployment start() → first step body (turbo uses the in-process fast path, non-turbo the dispatch path), and includes the VQS dispatch hop plus any /flow cold start. STSO/WO are measured between step bodies on the deployment. SL is measured inside the workflow (parallel reader/writer steps), so it no longer includes the api.vercel.com read path.

Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the /flow invocation for a large fraction of runs, inflating P75+; the Best column shows the fastest (warm-start) sample for comparison.

Comment threadpackages/core/src/runtime/quickjs-runtime.ts
Comment threadpackages/core/src/runtime/quickjs-entrypoint.ts
The capability gate in dispatchPendingOps is swallowed by the
terminal-drain call sites' try/catch (which exists to keep genuine
drain failures from masking the workflow's own outcome). A retained
hook that was never awaited only reaches the gate through that drain —
so on an unsupporting World, a fire-and-forget retained hook completed
the run normally with the retention silently dropped.
Hoist assertHookRetentionSupported above the drain try/catch in both
terminal branches. The drain runs before the run_completed / run_failed
write, so the FatalError escapes to the replay loop's catch while the
run is still non-terminal and records run_failed — mirroring the node
engine, where the same error throws synchronously inside createHook()
and a fire-and-forget retained hook can never complete the run.
Suggested by the Vercel agent review.
@github-actionsgithub-actionsBot mentioned this pull request Aug 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

No backport to stable for 27a3f15 (AI decision).

The entire QuickJS engine is absent from stablegit ls-tree origin/stable -- packages/core/src/runtime/ shows no quickjs-runtime.ts or quickjs-entrypoint.ts, and git grep quickjs origin/stable -- packages/core returns nothing. The experimental_minRetention Hook option this fix preserves also does not exist anywhere on stable. There is no defect on the maintenance line for this commit to fix.

To override, re-run the Backport to stable workflow manually via workflow_dispatch and paste this commit SHA into the ref input:

27a3f15a7b08659f2b0e69e12e0c4ad923d7f4e5

Sign up for freeto 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.

3 participants

@NathanColosimo@TooTallNate@VaguelySerious