Uh oh!
There was an error while loading. Please reload this page.
fix(cli): compare versions by semver in the update check - #2394
fix(cli): compare versions by semver in the update check#2394aryanku-dev wants to merge 1 commit into
Conversation
The update check derived "how far behind" from `versions.indexOf(pkg.version)` against a list with prereleases filtered out. Anything absent from that list returned -1, which failed the `age > 0 && age < 10` guard and fell through to "more than 10 releases behind" — so three unrelated situations all produced the same misleading warning: - any prerelease build, since betas are filtered out of the list and are therefore never found (`1.32.6-beta.3` -> "more than 10 releases behind") - any version newer than the latest release, which told users to downgrade - versions only a few releases old, because the API page holds 30 releases of which just 8 are stable, so the "10" was unrelated to the real distance Two latent bugs went with it: `tag.substr(1)` blindly stripped the first character, mangling the release tags that are published without a `v` prefix, and "latest" was whichever release GitHub listed first rather than the highest version. Replace the index arithmetic with semver parsing and comparison, take the latest release as the semver maximum, and request a full page of releases so the window of stable releases (8 -> 22) is wide enough for the count to be real. Messages now describe the actual situation: prereleases are told the latest stable version, versions ahead of the latest warn nothing, and a count of releases behind is only printed when it is exact rather than a lower bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
630d3e7 to
a8d13c5Compare
aryanku-dev
left a comment
There was a problem hiding this comment.
Claude Code Review (automated) — 2 inline finding(s). Full report in the PR comment below. Verdict: Passed.
| // a `v`, so the prefix is optional. Returns null for anything unparseable so callers can bail out | ||
| // rather than warn about a comparison that cannot be trusted. | ||
| function parseVersion(version) { | ||
| let match = /^v?(\d+)\.(\d+)\.(\d+)(?:-([\w.-]+))?$/.exec(String(version).trim()); |
There was a problem hiding this comment.
[Low] parseVersion drops tags carrying semver build metadata
The pattern has no branch for a +build suffix, so such a tag is silently excluded from the comparison set. More importantly, if the installed version ever carried build metadata, the check bails out at Unable to parse the current version and the user is never told an update exists. No current percy/cli tag uses +, so this is latent rather than active.
Suggestion: tolerate and discard it, or note the limitation in a comment.
| letmatch=/^v?(\d+)\.(\d+)\.(\d+)(?:-([\w.-]+))?$/.exec(String(version).trim()); | |
| letmatch=/^v?(\d+)\.(\d+)\.(\d+)(?:-([\w.-]+))?(?:\+[\w.-]+)?$/.exec(String(version).trim()); |
Reviewer: stack-code-reviewer
| // only compare against stable releases - alpha/beta versions are excluded both by the release | ||
| // flag and by their own version, since the flag is set by hand and is sometimes wrong | ||
| let versions = releases.reduce((acc, r) => { | ||
| let parsed = !r.prerelease && parseVersion(r.tag); |
There was a problem hiding this comment.
[Low] Mixed boolean/null short-circuit reads awkwardly
parsed ends up as false, null, or an object, conflating a short-circuited boolean with a parse result. Functionally correct — the prerelease-flag/tag mismatch spec covers it — but an early return separates the two concerns:
| letparsed=!r.prerelease&&parseVersion(r.tag); | |
| if(r.prerelease)returnacc; | |
| letparsed=parseVersion(r.tag); |
Reviewer: stack-code-reviewer
aryanku-dev
commented
Aug 22, 2026
Claude Code PR ReviewPR:#2394 • Head:a8d13c5 • Reviewers: stack-code-reviewer SummaryReplaces the CLI update check's list-index distance calculation ( Review Table
Findings
Notes (not defects)
Verdict: PASS — no correctness defects found; three Low/nit polish items, none blocking. |
Problem
The update check tells you the wrong thing in several common situations. Most visibly, any beta build reports being more than ten releases behind:
All of it traces back to one line, which uses a list index as a distance metric:
indexOfreturns-1for anything not in the stable list, and-1fails theage > 0 && age < 10guard, so it falls through to the "more than 10 releases behind" branch. Three unrelated situations land there:1.32.6-beta.31.32.8(ahead of latest)1.32.8 -> 1.32.71.32.0(8 releases back)Two latent bugs came along with it:
tag.substr(1)strips the first character unconditionally. Percy publishes tags both with and without avprefix, so1.32.5-beta.1becomes.32.5-beta.1.versions[0]— whichever release GitHub listed first by publish date, not the highest version. A backported patch published after a newer release would be reported as latest.Fix
parseVersion/compareVersions) instead of doing index arithmetic. Thevprefix is optional, unparseable input returnsnulland skips the check rather than warning on a comparison that cannot be trusted.latestas the semver maximum rather than trusting publish order.per_page=100, which widens the window of stable releases from 8 to 22 so a real count is available for most versions in the wild.1.32.6-beta.3You are using a pre-release build of @percy/cli. 1.32.6-beta.3 -> 1.32.7 (latest stable)1.32.61.32.0(8 back)A new version of @percy/cli is available! 1.32.0 -> 1.32.7Heads up! Your @percy/cli is 12 releases behind the latest release.+ releases link1.30.2(outside window)Heads up! Your @percy/cli is significantly out of date. 1.30.2 -> 1.32.7+ releases link1.32.8(ahead)One deliberate call worth a reviewer's attention: when the installed version predates every release fetched, the message says "significantly out of date" with no number, because any count there is only a lower bound — stating a floor as though it were exact is what made the original warning misleading. The two version numbers carry the real information. Counts are printed only when exact.
The cache format is unchanged, so existing
.releasesfiles keep working.Testing
packages/cli— 38 specs pass at 100% statement/branch/function/line coverage (the package gate requires 100%).New specs cover: prerelease in use, prerelease ahead of latest stable, version ahead of latest, exact count when far behind, escalation one major behind, no count when outside the fetched window, semver ordering vs publish ordering, tags without a
vprefix, tags whose prerelease flag disagrees with the tag, unparseable current version, and unparseable release tags.The logic was also replayed verbatim against the live
percy/clireleases API across 12 version scenarios.