fix(core): start forwarded writable key lookup on first write (backport of #4175) - #4176
Merged
pranaygp merged 2 commits intoSep 15, 2026
Conversation
Backport of the `main` fix for #3935, hand-ported because the 4.8.x `getForwardedWritableEncryptionKey` takes two parameters and returns a `CryptoKey`, so the commit does not cherry-pick cleanly. Hydrating a payload that carries a writable forwarded from another run called `getForwardedWritableEncryptionKey()` inside the reviver and stored the promise it returned. Nothing awaits that promise until the first chunk is written, so when the lookup failed first — e.g. the `runs.get` fallback for a descriptor from an older deployment timing out — the rejection had no handler and Node exited the process. A caller that never touched the stream lost its whole invocation. Wrap the lookup in a memoized thunk instead. This is the resolver form `EncryptionKeyParam` already documents ("avoids unobserved background lookups for empty or never-read streams"), so the lookup starts on the first write and a failure rejects that stream. Both reviver boundaries are covered: the client one (`getExternalRevivers`) and the step one (`getStepRevivers`). This line is more exposed than `main`: it has no `encryptionPublicKey` fast path, so every cross-run forwarded writable takes the failing `runs.get` path. Fixes #3935 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com> Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
2 of 3 tasks
Contributor
🦋 Changeset detectedLatest commit: 08886b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (106 failed)redis (21 failed):
turso (85 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
Author
CI triage — the two red jobs are pre-existing on
|
| Job | On e3862bc13 (base) |
Here |
|---|---|---|
E2E Community World (Turso) |
❌ fail | ❌ fail |
Vercel – workbench-python-workflow |
deployment failure | deployment failure |
That baseline run was red overall, with E2E Vercel Prod Tests (nextjs-turbopack, sveltekit, example), E2E Community World (Redis) and E2E Required Check failing too — none of which this diff touches. Neither red job here exercises cross-run forwarded writables.
The checks that this change can affect are green: DCO, Unit Tests (ubuntu-latest), CI Scripts Tests, No Test Overrides, Node.js Module Build Errors Test.
My token can't rerun --failed, so flagging for a maintainer if a clean run is wanted.
The backport asserted only that `writer.closed` rejects. Assert the same
`cause: { code: 'TIMEOUT' }` the `main` test does, so both branches pin
that the world error survives to the writer rather than just that some
error does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
Co-Authored-By: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
Contributor
Author
CI status — every gate passes; the 4 reds are chronically broken lanes on
|
| Lane | e3862bc (base) | 9595a5a | ee29d15 | a158f81 | d86d752 |
|---|---|---|---|---|---|
E2E Turso |
fail | fail | fail | fail | fail |
E2E Redis |
fail | fail | fail | cancelled | fail |
E2E MongoDB |
cancelled | cancelled | cancelled | cancelled | cancelled |
Vercel – workbench-python-workflow |
fail | — | — | — | — |
Turso is 5/5 red, Redis 4/5, MongoDB never completes, and the python workbench deployment fails on the base commit too. Retrying reproduces them, so I haven't burned CI on it — these need owners, not a re-run. None of them exercise cross-run forwarded writables.
For contrast, the main companion #4175 is now fully green (180/180) after two retriggers cleared genuinely transient Windows and Vercel-upload flakes.
One change since the first push
08886b86 tightens the backport's test to assert the same thing the main test does:
- await expect(writer.closed).rejects.toThrow();
+ await expect(writer.closed).rejects.toMatchObject({
+ cause: { code: 'TIMEOUT' },
+ });I verified the cause chain this branch actually produces before tightening it, so the assertion pins real behaviour rather than a guess:
WorkflowRuntimeError: Failed to serialize stream chunk…
└▶ WorkflowWorldError: GET /v2/runs/wrun_owner?remoteRefBehavior=resolve timed out after 71779ms (code=TIMEOUT)
Both branches now pin that the world error survives to the writer, not merely that some error does. 127 tests pass in serialization.test.ts; no new Biome findings (the 2 noUnusedVariables errors are pre-existing on stable).
TooTallNate
approved these changes
Sep 15, 2026
pranaygp
deleted the
fix/3935-forwarded-writable-key-unhandled-rejection-stable
branch
September 15, 2026 19:42
Merged
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport of #4175 to
stable. Fixes #3935 on the 4.8.x line.Opened by hand rather than left to the backport bot:
stable'sgetForwardedWritableEncryptionKeytakes two parameters and returns aCryptoKey, so themaincommit does not cherry-pick cleanly, and the exposure here is larger than onmain(below).When a step's arguments or a run's return value contain a writable forwarded from another run, the reviver called
getForwardedWritableEncryptionKey()immediately and stored the promise it returned. The only consumer of that promise is the serialize transform, which awaits it on the first chunk written — so on a writable nobody writes to, nothing ever observes it. When the lookup failed first, Node saw an unhandled rejection and killed the process, taking unrelated work in the same invocation with it.The failing path is
world.runs.get(ownerRunId)—GET /v2/runs/:id?remoteRefBehavior=resolve, the request that timed out after ~72s in the report. Only the Vercel World implementsgetEncryptionKeyForRun, so this is Vercel-only.The fix is a memoized thunk, which is the resolver form
EncryptionKeyParamon this branch already documents:The writable reviver was passing the
Promisearm where it wanted the resolver arm. Both boundaries now use the resolver:getExternalRevivers(the client path, reached fromawait run.returnValue) andgetStepRevivers(forwarded writables in step arguments). The lookup runs at most once, starts only when a write needs the key, and a failure rejects that stream instead of escaping.Important
stableis more exposed thanmain. The 4.8.x line has noencryptionPublicKeyfast path ingetForwardedWritableEncryptionKey, so every cross-run forwarded writable falls through toruns.get— not just descriptors minted by older deployments. Any app on 4.8.x that forwards a writable acrossstart()is exposed on every hydration of such a payload. The reporter hit this onworkflow4.8.5.Scope and history
AGENTS.md: a crash-class stability fix for a defect users hit in production, no new API, no dependency onmain-only code.How did you test your changes?
Four regression tests in
packages/core/src/serialization.test.ts, adapted to this branch'ssetWorld()harness and placed next to the existing forwarded-writable tests:unhandledRejection, and no request at allwriter.closed, andruns.getruns exactly onceEach mocks
runs.getto reject with theWorkflowWorldError { code: 'TIMEOUT' }shapeworld-vercel'smakeRequestraises.Verified the tests fail without the fix. Reverting only the two call sites (keeping the tests) fails both unhandled-rejection tests with
AssertionError: expected [ Array(1) ] to deeply equal [], the array holding the timeout message. The other two pass either way by design — they pin behaviour the fix must preserve.Green on this branch:
packages/coreunit: 866 passed, 3 expected-fail, 3 skipped (48 files)pnpm --filter @workflow/core typecheck: cleanbiome checkon both changed files: no new findings (2 pre-existingnoUnusedVariableserrors and 5 warnings, identical tostable)PR Checklist - Required to merge
pnpm changesetwas run to create a changelog for this PRpatch, matching themainchangesetgit commit --signoffon your commits)@vercel/workflowin a comment once the PR is ready, and the above checklist is complete🤖 Generated with Claude Code