[core] Allow settings attributes from inside step functions#2157
Conversation
🦋 Changeset detectedLatest commit: 79bbfce The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
❌ Some benchmark jobs failed:
Check the workflow run for details. |
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
| ## Usage | ||
|
|
||
| Call [`experimental_setAttributes`](/docs/api-reference/workflow/experimental-set-attributes) from a `"use workflow"` function. Calling it from a `"use step"` function or plain application code is not supported. | ||
| Call [`experimental_setAttributes`](/docs/api-reference/workflow/experimental-set-attributes) from a `"use workflow"` function or a `"use step"` function. Plain application code is not supported because there is no active workflow run to attach attributes to. |
There was a problem hiding this comment.
| Call [`experimental_setAttributes`](/docs/api-reference/workflow/experimental-set-attributes) from a `"use workflow"` function or a `"use step"` function. Plain application code is not supported because there is no active workflow run to attach attributes to. | |
| Call [`experimental_setAttributes`](/docs/api-reference/workflow/experimental-set-attributes) from a `"use workflow"` function or a `"use step"` function. |
| - Setting attributes is currently slower than the final API will be, because each write goes through an internal workflow step. Prefer batching related attributes in one call. | ||
| - Workflow-body storage errors are logged after retries, but do not fail the workflow run. | ||
| - Step-body storage errors throw from `experimental_setAttributes` like any other step-side network write. Catch the error inside the step if the attribute is best-effort. | ||
| - Setting attributes from a workflow body is currently slower than the final API will be, because each write goes through an internal workflow step. Step-body calls post directly to the World. Prefer batching related attributes in one call. |
There was a problem hiding this comment.
| - Setting attributes from a workflow body is currently slower than the final API will be, because each write goes through an internal workflow step. Step-body calls post directly to the World. Prefer batching related attributes in one call. | |
| - Setting attributes from within a workflow function (as opposed to a step function) is currently slower than the final API will be, because each write goes through an internal workflow step. Step-body calls post directly to the World. Prefer batching related attributes in one call. |
TooTallNate
left a comment
There was a problem hiding this comment.
Approve — clean follow-up to the attributes MVP, picks up the scope cut from #2088
This is the step-body support that #2088 deliberately left out, plus a nice refactor that pulls validation into a shared helper. The implementation is straightforward:
- Workflow-body path unchanged: still dispatches through
__builtin_set_attributesstep bridge for event-log materialization. - Step-body path is new: the host-side
set-attributes.ts(which used to throwFatalError) now readsworkflowRunIdfromcontextStorage, validates, and posts directly toworld.runs.experimentalSetAttributes(). No nested step needed — step bodies already run in host context. - Plain host code path: still throws
FatalErrorbecause there's no active workflow run.
The attribute-changes.ts shared helper extraction is exactly the right move — the workflow-side and host-side both need the same input normalization + validation + AttributeValidationError → FatalError conversion, and DRY'ing them up prevents future drift.
What I verified locally
pnpm install --frozen-lockfile✓pnpm turbo run build --filter @workflow/core✓pnpm --filter @workflow/core test✓ (1051/1051)- Specifically ran
set-attributes.test.ts— 5/5 pass, covering: plain host code throws FatalError, step context posts directly,allowReservedAttributesforwarded, validation rejects before world call, warn-once for unsupported worlds. - Confirmed
WorldCacheKeyis the same symbol (@workflow/world//cache) used by bothgetWorldLazy()and the existing__builtin_set_attributesbuiltin — no symbol-mismatch concern. - Rebased onto current
maincleanly. The "deletions" of trace-viewer files in the GitHub diff are stale-branch artifacts from being forked before #2144 landed; squash merge produces a clean commit.
E2E test design is sharp
The test specifically asserts that NO __builtin_set_attributes step events are created when calling from a step body:
expect(
events.some(
(e) => (e.eventType === 'step_created' || e.eventType === 'step_completed') &&
stepName.includes('__builtin_set_attributes')
)
).toBe(false);That's the right invariant — it documents that the step-body path is genuinely a direct write, not a nested step dispatch.
CI
109 success, 1 failure (Benchmark Vercel (nitro-v3) — recurring infra flake, fails regularly on main too).
One inline comment
The PR body acknowledges a forward-compat asymmetry that the docs don't yet capture — see inline. Step-body writes will be inherently non-deterministic from the workflow's perspective when V1's getAttribute() lands, while workflow-body writes will be cleanly event-sourceable. Worth documenting up-front so users don't write step-body setAttributes calls expecting deterministic reads later.
Non-blocker — the implementation is correct and the asymmetry isn't introduced by this PR (it's inherent to where step bodies execute). Just want the docs to match reality before users adopt the pattern.
Notable trade-off worth flagging (also non-blocker)
FatalError on validation still applies to both workflow-body and step-body calls (because both go through the shared normalizeAttributeChanges helper). Karthik flagged on PR #2088 that this is potentially too heavy-handed for telemetry — "I moved a tag call into a step → my prod run dies". The host-side wrong-call-site path now has a defensible answer (no run exists, so the call is meaningless), but the validation-error case still kills the run for what is fundamentally observability metadata. Not made worse by this PR. Worth a follow-up discussion eventually.
Approving.
| - World implementations emit a side-channel observability record per successful write (in `world-vercel`, this hooks into the same observability/analytics pipeline already used for other run lifecycle events) | ||
|
|
||
| Calling `experimental_setAttributes` from a step body or plain host code is intentionally not supported in the MVP — the host-side export throws `FatalError` directing callers back to a workflow body. This keeps the implementation a single dispatch path; step-body support can be added later without breaking the workflow-body contract. | ||
| Calling `experimental_setAttributes` from a step body was intentionally not supported in the MVP, but step-body calls are now supported as a follow-up. Plain host code remains unsupported because there is no active workflow run to attach attributes to. |
There was a problem hiding this comment.
Forward-compat caveat in the PR body should make it into the docs.
The PR description says:
This is forward-compatible with event-based attributes if we accept attributes set from a step-level potentially racing with other calls, and accept not supporting deterministic getAttribute calls from within a workflow context. This will need to be documented when we move to the new implementation.
That second part is the bigger semantic asymmetry — and it's the thing a user is most likely to be surprised by when V1 lands with getAttribute():
- Workflow-body writes go through
__builtin_set_attributes(step boundary), and when V1 lands they convert cleanly toattr_setevents that the VM replays deterministically.getAttribute()from inside the workflow body will see them. - Step-body writes post directly to the world from host context, outside the VM. When V1 lands, these can't be event-sourced (there's no spot in the workflow's event log where they were issued — they're step-side side effects). So
getAttribute()from inside the workflow body won't see them deterministically; they're a race with whatever else writes to the same key.
For the MVP this asymmetry is invisible because there's no getAttribute() yet. But the contract is being set now and will be load-bearing when V1 ships. Worth documenting up-front so users don't write step-body setAttributes calls and then expect deterministic reads from the workflow body later.
Suggested addition under the existing "When the full feature ships" paragraph or a new note in attributes-mvp.mdx:
Step-body vs workflow-body forward-compat asymmetry. Workflow-body writes go through a step boundary and will be event-sourced as
attr_setevents when V1 lands, so futuregetAttribute()calls from the same workflow body will see them deterministically. Step-body writes post directly to the World outside the VM and are inherently non-deterministic from the workflow's perspective — they won't appear in event replay. Use step-body writes for observability metadata that you don't intend to read back from inside the run; use workflow-body writes for anything you might want to read deterministically later.
Non-blocker (the implementation is correct and the asymmetry is real, not invented by this PR), but worth landing the docs change before users start adopting step-body writes for the wrong reasons.
There was a problem hiding this comment.
We likely won't ship getAttribute, so this is nil
|
No backport to This commit extends the To override, re-run the Backport to stable workflow manually via |
Expands on #2134 to allow running
experimental_setAttributes()from inside a"use step"function.Shares SDK-side attribute normalization/validation between workflow and step entrypoints. Add unit and e2e coverage for step-side writes, and updates docs.
This is forward-compatible with event-based attributes if we accept attributes set from a step-level potentially racing with other calls, and accept not supporting deterministic getAttribute calls from within a workflow context. This will need to be documented when we move to the new implementation.