[cli] [world-local] Ensure update checks don't suggest upgrading from stable release to pre-releases#1490
Conversation
…able releases Signed-off-by: Peter Wielander <mittgfu@gmail.com>
🦋 Changeset detectedLatest commit: c33475c 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 |
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express 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: Nitro | Express | 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 | Express | 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: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express 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: Nitro | Next.js (Turbopack) | Express workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | 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: Express | Nitro | 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: Next.js (Turbopack) | Express | Nitro Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | 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: Express | Nitro | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests▲ Vercel Production (1 failed)astro (1 failed):
🌍 Community Worlds (56 failed)mongodb (3 failed):
redis (2 failed):
turso (51 failed):
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
karthikscale3
left a comment
There was a problem hiding this comment.
Looks good to me
Here's my review of PR #1490:
Note: This PR is still in Draft status.
Summary
A small, focused bug fix PR by @VaguelySerious that prevents two update-check mechanisms from incorrectly suggesting users upgrade to pre-release versions when they're on stable releases. It touches two packages:
@workflow/cli(update-check.ts) - The CLI's "new version available" prompt@workflow/world-local(init.ts) - The data directory version migration logic
No Blockers
The code changes are correct and well-scoped. No regressions.
CI Status
CI is still running (most E2E/benchmark jobs are pending). What has completed so far:
Passing:
- Build Packages passes
- Unit Tests pass (ubuntu + windows)
- Vitest Plugin Tests pass
- All Vercel deployments succeed (all 13 projects)
- Docs code samples and links pass
- Community World (Turso) E2E passes
- Socket Security passes
Pending:
- All E2E suites (Vercel Prod, Local Dev/Prod/Postgres, Windows, MongoDB/Redis community worlds)
- All benchmarks
No failures so far. Given the narrow scope of the changes (version-checking logic only, no runtime behavior changes), I'd expect all E2E/benchmarks to pass cleanly.
Code Review
1. CLI update check (packages/cli/src/lib/update-check.ts) - Correct
Two call sites are patched, both with the same pattern:
const isPrerelease = latestVersion.includes('-');
const needsUpdate = !isPrerelease && compareVersions(latestVersion, currentVersion);This gates needsUpdate on the latest version not being a pre-release. The check is applied in both:
checkForUpdate()- the live npm registry checkcheckForUpdateCached()- the cached version check path
Simple and effective. The includes('-') check works because semver pre-release tags always contain a hyphen (e.g., 5.0.0-beta.1).
2. world-local data dir init (packages/world-local/src/init.ts) - Correct
The guard prevents a pre-release version from triggering a data directory upgrade/migration when the stored version has a different base version:
if (
currentVersion.prerelease &&
(currentVersion.major !== oldVersion.major ||
currentVersion.minor !== oldVersion.minor ||
currentVersion.patch !== oldVersion.patch)
) {
return;
}This means: if you're running 5.1.0-beta.3 and the data dir was created by 4.2.0, don't attempt the migration. But if you're running 5.1.0-beta.3 and the data dir was from 5.1.0-beta.1 (same base), the upgrade proceeds normally. This is the right behavior - pre-releases shouldn't perform cross-version migrations.
3. Tests (packages/world-local/src/init.test.ts) - Thorough
- Existing tests for "older version" and "newer version" upgrade scenarios are updated to be prerelease-aware, dynamically choosing stored versions that will trigger/bypass the guard appropriately
- A new test
'should skip upgrade when current version is prerelease with different base'specifically validates the guard, and correctly usesif (!current.prerelease) return;to skip when the current package isn't a prerelease (since the test is only meaningful in that context)
4. Changeset - Present and correct
Includes @workflow/world-local and @workflow/cli as patch changes.
Minor Observations
- The PR title says "don't suggest upgrading to pre-releases from stable releases" but the CLI fix actually prevents suggesting any upgrade when the latest is a pre-release (regardless of whether the user is on stable or pre-release). This seems intentional - if you're on a pre-release, you already know what you're doing, and if you're on stable, you definitely shouldn't be prompted to go to a pre-release.
- The
world-localfix is the inverse direction: it prevents a pre-release binary from upgrading data dirs created by a different-base stable version. So the two fixes complement each other nicely.
Verdict
No blockers or regressions. Clean, well-tested bug fix. Still in draft and CI is still running, but the changes are narrow and low-risk. Once CI completes and the PR is marked ready for review, it should be straightforward to merge.
|
@karthikscale3 I made some changes, please re-review |
karthikscale3
left a comment
There was a problem hiding this comment.
A few issues to address:
- Changeset description is inverted — says "from pre-release to stable" but the code blocks "from stable to pre-release"
@workflow/world-localin changeset has no corresponding code changes — will cause a spurious patch bump- No test coverage for
compareVersionsor the new pre-release guard
.changeset/great-kings-return.md
Outdated
| "@workflow/cli": patch | ||
| --- | ||
|
|
||
| Ensure update checks don't suggest upgrading from pre-release to stable releases |
There was a problem hiding this comment.
Two issues here:
-
Description is inverted: The code blocks updates when
current=stableANDlatest=prerelease(i.e. stable → prerelease). But this says "from pre-release to stable releases" which is the opposite direction. Should read something like: "Ensure update checks don't suggest upgrading to pre-release from stable releases" -
@workflow/world-localhas no changes in this PR — there are noworld-localfiles touched. This will cause a spurious patch version bump. Was there aninit.tschange that was intended but dropped?
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
|
@pranaygp @karthikscale3 Please re-review. I updated the changset and PR title. Code is the same |
pranaygp
left a comment
There was a problem hiding this comment.
Looks good — all prior feedback has been addressed. The guard is now precise (currentIsStable && latestIsPrerelease), the changeset description is correct, world-local was properly reverted, and the scope is clean.
Two minor nits remaining (non-blocking):
-
Line 140 comment —
// Always use 'latest' tag - even beta versions are published as latestis now misleading since it describes the root cause of the bug this PR fixes without noting the mitigation. @VaguelySerious posted a suggestion to delete it but it wasn't applied. Either remove it or update it to something like:// Always use 'latest' tag — beta versions may be published under this tag, so the caller filters out prerelease versions before prompting. -
Duplicated logic — the
currentIsStable && latestIsPrereleaseguard appears in bothcheckForUpdate()andcheckForUpdateCached(). A smallshouldSuggestUpdate()helper would keep them in sync. Not a blocker for this PR.
packages/cli/src/lib/update-check.ts
Outdated
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
…naygp-db9e68c1 * 'main' of https://github.com/vercel/workflow: (32 commits) chore: bump @nestjs/* to ^11.1.17 (#1497) chore: bump hono to ^4.12.8 (#1495) Revert "Inline class serialization registration to fix 3rd-party package supp…" (#1493) [world] Add stream pagination and metadata endpoints (#1470) [cli] [world-local] Ensure update checks don't suggest upgrading from stable release to pre-releases (#1490) Remove NestJS Vercel integration while in experimental phase (#1485) feat: export semantic error types and add API reference docs (#1447) feat: enforce max queue deliveries in handlers with graceful failure (#1344) [world-postgres] Migrate client from `postgres.js` to `pg` (#1484) Inline class serialization registration to fix 3rd-party package support (#1480) [ai] Add experimental_context to DurableAgentOptions (#1489) [ai] Expose configured tools on DurableAgent instances (#1488) fix(builders): catch node builtin usage when entry fields diverge (#1455) [web-shared] Fix timeline duration format and precision (#1482) [cli] Add bulk cancel, --status filter, fix step JSON hydration (#1467) [utils] Re-export parseName utilities and add workflow/observability module (#1453) [o11y] Polish display when run data has expired (#1438) Add CommonJS `require()` support for class serialization detection in SWC plugin (#1144) fix(next): stabilize deferred canary e2e in nextjs workbenches (#1468) [web] Support legacy newline-delimited stream format in `useStreamReader` (#1473) ...
…naygp-6fadd605 * 'main' of https://github.com/vercel/workflow: (73 commits) chore: bump next to 16.2.1 and fix deferred build (#1496) chore: bump nitropack to ^2.13.1 (#1501) chore: bump nuxt ecosystem dependencies (#1500) chore: bump sveltekit ecosystem (#1498) chore: bump express and fastify in workbenches (#1499) chore: bump @nestjs/* to ^11.1.17 (#1497) chore: bump hono to ^4.12.8 (#1495) Revert "Inline class serialization registration to fix 3rd-party package supp…" (#1493) [world] Add stream pagination and metadata endpoints (#1470) [cli] [world-local] Ensure update checks don't suggest upgrading from stable release to pre-releases (#1490) Remove NestJS Vercel integration while in experimental phase (#1485) feat: export semantic error types and add API reference docs (#1447) feat: enforce max queue deliveries in handlers with graceful failure (#1344) [world-postgres] Migrate client from `postgres.js` to `pg` (#1484) Inline class serialization registration to fix 3rd-party package support (#1480) [ai] Add experimental_context to DurableAgentOptions (#1489) [ai] Expose configured tools on DurableAgent instances (#1488) fix(builders): catch node builtin usage when entry fields diverge (#1455) [web-shared] Fix timeline duration format and precision (#1482) [cli] Add bulk cancel, --status filter, fix step JSON hydration (#1467) ... # Conflicts: # packages/core/src/runtime/start.ts
No description provided.