Uh oh!
There was an error while loading. Please reload this page.
refactor: use this value serialization for builtin step functions - #1413
Merged
Conversation
Refactor builtin step functions (__builtin_response_json, _text, _array_buffer) to use `this` instead of an explicit parameter, leveraging the useStep() proxy's this value serialization support. - Assign useStep() proxies directly onto Request.prototype and Response.prototype instead of wrapping them in class methods - Update builtin function signatures to use `this: Request | Response` - Remove unused duplicate builtins file from @workflow/core (the canonical copy lives in workflow/src/internal/builtins.ts)
Contributor
🦋 Changeset detectedLatest commit: 5ac88af 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
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (55 failed)mongodb (3 failed):
redis (2 failed):
turso (50 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the internal built-in step functions used by the workflow VM to rely on serialized this values (method-style invocation) instead of explicitly passing Request/Response as parameters, and removes an unused duplicate builtins entrypoint from @workflow/core.
Changes:
- Update
__builtin_response_*steps to usethis: Request | Responseand callthis.{arrayBuffer,json,text}(). - Switch VM
Request/Responseimplementations to attacharrayBuffer/json/textviauseStepon the prototypes (sothisis captured/serialized). - Remove the unused
@workflow/core./builtinsexport and source file.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/workflow/src/internal/builtins.ts | Builtin step functions now use the serialized this value instead of an explicit parameter. |
| packages/core/src/workflow.ts | VM Request/Response now wire body-reading methods directly from useStep, enabling this serialization. |
| packages/core/src/builtins.ts | Removes duplicate/unused builtin step definitions from core. |
| packages/core/package.json | Removes ./builtins export mapping from @workflow/core. |
| .changeset/hungry-spoons-attack.md | Publishes patch release notes for the refactor/removal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+467
to
+471
| Request.prototype.arrayBuffer = useStep<[], ArrayBuffer>( | ||
| '__builtin_response_array_buffer' | ||
| ); | ||
| Request.prototype.json = useStep<[], any>('__builtin_response_json'); | ||
| Request.prototype.text = useStep<[], string>('__builtin_response_text'); |
Comment on lines
+577
to
+581
| Response.prototype.arrayBuffer = useStep<[], ArrayBuffer>( | ||
| '__builtin_response_array_buffer' | ||
| ); | ||
| Response.prototype.json = useStep<[], any>('__builtin_response_json'); | ||
| Response.prototype.text = useStep<[], string>('__builtin_response_text'); |
pranaygp
approved these changes
Mar 17, 2026
pranaygp
left a comment
Contributor
There was a problem hiding this comment.
LGMTM if tests are passing :)
VaguelySerious
approved these changes
Mar 17, 2026
TooTallNate
enabled auto-merge (squash)
March 17, 2026 00:25
Uh oh!
There was an error while loading. Please reload this page.
pranaygp added a commit
that referenced
this pull request
Mar 17, 2026
The rebase onto main didn't fully adopt PR #1413's refactor of response builtins to use `this` instead of explicit parameters. The old pattern (resJson(this) wrappers) passed `this` as an argument, but the step functions now expect `this` to be set via method call context. Switch to Object.defineProperties on Request/Response prototypes, matching main's approach. Also document WORKFLOW_PUBLIC_MANIFEST=1 for local e2e testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygp added a commit
that referenced
this pull request
Mar 17, 2026
The rebase onto main didn't fully adopt PR #1413's refactor of response builtins to use `this` instead of explicit parameters. The old pattern (resJson(this) wrappers) passed `this` as an argument, but the step functions now expect `this` to be set via method call context. Switch to Object.defineProperties on Request/Response prototypes, matching main's approach. Also document WORKFLOW_PUBLIC_MANIFEST=1 for local e2e testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 tasks
pranaygp added a commit
that referenced
this pull request
Mar 18, 2026
The rebase onto main didn't fully adopt PR #1413's refactor of response builtins to use `this` instead of explicit parameters. The old pattern (resJson(this) wrappers) passed `this` as an argument, but the step functions now expect `this` to be set via method call context. Switch to Object.defineProperties on Request/Response prototypes, matching main's approach. Also document WORKFLOW_PUBLIC_MANIFEST=1 for local e2e testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygp added a commit
that referenced
this pull request
Mar 19, 2026
The rebase onto main didn't fully adopt PR #1413's refactor of response builtins to use `this` instead of explicit parameters. The old pattern (resJson(this) wrappers) passed `this` as an argument, but the step functions now expect `this` to be set via method call context. Switch to Object.defineProperties on Request/Response prototypes, matching main's approach. Also document WORKFLOW_PUBLIC_MANIFEST=1 for local e2e testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygp added a commit
that referenced
this pull request
Mar 20, 2026
* Add support for calling `start()` directly inside workflow functions
Enable `start()` to work in workflow context by routing through an
internal step (`__workflow_start`), reusing existing step infrastructure
with no new event types or server changes needed.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Address PR review feedback
- Use typeof check instead of truthiness for WORKFLOW_START symbol
- Validate start() options in workflow context (reject unsupported options like world)
- Set maxRetries=0 on __workflow_start step to prevent orphaned child runs
- Add unit tests for createStart factory (6 tests)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Make Run serializable in workflow context with step-backed methods
- Add Run serialization via __serializable marker + custom Run reducer/reviver
in the serialization module (avoids SWC plugin injecting class-serialization imports)
- Create WorkflowRun class factory (packages/core/src/workflow/run.ts) with
step-backed methods: cancel(), status, returnValue, workflowName, createdAt,
startedAt, completedAt, exists
- Register 8 built-in steps (__run_cancel, __run_status, etc.) in step-handler
- Update __workflow_start to return full Run object (serialized → WorkflowRun in VM)
- Update createStart to pass through step result directly
- Update docs to reflect full Run support in workflow context
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix start() in workflow VM by delegating from api-workflow stub
The workflow VM loads api-workflow.ts (via the "workflow" export condition)
which stubs all runtime functions. The start stub needs to check for the
injected WORKFLOW_START symbol and delegate to it, otherwise start() throws
"doesn't allow this runtime usage" in the workflow context.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Address PR review: fix stale WORKFLOW_SERIALIZE comments and register Run in host registry
- Update comments in step-handler.ts and start.ts to reference the actual
serialization mechanism (Run reducer with __serializable marker) instead
of the stale WORKFLOW_SERIALIZE reference
- Register Run class in the host's class registry from step-handler.ts so
the Run reviver can deserialize Run/WorkflowRun instances in step context
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add docs for recursive/repeating workflows and deploymentId: "latest"
- Document using start() for self-chaining workflows to avoid large event logs
- Add examples for batch processing and cron-like repeating patterns
- Document deploymentId: "latest" option with type safety warning
- Update skill file with same patterns
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Return full Run object from startFromWorkflow e2e workflow
Update the e2e workflow to return the childRun object directly instead of
just childRun.runId, exercising Run serialization across the workflow boundary.
Update e2e test assertions to match.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add recursive fibonacci e2e test for start() in workflow
Demonstrates recursive workflow composition: fibonacciWorkflow starts
new instances of itself via start() + Promise.all to compute fib(6)=8,
fanning out across independent workflow runs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Move Run method steps to builtins with "use step" directives
Refactor: instead of manually registering Run method steps via
registerStepFunction in step-handler.ts, define them as proper "use step"
functions in builtins.ts with __builtin_ prefix. This leverages the
existing SWC plugin infrastructure — functions starting with "__builtin"
get stable bare-name step IDs.
- Add __builtin_run_{cancel,status,return_value,...} to both builtins files
- Use dynamic import() for getRun inside step bodies to avoid pulling
Node.js modules into the workflow bundle
- Remove manual registerStepFunction calls from step-handler.ts
- Update WorkflowRun step references to __builtin_run_* names
- Fix step name display in web observability: fall back to raw name
instead of "?" for built-in steps that don't follow step//module//fn format
- Add fibonacciWorkflow default args for nextjs-turbopack workbench UI
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Render Run objects as clickable links in web observability UI
- Add RunRef type and Run reviver to observabilityRevivers so serialized
Run objects are hydrated as RunRef instead of showing raw Uint8Array
- Add RunRefInline component (purple badge with run ID) that navigates
to the target run on click, matching the StreamRef pattern
- Thread onRunClick callback through the component chain:
WorkflowTraceViewer → EntityDetailPanel → AttributePanel → DataInspector
- Wire up navigation in the web app's run-detail-view
- Add startFromWorkflow default args for workbench UI
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Throw error instead of silent fallback when Run class not in registry
Address PR review: the Run reviver now throws if the class isn't found
in the registry, instead of silently returning a plain { runId } object
that would break the assumption of getting a valid Run instance.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix e2e failures: allow retries on Run getter steps, fix docs code samples
- Remove maxRetries=0 from read-only Run getter steps (status, returnValue,
workflowName, etc.) — these are safe to retry and need retries when the
child workflow hasn't completed within the step timeout. Only cancel
keeps maxRetries=0.
- Fix docs code samples: use correct import path (workflow/api not workflow),
add declare statements for helper functions used in examples.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Use standard step//module//function naming for built-in steps
Update the SWC plugin's __builtin_ special case to generate proper
step//@workflow/core//{name} IDs instead of bare function names. This
makes parseStepName work correctly for built-in steps, showing:
- StepName: "Run#returnValue" (not "__builtin_run_return_value")
- ModuleSpecifier: "@workflow/core" (not the raw function name)
Convention: __builtin_Run_cancel → step//@workflow/core//Run#cancel
(uppercase prefix + underscore → instance method # notation)
- Move __workflow_start to builtins.ts as __builtin_start
- Rename __builtin_run_* to __builtin_Run_* for proper # notation
- Update WorkflowRun step refs to use full step// IDs
- Remove manual registerStepFunction from step-handler.ts
- Update SWC spec.md with new naming examples
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Remove SWC __builtin special case, use standard step naming for builtins
Remove the SWC plugin's __builtin_ special case so built-in steps get
standard step//{module}@{version}//{fn} IDs like any other step. This
makes parseStepName work correctly, showing proper StepName and
ModuleSpecifier in observability.
The VM reconstructs the same IDs via builtinStepId() which uses the
@workflow/core version to build: step//workflow/internal/builtins@{v}//{fn}
- Remove __builtin special case from SWC plugin (revert to original)
- Add builtinStepId() helper shared by workflow.ts, start.ts, run.ts
- Rename Run steps: __builtin_Run_cancel → Run_cancel, etc.
- Rename start step: __builtin_start → start
- Move start step from manual registerStepFunction to builtins.ts
- Keep __builtin_response_* names unchanged (pre-existing)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Use static class methods for Run steps to get Run.method naming
Refactor Run method steps from standalone functions (Run_cancel) to
static methods on a Run class, so the SWC plugin generates step IDs
with the standard static method convention: Run.cancel, Run.returnValue,
Run.status, etc.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Address PR review: tests, docs warnings, skill fix
- Add TODO on Run.returnValue about polling blocking (replace with system
hooks once AbortSignal/AbortController PR lands)
- Add docs callout warning about returnValue holding workers alive
- Fix SKILL.md contradiction that said start() can't be used in workflows
- Enhance suspension test to assert step arguments are forwarded
- Add WorkflowRun unit tests: serializable marker, runId, registry, delegation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix response builtins: adopt this-serialization from PR #1413
The rebase onto main didn't fully adopt PR #1413's refactor of response
builtins to use `this` instead of explicit parameters. The old pattern
(resJson(this) wrappers) passed `this` as an argument, but the step
functions now expect `this` to be set via method call context.
Switch to Object.defineProperties on Request/Response prototypes,
matching main's approach. Also document WORKFLOW_PUBLIC_MANIFEST=1
for local e2e testing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Address docs review: returnValue polling is temporary, link to start() API ref
- Update returnValue warning to note this is a temporary implementation
that will be replaced with internal hooks
- Replace inline deploymentId: "latest" docs with link to the existing
start() API reference which already covers it comprehensively
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix e2e tests: replace collectedRunIds with trackRun API
PR #1426 replaced the manual collectedRunIds array with a trackRun()
helper. The start() wrapper already auto-tracks, so just remove the
manual push calls and add trackRun for the child run.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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
__builtin_response_json,__builtin_response_text,__builtin_response_array_buffer) to usethisinstead of an explicitresparameter, leveraging theuseStep()proxy'sthisvalue serialization supportuseStep()proxies directly ontoRequest.prototypeandResponse.prototypeinstead of wrapping them in class methods that forwardthisas an argumentpackages/core/src/builtins.ts(the canonical copy lives inpackages/workflow/src/internal/builtins.ts)