From a8d13c5d142077f18ac2ad112edc5c91ac82a384 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sat, 22 Aug 2026 21:58:01 +0530 Subject: [PATCH] fix(cli): compare versions by semver in the update check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/cli/src/update.js | 104 ++++++++++++++++++--- packages/cli/test/update.test.js | 151 +++++++++++++++++++++++++++++-- 2 files changed, 236 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/update.js b/packages/cli/src/update.js index 22f406592..74b20d5d0 100644 --- a/packages/cli/src/update.js +++ b/packages/cli/src/update.js @@ -9,6 +9,10 @@ import { getPackageJSON } from '@percy/cli-command/utils'; const CACHE_FILE = path.resolve(url.fileURLToPath(import.meta.url), '../../.releases'); // max age the cache should be used for (3 days) const CACHE_MAX_AGE = 3 * 24 * 60 * 60 * 1000; +// how many stable releases behind before the warning escalates +const MANY_RELEASES_BEHIND = 10; +// where users are pointed to see what changed +const RELEASES_URL = 'https://github.com/percy/cli/releases'; // Safely read from CACHE_FILE and return an object containing `data` mirroring what was previously // written using `writeToCache(data)`. An empty object is returned when older than CACHE_MAX_AGE, @@ -44,12 +48,44 @@ function writeToCache(data) { } } +// Parse a version string into its comparable parts. Release tags are inconsistently prefixed with +// 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()); + if (!match) return null; + + return { + major: parseInt(match[1], 10), + minor: parseInt(match[2], 10), + patch: parseInt(match[3], 10), + prerelease: match[4] || null, + // a prerelease precedes the stable release of the same version, so it sorts lower + stable: match[4] ? 0 : 1, + // the version without any `v` prefix, for display + version: match[0].replace(/^v/, '') + }; +} + +// Compare two parsed versions, returning a negative number when `a` precedes `b`, a positive +// number when it follows, and zero when they are equal. Prerelease identifiers are not compared +// against each other, since only stable releases are ever compared - the sole prerelease involved +// is the version currently installed, and a prerelease always precedes its own stable release. +function compareVersions(a, b) { + for (let part of ['major', 'minor', 'patch', 'stable']) { + if (a[part] !== b[part]) return a[part] - b[part]; + } + + return 0; +} + // Fetch and return release information for @percy/cli. async function fetchReleases(pkg) { let { request } = await import('@percy/client/utils'); - // fetch releases from the github api without retries - let api = 'https://api.github.com/repos/percy/cli/releases'; + // fetch releases from the github api without retries. a full page is requested since the majority + // of releases are prereleases, and those are filtered out before comparing versions + let api = 'https://api.github.com/repos/percy/cli/releases?per_page=100'; let data = await request(api, { headers: { 'User-Agent': pkg.name }, retries: 0 @@ -78,19 +114,63 @@ export async function checkForUpdate() { // request new release information if needed if (!releases) { releases = await fetchReleases(pkg); - if (!cacheError) writeToCache(releases, log); + if (!cacheError) writeToCache(releases); + } + + let current = parseVersion(pkg.version); + + if (!current) { + log.debug(`Unable to parse the current version: ${pkg.version}`); + return; + } + + // 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); + if (parsed && !parsed.prerelease) acc.push(parsed); + return acc; + }, []); + + if (!versions.length) { + log.debug('No stable releases found to compare against'); + return; + } + + // sort newest first rather than trusting the order releases were published in + versions.sort((a, b) => compareVersions(b, a)); + let [latest] = versions; + + // already on the latest stable release, or ahead of it - nothing to warn about + if (compareVersions(current, latest) >= 0) { + log.debug(`Current version ${current.version} is up to date (latest is ${latest.version})`); + return; + } + + // a prerelease is intentionally not the latest stable release, so counting releases behind is + // meaningless. say what it actually is and what the latest stable release is instead + if (current.prerelease) { + log.warn('\nYou are using a pre-release build of @percy/cli. ' + + `${colors.red(current.version)} -> ${colors.green(latest.version)} (latest stable)\n`); + return; } - // check the current package version against released versions - // don't include prerelease - alpha/beta versions - let versions = releases.filter(r => !r.prerelease).map(r => r.tag.substr(1)); - let age = versions.indexOf(pkg.version); + // the number of stable releases newer than the current one. this is only a real count when the + // current version falls within the window of releases fetched - otherwise it is a lower bound, + // and reporting a lower bound as though it were exact is what made this warning misleading + let behind = versions.filter(v => compareVersions(v, current) > 0).length; + let known = compareVersions(current, versions[versions.length - 1]) >= 0; + let versionChange = `${colors.red(current.version)} -> ${colors.green(latest.version)}`; - // a new version is available - if (age !== 0) { - log.warn(`\n${age > 0 && age < 10 ? 'A new version of @percy/cli is available!' : ( - 'Heads up! The current version of @percy/cli is more than 10 releases behind!' - )} ${colors.red(pkg.version)} -> ${colors.green(versions[0])}\n`); + if (!known) { + log.warn('\nHeads up! Your @percy/cli is significantly out of date. ' + + `${versionChange}\nSee ${RELEASES_URL} for what changed.\n`); + } else if (behind >= MANY_RELEASES_BEHIND || current.major < latest.major) { + log.warn(`\nHeads up! Your @percy/cli is ${behind} ${behind === 1 ? 'release' : 'releases'} ` + + `behind the latest release. ${versionChange}\n` + + `See ${RELEASES_URL} for what changed.\n`); + } else { + log.warn(`\nA new version of @percy/cli is available! ${versionChange}\n`); } } catch (err) { log.debug('Unable to check for updates'); diff --git a/packages/cli/test/update.test.js b/packages/cli/test/update.test.js index 65cf2db9a..3a8c24011 100644 --- a/packages/cli/test/update.test.js +++ b/packages/cli/test/update.test.js @@ -5,9 +5,14 @@ import { checkForUpdate } from '../src/update.js'; describe('CLI update check', () => { let ghAPI; + // remocks package.json to simulate running a different version of the CLI. this resets the + // mocked filesystem, so it must be called before mocking the update cache + async function mockVersion(version) { + await mockfs({ './package.json': JSON.stringify({ name: '@percy/cli', version }) }); + } + beforeEach(async () => { - let pkg = { name: '@percy/cli', version: '1.0.0' }; - await mockfs({ './package.json': JSON.stringify(pkg) }); + await mockVersion('1.0.0'); ghAPI = await mockRequests('https://api.github.com'); await logger.mock(); }); @@ -84,14 +89,144 @@ describe('CLI update check', () => { expect(logger.stderr).toEqual([]); }); - it('warns when the current version is outdated', async () => { + it('warns with the exact number of releases behind when far behind', async () => { + await mockVersion('1.0.0'); + // 12 stable releases newer than 1.0.0, with 1.0.0 itself inside the fetched window + mockUpdateCache([ + ...Array.from({ length: 12 }, (_, i) => ({ tag: `v1.0.${12 - i}` })), + { tag: 'v1.0.0' } + ]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '\n[percy] Heads up! Your @percy/cli is 12 releases behind the latest release. ' + + '1.0.0 -> 1.0.12\nSee https://github.com/percy/cli/releases for what changed.\n' + ]); + }); + + it('escalates the warning when a major version behind, even by one release', async () => { + await mockVersion('1.0.0'); + mockUpdateCache([{ tag: 'v2.0.0' }, { tag: 'v1.0.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '\n[percy] Heads up! Your @percy/cli is 1 release behind the latest release. ' + + '1.0.0 -> 2.0.0\nSee https://github.com/percy/cli/releases for what changed.\n' + ]); + }); + + it('warns without a count when the current version predates every fetched release', async () => { + await mockVersion('1.0.0'); mockUpdateCache([{ tag: 'v2.0.2', prerelease: true }, { tag: 'v2.0.1', prerelease: false }, { tag: 'v2.0.0', prerelease: true }]); await checkForUpdate(); expect(logger.stdout).toEqual([]); expect(logger.stderr).toEqual([ - '\n[percy] Heads up! The current version of @percy/cli ' + - 'is more than 10 releases behind! 1.0.0 -> 2.0.1\n' + '\n[percy] Heads up! Your @percy/cli is significantly out of date. 1.0.0 -> 2.0.1\n' + + 'See https://github.com/percy/cli/releases for what changed.\n' + ]); + }); + + it('warns that a pre-release is in use rather than counting releases behind', async () => { + await mockVersion('1.1.0-beta.3'); + mockUpdateCache([ + { tag: 'v1.1.0' }, + { tag: 'v1.1.0-beta.3', prerelease: true }, + { tag: 'v1.0.0' } + ]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '\n[percy] You are using a pre-release build of @percy/cli. ' + + '1.1.0-beta.3 -> 1.1.0 (latest stable)\n' + ]); + }); + + it('does not warn when a pre-release is ahead of the latest stable release', async () => { + await mockVersion('1.2.0-beta.0'); + mockUpdateCache([{ tag: 'v1.1.0' }, { tag: 'v1.0.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([]); + }); + + it('does not warn when the current version is ahead of the latest release', async () => { + await mockVersion('1.2.0'); + mockUpdateCache([{ tag: 'v1.1.0' }, { tag: 'v1.0.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([]); + }); + + it('compares releases by version rather than by the order they were published', async () => { + await mockVersion('1.1.0'); + // a backported patch published after the newest release + mockUpdateCache([{ tag: 'v1.0.1' }, { tag: 'v1.2.0' }, { tag: 'v1.1.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '\n[percy] A new version of @percy/cli is available! 1.1.0 -> 1.2.0\n' + ]); + }); + + it('handles release tags that are not prefixed with a v', async () => { + await mockVersion('1.0.0'); + mockUpdateCache([{ tag: '1.1.0' }, { tag: '1.0.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '\n[percy] A new version of @percy/cli is available! 1.0.0 -> 1.1.0\n' + ]); + }); + + it('ignores releases whose tags are prereleases regardless of the release flag', async () => { + await mockVersion('1.0.0'); + // the prerelease flag is set by hand when publishing and is sometimes wrong + mockUpdateCache([{ tag: 'v1.1.0-beta.1', prerelease: false }, { tag: 'v1.0.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([]); + }); + + it('does not warn when no stable releases are found', async () => { + logger.loglevel('debug'); + mockUpdateCache([{ tag: 'v1.1.0', prerelease: true }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '[percy:cli:update] No stable releases found to compare against' + ]); + }); + + it('does not warn when the current version cannot be parsed', async () => { + await mockVersion('not-a-version'); + mockUpdateCache([{ tag: 'v1.1.0' }]); + logger.loglevel('debug'); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '[percy:cli:update] Unable to parse the current version: not-a-version' + ]); + }); + + it('ignores release tags that cannot be parsed', async () => { + await mockVersion('1.0.0'); + mockUpdateCache([{ tag: 'nightly' }, { tag: 'v1.1.0' }, { tag: 'v1.0.0' }]); + + await checkForUpdate(); + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '\n[percy] A new version of @percy/cli is available! 1.0.0 -> 1.1.0\n' ]); }); @@ -110,7 +245,8 @@ describe('CLI update check', () => { expect(logger.stdout).toEqual([]); expect(logger.stderr).toEqual([ '[percy:cli:update:cache] Unable to read from cache', - jasmine.stringContaining('[percy:cli:update:cache] Error: EACCES') + jasmine.stringContaining('[percy:cli:update:cache] Error: EACCES'), + '[percy:cli:update] Current version 1.0.0 is up to date (latest is 1.0.0)' ]); expect(ghAPI).toHaveBeenCalled(); @@ -130,7 +266,8 @@ describe('CLI update check', () => { expect(logger.stdout).toEqual([]); expect(logger.stderr).toEqual([ '[percy:cli:update:cache] Unable to write to cache', - jasmine.stringContaining('[percy:cli:update:cache] Error: EACCES') + jasmine.stringContaining('[percy:cli:update:cache] Error: EACCES'), + '[percy:cli:update] Current version 1.0.0 is up to date (latest is 1.0.0)' ]); expect(ghAPI).toHaveBeenCalled();