fix(core): make deploymentId 'latest' a no-op in non-Vercel worlds#2397
Open
pranaygp wants to merge 2 commits into
Open
fix(core): make deploymentId 'latest' a no-op in non-Vercel worlds#2397pranaygp wants to merge 2 commits into
pranaygp wants to merge 2 commits into
Conversation
Previously, start({ deploymentId: 'latest' }) threw a WorkflowRuntimeError
in any World that doesn't implement resolveLatestDeploymentId() (local dev,
Postgres). That meant a workflow which opts into 'latest' on Vercel would
fail outright in local development.
Resolving 'latest' only means something in worlds with atomic, immutable
deployments. In other worlds there is nothing to resolve between, so instead
of throwing we now log a warning and fall back to the current deployment,
making 'latest' an effective no-op there.
- start.ts: warn + fall back to currentDeploymentId instead of throwing
- start.test.ts: replace the "should throw" test with a warn + fallback test
- e2e.test.ts: assert 'latest' completes (no-op) on non-Vercel worlds
- docs: note the no-op behavior in v4 + v5 start.mdx
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 17604c2 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
📊 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: Nitro | Next.js (Turbopack) | Express workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro 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: Next.js (Turbopack) | Express | Nitro Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) 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: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | 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: Next.js (Turbopack) | Nitro | Express 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: Express | Nitro | 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: Nitro | Express | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express 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❌ Some tests failed Summary
❌ Failed Tests▲ Vercel Production (1 failed)hono (1 failed):
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts start({ deploymentId: 'latest' }) behavior so it no longer throws in Worlds that don’t implement resolveLatestDeploymentId() (e.g. local dev and Postgres). Instead, it logs a warning and targets the current deployment, making 'latest' an intentional no-op outside atomic-deployment platforms like Vercel.
Changes:
- Updated
start()to resolve'latest'when supported, otherwise warn + fall back tocurrentDeploymentIdinstead of throwing. - Reworked unit coverage to assert warn + fallback behavior, and added an e2e test ensuring non-Vercel worlds can start/complete runs using
deploymentId: 'latest'. - Updated v4/v5 docs and added a changeset documenting the no-op-with-warning behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/runtime/start.ts | Warn + fall back to current deployment when 'latest' is used in Worlds without resolveLatestDeploymentId(). |
| packages/core/src/runtime/start.test.ts | Replaces the previous “throws” expectation with a warn + fallback assertion (event payload + queue dispatch). |
| packages/core/e2e/e2e.test.ts | Adds a non-Vercel-gated e2e test verifying 'latest' doesn’t break local/Postgres runs. |
| docs/content/docs/v5/api-reference/workflow-api/start.mdx | Documents 'latest' being a no-op (with warning) outside atomic-deployment worlds. |
| docs/content/docs/v4/api-reference/workflow-api/start.mdx | Same documentation update for v4 docs. |
| .changeset/deployment-id-latest-noop-non-vercel.md | Patch changeset describing the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+518
to
+522
| it('should warn and fall back to the current deployment ID when "latest" is used with a World that does not implement resolveLatestDeploymentId', async () => { | ||
| const warnSpy = vi | ||
| .spyOn(runtimeLogger, 'warn') | ||
| .mockImplementation(() => {}); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Fixed in 17604c2. The block's afterEach now calls vi.restoreAllMocks() (in addition to vi.clearAllMocks()), so the runtimeLogger.warn spy is restored even if an assertion throws before the test's own cleanup. Dropped the manual warnSpy.mockRestore(), and the guard is reset in beforeEach so the warn path is exercisable regardless of test order.
Comment on lines
+207
to
+213
| runtimeLogger.warn( | ||
| "deploymentId: 'latest' has no effect in this world and was ignored. " + | ||
| 'It is only supported by worlds with atomic deployments, such as Vercel. ' + | ||
| 'The run will target the current deployment.', | ||
| { currentDeploymentId } | ||
| ); | ||
| deploymentId = currentDeploymentId; |
Contributor
Author
There was a problem hiding this comment.
Good call — done in 17604c2. The warning is now gated behind a once-per-process guard (hasWarnedLatestNoOp), mirroring the warnOnce pattern already used in constants.ts. Users still get the nudge that 'latest' is a no-op here, but a workflow that hardcodes 'latest' for Vercel won't flood a tight local/Postgres dev loop. Exposed _resetLatestNoOpWarnForTests() (@internal) so the warn path stays testable, and added a test asserting it fires exactly once across repeated 'latest' starts.
TooTallNate
approved these changes
Jun 13, 2026
TooTallNate
left a comment
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Approve — correct behavior change, and it quietly fixes a second bug too
Turning deploymentId: 'latest' from a hard throw into a warn-and-fall-back is the right call: a workflow that opts into 'latest' to target the newest Vercel deployment shouldn't be unrunnable in local dev or Postgres, where there's no deployment set to resolve between. The fix is minimal and the reasoning in both the code comment and the JSDoc is clear about why it's a no-op rather than just that it is.
What I verified:
WorkflowRuntimeErrorimport is not orphaned — still used at 8 other sites instart.ts;runtimeLoggerandcurrentDeploymentIdare both already in scope above the check.- The fallback also fixes a downstream interaction the PR doesn't mention. Setting
deploymentId = currentDeploymentId(not leaving it as the'latest'sentinel) means the laterif (deploymentId === currentDeploymentId)gate takes the same-deployment fast path — framing + compression enabled, no health-probe round-trip. That's exactly right for local/Postgres, where'latest'semantically means "right here." Had it fallen through with the sentinel still set, it would have taken the cross-deployment branch and either probed a nonexistent target or disabled framing. Worth a sentence in the PR body, but the code does the correct thing. - The unit test is the right replacement — asserts the warning (message +
currentDeploymentIdcontext) and that the fallback lands on both payloads (run_createdeventData and the queue dispatch'sdeploymentId), not just that it didn't throw. 27/27 pass locally; typecheck clean. - The e2e test is well-scoped: reuses
addTenWorkflow(no new workbench files), correctlyskipIf(WORKFLOW_VERCEL_ENV)so it exercises the real no-op on local/Postgres while leaving Vercel's actual resolve-API path alone, and asserts both the return value and the CLI-inspected run record. This is genuinely new coverage —'latest'previously had no e2e exercise on non-Vercel worlds. - Docs (v4 + v5) accurately describe the no-op-with-warning behavior and the "still runs unchanged locally" implication. Changeset is a correctly-scoped
patchon@workflow/core.
One small note (non-blocking)
The warning fires on every start({ deploymentId: 'latest' }) call in local dev — which, for a workflow that hardcodes 'latest' for its Vercel deployment, means every single local run logs it. That's arguably the intended nudge ("this option does nothing here"), but for a tight dev loop it could get noisy. Not worth gating or deduping for this PR; flagging only in case repeated-warning noise comes up — runtimeLogger.warn once-per-process would be a trivial follow-up if it does.
CI
The three red lanes are all unrelated to this change: E2E Windows Tests aborted in dev.test.ts on a "Next.js dev server unhealthy / Turbopack" startup failure (the new test never ran — the suite bailed before it), and Benchmark Vercel (express) + the E2E Required Check cascade are this cycle's known baseline flakes. The new test is gated off Vercel anyway. Nothing here implicates the PR.
LGTM.
VaguelySerious
approved these changes
Jun 13, 2026
…anup Address PR review: - Gate the 'latest'-has-no-effect warning behind a once-per-process guard (mirrors the warnOnce pattern in constants.ts) so a workflow that hardcodes 'latest' for Vercel doesn't flood local/Postgres dev logs on every run. Exposes _resetLatestNoOpWarnForTests() (@internal) for unit tests. - start.test.ts: reset the guard in beforeEach and restore spies in afterEach via vi.restoreAllMocks() so a throwing assertion can't leak the runtimeLogger.warn spy into later tests; drop the manual mockRestore(). - Add a test asserting the warning fires exactly once across repeated 'latest' starts while every run still falls back to the current deployment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
start({ deploymentId: 'latest' })previously threw aWorkflowRuntimeErrorin any World that doesn't implementresolveLatestDeploymentId()— i.e. local dev and Postgres. That meant a workflow which opts into'latest'to target the newest deployment on Vercel would fail outright in local development, even though there's nothing for it to do there.Resolving
'latest'only means something in worlds with atomic, immutable deployments (currently Vercel). Other worlds have no notion of multiple deployments to resolve between. So instead of throwing, the SDK now logs a warning (once per process) and falls back to the current deployment, making'latest'an effective no-op outside Vercel.Changes
packages/core/src/runtime/start.ts— whendeploymentId === 'latest'and the World has noresolveLatestDeploymentId(), warn viaruntimeLogger.warnand fall back tocurrentDeploymentIdinstead of throwing. The warning is gated behind a once-per-process guard (hasWarnedLatestNoOp, mirroring thewarnOncepattern inconstants.ts) so tight local/Postgres dev loops aren't flooded. Updated theStartOptionsWithDeploymentIdJSDoc.packages/core/src/runtime/start.test.ts— replaced the "should throw" unit test with one asserting the warning is logged and the run falls back to the current deployment ID (in both therun_createdevent and the queue dispatch); added a test asserting the warning fires exactly once across repeated'latest'starts. Spies are now restored inafterEachviavi.restoreAllMocks()so a throwing assertion can't leak theruntimeLogger.warnspy into later tests.packages/core/e2e/e2e.test.ts— new test gated to non-Vercel worlds (skipIf(WORKFLOW_VERCEL_ENV)) that starts a workflow withdeploymentId: 'latest'and asserts it completes — directly exercising the no-op against the real local/Postgres worlds. (This is the e2e coverage that was previously missing for this option.)deploymentId: "latest"section of bothv4andv5start.mdxdescribing the no-op-with-warning behavior in non-Vercel worlds.Implementation note
The fallback assigns
deploymentId = currentDeploymentId(rather than leaving the'latest'sentinel in place). This is deliberate: the downstreamif (deploymentId === currentDeploymentId)gate then takes the same-deployment fast path — byte-stream framing + compression enabled, no cross-deployment health-probe round-trip — which is exactly right for local/Postgres, where'latest'semantically means "right here." (h/t @TooTallNate for calling this out.)Behavior matrix
Testing
pnpm vitest run src/runtime/start.test.ts→ 28/28 pass (incl. warn+fallback and warn-once tests)pnpm --filter @workflow/core typecheck→ cleanDocs Preview
start— UsingdeploymentId: "latest"(Behind deployment protection — requires Vercel team access.)
🤖 Generated with Claude Code