From f49ada939d8fb193f86cb57dcba359fe5f69849c Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 4 May 2026 22:51:11 -0700 Subject: [PATCH 1/6] [swc-plugin] Capture lexical `this` for nested arrow step functions When a nested arrow `"use step"` references the enclosing function/method's `this`, plumb that `this` through the workflow runtime so the step body sees the correct receiver. - Workflow mode wraps the step proxy with `.bind(this)`, so invoking the proxy captures the caller's `this` as `thisVal` on the queue item. - Step mode hoists the body as a regular `function` (not an arrow) so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the hoisted body. Detection only fires for arrows, since arrows inherit `this` lexically. Nested non-arrow functions/methods/getters/setters introduce their own `this`, so the detector stops at those boundaries. The runtime already supported `thisVal` for instance-method steps; this PR is purely a compiler change to feed the existing pipeline. Caveat: capture works at runtime only when the captured value is serializable across the workflow->step boundary (i.e. the enclosing class implements `WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE`). Refs vercel/workflow#1865 --- .changeset/swc-lexical-this-capture.md | 5 + packages/core/src/step.test.ts | 43 +++++ packages/swc-plugin-workflow/spec.md | 78 ++++++++- .../swc-plugin-workflow/transform/src/lib.rs | 156 ++++++++++++++++-- .../input.js | 29 ++++ .../output-step.js | 50 ++++++ .../output-workflow.js | 36 ++++ .../nested-arrow-step-lexical-this/input.js | 31 ++++ .../output-step.js | 59 +++++++ .../output-workflow.js | 42 +++++ 10 files changed, 514 insertions(+), 15 deletions(-) create mode 100644 .changeset/swc-lexical-this-capture.md create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js diff --git a/.changeset/swc-lexical-this-capture.md b/.changeset/swc-lexical-this-capture.md new file mode 100644 index 0000000000..bf1f9dc4e3 --- /dev/null +++ b/.changeset/swc-lexical-this-capture.md @@ -0,0 +1,5 @@ +--- +"@workflow/swc-plugin": patch +--- + +Capture lexical `this` for nested arrow `"use step"` functions: workflow mode wraps the proxy with `.bind(this)`, and step mode hoists the body as a regular `function` so the runtime can rebind `this` via `stepFn.apply(thisVal, args)`. Requires the enclosing class to have custom serialization. diff --git a/packages/core/src/step.test.ts b/packages/core/src/step.test.ts index 3baea6f355..b747232d3d 100644 --- a/packages/core/src/step.test.ts +++ b/packages/core/src/step.test.ts @@ -284,6 +284,49 @@ describe('createUseStep', () => { }); }); + // The SWC plugin emits `useStep(stepId, closureFn).bind(this)` for nested + // arrow steps that lexically capture `this`. The runtime relies on the + // step proxy being a regular `function` (not an arrow) so that `.bind(this)` + // works and the bound `this` is recorded as `thisVal` on the queue item. + it('captures `this` via .bind(this) on the step proxy (lexical-this support)', async () => { + const ctx = setupWorkflowContext([]); + let workflowErrorReject: (err: Error) => void; + const workflowErrorPromise = new Promise((_, reject) => { + workflowErrorReject = reject; + }); + ctx.onWorkflowError = (err) => { + workflowErrorReject(err); + }; + + const useStep = createUseStep(ctx); + + // Simulate the SWC plugin output: + // globalThis[Symbol.for('WORKFLOW_USE_STEP')]("step_id").bind(this) + // executed inside an enclosing method whose `this` is `instance`. + const instance = { name: 'enclosing-this' }; + const boundStep = useStep('step//input.js//withThis').bind(instance); + + let error: Error | undefined; + try { + await Promise.race([boundStep(7), workflowErrorPromise]); + } catch (err_) { + error = err_ as Error; + } + + expect(error).toBeInstanceOf(WorkflowSuspension); + + // The bound `this` should have been captured on the queue item so the + // step runtime can `apply(thisVal, args)` when executing the step. + expect(ctx.invocationsQueue.size).toBe(1); + const queueItem = [...ctx.invocationsQueue.values()][0]; + expect(queueItem).toMatchObject({ + type: 'step', + stepName: 'step//input.js//withThis', + args: [7], + thisVal: instance, + }); + }); + it('should handle empty closure variables', async () => { // Use empty events to check queue state before step completes const ctx = setupWorkflowContext([]); diff --git a/packages/swc-plugin-workflow/spec.md b/packages/swc-plugin-workflow/spec.md index 3237a746de..13f680608a 100644 --- a/packages/swc-plugin-workflow/spec.md +++ b/packages/swc-plugin-workflow/spec.md @@ -1158,10 +1158,86 @@ The plugin detects this pattern and correctly identifies the directive inside th --- +## Lexical `this` Capture in Nested Arrow Steps + +When a nested arrow-function step references `this` from an enclosing +function/method scope, the plugin captures that `this` so the workflow +runtime can rebind it inside the executing step body. This makes the +following pattern work — the user's class is responsible for providing +custom serialization (`WORKFLOW_SERIALIZE` / `WORKFLOW_DESERIALIZE`) so the +captured `this` can survive the workflow→step boundary: + +Input: +```javascript +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { service: instance.service }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service) { + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input) => { + 'use step'; + return this.service.readFileContent(input, context); + }, + }); + } +} +``` + +Output (Workflow Mode) — the proxy reference is wrapped with `.bind(this)` +so the runtime's step proxy captures the caller's `this` as `thisVal` on the +invocation queue item: +```javascript +createTool(context) { + return tool({ + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]( + "step//./input//_anonymousStep0", + () => ({ context }) + ).bind(this), + }); +} +``` + +Output (Step Mode) — the step body is hoisted as a regular `function` (not +an arrow) so the runtime's `stepFn.apply(thisVal, args)` can rebind `this` +to the value that was captured at call time: +```javascript +async function _anonymousStep0(input) { + const { context } = (function() { /* closure-var IIFE */ })(); + return this.service.readFileContent(input, context); +} +``` + +Detection rules: +- Only `this` references that are **lexically captured by an arrow** count. + An arrow function inherits `this` from its enclosing scope; a nested + `function`/method/getter/setter introduces its own `this` and is therefore + not traversed by the detector. +- The detector only flags arrows that are themselves step functions. A + `this` reference inside a non-step nested arrow inside a step does still + count, because the inner arrow inherits `this` from the step function + body, which in turn inherits from the enclosing function. + +Caveat: capturing `this` only works at runtime if the captured value is +serializable across the workflow→step boundary. Classes registered with +`WORKFLOW_SERIALIZE` / `WORKFLOW_DESERIALIZE` work; ordinary class +instances without custom serialization will fail at proxy-invocation time. + +--- + ## Notes - Arguments and return values must be serializable (JSON-compatible or using custom serialization) -- The `this` keyword and `arguments` object are not allowed in step functions +- `this` is allowed in step functions; arrow-function steps that capture `this` lexically (see above) are rebound at runtime via `stepFn.apply(thisVal, args)` +- The `arguments` object is not allowed in step functions - `super` calls are not allowed in step functions - Imports from the module are excluded from closure variable detection - Module-level declarations (functions, variables, classes) are excluded from closure variable detection, since they are available directly in the step bundle and should not be serialized as closure values diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs index f299d1904a..c5304ba011 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -3,10 +3,10 @@ mod naming; use serde::Deserialize; use std::collections::{HashMap, HashSet}; use swc_core::{ - common::{errors::HANDLER, SyntaxContext, DUMMY_SP}, + common::{DUMMY_SP, SyntaxContext, errors::HANDLER}, ecma::{ ast::*, - visit::{noop_visit_mut_type, noop_visit_type, Visit, VisitMut, VisitMutWith, VisitWith}, + visit::{Visit, VisitMut, VisitMutWith, VisitWith, noop_visit_mut_type, noop_visit_type}, }, }; @@ -360,7 +360,14 @@ pub struct StepTransform { object_property_step_functions: Vec<(String, String, FnExpr, swc_core::common::Span, String, bool)>, // Track nested step functions inside workflow functions for hoisting in step mode - // (fn_name, fn_expr, span, closure_vars, was_arrow, parent_workflow_name) + // (fn_name, fn_expr, span, closure_vars, was_arrow, parent_workflow_name, references_lexical_this) + // + // `references_lexical_this` is set when the original step function was an + // arrow whose body references the enclosing `this`. In step mode such a + // step is hoisted as a regular `function` (not an arrow) so the runtime + // can rebind `this` via `stepFn.apply(thisVal, args)`. In workflow mode + // the proxy reference is wrapped with `.bind(this)` so the step proxy + // captures the caller's `this` and forwards it as `thisVal`. nested_step_functions: Vec<( String, FnExpr, @@ -368,6 +375,7 @@ pub struct StepTransform { Vec, bool, String, + bool, )>, // Counter for anonymous function names #[allow(dead_code)] @@ -1453,6 +1461,50 @@ impl VisitMut for ClosureVariableNormalizer { noop_visit_mut_type!(); } +// Visitor that detects whether an arrow-function body references its lexical +// `this` (i.e. a `this` whose binding comes from an enclosing function/method). +// +// Arrow functions inherit `this` lexically; regular `function` expressions and +// methods introduce their own `this` binding. So when scanning an arrow body +// we recurse into nested arrows but stop at non-arrow function/method/class +// member boundaries (a `this` inside one of those is bound by the inner +// function, not by the outer scope). +struct LexicalThisDetector { + found: bool, +} + +impl LexicalThisDetector { + fn new() -> Self { + Self { found: false } + } + + fn detect_in_arrow(arrow: &ArrowExpr) -> bool { + let mut detector = Self::new(); + arrow.body.visit_with(&mut detector); + detector.found + } +} + +impl Visit for LexicalThisDetector { + noop_visit_type!(); + + fn visit_this_expr(&mut self, _: &ThisExpr) { + self.found = true; + } + + // Do not descend into nested non-arrow functions / methods / getters / + // setters / constructors / class static blocks — those introduce their + // own `this` binding. + fn visit_function(&mut self, _: &Function) {} + fn visit_constructor(&mut self, _: &Constructor) {} + fn visit_getter_prop(&mut self, _: &GetterProp) {} + fn visit_setter_prop(&mut self, _: &SetterProp) {} + fn visit_method_prop(&mut self, _: &MethodProp) {} + fn visit_class_method(&mut self, _: &ClassMethod) {} + fn visit_private_method(&mut self, _: &PrivateMethod) {} + fn visit_static_block(&mut self, _: &StaticBlock) {} +} + impl StepTransform { fn process_stmt(&mut self, stmt: &mut Stmt) { match stmt { @@ -1498,6 +1550,7 @@ impl StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + false, // Regular functions have their own `this` )); // Keep the original function declaration with the directive stripped, @@ -3872,6 +3925,42 @@ impl StepTransform { }) } + // Wrap an expression with `.bind(this)` so the resulting proxy captures + // the caller's lexical `this`. Used when the original step was an arrow + // function whose body referenced the enclosing function's `this`. + fn wrap_with_bind_this(expr: Expr) -> Expr { + Expr::Call(CallExpr { + span: DUMMY_SP, + ctxt: SyntaxContext::empty(), + callee: Callee::Expr(Box::new(Expr::Member(MemberExpr { + span: DUMMY_SP, + obj: Box::new(expr), + prop: MemberProp::Ident(IdentName::new("bind".into(), DUMMY_SP)), + }))), + args: vec![ExprOrSpread { + spread: None, + expr: Box::new(Expr::This(ThisExpr { span: DUMMY_SP })), + }], + type_args: None, + }) + } + + // Same as `create_step_proxy_reference` but wrapped with `.bind(this)` + // when `bind_this` is true. + fn create_step_proxy_reference_maybe_bound( + &self, + step_id: &str, + closure_vars: &[String], + bind_this: bool, + ) -> Expr { + let proxy = self.create_step_proxy_reference(step_id, closure_vars); + if bind_this { + Self::wrap_with_bind_this(proxy) + } else { + proxy + } + } + // Create a proxy reference: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step_id", closure_fn) (workflow mode) fn create_step_proxy_reference(&self, step_id: &str, closure_vars: &[String]) -> Expr { let mut args = vec![ExprOrSpread { @@ -4852,6 +4941,7 @@ impl VisitMut for StepTransform { closure_vars, was_arrow, parent_workflow_name, + references_lexical_this, ) in nested_functions { // Generate hoisted name including parent workflow function name @@ -4914,8 +5004,13 @@ impl VisitMut for StepTransform { } } - // Create the appropriate hoisted declaration based on original function type - let hoisted_decl = if was_arrow { + // Create the appropriate hoisted declaration based on original function type. + // + // If the original arrow body referenced lexical `this`, we + // hoist as a regular `function` (not an arrow) so that the + // workflow runtime's `stepFn.apply(thisVal, args)` can + // rebind `this` to the value captured at call time. + let hoisted_decl = if was_arrow && !references_lexical_this { // Convert back to arrow function: var name = async () => { ... }; let arrow_expr = self.convert_fn_expr_to_arrow(&fn_expr); ModuleItem::Stmt(Stmt::Decl(Decl::Var(Box::new(VarDecl { @@ -7308,9 +7403,9 @@ impl VisitMut for StepTransform { if let Some(body) = &mut fn_expr.function.body { let error_msg = format!( - "You attempted to execute workflow {} function directly. To start a workflow, use start({}) from workflow/api", - name, name - ); + "You attempted to execute workflow {} function directly. To start a workflow, use start({}) from workflow/api", + name, name + ); let error_expr = Expr::New(NewExpr { span: DUMMY_SP, ctxt: SyntaxContext::empty(), @@ -7657,6 +7752,13 @@ impl VisitMut for StepTransform { // Check if we're inside any function (nested), not just workflow functions if !self.in_module_level { + // Detect whether the arrow body references the + // enclosing function's `this`. If so, we need to + // hoist as a regular function (so `apply(thisVal, + // args)` rebinds) and `.bind(this)` the workflow- + // mode proxy reference. + let references_lexical_this = + LexicalThisDetector::detect_in_arrow(arrow_expr); match self.mode { TransformMode::Step => { // Hoist arrow function to module scope @@ -7727,6 +7829,7 @@ impl VisitMut for StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + references_lexical_this, )); // Keep the original arrow with the directive stripped, @@ -7759,10 +7862,13 @@ impl VisitMut for StepTransform { &self.module_imports, &self.declared_identifiers, ); - *init = Box::new(self.create_step_proxy_reference( - &step_id, - &closure_vars, - )); + *init = Box::new( + self.create_step_proxy_reference_maybe_bound( + &step_id, + &closure_vars, + references_lexical_this, + ), + ); } TransformMode::Detect => {} } @@ -8611,6 +8717,7 @@ impl VisitMut for StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + false, // Regular functions have their own `this` )); // Keep the original function with the directive stripped, @@ -8659,6 +8766,11 @@ impl VisitMut for StepTransform { self.anonymous_fn_counter += 1; self.step_function_names.insert(name.clone()); + // Detect whether the arrow body references the + // enclosing function's `this` (lexical capture). + let references_lexical_this = + LexicalThisDetector::detect_in_arrow(arrow_expr); + match self.mode { TransformMode::Step => { // Hoist to module scope @@ -8719,6 +8831,7 @@ impl VisitMut for StepTransform { self.current_parent_function_name .clone() .unwrap_or_default(), + references_lexical_this, )); // Keep the original arrow with the directive stripped, @@ -8749,7 +8862,11 @@ impl VisitMut for StepTransform { &self.module_imports, &self.declared_identifiers, ); - *expr = self.create_step_proxy_reference(&step_id, &closure_vars); + *expr = self.create_step_proxy_reference_maybe_bound( + &step_id, + &closure_vars, + references_lexical_this, + ); return; // Don't visit children since we replaced the expr } TransformMode::Detect => {} @@ -9211,6 +9328,13 @@ impl VisitMut for StepTransform { self.anonymous_fn_counter += 1; self.step_function_names.insert(generated_name.clone()); + // Detect lexical `this` capture so we + // can hoist as a regular function (step + // mode) and `.bind(this)` the proxy + // (workflow mode). + let references_lexical_this = + LexicalThisDetector::detect_in_arrow(arrow_expr); + match self.mode { TransformMode::Step => { // Hoist to module scope @@ -9280,6 +9404,7 @@ impl VisitMut for StepTransform { self.current_workflow_function_name .clone() .unwrap_or_default(), + references_lexical_this, )); // Keep the original arrow with the directive stripped @@ -9309,9 +9434,10 @@ impl VisitMut for StepTransform { // Collect closure variables let closure_vars = ClosureVariableCollector::collect_from_arrow_expr(&arrow_expr, &self.module_imports, &self.declared_identifiers); *kv_prop.value = self - .create_step_proxy_reference( + .create_step_proxy_reference_maybe_bound( &step_id, &closure_vars, + references_lexical_this, ); } TransformMode::Detect => {} @@ -9357,6 +9483,7 @@ impl VisitMut for StepTransform { self.current_workflow_function_name .clone() .unwrap_or_default(), + false, // Regular functions have their own `this` )); // Keep the original function with the directive stripped @@ -9442,6 +9569,7 @@ impl VisitMut for StepTransform { self.current_workflow_function_name .clone() .unwrap_or_default(), + false, // Methods have their own `this` )); // Keep the original method with the directive stripped diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js new file mode 100644 index 0000000000..4a251616ea --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/input.js @@ -0,0 +1,29 @@ +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { value: instance.value }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value) { + this.value = value; + } + + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = async (delta) => { + 'use step'; + return this.value + delta; + }; + + return addToValue(amount); + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js new file mode 100644 index 0000000000..6db5d11ee9 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-step.js @@ -0,0 +1,50 @@ +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"addToValue":{"stepId":"step//./input//addToValue"}}},"classes":{"input.js":{"Counter":{"classId":"class//./input//Counter"}}}}*/; +async function addToValue(delta) { + return this.value + delta; +} +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; + Object.defineProperty(__wf_fn, "name", { + value: "addToValue", + configurable: true + }); +})(addToValue, "step//./input//addToValue"); +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { + value: instance.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value){ + this.value = value; + } + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = async (delta)=>{ + return this.value + delta; + }; + return addToValue(amount); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Counter, "class//./input//Counter"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js new file mode 100644 index 0000000000..3b9f85ca9a --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this-var-decl/output-workflow.js @@ -0,0 +1,36 @@ +// A nested arrow-function step assigned to a `const` inside a method. The +// arrow body references the enclosing method's `this`, so the compiler should +// hoist as a regular function (step mode) and `.bind(this)` the proxy +// (workflow mode). +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"addToValue":{"stepId":"step//./input//addToValue"}}},"classes":{"input.js":{"Counter":{"classId":"class//./input//Counter"}}}}*/; +export class Counter { + static [WORKFLOW_SERIALIZE](instance) { + return { + value: instance.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Counter(data.value); + } + constructor(value){ + this.value = value; + } + // The nested step is assigned to a `const`, exercising the var-declarator + // code path (separate from the in-expression arrow path used for object + // literals). + async run(amount) { + const addToValue = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//addToValue").bind(this); + return addToValue(amount); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Counter, "class//./input//Counter"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js new file mode 100644 index 0000000000..10a4d6be34 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/input.js @@ -0,0 +1,31 @@ +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { service: instance.service }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service) { + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input) => { + 'use step'; + return this.service.readFileContent(input, context); + }, + }); + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js new file mode 100644 index 0000000000..bbcafef2c1 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-step.js @@ -0,0 +1,59 @@ +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"}}},"classes":{"input.js":{"ReadFileTool":{"classId":"class//./input//ReadFileTool"}}}}*/; +async function _anonymousStep0(input) { + const { context } = function() { + var __wf_ctx = globalThis[Symbol.for("WORKFLOW_STEP_CONTEXT_STORAGE")], __wf_store = __wf_ctx && __wf_ctx.getStore(); + if (!__wf_store) throw new Error("Closure variables can only be accessed inside a step function"); + return __wf_store.closureVars || {}; + }(); + return this.service.readFileContent(input, context); +} +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; + Object.defineProperty(__wf_fn, "name", { + value: "_anonymousStep0", + configurable: true + }); +})(_anonymousStep0, "step//./input//_anonymousStep0"); +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { + service: instance.service + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service){ + this.service = service; + } + createTool(context) { + return tool({ + execute: async (input)=>{ + return this.service.readFileContent(input, context); + } + }); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(ReadFileTool, "class//./input//ReadFileTool"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js new file mode 100644 index 0000000000..79c6a4abe9 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-arrow-step-lexical-this/output-workflow.js @@ -0,0 +1,42 @@ +// A nested arrow-function step that references `this` from the enclosing +// method. The class is serializable, so the workflow runtime can carry the +// captured `this` across the workflow→step boundary. +// +// The compiler should: +// - Workflow mode: emit `...WORKFLOW_USE_STEP(...).bind(this)` so the step +// proxy captures the caller's `this` and forwards it as `thisVal`. +// - Step mode: hoist the step body as a regular `function` (not an arrow) +// so the runtime's `stepFn.apply(thisVal, args)` rebinds `this` inside the +// hoisted body. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"}}},"classes":{"input.js":{"ReadFileTool":{"classId":"class//./input//ReadFileTool"}}}}*/; +export class ReadFileTool { + static [WORKFLOW_SERIALIZE](instance) { + return { + service: instance.service + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new ReadFileTool(data.service); + } + constructor(service){ + this.service = service; + } + createTool(context) { + return tool({ + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//_anonymousStep0", ()=>({ + context + })).bind(this) + }); + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(ReadFileTool, "class//./input//ReadFileTool"); From c6d6afb56e11eec1b2a21374324a3b7adc22febe Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 4 May 2026 23:07:00 -0700 Subject: [PATCH 2/6] Address PR review: preserve step proxy metadata + tighter `this` detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - core: Override `.bind` on step proxies so the bound function retains `stepId` and `__closureVarsFn`. Without this, a bound proxy that flows through workflow serialization (e.g. as a step argument) would be treated as a non-serializable plain function by `getStepFunctionReducer`. - swc-plugin: Detector now also walks `arrow.params` so `this` references in default values / destructuring initializers (e.g. `(x = this.foo) => ...`) trigger the `.bind(this)` path. - swc-plugin: Class bodies inside the arrow body are now treated as `this`-binding boundaries — `this` inside class field initializers, methods, etc. is bound to the class instance, not the outer arrow. The detector still walks `extends` clauses and computed property keys because those are evaluated in the surrounding scope. - spec.md: Sharpen the note about `this` in step bodies — it's syntactically allowed but only meaningful for instance-method steps and lexical-`this` arrow steps; other shapes compile but `this` will be whatever the caller of the step proxy passes. - Add `lexical-this-detector-edge-cases` fixture covering both the default-param positive case and the inner-class false-positive guard. - Strengthen the runtime test to assert `stepId` / `__closureVarsFn` survive `.bind(...)`. --- .changeset/step-bind-preserves-metadata.md | 5 ++ packages/core/src/step.test.ts | 14 ++- packages/core/src/step.ts | 40 +++++++++ packages/swc-plugin-workflow/spec.md | 6 +- .../swc-plugin-workflow/transform/src/lib.rs | 47 ++++++++++ .../lexical-this-detector-edge-cases/input.js | 52 +++++++++++ .../output-step.js | 90 +++++++++++++++++++ .../output-workflow.js | 50 +++++++++++ 8 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 .changeset/step-bind-preserves-metadata.md create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js diff --git a/.changeset/step-bind-preserves-metadata.md b/.changeset/step-bind-preserves-metadata.md new file mode 100644 index 0000000000..0d2a2ab79e --- /dev/null +++ b/.changeset/step-bind-preserves-metadata.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Preserve `stepId` and `__closureVarsFn` metadata when calling `.bind(thisArg)` on a step proxy, so bound proxies still serialize correctly through the `StepFunction` reducer (e.g. when passed as step arguments). diff --git a/packages/core/src/step.test.ts b/packages/core/src/step.test.ts index b747232d3d..f56f2e53be 100644 --- a/packages/core/src/step.test.ts +++ b/packages/core/src/step.test.ts @@ -304,7 +304,18 @@ describe('createUseStep', () => { // globalThis[Symbol.for('WORKFLOW_USE_STEP')]("step_id").bind(this) // executed inside an enclosing method whose `this` is `instance`. const instance = { name: 'enclosing-this' }; - const boundStep = useStep('step//input.js//withThis').bind(instance); + const stepProxy = useStep('step//input.js//withThis', () => ({ x: 42 })); + const boundStep = stepProxy.bind(instance); + + // The bound proxy MUST retain the `stepId` and `__closureVarsFn` + // metadata that `getStepFunctionReducer` reads when serializing step + // function references — otherwise a bound proxy that flows through + // workflow serialization (e.g. as a step argument or return value) + // would be treated as a non-serializable plain function. + expect((boundStep as any).stepId).toBe('step//input.js//withThis'); + expect((boundStep as any).__closureVarsFn).toBe( + (stepProxy as any).__closureVarsFn + ); let error: Error | undefined; try { @@ -324,6 +335,7 @@ describe('createUseStep', () => { stepName: 'step//input.js//withThis', args: [7], thisVal: instance, + closureVars: { x: 42 }, }); }); diff --git a/packages/core/src/step.ts b/packages/core/src/step.ts index 5ee265beb6..d8e19b8786 100644 --- a/packages/core/src/step.ts +++ b/packages/core/src/step.ts @@ -221,6 +221,46 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { }); } + // Override `.bind` so the bound function preserves the step proxy + // metadata that `getStepFunctionReducer` relies on for serialization + // (`stepId`, `__closureVarsFn`). Without this override, the SWC plugin + // emitting `useStep(...).bind(this)` for nested arrow steps that + // lexically capture `this` would produce a bound function whose + // `.stepId` is `undefined`, causing the StepFunction reducer to treat + // it as a non-serializable plain function if it ever flows through + // workflow serialization (e.g. as a step argument or return value). + Object.defineProperty(stepFunction, 'bind', { + value: function ( + this: typeof stepFunction, + thisArg: unknown, + ...partialArgs: unknown[] + ) { + const bound = Function.prototype.bind.call( + this, + thisArg, + ...partialArgs + ); + Object.defineProperty(bound, 'stepId', { + value: stepName, + writable: false, + enumerable: false, + configurable: false, + }); + if (closureVarsFn) { + Object.defineProperty(bound, '__closureVarsFn', { + value: closureVarsFn, + writable: false, + enumerable: false, + configurable: false, + }); + } + return bound; + }, + writable: false, + enumerable: false, + configurable: false, + }); + return stepFunction; }; } diff --git a/packages/swc-plugin-workflow/spec.md b/packages/swc-plugin-workflow/spec.md index 13f680608a..8afdcd36b4 100644 --- a/packages/swc-plugin-workflow/spec.md +++ b/packages/swc-plugin-workflow/spec.md @@ -1236,7 +1236,11 @@ instances without custom serialization will fail at proxy-invocation time. ## Notes - Arguments and return values must be serializable (JSON-compatible or using custom serialization) -- `this` is allowed in step functions; arrow-function steps that capture `this` lexically (see above) are rebound at runtime via `stepFn.apply(thisVal, args)` +- `this` is syntactically allowed inside step bodies, but it only carries a meaningful value in two shapes that both flow through the runtime's `thisVal` plumbing: + 1. **Instance-method steps** on a class with custom serialization (e.g. `Counter#add`). Calling `instance.add(...)` captures `instance` as `thisVal` so the step body sees `this === instance`. + 2. **Nested arrow steps that lexically capture `this`** (see "Lexical `this` Capture in Nested Arrow Steps" above). The compiler emits `.bind(this)` on the proxy in workflow mode and hoists the body as a regular `function` in step mode so `stepFn.apply(thisVal, args)` rebinds correctly. + + Other shapes (a top-level `async function` step that references `this`, an arrow step assigned to a module-level variable, etc.) compile without error but `this` will be whatever the caller of the step proxy passes — typically `null`/`undefined` — so referencing it is rarely useful. - The `arguments` object is not allowed in step functions - `super` calls are not allowed in step functions - Imports from the module are excluded from closure variable detection diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs index c5304ba011..a4dd86628d 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -1469,6 +1469,12 @@ impl VisitMut for ClosureVariableNormalizer { // we recurse into nested arrows but stop at non-arrow function/method/class // member boundaries (a `this` inside one of those is bound by the inner // function, not by the outer scope). +// +// Class bodies are also boundaries: `this` inside class property initializers, +// methods, getters/setters, constructors, and static blocks is bound to the +// class instance/static class, not to the enclosing arrow. We still traverse +// `extends` expressions and computed-key expressions because those are +// evaluated in the surrounding scope. struct LexicalThisDetector { found: bool, } @@ -1480,6 +1486,11 @@ impl LexicalThisDetector { fn detect_in_arrow(arrow: &ArrowExpr) -> bool { let mut detector = Self::new(); + // Walk both params (default values, destructuring initializers can + // contain `this`, e.g. `(x = this.foo) => ...`) and the body. + for param in &arrow.params { + param.visit_with(&mut detector); + } arrow.body.visit_with(&mut detector); detector.found } @@ -1503,6 +1514,42 @@ impl Visit for LexicalThisDetector { fn visit_class_method(&mut self, _: &ClassMethod) {} fn visit_private_method(&mut self, _: &PrivateMethod) {} fn visit_static_block(&mut self, _: &StaticBlock) {} + + // Class declarations / expressions: `this` inside a class body member is + // bound to the class instance/static class, not the enclosing arrow. We + // still need to visit `extends` clauses and computed property keys + // because those are evaluated in the outer scope. + fn visit_class(&mut self, class: &Class) { + if let Some(super_class) = &class.super_class { + super_class.visit_with(self); + } + for member in &class.body { + // For each member, only walk the parts evaluated in the outer + // scope (computed keys); the value/body is evaluated with `this` + // bound to the class. + match member { + ClassMember::ClassProp(p) => { + if let PropName::Computed(computed) = &p.key { + computed.expr.visit_with(self); + } + } + ClassMember::PrivateProp(_) => { /* private name keys are not expressions */ } + ClassMember::Method(m) => { + if let PropName::Computed(computed) = &m.key { + computed.expr.visit_with(self); + } + } + ClassMember::PrivateMethod(_) => { /* private name keys are not expressions */ } + ClassMember::Constructor(c) => { + if let PropName::Computed(computed) = &c.key { + computed.expr.visit_with(self); + } + } + ClassMember::StaticBlock(_) => { /* `this` inside is class itself */ } + ClassMember::Empty(_) | ClassMember::TsIndexSignature(_) | ClassMember::AutoAccessor(_) => {} + } + } + } } impl StepTransform { diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js new file mode 100644 index 0000000000..5177c86d01 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/input.js @@ -0,0 +1,52 @@ +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; + +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { value: inst.value }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value) { + this.value = value; + } + + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: async (input = this.value) => { + 'use step'; + return input + 1; + }, + }; + } + + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: async () => { + 'use step'; + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); + }, + }; + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js new file mode 100644 index 0000000000..228a2c793f --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-step.js @@ -0,0 +1,90 @@ +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"},"_anonymousStep1":{"stepId":"step//./input//_anonymousStep1"}}},"classes":{"input.js":{"Edge":{"classId":"class//./input//Edge"}}}}*/; +async function _anonymousStep0(input = this.value) { + return input + 1; +} +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; + Object.defineProperty(__wf_fn, "name", { + value: "_anonymousStep0", + configurable: true + }); +})(_anonymousStep0, "step//./input//_anonymousStep0"); +var _anonymousStep1 = async ()=>{ + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); +}; +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; + Object.defineProperty(__wf_fn, "name", { + value: "_anonymousStep1", + configurable: true + }); +})(_anonymousStep1, "step//./input//_anonymousStep1"); +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { + value: inst.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value){ + this.value = value; + } + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: async (input = this.value)=>{ + return input + 1; + } + }; + } + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: async ()=>{ + class Inner { + self = this; + getThis() { + return this; + } + } + return new Inner(); + } + }; + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Edge, "class//./input//Edge"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js new file mode 100644 index 0000000000..3d5b438b12 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/lexical-this-detector-edge-cases/output-workflow.js @@ -0,0 +1,50 @@ +// Edge cases for the lexical-`this` detector that drives `.bind(this)` and +// the arrow→function hoisting choice. +// +// - default parameter initializers see lexical `this` ⇒ should bind +// - destructuring parameter defaults see lexical `this` ⇒ should bind +// - class field initializers / methods inside the arrow body bind their own +// `this` (the class instance), so they should NOT trigger the detector +// - `extends` clauses and computed property keys inside such a class are +// evaluated in the outer scope, so `this` references there SHOULD trigger +// the detector. +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"_anonymousStep0":{"stepId":"step//./input//_anonymousStep0"},"_anonymousStep1":{"stepId":"step//./input//_anonymousStep1"}}},"classes":{"input.js":{"Edge":{"classId":"class//./input//Edge"}}}}*/; +export class Edge { + static [WORKFLOW_SERIALIZE](inst) { + return { + value: inst.value + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Edge(data.value); + } + constructor(value){ + this.value = value; + } + // `this` appears only in a default parameter (no body reference). + // Detector should still flag this and emit `.bind(this)`. + withThisInDefaultParam() { + return { + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//_anonymousStep0").bind(this) + }; + } + // `this` appears only inside a class body declared *inside* the arrow. + // The class field initializer's `this` is the new instance, NOT the + // outer arrow's lexical `this`. Detector should NOT flag this. + withClassBodyOnly() { + return { + execute: globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//_anonymousStep1") + }; + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Edge, "class//./input//Edge"); From 7e3bd940db05b5a361be1cf08db6a5c4285f4e69 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 4 May 2026 23:30:10 -0700 Subject: [PATCH 3/6] [swc-plugin] Fix `arguments` closure-var capture; drop dead `this`/`arguments` checks - Add `arguments` to `is_global_identifier` so it's not captured as a closure variable. Previously a nested `function`-form step like function step() { 'use step'; return arguments[0]; } was hoisted with `const { arguments } = ...` (a strict-mode syntax error) and the body's `arguments[0]` resolved against the destructured binding instead of the function's intrinsic `arguments` object. - Remove dead `ForbiddenExpression` checks for `this` and `arguments` in `visit_mut_this_expr` / `visit_mut_ident`. The `'use step'` / `'use workflow'` directives are stripped during the module-level traversal before children are visited, so `in_step_function` / `in_workflow_function` are never observed as true here in practice. The existing `step-with-this-arguments-super` fixture explicitly documents that all three identifiers are allowed in step bodies. - Tighten the spec note about `arguments` accordingly: it works in `function`-form steps (reflecting positional args) but is not captured for arrow-form steps; use `...args` for that case. - Add `nested-step-arguments` fixture pinning down the new behavior. --- .changeset/swc-arguments-not-closure.md | 5 ++ packages/swc-plugin-workflow/spec.md | 2 +- .../swc-plugin-workflow/transform/src/lib.rs | 57 +++++++------------ .../fixture/nested-step-arguments/input.js | 16 ++++++ .../nested-step-arguments/output-step.js | 23 ++++++++ .../nested-step-arguments/output-workflow.js | 13 +++++ 6 files changed, 79 insertions(+), 37 deletions(-) create mode 100644 .changeset/swc-arguments-not-closure.md create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js diff --git a/.changeset/swc-arguments-not-closure.md b/.changeset/swc-arguments-not-closure.md new file mode 100644 index 0000000000..45d7576dfa --- /dev/null +++ b/.changeset/swc-arguments-not-closure.md @@ -0,0 +1,5 @@ +--- +"@workflow/swc-plugin": patch +--- + +Stop treating `arguments` as a closure variable when it appears inside a nested `function`-form step body. Previously the hoisted body got an invalid `const { arguments } = ...` destructuring and the original `arguments[N]` access silently broke; now `arguments` reflects the positional args the runtime passes via `stepFn.apply(thisVal, args)`. Also drop dead `ForbiddenExpression` checks for `this`/`arguments` (the directives are stripped before the visitor reaches them, so they were never actually triggered). diff --git a/packages/swc-plugin-workflow/spec.md b/packages/swc-plugin-workflow/spec.md index 8afdcd36b4..b4f71f6eab 100644 --- a/packages/swc-plugin-workflow/spec.md +++ b/packages/swc-plugin-workflow/spec.md @@ -1241,7 +1241,7 @@ instances without custom serialization will fail at proxy-invocation time. 2. **Nested arrow steps that lexically capture `this`** (see "Lexical `this` Capture in Nested Arrow Steps" above). The compiler emits `.bind(this)` on the proxy in workflow mode and hoists the body as a regular `function` in step mode so `stepFn.apply(thisVal, args)` rebinds correctly. Other shapes (a top-level `async function` step that references `this`, an arrow step assigned to a module-level variable, etc.) compile without error but `this` will be whatever the caller of the step proxy passes — typically `null`/`undefined` — so referencing it is rarely useful. -- The `arguments` object is not allowed in step functions +- `arguments` is allowed inside `function`-form step bodies (it reflects the positional arguments the runtime passes via `stepFn.apply(thisVal, args)`). It does **not** work inside arrow-form steps — arrows don't have their own `arguments` binding, and the compiler doesn't capture the enclosing scope's `arguments` the way it does for `this`. Use rest parameters (`...args`) instead if you need that pattern in an arrow step. - `super` calls are not allowed in step functions - Imports from the module are excluded from closure variable detection - Module-level declarations (functions, variables, classes) are excluded from closure variable detection, since they are available directly in the step bundle and should not be serialized as closure values diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs index a4dd86628d..8557e4750c 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -1327,7 +1327,15 @@ impl ClosureVariableCollector { fn is_global_identifier(name: &str) -> bool { matches!( name, - "console" + // `arguments` is a per-function intrinsic binding, not a closure + // variable. Treat it as a global so the closure-variable collector + // doesn't try to serialize it and the hoisted body doesn't end up + // with `const { arguments } = ...` (which is a syntax error in + // strict mode anyway). Hoisted `function`-form steps see their own + // `arguments` reflecting the positional args passed via + // `stepFn.apply(thisVal, args)`. + "arguments" + | "console" | "process" | "global" | "globalThis" @@ -6208,23 +6216,18 @@ impl VisitMut for StepTransform { self.in_module_level = old_in_module; } - // Add forbidden expression checks - fn visit_mut_this_expr(&mut self, expr: &mut ThisExpr) { - if self.in_step_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: expr.span, - expr: "this", - directive: "use step", - }); - } else if self.in_workflow_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: expr.span, - expr: "this", - directive: "use workflow", - }); - } - } - + // Forbidden-expression check for `super` inside step/workflow bodies. + // + // Note: corresponding checks for `this` and `arguments` were removed + // because they were dead in practice (the `'use step'` / `'use workflow'` + // directives are stripped during the module-level traversal before this + // visitor runs over function bodies, so `in_step_function` / + // `in_workflow_function` are never observed as `true` here for those + // expressions). The existing `step-with-this-arguments-super` fixture + // documents that `this`, `arguments`, and `super` are allowed in step + // bodies. `super` is left flagged here for now — the same caveat applies + // — but the wording in `spec.md` reflects the actual runtime behavior + // for the other two. fn visit_mut_super(&mut self, sup: &mut Super) { if self.in_step_function { emit_error(WorkflowErrorKind::ForbiddenExpression { @@ -6241,24 +6244,6 @@ impl VisitMut for StepTransform { } } - fn visit_mut_ident(&mut self, ident: &mut Ident) { - if ident.sym == *"arguments" { - if self.in_step_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: ident.span, - expr: "arguments", - directive: "use step", - }); - } else if self.in_workflow_function { - emit_error(WorkflowErrorKind::ForbiddenExpression { - span: ident.span, - expr: "arguments", - directive: "use workflow", - }); - } - } - } - // Track when we're in a callee position fn visit_mut_callee(&mut self, callee: &mut Callee) { let old_in_callee = self.in_callee; diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js new file mode 100644 index 0000000000..5832e5d19b --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/input.js @@ -0,0 +1,16 @@ +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + 'use workflow'; + + async function step() { + 'use step'; + return arguments[0]; + } + + return step('hello'); +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js new file mode 100644 index 0000000000..d1759676ad --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-step.js @@ -0,0 +1,23 @@ +/**__internal_workflows{"workflows":{"input.js":{"outer":{"workflowId":"workflow//./input//outer"}}},"steps":{"input.js":{"step":{"stepId":"step//./input//step"}}}}*/; +async function outer$step() { + return arguments[0]; +} +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; + Object.defineProperty(__wf_fn, "name", { + value: "outer$step", + configurable: true + }); +})(outer$step, "step//./input//outer/step"); +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + throw new Error("You attempted to execute workflow outer function directly. To start a workflow, use start(outer) from workflow/api"); +} +outer.workflowId = "workflow//./input//outer"; diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js new file mode 100644 index 0000000000..dda9655df2 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/nested-step-arguments/output-workflow.js @@ -0,0 +1,13 @@ +/**__internal_workflows{"workflows":{"input.js":{"outer":{"workflowId":"workflow//./input//outer"}}},"steps":{"input.js":{"step":{"stepId":"step//./input//step"}}}}*/; +// `arguments` inside a nested `function`-form step body must NOT be treated +// as a closure variable — it's a per-function intrinsic binding that +// reflects the positional args the runtime passes via `stepFn.apply(thisVal, +// args)`. Without the `is_global_identifier("arguments")` exclusion, the +// hoisted body would gain `const { arguments } = ...` (a syntax error in +// strict mode) and the original `arguments[0]` access would silently break. +export async function outer() { + var step = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//outer/step"); + return step('hello'); +} +outer.workflowId = "workflow//./input//outer"; +globalThis.__private_workflows.set("workflow//./input//outer", outer); From 753318e8e5247d5ac0e3da1d619032634145c711 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Tue, 5 May 2026 10:43:41 -0700 Subject: [PATCH 4/6] [core] Round-trip bound step proxy `this` through serialization + e2e coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this, a `useStep(...).bind(thisArg)` proxy that flows through workflow serialization (e.g. passed as a step argument) would lose its receiver: the reducer captured `stepId` but not the bound `this`, and the step-bundle reviver returned the raw registered step body which ignores any `this` the caller passes. Now: - step.ts `.bind` override stashes the bound value on the result as `__boundThis` so the reducer can see it. - The reducer serializes `boundThis` (using property presence so `bind(null)`/`bind(undefined)` round-trip faithfully). - The workflow-bundle reviver re-binds the freshly created proxy. - The step-bundle reviver wraps the registered body so it's invoked with `apply(boundThis, args)` (and still runs inside the closure-vars AsyncLocalStorage frame when `closureVars` is present). E2E coverage extends `instanceMethodStepWorkflow` with two new shapes: - `counter.makeAdder(7).add(2)` — direct invocation of a lexical-`this` arrow step. Verifies `bind(this)` carries `thisVal` to the queue and the step body sees `this.value` correctly. - `invokeAdderFromStep(adder.add, 3)` — passes the bound proxy as a step argument so the round-trip path exercises both the reducer and the step-bundle reviver, with the inner call running inline. Without the new `boundThis` plumbing this previously failed with `Cannot read properties of undefined (reading 'value')`. --- .changeset/step-bind-preserves-metadata.md | 2 +- packages/core/e2e/e2e.test.ts | 41 ++++++++++- packages/core/src/serialization.ts | 69 ++++++++++-------- .../serialization/reducers/step-function.ts | 54 ++++++++++---- .../src/serialization/serialization.test.ts | 71 +++++++++++++++++++ packages/core/src/serialization/types.ts | 8 +++ packages/core/src/step.test.ts | 4 ++ packages/core/src/step.ts | 26 +++++-- workbench/example/workflows/99_e2e.ts | 59 +++++++++++++++ 9 files changed, 286 insertions(+), 48 deletions(-) diff --git a/.changeset/step-bind-preserves-metadata.md b/.changeset/step-bind-preserves-metadata.md index 0d2a2ab79e..07bdcdd799 100644 --- a/.changeset/step-bind-preserves-metadata.md +++ b/.changeset/step-bind-preserves-metadata.md @@ -2,4 +2,4 @@ "@workflow/core": patch --- -Preserve `stepId` and `__closureVarsFn` metadata when calling `.bind(thisArg)` on a step proxy, so bound proxies still serialize correctly through the `StepFunction` reducer (e.g. when passed as step arguments). +Round-trip the `this` binding of bound step proxies through workflow serialization. Calling `.bind(thisArg)` on a step proxy now stashes `__boundThis` on the bound function in addition to preserving `stepId`/`__closureVarsFn`; the workflow-side reducer serializes that as `boundThis`, and the workflow- and step-bundle revivers re-bind / `apply()` the deserialized step body to the captured value. Without this, a `useStep(...).bind(this)` proxy passed as a step argument would lose its receiver and the body would see `this === undefined` when invoked from the step bundle. diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts index e10b0f3ab5..085b4f96cd 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -2112,6 +2112,11 @@ describe('e2e', () => { // 3. counter.multiply(3) -> 5 * 3 = 15 // 4. counter.describe('test counter') -> { label: 'test counter', value: 5 } // 5. Create Counter(100), call counter2.add(50) -> 100 + 50 = 150 + // 6. counter.makeAdder(7).add(2) -> 5 + 2 + 7 = 14 (lexical `this`, + // direct invocation — `bind(this)` carries `thisVal` to the queue) + // 7. invokeAdderFromStep(counter.makeAdder(7).add, 3) -> 5 + 3 + 7 = 15 + // (lexical `this` round-tripped through step-arg serialization — + // the reducer captures `__boundThis`, the reviver re-binds) const run = await start(await e2e('instanceMethodStepWorkflow'), [5]); const returnValue = await run.returnValue; @@ -2121,6 +2126,8 @@ describe('e2e', () => { multiplied: 15, // 5 * 3 description: { label: 'test counter', value: 5 }, added2: 150, // 100 + 50 + adderResult: 14, // 5 + 2 + 7 (lexical `this` capture) + adderViaStep: 15, // 5 + 3 + 7 (lexical `this` survives serialization) }); // Verify the run completed successfully @@ -2134,9 +2141,15 @@ describe('e2e', () => { multiplied: 15, description: { label: 'test counter', value: 5 }, added2: 150, + adderResult: 14, + adderViaStep: 15, }); - // Verify the steps were executed (should have 4 steps: add, multiply, describe, add) + // Verify the steps were executed: + // - 4 Counter instance method steps (add, multiply, describe, add) + // - 2 lexical-`this` arrow steps from `makeAdder` (direct + via-step) + // - 1 invokeAdderFromStep wrapper (which itself triggers another + // makeAdder arrow step inside it) const { json: steps } = await cliInspectJson( `steps --runId ${run.runId}` ); @@ -2151,6 +2164,32 @@ describe('e2e', () => { expect(counterSteps.every((s: any) => s.status === 'completed')).toBe( true ); + + // The lexical-`this` arrow step inside `Counter#makeAdder` is + // hoisted by the SWC plugin under an `_anonymousStep` name. It + // ran once as its own step (`adder.add(2)` invoked directly from + // the workflow). The second call (inside `invokeAdderFromStep`) + // executes inline because steps invoked from another step body + // run inline rather than queueing a new step — so we only see one + // `_anonymousStep` event in the log, even though the body executed + // twice. Asserting `=== 1` here pins down both: + // 1. the direct invocation actually creates a step (i.e. the + // `bind(this)` proxy still goes through `useStep`), and + // 2. the round-tripped proxy correctly runs inline rather than + // somehow re-queuing a duplicate step. + const adderArrowSteps = steps.filter((s: any) => + s.stepName.includes('_anonymousStep') + ); + expect(adderArrowSteps.length).toBe(1); + expect(adderArrowSteps[0].status).toBe('completed'); + + // The `invokeAdderFromStep` wrapper itself runs as its own step; + // its body invokes the round-tripped `add` proxy inline. + const invokeAdderSteps = steps.filter((s: any) => + s.stepName.includes('invokeAdderFromStep') + ); + expect(invokeAdderSteps.length).toBe(1); + expect(invokeAdderSteps[0].status).toBe('completed'); } ); diff --git a/packages/core/src/serialization.ts b/packages/core/src/serialization.ts index 1ec66d0cb7..7d03a7bd5a 100644 --- a/packages/core/src/serialization.ts +++ b/packages/core/src/serialization.ts @@ -1001,10 +1001,24 @@ function getStepRevivers( ...getCommonRevivers(global), // StepFunction reviver for step context - returns raw step function - // with closure variable support via AsyncLocalStorage + // with closure variable support via AsyncLocalStorage. + // + // Handles three independent flags from the serialized payload: + // - `closureVars`: invoke the body inside an AsyncLocalStorage frame + // so the SWC-emitted `WORKFLOW_STEP_CONTEXT_STORAGE` IIFE in the + // hoisted body can pull the closure variables back out. + // - `boundThis`: a `this` value captured by `useStep(...).bind(this)` + // in the workflow bundle (lexical-`this` arrow steps). The + // wrapper invokes the body via `stepFn.apply(boundThis, args)` so + // the body sees the same `this` it would have had in the workflow + // bundle. Property presence — not truthiness — is significant + // because `bind(null)` and `bind(undefined)` are both legal and + // should round-trip faithfully. StepFunction: (value) => { const stepId = value.stepId; const closureVars = value.closureVars; + const hasBoundThis = 'boundThis' in value; + const boundThis = hasBoundThis ? value.boundThis : undefined; const stepFn = getStepFunction(stepId); if (!stepFn) { @@ -1016,47 +1030,46 @@ function getStepRevivers( ); } - // If closure variables were serialized, return a wrapper function - // that sets up AsyncLocalStorage context when invoked - if (closureVars) { - const wrappedStepFn = ((...args: any[]) => { - // Get the current context from AsyncLocalStorage - const currentContext = contextStorage.getStore(); + // Fast path: nothing to wrap. + if (!closureVars && !hasBoundThis) { + return stepFn; + } + const wrappedStepFn = function (this: unknown, ...args: any[]) { + const callThis = hasBoundThis ? boundThis : this; + if (closureVars) { + const currentContext = contextStorage.getStore(); if (!currentContext) { throw new WorkflowRuntimeError( 'Cannot call step function with closure variables outside step context' ); } - - // Create a new context with the closure variables merged in const newContext = { ...currentContext, closureVars, }; - - // Run the step function with the new context that includes closure vars - return contextStorage.run(newContext, () => stepFn(...args)); - }) as any; - - // Copy properties from original step function - Object.defineProperty(wrappedStepFn, 'name', { - value: stepFn.name, - }); - Object.defineProperty(wrappedStepFn, 'stepId', { - value: stepId, - writable: false, - enumerable: false, - configurable: false, - }); - if (stepFn.maxRetries !== undefined) { - wrappedStepFn.maxRetries = stepFn.maxRetries; + return contextStorage.run(newContext, () => + stepFn.apply(callThis, args) + ); } + return stepFn.apply(callThis, args); + } as any; - return wrappedStepFn; + // Copy properties from original step function + Object.defineProperty(wrappedStepFn, 'name', { + value: stepFn.name, + }); + Object.defineProperty(wrappedStepFn, 'stepId', { + value: stepId, + writable: false, + enumerable: false, + configurable: false, + }); + if (stepFn.maxRetries !== undefined) { + wrappedStepFn.maxRetries = stepFn.maxRetries; } - return stepFn; + return wrappedStepFn; }, WorkflowFunction: (value) => diff --git a/packages/core/src/serialization/reducers/step-function.ts b/packages/core/src/serialization/reducers/step-function.ts index 8b2f521ed1..4b0e27eebb 100644 --- a/packages/core/src/serialization/reducers/step-function.ts +++ b/packages/core/src/serialization/reducers/step-function.ts @@ -4,10 +4,16 @@ * In workflow mode, step functions are replaced by the SWC plugin with * proxies created by `globalThis[Symbol.for("WORKFLOW_USE_STEP")]("stepId")`. * These proxies have a `.stepId` property and optionally a `.__closureVarsFn` - * for captured closure variables. + * for captured closure variables. They may additionally have a + * `.__boundThis` property when the SWC plugin emitted `useStep(...).bind(this)` + * for a nested arrow step that lexically captured `this` (see + * `packages/swc-plugin-workflow/spec.md` → "Lexical `this` Capture in + * Nested Arrow Steps"). * - * The reducer serializes them as `{ stepId, closureVars? }`. - * The reviver reconstructs them by calling WORKFLOW_USE_STEP. + * The reducer serializes them as `{ stepId, closureVars?, boundThis? }`. + * The reviver reconstructs them by calling WORKFLOW_USE_STEP and, when + * `boundThis` is present, re-binding the resulting proxy so the caller's + * captured `this` survives the round trip. */ import type { Reducers, Revivers } from '../types.js'; @@ -22,12 +28,27 @@ export function getStepFunctionReducer(): Partial { if (typeof stepId !== 'string') return false; const closureVarsFn = (value as any).__closureVarsFn; - if (closureVarsFn && typeof closureVarsFn === 'function') { - const closureVars = closureVarsFn(); - return { stepId, closureVars }; - } + const closureVars = + closureVarsFn && typeof closureVarsFn === 'function' + ? closureVarsFn() + : undefined; + + // `__boundThis` is a marker property added by the step proxy's + // overridden `.bind` (see step.ts) to record the captured lexical + // `this`. Use `in` so we round-trip even when the bound `this` is + // `undefined`/`null`. + const hasBoundThis = '__boundThis' in (value as any); + const boundThis = hasBoundThis ? (value as any).__boundThis : undefined; - return { stepId }; + const payload: { + stepId: string; + closureVars?: Record; + boundThis?: unknown; + } = { stepId }; + if (closureVars !== undefined) payload.closureVars = closureVars; + if (hasBoundThis) payload.boundThis = boundThis; + + return payload; }, }; } @@ -38,7 +59,12 @@ export function getStepFunctionReducer(): Partial { * Create the StepFunction reviver for workflow context. * * The reviver calls WORKFLOW_USE_STEP to create the step proxy, - * restoring the ability to call the step from workflow code. + * restoring the ability to call the step from workflow code. If the + * serialized payload includes `boundThis`, the reviver also re-binds the + * freshly-created proxy so a step proxy that was constructed with + * `.bind(this)` in the workflow bundle continues to carry that `this` + * after being deserialized in another bundle (e.g. when passed as a step + * argument). */ export function getStepFunctionReviver( global: Record = globalThis @@ -61,10 +87,14 @@ export function getStepFunctionReviver( ); } - if (closureVars) { - return useStep(stepId, () => closureVars); + const proxy = closureVars + ? useStep(stepId, () => closureVars) + : useStep(stepId); + + if ('boundThis' in value) { + return (proxy as any).bind(value.boundThis); } - return useStep(stepId); + return proxy; }, }; } diff --git a/packages/core/src/serialization/serialization.test.ts b/packages/core/src/serialization/serialization.test.ts index ace5b0f8e3..c28af97b0d 100644 --- a/packages/core/src/serialization/serialization.test.ts +++ b/packages/core/src/serialization/serialization.test.ts @@ -755,6 +755,45 @@ describe('step function reducer', () => { it('should return false for functions without stepId', () => { expect(reducers.StepFunction!(() => {})).toBe(false); }); + + it('should include `boundThis` when the proxy has the marker property', () => { + // Simulates a step proxy that was created via `useStep(...).bind(thisArg)`. + // The actual `.bind` override on real proxies (in step.ts) attaches + // `__boundThis` so the reducer can serialize the captured `this`. + const instance = { kind: 'workflow-side' }; + const fn = Object.defineProperties(() => {}, { + stepId: { value: 'step//test//withBoundThis' }, + __boundThis: { value: instance }, + }); + const result = reducers.StepFunction!(fn); + expect(result).toEqual({ + stepId: 'step//test//withBoundThis', + boundThis: instance, + }); + }); + + it('should round-trip a `null`/`undefined` bound `this`', () => { + // `Function.prototype.bind(null)` and `bind(undefined)` are valid; the + // reducer must use property presence rather than truthiness so the + // distinction round-trips faithfully. + const fnNull = Object.defineProperties(() => {}, { + stepId: { value: 'step//test//boundNull' }, + __boundThis: { value: null }, + }); + expect(reducers.StepFunction!(fnNull)).toEqual({ + stepId: 'step//test//boundNull', + boundThis: null, + }); + + const fnUndef = Object.defineProperties(() => {}, { + stepId: { value: 'step//test//boundUndef' }, + __boundThis: { value: undefined }, + }); + expect(reducers.StepFunction!(fnUndef)).toEqual({ + stepId: 'step//test//boundUndef', + boundThis: undefined, + }); + }); }); describe('step function reviver', () => { @@ -798,6 +837,38 @@ describe('step function reviver', () => { revivers.StepFunction!({ stepId: 'step//test//myStep' }) ).toThrow(/WORKFLOW_USE_STEP not found/); }); + + it('should re-bind the proxy when `boundThis` is present in the payload', () => { + // Simulates the round trip for a step proxy that was created via + // `useStep(...).bind(thisArg)` in the workflow bundle and then passed + // as a step argument: the reducer captures `boundThis`, and the + // reviver in the step bundle must re-bind the freshly created proxy + // so the original `this` survives the serialization round trip. + const recordedThisVals: unknown[] = []; + const proxy = function (this: unknown) { + recordedThisVals.push(this); + return 'result'; + }; + const mockUseStep = vi.fn().mockReturnValue(proxy); + const global = { + [Symbol.for('WORKFLOW_USE_STEP')]: mockUseStep, + }; + + const instance = { restored: true }; + const revivers = getStepFunctionReviver(global); + const result = revivers.StepFunction!({ + stepId: 'step//test//withBoundThis', + boundThis: instance, + }) as () => unknown; + + expect(typeof result).toBe('function'); + expect(mockUseStep).toHaveBeenCalledWith('step//test//withBoundThis'); + + // Invoking the revived proxy without an explicit receiver should + // observe the bound `this`, not `undefined`. + result(); + expect(recordedThisVals).toEqual([instance]); + }); }); // ============================================================================ diff --git a/packages/core/src/serialization/types.ts b/packages/core/src/serialization/types.ts index 4c14f82bff..b7518e723c 100644 --- a/packages/core/src/serialization/types.ts +++ b/packages/core/src/serialization/types.ts @@ -107,6 +107,14 @@ export interface SerializableSpecial { StepFunction: { stepId: string; closureVars?: Record; + /** + * Captured lexical `this` for step proxies that were created via + * `useStep(...).bind(thisArg)` (the SWC plugin emits this for nested + * arrow steps that close over their enclosing function's `this`). + * The reviver re-binds the freshly-created proxy to this value so the + * binding survives serialization round-trips. + */ + boundThis?: unknown; }; TypeError: { message: string; stack?: string; cause?: unknown }; URIError: { message: string; stack?: string; cause?: unknown }; diff --git a/packages/core/src/step.test.ts b/packages/core/src/step.test.ts index f56f2e53be..4152562b66 100644 --- a/packages/core/src/step.test.ts +++ b/packages/core/src/step.test.ts @@ -316,6 +316,10 @@ describe('createUseStep', () => { expect((boundStep as any).__closureVarsFn).toBe( (stepProxy as any).__closureVarsFn ); + // `__boundThis` is the marker the reducer uses to serialize the + // captured `this`, so a deserialized proxy in another bundle can + // re-bind to the same value. + expect((boundStep as any).__boundThis).toBe(instance); let error: Error | undefined; try { diff --git a/packages/core/src/step.ts b/packages/core/src/step.ts index d8e19b8786..b99db5b226 100644 --- a/packages/core/src/step.ts +++ b/packages/core/src/step.ts @@ -223,12 +223,20 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { // Override `.bind` so the bound function preserves the step proxy // metadata that `getStepFunctionReducer` relies on for serialization - // (`stepId`, `__closureVarsFn`). Without this override, the SWC plugin - // emitting `useStep(...).bind(this)` for nested arrow steps that - // lexically capture `this` would produce a bound function whose - // `.stepId` is `undefined`, causing the StepFunction reducer to treat - // it as a non-serializable plain function if it ever flows through - // workflow serialization (e.g. as a step argument or return value). + // (`stepId`, `__closureVarsFn`, `__boundThis`). Without this override, + // the SWC plugin emitting `useStep(...).bind(this)` for nested arrow + // steps that lexically capture `this` would produce a bound function + // whose `.stepId` is `undefined`, causing the StepFunction reducer to + // treat it as a non-serializable plain function. + // + // We also stash the bound `this` value as `__boundThis` so the bound + // proxy can round-trip through serialization with the binding intact: + // when the bound proxy is passed as a step argument, the reducer + // serializes the captured `this` alongside the `stepId`, and the + // reviver in the step bundle re-binds a fresh proxy to the same + // `this`. Without this, a deserialized proxy would lose its binding + // and the step body would see `this === undefined` when the + // downstream caller invokes it without an explicit receiver. Object.defineProperty(stepFunction, 'bind', { value: function ( this: typeof stepFunction, @@ -254,6 +262,12 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { configurable: false, }); } + Object.defineProperty(bound, '__boundThis', { + value: thisArg, + writable: false, + enumerable: false, + configurable: false, + }); return bound; }, writable: false, diff --git a/workbench/example/workflows/99_e2e.ts b/workbench/example/workflows/99_e2e.ts index 54c46fe725..ad3826b826 100644 --- a/workbench/example/workflows/99_e2e.ts +++ b/workbench/example/workflows/99_e2e.ts @@ -1543,6 +1543,45 @@ export class Counter { 'use step'; return { label, value: this.value }; } + + /** + * Returns a "tool"-shaped object whose `add` property is a nested arrow + * step that lexically captures `this`. + * + * This exercises the SWC plugin's lexical-`this` capture for nested arrow + * step functions: in workflow mode the proxy is wrapped with + * `.bind(this)` (so the queue item carries `thisVal`), and in step mode + * the body is hoisted as a regular `function` (so `stepFn.apply(thisVal, + * args)` can rebind). The captured `delta` exercises the closure-vars + * path alongside it. + * + * This is structurally identical to the AI SDK `tool({ execute: async + * (input) => { 'use step'; return this.x; } })` pattern from the + * upstream issue (vercel/workflow#1865). + */ + makeAdder(delta: number): { + add: (amount: number) => Promise; + } { + return { + add: async (amount: number) => { + 'use step'; + return this.value + amount + delta; + }, + }; + } +} + +/** + * Step that takes a step-function reference and invokes it. Used by + * `instanceMethodStepWorkflow` to exercise the workflow→step→step + * round-trip serialization of a `bind(this)`-wrapped step proxy. + */ +async function invokeAdderFromStep( + add: (amount: number) => Promise, + amount: number +): Promise { + 'use step'; + return add(amount); } /** @@ -1566,12 +1605,32 @@ export async function instanceMethodStepWorkflow(initialValue: number) { const counter2 = new Counter(100); const added2 = await counter2.add(50); + // Lexical-`this` capture in a nested arrow step: + // + // `counter.makeAdder(7).add` is a step proxy that the SWC plugin wraps + // with `.bind(this)` in workflow mode. Invoking it directly should + // capture the Counter instance as `thisVal` so the step body sees + // `this.value` correctly. + const adder = counter.makeAdder(7); + const adderResult = await adder.add(2); // initialValue + 2 + 7 + + // Round-trip the bound step proxy through another step boundary: + // + // Passing `adder.add` as a step argument forces the + // `getStepFunctionReducer` to capture the bound `this` (`__boundThis`) + // alongside the `stepId`, and the step bundle's reviver must re-bind + // the freshly created proxy so the inner step still sees the original + // Counter instance via `this`. + const adderViaStep = await invokeAdderFromStep(adder.add, 3); // initialValue + 3 + 7 + return { initialValue, added, // initialValue + 10 multiplied, // initialValue * 3 description, // { label: 'test counter', value: initialValue } added2, // 100 + 50 = 150 + adderResult, // initialValue + 2 + 7 + adderViaStep, // initialValue + 3 + 7 }; } From 993055c4799ff30e65d73fb5710b23b75e20b18c Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Tue, 5 May 2026 10:53:18 -0700 Subject: [PATCH 5/6] Trim changeset descriptions --- .changeset/step-bind-preserves-metadata.md | 2 +- .changeset/swc-arguments-not-closure.md | 2 +- .changeset/swc-lexical-this-capture.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/step-bind-preserves-metadata.md b/.changeset/step-bind-preserves-metadata.md index 07bdcdd799..ead2f02f04 100644 --- a/.changeset/step-bind-preserves-metadata.md +++ b/.changeset/step-bind-preserves-metadata.md @@ -2,4 +2,4 @@ "@workflow/core": patch --- -Round-trip the `this` binding of bound step proxies through workflow serialization. Calling `.bind(thisArg)` on a step proxy now stashes `__boundThis` on the bound function in addition to preserving `stepId`/`__closureVarsFn`; the workflow-side reducer serializes that as `boundThis`, and the workflow- and step-bundle revivers re-bind / `apply()` the deserialized step body to the captured value. Without this, a `useStep(...).bind(this)` proxy passed as a step argument would lose its receiver and the body would see `this === undefined` when invoked from the step bundle. +Preserve the `this` binding of bound step proxies across workflow serialization, so passing `useStep(...).bind(thisArg)` as a step argument no longer loses the receiver. diff --git a/.changeset/swc-arguments-not-closure.md b/.changeset/swc-arguments-not-closure.md index 45d7576dfa..02aed4b7a0 100644 --- a/.changeset/swc-arguments-not-closure.md +++ b/.changeset/swc-arguments-not-closure.md @@ -2,4 +2,4 @@ "@workflow/swc-plugin": patch --- -Stop treating `arguments` as a closure variable when it appears inside a nested `function`-form step body. Previously the hoisted body got an invalid `const { arguments } = ...` destructuring and the original `arguments[N]` access silently broke; now `arguments` reflects the positional args the runtime passes via `stepFn.apply(thisVal, args)`. Also drop dead `ForbiddenExpression` checks for `this`/`arguments` (the directives are stripped before the visitor reaches them, so they were never actually triggered). +Fix `arguments` being incorrectly captured as a closure variable in nested `function`-form step bodies, which previously produced invalid output. diff --git a/.changeset/swc-lexical-this-capture.md b/.changeset/swc-lexical-this-capture.md index bf1f9dc4e3..9661a02dcd 100644 --- a/.changeset/swc-lexical-this-capture.md +++ b/.changeset/swc-lexical-this-capture.md @@ -2,4 +2,4 @@ "@workflow/swc-plugin": patch --- -Capture lexical `this` for nested arrow `"use step"` functions: workflow mode wraps the proxy with `.bind(this)`, and step mode hoists the body as a regular `function` so the runtime can rebind `this` via `stepFn.apply(thisVal, args)`. Requires the enclosing class to have custom serialization. +Support `this` references inside nested arrow `"use step"` functions. Requires the enclosing class to have custom serialization. From a19805f337baec7f4cd08df789ef014f8a35bcdb Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Tue, 5 May 2026 11:37:56 -0700 Subject: [PATCH 6/6] Address PR review: round-trip `boundArgs` + unit test for closureVars+boundThis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - step.ts `.bind` override now also stashes `__boundArgs` when the caller supplied prefilled args (`useStep(...).bind(thisArg, x, y)`). The reducer serializes them as `boundArgs`, and both the workflow- and step-bundle revivers re-apply them (the workflow reviver via `bind(boundThis, ...boundArgs)`, the step-bundle reviver by prepending to the runtime args). The SWC plugin only ever emits `.bind(this)` today, but this keeps partial application faithful in case hand-written code ever calls `.bind` with extra args. - Add two new unit tests in `serialization.test.ts`: - `closureVars + boundThis` combo: exercises the step-bundle reviver's inner branch (`contextStorage.run(newContext, () => stepFn.apply(callThis, callArgs))`) in isolation — previously only covered end-to-end by the `instanceMethodStepWorkflow` e2e test. - `boundArgs` round-trip: codifies that prefilled args survive serialization. --- packages/core/src/serialization.test.ts | 142 ++++++++++++++++++ packages/core/src/serialization.ts | 28 ++-- .../serialization/reducers/step-function.ts | 46 +++--- packages/core/src/serialization/types.ts | 10 ++ packages/core/src/step.ts | 44 ++++-- 5 files changed, 228 insertions(+), 42 deletions(-) diff --git a/packages/core/src/serialization.test.ts b/packages/core/src/serialization.test.ts index 4c86d66739..d8ac2b4a13 100644 --- a/packages/core/src/serialization.test.ts +++ b/packages/core/src/serialization.test.ts @@ -2299,6 +2299,148 @@ describe('step function serialization', () => { expect(result).toBe('Result: 21'); }); + it('should dehydrate and hydrate step function with closureVars + boundThis combined', async () => { + // The end-to-end shape exercised by the SWC plugin's lexical-`this` + // capture: a nested arrow step closes over both lexical `this` AND a + // surrounding closure variable. After serialization, the step-bundle + // reviver must run the registered body inside the closure-vars + // AsyncLocalStorage frame *and* invoke it with `apply(boundThis, + // args)`. + const stepName = 'step//workflows/test.ts//addToInstance'; + + const { __private_getClosureVars } = await import( + './step/get-closure-vars.js' + ); + const { contextStorage } = await import('./step/context-storage.js'); + + const stepFn = async function (this: { value: number }, amount: number) { + const { delta } = __private_getClosureVars() as { delta: number }; + return this.value + amount + delta; + }; + registerStepFunction(stepName, stepFn); + + Object.defineProperty(stepFn, 'stepId', { + value: stepName, + writable: false, + enumerable: false, + configurable: false, + }); + Object.defineProperty(stepFn, '__closureVarsFn', { + value: () => ({ delta: 7 }), + writable: false, + enumerable: false, + configurable: false, + }); + // Simulate the `__boundThis` marker that the step proxy's overridden + // `.bind` (in step.ts) would attach. Plain object instead of a real + // class instance so the test focuses on the reducer/reviver plumbing. + const instance = { value: 5 }; + Object.defineProperty(stepFn, '__boundThis', { + value: instance, + writable: false, + enumerable: false, + configurable: false, + }); + + // Round-trip through the workflow→step serialization pipeline. + const dehydrated = await dehydrateStepArguments( + [stepFn, 3], + mockRunId, + noEncryptionKey, + globalThis + ); + const hydrated = (await hydrateStepArguments( + dehydrated, + 'test-run-123', + noEncryptionKey, + [] + )) as [(amount: number) => Promise, number]; + + expect(typeof hydrated[0]).toBe('function'); + expect(hydrated[1]).toBe(3); + + // Invoke the rehydrated step function inside a step-context frame + // (otherwise the closure-vars wrapper throws). The wrapper must + // bind `this` to `instance` *and* expose `delta = 7` via + // `__private_getClosureVars()`. + const result = await contextStorage.run( + { + stepMetadata: { + stepName, + stepId: 'test-step', + stepStartedAt: new Date(), + attempt: 1, + }, + workflowMetadata: { + workflowName: 'workflow//workflows/test.ts//testWorkflow', + workflowRunId: 'test-run', + workflowStartedAt: new Date(), + url: 'http://localhost:3000', + features: { encryption: false }, + }, + ops: [], + }, + () => hydrated[0](3) + ); + + // value(5) + amount(3) + delta(7) = 15 + expect(result).toBe(15); + }); + + it('should preserve `boundArgs` (partial application) across serialization', async () => { + // The step proxy's overridden `.bind` also stashes prefilled args + // (`useStep(...).bind(thisArg, x)`) so partial application survives + // the round trip. The SWC plugin only ever emits `.bind(this)` today, + // so this codifies the safety net for future hand-written callers. + const stepName = 'step//workflows/test.ts//partialApply'; + + const stepFn = async function ( + this: { tag: string }, + prefilled: number, + runtimeArg: number + ) { + return `${this.tag}:${prefilled}+${runtimeArg}`; + }; + registerStepFunction(stepName, stepFn); + + Object.defineProperty(stepFn, 'stepId', { + value: stepName, + writable: false, + enumerable: false, + configurable: false, + }); + Object.defineProperty(stepFn, '__boundThis', { + value: { tag: 'bound' }, + writable: false, + enumerable: false, + configurable: false, + }); + Object.defineProperty(stepFn, '__boundArgs', { + value: [10], + writable: false, + enumerable: false, + configurable: false, + }); + + const dehydrated = await dehydrateStepArguments( + [stepFn], + mockRunId, + noEncryptionKey, + globalThis + ); + const hydrated = (await hydrateStepArguments( + dehydrated, + 'test-run-123', + noEncryptionKey, + [] + )) as [(runtimeArg: number) => Promise]; + + // The hydrated proxy should already have `prefilled = 10` baked in, + // so the caller only supplies `runtimeArg`. + const result = await hydrated[0](32); + expect(result).toBe('bound:10+32'); + }); + it('should serialize step function to object through reducer', () => { const stepName = 'step//workflows/test.ts//anotherStep'; const stepFn = async () => 'result'; diff --git a/packages/core/src/serialization.ts b/packages/core/src/serialization.ts index 7d03a7bd5a..ec67ef5140 100644 --- a/packages/core/src/serialization.ts +++ b/packages/core/src/serialization.ts @@ -1003,22 +1003,27 @@ function getStepRevivers( // StepFunction reviver for step context - returns raw step function // with closure variable support via AsyncLocalStorage. // - // Handles three independent flags from the serialized payload: + // Handles four independent flags from the serialized payload: // - `closureVars`: invoke the body inside an AsyncLocalStorage frame // so the SWC-emitted `WORKFLOW_STEP_CONTEXT_STORAGE` IIFE in the // hoisted body can pull the closure variables back out. - // - `boundThis`: a `this` value captured by `useStep(...).bind(this)` - // in the workflow bundle (lexical-`this` arrow steps). The - // wrapper invokes the body via `stepFn.apply(boundThis, args)` so - // the body sees the same `this` it would have had in the workflow - // bundle. Property presence — not truthiness — is significant - // because `bind(null)` and `bind(undefined)` are both legal and - // should round-trip faithfully. + // - `boundThis`: a `this` value captured by + // `useStep(...).bind(this)` in the workflow bundle (lexical-`this` + // arrow steps). The wrapper invokes the body via + // `stepFn.apply(boundThis, args)` so the body sees the same + // `this` it would have had in the workflow bundle. Property + // presence — not truthiness — is significant because + // `bind(null)` and `bind(undefined)` are both legal and should + // round-trip faithfully. + // - `boundArgs`: prefilled args from + // `useStep(...).bind(thisArg, x, y)`. Prepended to the call args + // so partial application survives serialization. StepFunction: (value) => { const stepId = value.stepId; const closureVars = value.closureVars; const hasBoundThis = 'boundThis' in value; const boundThis = hasBoundThis ? value.boundThis : undefined; + const boundArgs = Array.isArray(value.boundArgs) ? value.boundArgs : []; const stepFn = getStepFunction(stepId); if (!stepFn) { @@ -1031,12 +1036,13 @@ function getStepRevivers( } // Fast path: nothing to wrap. - if (!closureVars && !hasBoundThis) { + if (!closureVars && !hasBoundThis && boundArgs.length === 0) { return stepFn; } const wrappedStepFn = function (this: unknown, ...args: any[]) { const callThis = hasBoundThis ? boundThis : this; + const callArgs = boundArgs.length > 0 ? [...boundArgs, ...args] : args; if (closureVars) { const currentContext = contextStorage.getStore(); if (!currentContext) { @@ -1049,10 +1055,10 @@ function getStepRevivers( closureVars, }; return contextStorage.run(newContext, () => - stepFn.apply(callThis, args) + stepFn.apply(callThis, callArgs) ); } - return stepFn.apply(callThis, args); + return stepFn.apply(callThis, callArgs); } as any; // Copy properties from original step function diff --git a/packages/core/src/serialization/reducers/step-function.ts b/packages/core/src/serialization/reducers/step-function.ts index 4b0e27eebb..59a2e31ff0 100644 --- a/packages/core/src/serialization/reducers/step-function.ts +++ b/packages/core/src/serialization/reducers/step-function.ts @@ -4,16 +4,18 @@ * In workflow mode, step functions are replaced by the SWC plugin with * proxies created by `globalThis[Symbol.for("WORKFLOW_USE_STEP")]("stepId")`. * These proxies have a `.stepId` property and optionally a `.__closureVarsFn` - * for captured closure variables. They may additionally have a - * `.__boundThis` property when the SWC plugin emitted `useStep(...).bind(this)` - * for a nested arrow step that lexically captured `this` (see - * `packages/swc-plugin-workflow/spec.md` → "Lexical `this` Capture in - * Nested Arrow Steps"). + * for captured closure variables. They may additionally have `.__boundThis` + * (and rarely `.__boundArgs`) when the SWC plugin emitted + * `useStep(...).bind(this)` for a nested arrow step that lexically + * captured `this` (see `packages/swc-plugin-workflow/spec.md` → "Lexical + * `this` Capture in Nested Arrow Steps"). * - * The reducer serializes them as `{ stepId, closureVars?, boundThis? }`. + * The reducer serializes them as + * `{ stepId, closureVars?, boundThis?, boundArgs? }`. * The reviver reconstructs them by calling WORKFLOW_USE_STEP and, when - * `boundThis` is present, re-binding the resulting proxy so the caller's - * captured `this` survives the round trip. + * `boundThis` (or `boundArgs`) is present, re-binding the resulting + * proxy so the caller's captured `this` (and prefilled args) survive the + * round trip. */ import type { Reducers, Revivers } from '../types.js'; @@ -33,20 +35,28 @@ export function getStepFunctionReducer(): Partial { ? closureVarsFn() : undefined; - // `__boundThis` is a marker property added by the step proxy's - // overridden `.bind` (see step.ts) to record the captured lexical - // `this`. Use `in` so we round-trip even when the bound `this` is - // `undefined`/`null`. + // `__boundThis` / `__boundArgs` are marker properties added by the + // step proxy's overridden `.bind` (see step.ts) to record the + // bound receiver and any prefilled arguments. Use `in` for + // `__boundThis` so we round-trip even when the bound `this` is + // `undefined`/`null`. `__boundArgs` is only set when the user + // actually supplied prefilled args, so a missing property means + // "no prefilled args". const hasBoundThis = '__boundThis' in (value as any); const boundThis = hasBoundThis ? (value as any).__boundThis : undefined; + const boundArgs = (value as any).__boundArgs as unknown[] | undefined; const payload: { stepId: string; closureVars?: Record; boundThis?: unknown; + boundArgs?: unknown[]; } = { stepId }; if (closureVars !== undefined) payload.closureVars = closureVars; if (hasBoundThis) payload.boundThis = boundThis; + if (Array.isArray(boundArgs) && boundArgs.length > 0) { + payload.boundArgs = boundArgs; + } return payload; }, @@ -60,10 +70,11 @@ export function getStepFunctionReducer(): Partial { * * The reviver calls WORKFLOW_USE_STEP to create the step proxy, * restoring the ability to call the step from workflow code. If the - * serialized payload includes `boundThis`, the reviver also re-binds the - * freshly-created proxy so a step proxy that was constructed with - * `.bind(this)` in the workflow bundle continues to carry that `this` - * after being deserialized in another bundle (e.g. when passed as a step + * serialized payload includes `boundThis` (and optionally `boundArgs`), + * the reviver also re-binds the freshly-created proxy so a step proxy + * that was constructed with `.bind(this, …)` in the workflow bundle + * continues to carry that receiver and any prefilled arguments after + * being deserialized in another bundle (e.g. when passed as a step * argument). */ export function getStepFunctionReviver( @@ -92,7 +103,8 @@ export function getStepFunctionReviver( : useStep(stepId); if ('boundThis' in value) { - return (proxy as any).bind(value.boundThis); + const boundArgs = Array.isArray(value.boundArgs) ? value.boundArgs : []; + return (proxy as any).bind(value.boundThis, ...boundArgs); } return proxy; }, diff --git a/packages/core/src/serialization/types.ts b/packages/core/src/serialization/types.ts index b7518e723c..3ec14b07ef 100644 --- a/packages/core/src/serialization/types.ts +++ b/packages/core/src/serialization/types.ts @@ -115,6 +115,16 @@ export interface SerializableSpecial { * binding survives serialization round-trips. */ boundThis?: unknown; + /** + * Prefilled arguments captured when the user (rather than the SWC + * plugin) called `useStep(...).bind(thisArg, x, y)`. The reviver + * re-applies these alongside `boundThis` so partial application + * survives serialization. The SWC plugin only ever emits + * `.bind(this)` with no extra args today; this slot exists so a + * hand-written `.bind(thisArg, x)` doesn't silently lose `x` after + * round-tripping through the reducer/reviver. + */ + boundArgs?: unknown[]; }; TypeError: { message: string; stack?: string; cause?: unknown }; URIError: { message: string; stack?: string; cause?: unknown }; diff --git a/packages/core/src/step.ts b/packages/core/src/step.ts index b99db5b226..9a03a2c0d9 100644 --- a/packages/core/src/step.ts +++ b/packages/core/src/step.ts @@ -222,21 +222,29 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { } // Override `.bind` so the bound function preserves the step proxy - // metadata that `getStepFunctionReducer` relies on for serialization - // (`stepId`, `__closureVarsFn`, `__boundThis`). Without this override, - // the SWC plugin emitting `useStep(...).bind(this)` for nested arrow - // steps that lexically capture `this` would produce a bound function - // whose `.stepId` is `undefined`, causing the StepFunction reducer to - // treat it as a non-serializable plain function. + // metadata that `getStepFunctionReducer` relies on for serialization. + // Without this override, `Function.prototype.bind` would return a new + // function that doesn't inherit `stepId`, `__closureVarsFn`, or any + // other own properties of the original proxy — so the StepFunction + // reducer would refuse to serialize it (it'd look like a plain + // function), and a `useStep(...).bind(this)` proxy that flowed + // through workflow serialization would silently break. // - // We also stash the bound `this` value as `__boundThis` so the bound - // proxy can round-trip through serialization with the binding intact: - // when the bound proxy is passed as a step argument, the reducer - // serializes the captured `this` alongside the `stepId`, and the - // reviver in the step bundle re-binds a fresh proxy to the same - // `this`. Without this, a deserialized proxy would lose its binding - // and the step body would see `this === undefined` when the - // downstream caller invokes it without an explicit receiver. + // The override stashes three pieces of state on the bound function so + // the round trip is faithful: + // - `stepId` — already set on the original proxy. + // - `__closureVarsFn` — only when the original proxy had one. + // - `__boundThis` — the receiver passed to `.bind(thisArg, …)`. + // Always set (even when `thisArg` is + // `null`/`undefined`) so the reducer can + // distinguish "was bound" from "wasn't". + // - `__boundArgs` — only when the user supplied prefilled + // arguments (`.bind(thisArg, x, y)`). The + // SWC plugin only ever emits `.bind(this)` + // today, so this is rare in practice; we + // still capture it so the partial args + // survive serialization rather than + // silently disappearing on the step side. Object.defineProperty(stepFunction, 'bind', { value: function ( this: typeof stepFunction, @@ -268,6 +276,14 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { enumerable: false, configurable: false, }); + if (partialArgs.length > 0) { + Object.defineProperty(bound, '__boundArgs', { + value: partialArgs, + writable: false, + enumerable: false, + configurable: false, + }); + } return bound; }, writable: false,