Uh oh!
There was an error while loading. Please reload this page.
Backport #2191: fix forwarded stream encryption across deployments - #2202
Merged
Conversation
Contributor
🦋 Changeset detectedLatest commit: 0c7c38c 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 |
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (92 failed)mongodb (14 failed):
redis (9 failed):
turso-dev (1 failed):
turso (68 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
Check the workflow run for details. |
(cherry picked from commit 8f68d35)
pranaygpforce-pushed
the
pranaygp/codex/backport-pr-2191-to-stable
branch
from
June 1, 2026 19:13
31e8bf0 to
0c7c38cCompareContributor
There was a problem hiding this comment.
Pull request overview
Backports PR #2191 to stable, fixing forwarded writable stream encryption when a child workflow runs on a newer deployment than its parent. The fix stamps the owning deployment ID onto forwarded writable descriptors so the child can resolve the parent run's encryption key via getEncryptionKeyForRun(runId, { deploymentId }), with a fallback path that loads the owning run for legacy descriptors.
Changes:
- Adds
STREAM_SERVER_DEPLOYMENT_ID_SYMBOLand threadsworkflowDeploymentIdthroughStepContext,getWritable, serialization reducers/revivers, andhydrateStepArguments. - Routes forwarded writable key resolution through a new
getForwardedWritableEncryptionKeyhelper that prefers{ deploymentId }context and falls back toworld.runs.get(runId). - Adds regression tests for both new and legacy descriptor paths plus a changeset.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/symbols.ts | Adds STREAM_SERVER_DEPLOYMENT_ID_SYMBOL. |
| packages/core/src/step/context-storage.ts | Adds optional workflowDeploymentId to StepContext. |
| packages/core/src/step/writable-stream.ts | Stamps the deployment ID on the writable when available. |
| packages/core/src/step/writable-stream.test.ts | Verifies the deployment-ID stamping. |
| packages/core/src/serialization.ts | Carries deploymentId through descriptors; centralizes forwarded key resolution; threads it through step revivers and hydrateStepArguments. |
| packages/core/src/serialization.test.ts | Covers new descriptor key lookup and legacy fallback via runs.get. |
| packages/core/src/runtime/step-handler.ts | Passes VERCEL_DEPLOYMENT_ID into hydration and step context. |
| packages/world/src/interfaces.ts | Documents expanded usage of getEncryptionKeyForRun for forwarded streams. |
| .changeset/cross-deployment-stream-keys.md | Patch changeset for @workflow/core and @workflow/world. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TooTallNate
approved these changes
Jun 1, 2026
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Approve — faithful backport with appropriate stable-shape adaptations
I approved the original #2191 last week. This is the manual backport now that #2070 has landed on stable (which provided the prerequisite forwarded-stream plumbing). Comparing file-by-file:
Byte-identical to original
changeset— identicalpackages/core/src/symbols.ts— addsSTREAM_SERVER_DEPLOYMENT_ID_SYMBOLidenticallypackages/core/src/step/writable-stream.ts— adds the symbol-stamping block identicallypackages/core/src/step/context-storage.ts— addsworkflowDeploymentId?: stringidenticallypackages/world/src/interfaces.ts— same JSDoc tweak ongetEncryptionKeyForRun
Justified stable-shape adaptations (all flagged in PR body)
SerializableSpecial['WritableStream']inline inserialization.ts: stable doesn't have theserialization/types.tssplit that main does (a main-only refactor). ThedeploymentId?: stringfield is correctly added to the inline declaration with the same JSDoc.getWorld()(sync) vsgetWorldLazy()(async): stable still uses the sync version. The newgetForwardedWritableEncryptionKeyhelper correctly usesgetWorld()for stable.getStreamAndRequestRevivers(getStepRevivers(...))wrapper missing on stable: another main-only refactor. The backport correctly threadsdeploymentIdthroughgetStepReviversdirectly without the wrapper.Test fixture differences: stable doesn't have
makeStepCtx()/makeMockWorld()helpers (those are main-only). Tests inline the equivalent construction. Assertions are functionally identical.Omitted files:
runtime.tsandstep-executor.ts(V2 runtime, main-only) andserialization/types.ts(main-only split) are correctly absent from the backport.
What I verified locally
pnpm install --frozen-lockfile✓pnpm turbo run build --filter @workflow/core✓pnpm --filter @workflow/core test src/serialization.test.ts src/step/writable-stream.test.ts✓ (125 passing — 122 serialization + 3 writable-stream)- All three new tests pass specifically:
uses the forwarded stream deployment to resolve its encryption key✓loads the owner run for forwarded descriptors from older deployments✓tags a writable with its owning deployment for child workflow forwarding✓
CI noise
5 failures, all pre-existing on stable or infrastructure:
E2E Community World (Turso / MongoDB / Redis)— same 3 tests fail on stable's latest baseline run (26776100981), so pre-existingE2E Vercel Prod Tests (astro)— single workflow run flake (wrun_01KT29TFQKXARPV5DNYXDNB786failed on Vercel infra)E2E Required Check— cascading from astro
None are caused by this PR.
Carry-over concern from the original
The step-handler.ts change sources workflowDeploymentId from process.env.VERCEL_DEPLOYMENT_ID (current runtime deployment), not the workflow's actual deployment. I flagged this on #2191 and the same analysis applies here: it's consistent with how step encryption already resolves keys on stable (also assumes current-deployment), so it doesn't make any existing behavior worse, but doesn't fix it either. The parent/child cross-deployment case (the PR's actual scope) is handled correctly because forwarded writables carry their deployment ID explicitly.
Backport stance
Faithful backport. Ready to merge once it comes out of draft.
TooTallNate
enabled auto-merge (squash)
June 1, 2026 19:56
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Summary
stablenow that prerequisite Backport #2059: feat(core): support passing parent WritableStream to child workflow via start() #2070 has merged.Why this needed manual resolution
The automated #2191 backport failed with conflicts (comment, run). PR #2070 has now merged the direct forwarded-stream path into
stable, so this PR has been rebased to contain only the targeted encryption fix.The remaining conflict resolution is specific to the
stablelayout: step execution remains inpackages/core/src/runtime/step-handler.ts, andSerializableSpecialremains inline inpackages/core/src/serialization.tsrather than bringing over main-only split modules.Validation
git diff --check origin-https/stable...HEADfnm exec --using=22.22.0 -- pnpm turbo build --filter=@workflow/core --filter=@workflow/worldfnm exec --using=22.22.0 -- pnpm --filter @workflow/core test(645 tests passed)Note: the local shell defaults to unsupported Node
v25.2.1, where the untouchedsrc/vm/uint8array-base64.test.tsreports cross-realm error-constructor assertion failures. The full suite passes on supported Nodev22.22.0.