Add native v4 workflow attribute events#2226
Draft
pranaygp wants to merge 3 commits into
Draft
Conversation
🦋 Changeset detectedLatest commit: 17ccd34 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro 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. |
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests▲ Vercel Production (68 failed)astro (6 failed):
example (6 failed):
express (6 failed):
fastify (7 failed):
hono (6 failed):
nextjs-turbopack (6 failed):
nextjs-webpack (7 failed):
nitro (6 failed):
nuxt (6 failed):
sveltekit (6 failed):
vite (6 failed):
📋 Other (6 failed)e2e-vercel-prod-tanstack-start (6 failed):
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
Contributor
karthikscale3
left a comment
karthikscale3
left a comment
There was a problem hiding this comment.
Reviewed the SDK + world changes. Nicely tested across all three worlds, and the step-vs-workflow dedup is consistent end-to-end (NULL correlationId keeps step writes repeatable; workflow writes are correlated one-shots, matching the DynamoDB guard). I also confirmed the attribute events are awaited via await Promise.all(ops) before the suspension returns, so the immediate re-invoke on hasAttributeEvents isn't racy. A few inline notes — none blocking.
| ? { allowReservedAttributes: true } | ||
| : {} | ||
| ); | ||
| await world.events.create(runId, { |
Contributor
There was a problem hiding this comment.
start() got an explicit specVersion < SPEC_VERSION_SUPPORTS_ATTRIBUTES throw with a clear message, but the step-body (and workflow-body) experimental_setAttributes paths call world.events.create({ eventType: 'attr_set' }) with no spec-version check. On a pre-v4 world this surfaces as a raw world/server rejection rather than the friendly "requires spec version 4" error. Looks intentional given the docs now state attributes require v4 and errors should surface rather than no-op — just flagging the asymmetry in error quality vs. start().
Contributor
Author
There was a problem hiding this comment.
Yes, this is intentional for the v4-only feature contract. start() rejects before creating a run because it can determine capability up front; workflow/step body writes emit the native v4 attr_set event and let an unsupported World reject it. We explicitly do not want a v3 fallback for this rollout.
|
|
||
| // Native workflow attribute events are resolved through | ||
| // replay; re-invoke now that their events are durable. | ||
| if (suspensionResult.hasAttributeEvents) { |
Contributor
There was a problem hiding this comment.
This mirrors the hasHookConflict early-return, but note it preempts the pendingSteps handling below: a suspension batch containing both an attribute write and pending steps will re-invoke immediately (timeoutSeconds: 0) and defer step handling to the next replay. Hook conflicts are rare so that precedent is cheap; attribute writes commonly co-occur with steps, so this adds an extra re-invoke cycle to those workflows. Is deferring steps here intended, or should attribute-driven re-invoke fold into the existing step/timeout path instead of short-circuiting ahead of it?
Contributor
Author
There was a problem hiding this comment.
Deferring is intentional: with Promise.race([experimental_setAttributes(...), slowStep()]), the persisted attribute event can win on replay without executing the losing step. Processing pendingSteps first would introduce a side effect that the deterministic replay says lost. I documented that decision in runtime.ts and added replays attribute events before executing a step that loses the same race in runtime.test.ts in 17ccd34.
| }), | ||
| z.object({ | ||
| type: z.literal('step'), | ||
| stepId: z.string(), |
Contributor
There was a problem hiding this comment.
Nit: the client writer schema uses stepId: z.string() / attempt: z.number(), while the server validates StepId.schema + attempt: z.number().int().min(0). The server is authoritative so this is safe, just an asymmetry — the client would accept a non-int/negative attempt that the server then rejects.
Contributor
Author
There was a problem hiding this comment.
Agreed that the server is stricter here. @workflow/world already models the existing public step event/payload surfaces with generic string IDs and number attempts (StepStartedEventSchema and queue payloads), while the server remains authoritative for platform ID and attempt constraints. I am keeping attr_set consistent with that client-side contract rather than tightening only this event.
| runInputData.workflowName && | ||
| runInputData.input !== undefined | ||
| ) { | ||
| validateAttributeChanges( |
Member
There was a problem hiding this comment.
Might allow SQL injection since we don't validate since validateAttributeChanges doesn't do special character checks , though maybe sql has some built in defenses there, probably good to sanitize more strictly
| '@workflow/world-postgres': minor | ||
| --- | ||
|
|
||
| Add native v4 attribute events and seed run attributes through `start()`. |
Member
There was a problem hiding this comment.
Suggested change
| Add native v4 attribute events and seed run attributes through `start()`. | |
| Add native event-based attribute setting and allow passing initial run attributes through `start()`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
attr_setevents in workflow and step contextsstart(..., { attributes })so initial run attributes are seeded during creationRollout
Testing
pnpm --filter @workflow/core testpnpm --filter @workflow/world-local testcd packages/world-postgres && pnpm vitest run test/storage.test.tspnpm exec turbo run build --filter=nextjs-turbopack... --output-logs=errors-onlyDEPLOYMENT_URL=http://localhost:3000 APP_NAME=nextjs-turbopack pnpm vitest run packages/core/e2e/e2e.test.ts -t experimental_setAttributes