Uh oh!
There was an error while loading. Please reload this page.
feat(core): add optional namespace for queue topic prefix - #2305
Merged
Conversation
🦋 Changeset detectedLatest commit: 292ebf2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
willsatherforce-pushed
the
willsather/add-queue-prefix-namespace
branch
from
June 9, 2026 03:37
187c205 to
805bea2CompareUh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Peter Wielander <mittgfu@gmail.com> Signed-off-by: Will Sather <56037657+willsather@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Additional Suggestion:
With WORKFLOW_QUEUE_NAMESPACE set, parseQueueName/getQueueRoute/createTaskHandler in world-postgres still match only the non-namespaced default prefixes, so namespaced queue names produced by the core producer throw Invalid queue name (enqueue fails) and lose workflow-run serialization.
* Fix namespaced queue routing reliability * Inline queue namespace in generated routes * Avoid loading Vercel functions during runtime import
ijjk
approved these changes
Jun 11, 2026
TooTallNate
approved these changes
Jun 11, 2026
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Approve — solid contract design, thorough iteration; main ask is documentation
I built all five packages from this branch and ran the suites locally: 50 world tests, 6 builders constants tests, 8 world-local queue tests, 8 world-postgres queue tests, and the full core unit suite (1110 tests) — all green.
What's good
- The naming contract is well-designed. Single-label namespace (
[a-z][a-z0-9]*— no underscores) makesparseQueueNamedeterministic: I worked through the adversarial cases (__wkf_workflow_foovs a hypotheticalwkfnamespace, multi-underscore labels, ids containingwkf_workflow_) and the regex backtracking always lands on a single unambiguous parse. The prefix/kind/id helpers centralize what was previously stringly-typedstartsWithlogic scattered across three packages. - The review iteration addressed the real correctness gaps. The original producer/consumer mismatch (entrypoint consuming a namespaced topic while
start()/requeue paths enqueued un-namespaced) and the world-postgresQueuesmap throwing on namespaced prefixes were both caught and fixed; the follow-up routing-reliability commit routes every producer, consumer, and trigger site throughresolveQueueNamespace, and bakes the namespace into generated routes at build time so the flow route doesn't depend on the runtime env. runtime-import.test.tsis a good regression pin — mocking@vercel/functionsto throw on load and asserting the runtime module still imports proves the lazy-loading actually holds, rather than depending on bundler luck.- Contract-level test coverage is comprehensive: prefix construction, namespace validation (digit-start/uppercase/empty/special chars rejected), schema acceptance/rejection, kind extraction, name parsing, and the world-local namespaced direct-handler routing test.
Main ask: documentation (should-fix, can be a fast-follow)
This PR introduces a user-facing env var (WORKFLOW_QUEUE_NAMESPACE) with a non-obvious operational requirement and ships zero docs. The critical thing to document is the build-time/runtime consistency requirement: the flow-route consumer gets the namespace baked at build, but producers (start(), resumeHook(), step-handler requeues) resolve it from the runtime env. If the env var is set in one phase but not the other (build-only env vars in some CI setups, or added to a running deployment without a rebuild), messages get enqueued to a topic no trigger listens on — silently. A short docs section ("set it as a regular project env var so build and runtime agree; changing it requires a redeploy; in-flight messages on the old topic are stranded") would save someone a very confusing afternoon. The migration story for an existing deployment that adopts a namespace (in-flight un-namespaced messages) deserves a sentence too.
Smaller notes (non-blocking)
- world-postgres namespace isolation is partial.
getJobQueueNamecollapses every namespace into the same graphile job names (workflow_flows/workflow_steps). One app + one namespace works correctly (the task handler reconstructs queue names from its own configured prefix), but two namespaces sharing a database andjobPrefix— the closest analog of the stated multi-framework use case — would consume each other's jobs and reconstruct queue names under the wrong prefix. Either include the namespace in the job name (workflow_{ns}_flows) or document that postgres isolation requires distinctjobPrefix/databases. - Module-load-time env resolution in two places:
WORKFLOW_QUEUE_TRIGGER = createWorkflowQueueTrigger()andconst stepHandler = createStepHandler(). Both bakeprocess.env.WORKFLOW_QUEUE_NAMESPACEat import time. This works for the current build/runtime flows, but the trigger constant's docstring ("Default queue trigger (no namespace). Backward compatible.") is inaccurate whenever the env var is set in the importing process — worth rewording to "namespace resolved from the environment at import time." - The lazy
waitUntilhas a small unhandled-rejection window:void import('@vercel/functions').then(({ waitUntil }) => waitUntil(promise))— if the platform'swaitUntilthrows (e.g. called after response finalization on the cold path where the dynamic import does real work), that rejection is unobserved. A.catchwith a debug log would make the failure mode visible. The lazy-loading itself is clearly deliberate (and pinned by the new test), so this is just hardening. - Duplicated namespace helpers in
builders/src/constants.tsvsworld/src/queue.ts— clearly deliberate (the@workflow/worlddependency was removed from builders in the same commit), but the two regex patterns must now stay in sync by hand. A cross-referencing comment on both sides ("mirror of …, keep in sync") would cheapen future drift. createStepHandler(namespace?)'s parameter is currently never passed explicitly (only the module-levelcreateStepHandler()exists), so the V1 split-route step handler is env-only while the V2 combined route gets the baked option. Fine — V1 is legacy — but if the param is meant for future use a one-line comment saying so would keep someone from "cleaning it up."
CI
Checks were re-running on the head commit when I reviewed (post-fix-up push); nothing failing, and the changeset correctly covers all five touched packages with minor. Approving — the docs ask is the one thing I'd really like to see land before (or immediately after) this merges.
Contributor
Backport PR opened against |
This was referenced Jun 11, 2026
Merged
5 tasks
This was referenced Jun 11, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
namespaceconfiguration to queue topic prefixesQueuePrefix/ValidQueueNameZod schemas and world-local routing to accept namespaced prefixesworld-postgresjob queue prefix parsing