Skip to content

Validate unique workflow step IDs at build time - #2018

Merged
pranaygp merged 7 commits into
mainfrom
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged

Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom
pranaygp/codex/validate-step-id-duplicates-build

Conversation

@pranaygp

@pranaygppranaygp commented May 19, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection

Root Cause

Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.

Validation

  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main

@changeset-bot

changeset-botBot commented May 19, 2026•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 01f9bac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch

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

@vercel

vercelBot commented May 19, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am

@github-actions

github-actionsBot commented May 19, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

✅ All tests passed

Summary

PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991

Details by Category

✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026

📋 View full workflow run

@pranaygp
pranaygpforce-pushed the pranaygp/codex/validate-step-id-duplicates-build branch from a862049 to f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp changed the base branch from backport/pr-2012-to-stable to mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed the pranaygp/codex/validate-step-id-duplicates-build branch from be57b18 to cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate reviewed May 19, 2026
View reviewed changes

@TooTallNateTooTallNate left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review

The core diagnosis is right and the two-part fix is well-targeted:

  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.

Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.

I think there's a real choice to make here about how the specifier should behave for non-exported package files:

  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code

Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.

What I verified

  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.

Two inline comments

  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting

The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.

Comment threadpackages/builders/src/module-specifier.ts Outdated
Comment threadpackages/builders/src/base-builder.ts Outdated
@TooTallNate

TooTallNate commented May 19, 2026

Copy link
Copy Markdown
Member

Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.

What changed in e86a96d

File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo

This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.

Dropped from the PR

  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.

Kept from the PR

  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).

Tradeoff

The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.

Validation

  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)

CopilotAI reviewed May 19, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.

Changes:

  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadpackages/world-postgres/src/queue.ts Outdated
Comment threadpackages/world-postgres/src/queue.ts Outdated
pranaygpand others added 3 commits May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate

TooTallNate commented May 20, 2026

Copy link
Copy Markdown
Member

Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.

Where this PR landed

Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.

File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in

Why option 2 was wrong

I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:

  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep

→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.

The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.

Current commit list

  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO

The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.

Tradeoff

The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.

Validation

  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)

@TooTallNate

TooTallNate commented May 20, 2026

Copy link
Copy Markdown
Member

Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.

Root cause

packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.

That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:

  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep

Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.

Fix in be40f72

Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.

Final state

The PR is now four commits. None of them ship src/dist hardcoding:

  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder

Diff vs main:

  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets

Validation

  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓

Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.

@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious approved these changes May 20, 2026
View reviewed changes

@VaguelySeriousVaguelySerious left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues

Comment thread.changeset/validate-step-id-duplicates.md Outdated
Comment threadpackages/builders/src/base-builder.ts Outdated
Comment threadpackages/builders/src/swc-esbuild-plugin.ts Outdated
vercel[bot]
vercelBot reviewed May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts
This was referenced Jun 10, 2026
Merged
Closed
Open
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Validate unique workflow step IDs at build time by pranaygp · Pull Request #2018 · vercel/workflow · GitHub
Skip to content
Validate unique workflow step IDs at build time - #2018
Merged
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Jun 10, 2026
Merged
Validate unique workflow step IDs at build time#2018
pranaygp merged 7 commits into
mainfrom 
pranaygp/codex/validate-step-id-duplicates-build
Conversation
@pranaygp
@pranaygppranaygp
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Summary
  • keep non-exported workspace package files on file-scoped workflow IDs instead of collapsing them to package@version
  • fail the build when two transformed files emit the same step ID, before runtime registration can overwrite one implementation
  • add focused builders coverage for workspace package root vs deep-file ID resolution and duplicate step ID detection
Root Cause
Workspace package files that did not match an export subpath still received a package-level moduleSpecifier like vade@0.0.0. That made same-named step functions in different package files emit the same ID, for example step//vade@0.0.0//sendMessage, and runtime registration used last-write-wins semantics.
Validation
  • fnm exec --using 24 pnpm --filter @workflow/builders exec vitest run src/module-specifier.test.ts src/swc-esbuild-plugin.test.ts
  • fnm exec --using 24 pnpm --filter @workflow/builders typecheck
  • fnm exec --using 24 pnpm --filter @workflow/builders test
  • git diff --check
  • fnm exec --using 24 pnpm changeset status --since=origin/main
@changeset-bot
changeset-botBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 01f9bac
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 16 packages
NameType
@workflow/nextPatch
@workflow/buildersPatch
workflowPatch
@workflow/astroPatch
@workflow/cliPatch
@workflow/nestPatch
@workflow/nitroPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/vitestPatch
@workflow/world-testingPatch
@workflow/nuxtPatch
@workflow/corePatch
@workflow/web-sharedPatch
@workflow/webPatch
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
@vercel
vercelBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-nextjs-workflow-webpackReadyReadyPreview, CommentMay 22, 2026 12:06am
example-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-astro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-express-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-fastify-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-hono-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nitro-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-nuxt-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-sveltekit-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-tanstack-start-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workbench-vite-workflowReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-docsReadyReadyPreview, Comment, Open in v0May 22, 2026 12:06am
workflow-swc-playgroundReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-tarballsReadyReadyPreview, CommentMay 22, 2026 12:06am
workflow-webReadyReadyPreview, CommentMay 22, 2026 12:06am
@github-actions
github-actionsBot
commented
May 19, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
🧪 E2E Test Results
✅ All tests passed
Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production122202191441
✅ 💻 Local Development161502191834
✅ 📦 Local Production161502191834
✅ 🐘 Local Postgres161502191834
✅ 🪟 Windows13100131
✅ 📋 Other7410176917
Total6939010527991
Details by Category
✅ ▲ Vercel Production
AppPassedFailedSkipped
✅ astro105026
✅ example105026
✅ express105026
✅ fastify105026
✅ hono105026
✅ nextjs-turbopack12902
✅ nextjs-webpack12902
✅ nitro105026
✅ nuxt105026
✅ sveltekit12407
✅ vite105026
✅ 💻 Local Development
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 📦 Local Production
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🐘 Local Postgres
AppPassedFailedSkipped
✅ astro-stable106025
✅ express-stable106025
✅ fastify-stable106025
✅ hono-stable106025
✅ nextjs-turbopack-canary112019
✅ nextjs-turbopack-stable-lazy-discovery-disabled13100
✅ nextjs-turbopack-stable-lazy-discovery-enabled13100
✅ nextjs-webpack-canary112019
✅ nextjs-webpack-stable-lazy-discovery-disabled13100
✅ nextjs-webpack-stable-lazy-discovery-enabled13100
✅ nitro-stable106025
✅ nuxt-stable106025
✅ sveltekit-stable12506
✅ vite-stable106025
✅ 🪟 Windows
AppPassedFailedSkipped
✅ nextjs-turbopack13100
✅ 📋 Other
AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable106025
✅ e2e-local-dev-tanstack-start-106025
✅ e2e-local-postgres-nest-stable106025
✅ e2e-local-postgres-tanstack-start-106025
✅ e2e-local-prod-nest-stable106025
✅ e2e-local-prod-tanstack-start-106025
✅ e2e-vercel-prod-tanstack-start105026
📋 View full workflow run
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
a862049 to
f7f3f45CompareMay 19, 2026 05:43
@pranaygp
pranaygp
changed the base branch from
backport/pr-2012-to-stable
to
mainMay 19, 2026 05:43
@pranaygp
pranaygpforce-pushed
the
pranaygp/codex/validate-step-id-duplicates-build
branch
from
be57b18 to
cc23370CompareMay 19, 2026 06:11
TooTallNate
TooTallNate
reviewed
May 19, 2026
View reviewed changes
@TooTallNateTooTallNate
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment-only review
The core diagnosis is right and the two-part fix is well-targeted:
  1. Build-time duplicate-ID detection in assertUniqueManifestIds — turns a silent last-write-wins runtime bug into a loud build-time WorkflowBuildError with a helpful hint. This is excellent regardless of how the specifier scheme evolves. Per-build state via build.onStart correctly handles esbuild rebuilds, the early-return for same-filePath+name avoids false positives on same-file-twice, and the error message reports both conflicting locations.
  2. base-builder.ts carve-outs for the workflow builtins (pseudo-use step runtime intrinsics) and src-backed-by-dist deduplication. Both make sense.
Where I'd push back is on the synthetic specifier scheme for private workspace files (see inline). The src/→dist/ rewrite assumes every workspace package uses dist/ as its compiled output dir, which holds for this repo's internal packages and the motivating case (@internal/agent), but not universally. Packages using lib/, build/, out/, or no separate output dir at all would get the wrong dev↔prod ID mapping under this PR. The build-time check would catch the symptom (duplicate IDs across two file paths emitting the same name), but the cause (the synthetic specifier not matching across dev/prod) is silent.
I think there's a real choice to make here about how the specifier should behave for non-exported package files:
  • Synthetic name/dist/path@version (this PR): stable dev↔prod IDs for dist/-using packages, broken for everything else
  • Filepath-based ./packages/foo/src/path (the moduleSpecifier: undefined fallback the SWC plugin already supports via naming.rs::get_module_path): never stable across dev/prod for any package, but always at least internally consistent
  • Inferred output dir (read from pkg.exports / pkg.main / pkg.module): stable for any package, more code
Worth thinking through which trade-off matches the actual use cases. Inline comment has more detail.
What I verified
  • 167 builder tests pass (+6 new) including the duplicate-ID detection for both step and workflow modes, the builtins exclusion, and the src↔dist dedup
  • Build + workspace typecheck clean
  • CI: 80 success, 0 failures, 15 in progress
  • Module-specifier semantics traced end-to-end: apply-swc-transform.ts passes moduleSpecifier to the SWC plugin, which uses it as the middle segment of the step ID via naming.rs::format_name ({prefix}//{module_path}//{identifier}). When moduleSpecifier is None, falls back to ./{filepath_without_ext} — so the moduleSpecifier: undefined fallback path IS reachable in practice when needed.
  • Per-build state isolation: stepIdsForCurrentBuild and workflowIdsForCurrentBuild are reset on build.onStart, so the duplicate-ID check correctly handles incremental builds and watch mode.
Two inline comments
  1. Hardcoded dist/ convention in the specifier scheme — the main design concern
  2. The isWorkflowInternalBuiltinsFile regex is brittle to a future workflow package rename — non-blocking but worth noting
The duplicate-ID check is a strong improvement on its own, and the specifier-scheme question is more of a design question than a correctness one — happy to approve once we've talked through the dev↔prod stability story for non-dist/ packages.
Comment threadpackages/builders/src/module-specifier.ts
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

@TooTallNate
TooTallNate
commented
May 19, 2026

Copy link


Copy Markdown

Member
Took over this PR to address the dist/ convention concern. Going with option 2 from that comment: drop the synthetic name/<path>@version specifier for non-exported package files entirely and let the SWC plugin's existing ./{filepath} fallback handle them — the same way local app files have always worked.
What changed in e86a96d
File situationSpecifier (before PR #2018)Specifier (PR as-was)Specifier (now)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist-convention-bakedundefined ← falls back to ./packages/vade/src/foo
This is consistent with how the codebase already handles duplicate workflow function names across local files — see addTenWorkflow in workbench/example/workflows/99_e2e.ts and 98_duplicate_case.ts, which coexist via distinct file-path-based IDs and have an e2e test covering both.
Dropped from the PR
  • resolvePrivatePackageSubpath / stripKnownJsTsExtension in module-specifier.ts — the synthetic specifier construction.
  • dropSourcePackageFilesBackedByDist in base-builder.ts — deduped src/dist copies that shared a synthetic specifier; with no synthetic specifier there's nothing to dedupe.
  • isWorkflowInternalBuiltinsFile in base-builder.ts — regex carve-out for workflow's internal/builtins. Those files have a real ./internal/builtins export and resolve through the normal subpath branch now, so the carve-out isn't needed.
  • packages/builders/src/base-builder.test.ts — both tests exercised the removed src/dist dedup behavior.
Kept from the PR
  • Build-time duplicate ID check (assertUniqueManifestIds + mergeWorkflowManifest in swc-esbuild-plugin.ts) and its two tests. Still useful as a safety net.
  • isRootEntrypointFile root-entry detection in module-specifier.ts and the new "uses package specifiers for workspace package root entrypoint ids" test.
  • The unrelated postgres queue advisory-lock fix (packages/world-postgres/).
Tradeoff
The synthetic-specifier approach would have kept a non-exported file's ID stable across directory renames (within the same package + version). The file-path fallback embeds the relative path, so moving packages/vade/src/foo.ts → packages/vade/src/internal/foo.ts changes the ID and breaks in-flight runs referencing the old one. But this matches local app file behavior (moving workflows/order.ts already breaks in-flight runs), so it's at least consistent, and avoids baking in the dist/ convention.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (164 tests pass, including the new duplicate-ID-check tests)
  • pnpm test ✓ (43/43 tasks)
CopilotAI
reviewed
May 19, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
This PR improves build-time correctness for workflow/step ID generation in @workflow/builders (preventing silent runtime overwrites) and hardens @workflow/world-postgres startup by serializing Graphile Worker initialization to avoid concurrent migration races.
Changes:
  • Generate file-scoped IDs for non-exported workspace package files by returning moduleSpecifier: undefined for deep/non-exported package paths.
  • Validate that workflow/step IDs are unique across the build when merging per-file manifests in the SWC esbuild plugin, failing early with a WorkflowBuildError.
  • Serialize Graphile Worker initialization in world-postgres using a Postgres advisory lock; add targeted tests for both the lock and the new ID behaviors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
FileDescription
packages/world-postgres/src/reenqueue.test.tsUpdates pool mocks to support advisory-lock startup path (pool.connect()).
packages/world-postgres/src/queue.tsAdds startup advisory lock around Graphile Worker init/migrate to reduce migration race conditions.
packages/world-postgres/src/queue.test.tsAdds advisory-lock ordering test and updates pool mocks to include a connected client.
packages/builders/src/swc-esbuild-plugin.tsAdds manifest ID uniqueness validation and switches manifest merging to a checked merge.
packages/builders/src/swc-esbuild-plugin.test.tsAdds tests asserting the build fails on duplicate step/workflow IDs across files.
packages/builders/src/module-specifier.tsChanges workspace deep/non-exported files to return moduleSpecifier: undefined to keep IDs file-scoped.
packages/builders/src/module-specifier.test.tsAdds coverage for workspace package root entrypoints vs non-exported deep files.
.changeset/validate-step-id-duplicates.mdChangeset for builders behavior change + build-time duplicate ID failure.
.changeset/fix-postgres-graphile-startup-lock.mdChangeset for world-postgres advisory-lock startup serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadpackages/world-postgres/src/queue.ts
Outdated

Comment threadpackages/world-postgres/src/queue.ts
Outdated

pranaygpand others
added 3 commits
May 19, 2026 16:32
@pranaygp@TooTallNate
Validate unique step ids at build time
da307a8
@TooTallNate
Fall back to file-path IDs for non-exported package files
ef4c433
Instead of synthesizing a 'name/dist/<path>@Version' specifier (which
hardcoded the dist/ output convention), non-exported workspace/node_modules
files now return moduleSpecifier: undefined and let the SWC plugin's
'./{filepath}' fallback produce per-file IDs. This is the same path local
app files have always taken and avoids the dist/ assumption flagged in
review. The build-time duplicate-ID check stays as the safety net.
@TooTallNate
Dedupe virtual-entry imports by canonical module identity
63d27e5
When both the source and the compiled-dist copies of the same workspace
package export end up in discoveredSteps/discoveredWorkflows (e.g. the
'workflow' package's internal/builtins in monorepo dev), they resolve to
the same module via esbuild's package resolution. The virtual entry was
emitting BOTH 'import "workflow/internal/builtins";' (the built-in
preamble) and 'import "../../packages/workflow/src/internal/builtins.ts";'
(via the isWorkspaceSourceBackedPackageFile carve-out in createImport),
which made the swc plugin transform both copies and generate duplicate
step IDs.
Track a per-bundle set of emitted module identities (package specifier
when reachable, otherwise the file path) and skip files whose identity
has already been imported. The steps bundle pre-seeds the set with the
built-in steps specifier so workspace step files at that path don't
emit a competing relative-path import.
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update: walking back the previous comment — option 2 (relative-path fallback) turned out to be worse than I claimed, not equivalent. Here's the final state.
Where this PR landed
Restored the synthetic {name}/{path}@{version} specifier for non-exported deep package files (this is the original PR's approach, kept as-is). Added a TODO about generalizing the hardcoded dist/ rewrite. So I ended up at option 3 from my original review, not option 2.
File situationSpecifier (before #2018)Specifier (final)
Matches exports subpathvade/foo@0.0.0vade/foo@0.0.0 (unchanged)
Package root entry (via exports['.'] / main / module / index)vade@0.0.0vade@0.0.0 (unchanged)
Non-exported, deep package filevade@0.0.0 ← bugvade/dist/foo@0.0.0 ← synthetic, dist/ baked in
Why option 2 was wrong
I had assumed the relative-path fallback (moduleSpecifier: undefined → SWC plugin emits ./{filepath} IDs) was internally consistent for non-exported package files. It isn't. Different bundlers — and even the same bundler in different modes — resolve the same logical file to different on-disk paths via workspace symlinks vs. pkg.exports. Concretely: Next.js dev resolves @workflow/ai/agent through the workspace symlink to packages/ai/src/agent/durable-agent.ts and transitively reaches src/agent/do-stream-step.ts, while the same import via pkg.exports resolves to dist/agent/do-stream-step.js. With path-based IDs:
  • Workflow proxy ID: step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration ID: step//./packages/ai/src/agent/do-stream-step//doStreamStep
→ "Step function not registered" at runtime. This broke agentBasicE2e on Windows + Vercel (and locally against pnpm dev, which is what reproduced it for me). The synthetic-specifier scheme avoids this because it normalizes src/ ↔ dist/ so both copies collapse to the same ID.
The build-time duplicate-ID check doesn't catch this — both bundles see the proxy/registration as the only entry for their (divergent) ID, so neither bundle's check fires.
Current commit list
  • da307a86b — original PR by @pranaygp (synthetic specifier + duplicate-ID check + new tests)
  • ef4c433ed — me: ❌ dropped synthetic specifier (regression source)
  • 63d27e59f — me: virtual-entry import dedup by canonical module identity (independently useful, kept)
  • bf74577ce — me: restored synthetic specifier with TODO
The diff vs. main is effectively the original PR's specifier scheme + duplicate-ID build-time check + virtual-entry import dedup. The dropSourcePackageFilesBackedByDist and isWorkflowInternalBuiltinsFile helpers from the original PR are still gone — their work is now done by the virtual-entry dedup (keyed on moduleIdentityKey from 63d27e5), which is more general and avoids the workflow-package-specific regex.
Tradeoff
The dist/ hardcoding is still there. Packages using lib/, build/, out/ would get incorrect ID mapping between dev and prod. None of this repo's internal packages hit that — they all use dist/ — but it's worth doing the proper inference in a follow-up. There's a TODO comment in module-specifier.ts pointing at this.
Validation
  • pnpm --filter @workflow/builders typecheck ✓
  • pnpm --filter @workflow/builders test ✓ (165 tests including a new test that source and dist copies of the same file produce the same specifier)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the failing agentBasicE2e)
@TooTallNate
TooTallNate
commented
May 20, 2026

Copy link


Copy Markdown

Member
Update — fixed at the right layer this time. You were right that this is a Next.js builder problem, not a resolveModuleSpecifier problem.
Root cause
packages/next/src/builder-deferred.ts had resolveSourceBackedPackagePath (introduced in #1338) that explicitly rewrote /dist/ paths to /src/ for workspace packages before they entered the discovery pipeline. App code imports @workflow/ai/agent; createRequire().resolve() correctly resolves it to packages/ai/dist/agent/durable-agent.js via pkg.exports; then the rewrite mangled that into packages/ai/src/agent/durable-agent.ts. From there the chain stayed in src/, all the way down to do-stream-step.ts.
That alone wouldn't break anything, but base-builder's esbuild bundle (which produces the workflow VM and step registrations) doesn't go through this rewrite — it resolves @workflow/ai/agent straight through pkg.exports to dist/. So:
  • Workflow proxy ID (computed from base-builder's view): step//./packages/ai/dist/agent/do-stream-step//doStreamStep
  • Step bundle registration (computed from Next.js builder's rewritten src view): step//./packages/ai/src/agent/do-stream-step//doStreamStep
Different IDs → "Step function not registered" at runtime. The synthetic-specifier scheme I was patching back in masked this by mapping both sides to the same ID, but it didn't fix the real problem and shipped the dist/ hardcoding you objected to.
Fix in be40f72
Dropped resolveSourceBackedPackagePath and resolveCopiedStepImportTargetPath from the Next.js deferred builder. Workspace packages are still discovered (the inclusion predicate shouldPreferSourceBackedPackagePath stays — only the path translation is gone). Their dist paths flow through unmodified, so the two bundles agree.
Final state
The PR is now four commits. None of them ship src/dist hardcoding:
  • da307a86b — original PR by @pranaygp (build-time duplicate-ID check + tests)
  • ef4c433ed — drop synthetic name/<path>@version specifier; non-exported package files use the SWC plugin's file-path fallback
  • 63d27e59f — dedupe virtual-entry imports by canonical module identity (safety net for src/dist twin-imports)
  • be40f728b — the actual fix: stop the dist→src rewrite in the Next.js builder
Diff vs main:
  • packages/builders/src/base-builder.ts (+50/-)
  • packages/builders/src/module-specifier.ts (+9/-)
  • packages/builders/src/module-specifier.test.ts (+24/-)
  • packages/builders/src/swc-esbuild-plugin.ts (+86/-)
  • packages/builders/src/swc-esbuild-plugin.test.ts (+82/-)
  • packages/next/src/builder-deferred.ts (+1/-60)
  • 2 changesets
Validation
  • pnpm --filter @workflow/builders test ✓ (164 tests)
  • pnpm test ✓ (43/43 tasks)
  • nextjs-turbopack agent e2e suite against pnpm dev ✓ (all 19 tests, including the previously failing agentBasicE2e)
  • addTenWorkflow e2e (the duplicate-workflow-name case) ✓
Apologies for the runaround on the previous iterations — your push to look at the resolution layer instead of working around it was right.
@TooTallNate
Stop rewriting workspace package /dist/ -> /src/ during Next.js disco…
4cc5002
…very
The Next.js deferred builder's `resolveSourceBackedPackagePath` rewrote
any discovered `/dist/` path to its `/src/` sibling for workspace
packages and for `workflow`/`@workflow/*` tarballs. That made the
discovered step file list point at source files while base-builder's
esbuild bundle (which builds the workflow VM and step registrations)
resolved the same package imports through `pkg.exports` to
`/dist/`. The workflow proxy ID — generated from the dist path —
didn't match the step bundle's registration ID — generated from the
src path — producing "Step function not registered" failures at
runtime, most visibly with @workflow/ai's doStreamStep on Vercel and
Windows Next.js deployments.
App code that imports a package by name should resolve naturally
through pkg.exports; the loader has no business reaching into the
package's source tree. Drop the rewrite (and the now-unused
`resolveCopiedStepImportTargetPath` helper that supported it).
Workspace packages are still discovered — that's a separate predicate
(`shouldPreferSourceBackedPackagePath`) which only gates inclusion,
not path translation.
Verified locally with the nextjs-turbopack workbench: agent e2e suite
(19 tests, including the failing `agentBasicE2e`) and the
addTenWorkflow duplicate-name suite all pass.
VaguelySerious
VaguelySerious
approved these changes
May 20, 2026
View reviewed changes
@VaguelySeriousVaguelySerious
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: no blocking issues
Comment thread.changeset/validate-step-id-duplicates.md
Outdated

Comment threadpackages/builders/src/base-builder.ts
Outdated

Comment threadpackages/builders/src/swc-esbuild-plugin.ts
Outdated

vercel[bot]
vercelBot
reviewed
May 20, 2026
View reviewed changes
Comment threadpackages/builders/src/swc-esbuild-plugin.ts

This was referenced Jun 10, 2026
 Merged

 Closed

 Open

 Open

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TooTallNateTooTallNateTooTallNate left review comments
@vercelvercel[bot]vercel[bot] left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VaguelySeriousVaguelySeriousVaguelySerious approved these changes
@ijjkijjkAwaiting requested review from ijjk

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@pranaygp@TooTallNate@VaguelySerious