fix(swc-plugin): remove __builtin special-case step ID generation#1607
Merged
VaguelySerious merged 1 commit intoApr 3, 2026
Merged
Conversation
The SWC plugin had a hardcoded special case that generated bare function
names (no step// prefix, no module specifier) for any function whose name
starts with "__builtin". This was originally intended to produce stable,
version-independent IDs for the workflow VM.
With the introduction of builtinStepId() in @workflow/core, the VM now
constructs fully-qualified step IDs (step//workflow/internal/builtins@{version}//name).
The bare-name special case causes a mismatch: the VM requests a qualified ID
but only a bare name is registered, causing StepNotRegisteredError.
Remove the special case so __builtin functions follow the standard step ID
generation path, receiving proper step//moduleSpecifier//name IDs like all
other step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (63 failed)mongodb (3 failed):
redis (2 failed):
turso (58 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
karthikscale3
left a comment
karthikscale3
left a comment
There was a problem hiding this comment.
AI Review:
Assessment: Low risk, architecturally correct — no blockers
What this does
Deletes 7 lines from the SWC Rust plugin's create_id function — a special case that generated bare function names (e.g. __builtin_response_text) for any function starting with __builtin, instead of the standard step//{moduleSpecifier}//{name} format.
Why it's needed
The base branch (pgp/revert-revert-start) introduces builtinStepId() in packages/core/src/workflow/builtin-step-id.ts, which changes the VM to look up builtins using fully-qualified IDs:
// VM now requests:
step//workflow/internal/builtins@{version}//__builtin_response_textWithout this PR, the SWC plugin still registers the bare name __builtin_response_text, creating a mismatch.
No regressions expected
-
Safety net exists: The base branch already adds
getBuiltinStepAlias()which handles both directions — bare-name lookup of qualified registrations AND qualified lookup of bare-name registrations. Even without this PR the system works; this PR makes the direct match path work, eliminating the need for the fallback. -
No event replay concerns: The workflow VM uses a seeded PRNG (
vmGlobalThis.Math.random()) for deterministic ULID generation, so correlationIds match during replay. ThestepNameis only used for step function dispatch, not event matching. And dispatch is guarded by thegetBuiltinStepAliasfallback. -
No user-facing
__builtinconvention: The__builtinprefix is internal SDK convention. User code should never use it. -
Spec-aligned:
spec.mdon the base branch already documents builtins with qualified IDs (step//workflow/internal/builtins@4.0.0//start), so this PR aligns the implementation with the spec.
Minor callout
Missing changeset: The changeset-bot flagged this. Since this changes SWC plugin behavior (step ID generation for __builtin functions), it should include a patch changeset for @workflow/swc-plugin before merge.
VaguelySerious
merged commit b394412
into
pgp/revert-revert-start
78 of 85 checks passed
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
2 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
step//prefix, no module specifier) for any function whose name starts with__builtin__builtin_response_*functions get properstep//workflow/internal/builtins@{version}//{name}IDs, matching whatbuiltinStepId()constructs in the workflow VMContext
The
__builtinspecial case (attransform/src/lib.rs:1581) was added so the workflow VM could look up response builtins by bare name. Since #1491 switches the VM to fully-qualified IDs viabuiltinStepId(), the special case causes a mismatch:__builtin_response_text__builtin_response_text__builtin_response_textstep//workflow/internal/builtins@4.2.0-beta.76//__builtin_response_textstep//workflow/internal/builtins@4.2.0-beta.76//__builtin_response_textstep//workflow/internal/builtins@4.2.0-beta.76//__builtin_response_textThe parent commit (Fix 1) adds a fallback alias in
getStepFunction()that unblocks the tests even without this fix, but this fix makes the IDs match directly — which is the architecturally correct solution.Test plan
cargo testin transform crate)@workflow/coretests pass@workflow/builderstests passregisterStepFunctionoutput in example workbench build — all 12 builtins now have properstep//workflow/internal/builtins@{version}//...IDs🤖 Generated with Claude Code