fix(core): actionable errors when a custom world package can't load - #3143
Draft
VaguelySerious wants to merge 2 commits into
Draft
fix(core): actionable errors when a custom world package can't load#3143VaguelySerious wants to merge 2 commits into
VaguelySerious wants to merge 2 commits into
Conversation
This reverts the static world-target injection and restores runtime resolution of the world package from `WORKFLOW_TARGET_WORLD`. Because 141 commits have landed on top of #2752, this is a surgical revert rather than a mechanical one. Removed: the `world-target` modules in core/builders/utils, the bundler aliases and `define`s wired through next/nitro/nuxt/astro/sveltekit/rollup/nest, the `@workflow/core/runtime/world-target` export, and the pg-native/node-compat-banner helpers that only existed to support the statically bundled world. Kept (later work that builds on #2752): - `createWorldFromModule` / `WorldFactoryModule` in core (used by world-testing and the nitro dev handler) - `world-init` / `world-init-stub` / `getWorldLazy` and their registration of `getWorld` on globalThis, plus the #3001 isolated route bundle regression test - #2804 (web), #2806 (cli dynamic community backends), #3112 (world factory alias removal), #2988 (nest vercel builder), #2925/#2908 (nitro), #2799 (sveltekit typescript stub is dropped with the injection it existed for), next basePath support, and the windows / canary test timeout tuning Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the #2752 revert. `WORKFLOW_TARGET_WORLD` resolves a custom world package from the app root at runtime, which fails in two ways whose raw errors say nothing about workflows or about the fix: - the package isn't resolvable from the running output (self-contained server bundles, container images built without it) -> ERR_MODULE_NOT_FOUND - a bundler dropped the webpackIgnore/turbopackIgnore comments and replaced the dynamic import with a stub -> "expression is too dynamic", the o2flow beta.26 failure signature Both now fail with the specifier, every resolution attempt made, and the static `setWorld()` registration that fixes them, with the original error kept as `cause`. Adds a source-level guard test asserting the ignore comments stay attached to every non-literal import/require in world.ts, and documents explicit world registration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 72c6113 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
🧪 E2E Test Results✅ All tests passed E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
VaguelySerious
force-pushed
the
peter/revert-static-world-target
branch
from
July 29, 2026 06:14
003f800 to
164e7d3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — proposal, stacked on #3142. Base is
peter/revert-static-world-target; review that PR first.#2752 was shipped to fix the o2flow
workflow@5.0.0-beta.26incident:sandboxDoneHook.resume()from a plain Next.js API route failed withThe static world-target injection was one way to make that go away, but it wasn't the cause. The cause, as documented in #3001's regression test:
defineHookcame from the rootworkflowentry, which did not carry the@workflow/core/runtime/world-initside-effect import (onlyworkflow/apidid).world.ts— and itsglobalThis[GetWorldFnKey] ??= getWorldregistration — out of the isolated route bundle.getWorldLazy()fell through to a last-resortawait import(['./world', 'js'].join('.'))— an obfuscated specifier with no ignore comments, which Turbopack compiles into a throwing stub.Steps 1–3 are all fixed by machinery that survives the revert:
world-initis imported byworkflow,workflow/api, andworkflow/runtime; the obfuscated fallback is gone andgetWorldLazy()now throwsworld runtime was not initializedinstead; and #3001'sroute-bundle-isolation.test.tsbuilds a real Turbopack bundle and asserts the o2flow failure can't come back. o2flow itself runs on the Vercel world, which core imports statically, so it never touches dynamic resolution at all.So this PR does not try to re-solve o2flow. It covers what the revert genuinely gives up: apps on a custom world package, where the world is resolved at runtime and is not part of the host bundle.
What this changes
createWorld()'s custom-world path now reports failures usefully instead of leaking whatever the loader threw:.output/server, an esbuild bundle, a container image built without the world package) — previously a bareERR_MODULE_NOT_FOUNDfor a path the user never wrote.Both now name the specifier, list every resolution attempt, and give the fix — a static import plus
setWorld()— with the original error preserved ascause. TheInvalid target world moduleerror also keeps its cause now, so "resolved but exported the wrong shape" stops looking like "couldn't resolve".Also added:
world-bundler-safety.test.ts— source-level guard that every non-literalimport()/require()inworld.tskeeps bothwebpackIgnoreandturbopackIgnore. Those comments are the only thing preventing a bundler stub, they are invisible to typecheck, and no test that runs under Node's own loader can catch their loss. Also assertsremoveCommentsstays off so they survive intodist/.world-resolution.test.ts— loads a world by absolute path, and asserts the two failure messages above (the stub case is simulated with a module that throws Turbopack's error text).setWorld(), and when you need to.Alternatives considered
world-targetmodule plus alias wiring in seven framework integrations, and it made the world package a static dependency of every host bundle (which is what dragged the TypeScript compiler andpg-nativeinto the SvelteKit server output and needed stub aliases of its own).import()per known world in coreworld-package/registerside-effect entrysetWorld(), but it's additive sugar over the same mechanism and needs a change in every world package. Worth doing separately if we like the direction.setWorld()for all custom worlds, drop dynamic resolutionimport()from core, but it breaks every app currently settingWORKFLOW_TARGET_WORLDto a package. Only worth it as a v5 breaking change, and this PR's error message is the migration path for it.Happy to take this in any of those directions — the point of the draft is the diagnosis above, not the specific patch.
Docs Preview
Verification
pnpm typecheckgreen;@workflow/coreunit tests green (1580 passed, 3 expected-fail)world.ts, 3 of the 4world-resolution.test.tscases fail (the absolute-path load passes on both).world-bundler-safety.test.tspasses on both by design — it's a guard against future regressions, not a repro.