Skip to content

Commit 3171fd0

Browse files
committed
fix(upgrade): detect downgrades and skip delta attempt
Two UX issues when running `sentry cli upgrade <older-version>`: 1. Messaging always said 'Upgrading to' even when the target is older. Now says 'Downgrading to' / 'Successfully downgraded to' when the target version is older than the current one. 2. Delta upgrade was attempted for downgrades, wasting 22+ seconds building the patch graph before discovering no forward path exists. bsdiff patches are one-directional (old→new only), so downgrades can never use deltas. `canAttemptDelta()` now returns false immediately for downgrades. Adds `compareVersions()` and `isDowngrade()` helpers to binary.ts using `Bun.semver.order` (polyfilled for Node.js via node:semver).
1 parent bb04650 commit 3171fd0

File tree

4 files changed

+163
-41
lines changed

4 files changed

+163
-41
lines changed

src/commands/cli/upgrade.ts

Lines changed: 71 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717
import { homedir } from "node:os";
1818
import { dirname } from "node:path";
1919
import type { SentryContext } from "../../context.js";
20-
import { determineInstallDir, releaseLock } from "../../lib/binary.js";
20+
import {
21+
determineInstallDir,
22+
isDowngrade,
23+
releaseLock,
24+
} from "../../lib/binary.js";
2125
import { buildCommand } from "../../lib/command.js";
2226
import { CLI_VERSION } from "../../lib/constants.js";
2327
import {
@@ -209,6 +213,58 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise<void> {
209213
}
210214
}
211215

216+
/**
217+
* Execute the standard upgrade path: download via curl or package manager,
218+
* then run setup on the new binary.
219+
*/
220+
async function executeStandardUpgrade(opts: {
221+
method: InstallationMethod;
222+
channel: ReleaseChannel;
223+
versionArg: string | undefined;
224+
target: string;
225+
execPath: string;
226+
}): Promise<void> {
227+
const { method, channel, versionArg, target, execPath } = opts;
228+
229+
// Use the rolling "nightly" tag only when upgrading to latest nightly
230+
// (no specific version was requested). A specific version arg always
231+
// uses its own tag so the correct release is downloaded.
232+
const downloadTag =
233+
channel === "nightly" && !versionArg ? NIGHTLY_TAG : undefined;
234+
const downloadResult = await executeUpgrade(method, target, downloadTag);
235+
236+
// Run setup on the new binary to update completions, agent skills,
237+
// and record installation metadata.
238+
if (downloadResult) {
239+
// Curl: new binary is at temp path, setup --install will place it.
240+
// Pin the install directory via SENTRY_INSTALL_DIR so the child's
241+
// determineInstallDir() doesn't relocate to a different directory.
242+
// Release the download lock after the child exits — if the child used
243+
// the same lock path (ppid takeover), this is a harmless no-op.
244+
const currentInstallDir = dirname(getCurlInstallPaths().installPath);
245+
try {
246+
await runSetupOnNewBinary({
247+
binaryPath: downloadResult.tempBinaryPath,
248+
method,
249+
channel,
250+
install: true,
251+
installDir: currentInstallDir,
252+
});
253+
} finally {
254+
releaseLock(downloadResult.lockPath);
255+
}
256+
} else if (method !== "brew") {
257+
// Package manager: binary already in place, just run setup.
258+
// Skip brew — Homebrew's post_install hook already runs setup.
259+
await runSetupOnNewBinary({
260+
binaryPath: execPath,
261+
method,
262+
channel,
263+
install: false,
264+
});
265+
}
266+
}
267+
212268
/**
213269
* Migrate from a package-manager or Homebrew install to a standalone binary
214270
* when the user switches to the nightly channel.
@@ -382,7 +438,10 @@ export const upgradeCommand = buildCommand({
382438
return;
383439
}
384440

385-
stdout.write(`\nUpgrading to ${target}...\n`);
441+
const downgrade = isDowngrade(CLI_VERSION, target);
442+
stdout.write(
443+
`\n${downgrade ? "Downgrading" : "Upgrading"} to ${target}...\n`
444+
);
386445

387446
// Nightly is GitHub-only. If the current install method is not curl,
388447
// migrate to a standalone binary first then return — the migration
@@ -393,45 +452,16 @@ export const upgradeCommand = buildCommand({
393452
return;
394453
}
395454

396-
// Standard upgrade path: download (curl) or package manager.
397-
// Use the rolling "nightly" tag only when upgrading to latest nightly
398-
// (no specific version was requested). A specific version arg always
399-
// uses its own tag so the correct release is downloaded.
400-
const downloadTag =
401-
channel === "nightly" && !versionArg ? NIGHTLY_TAG : undefined;
402-
const downloadResult = await executeUpgrade(method, target, downloadTag);
403-
404-
// Run setup on the new binary to update completions, agent skills,
405-
// and record installation metadata.
406-
if (downloadResult) {
407-
// Curl: new binary is at temp path, setup --install will place it.
408-
// Pin the install directory via SENTRY_INSTALL_DIR so the child's
409-
// determineInstallDir() doesn't relocate to a different directory.
410-
// Release the download lock after the child exits — if the child used
411-
// the same lock path (ppid takeover), this is a harmless no-op.
412-
const currentInstallDir = dirname(getCurlInstallPaths().installPath);
413-
try {
414-
await runSetupOnNewBinary({
415-
binaryPath: downloadResult.tempBinaryPath,
416-
method,
417-
channel,
418-
install: true,
419-
installDir: currentInstallDir,
420-
});
421-
} finally {
422-
releaseLock(downloadResult.lockPath);
423-
}
424-
} else if (method !== "brew") {
425-
// Package manager: binary already in place, just run setup.
426-
// Skip brew — Homebrew's post_install hook already runs setup.
427-
await runSetupOnNewBinary({
428-
binaryPath: this.process.execPath,
429-
method,
430-
channel,
431-
install: false,
432-
});
433-
}
455+
await executeStandardUpgrade({
456+
method,
457+
channel,
458+
versionArg,
459+
target,
460+
execPath: this.process.execPath,
461+
});
434462

435-
stdout.write(`\nSuccessfully upgraded to ${target}.\n`);
463+
stdout.write(
464+
`\nSuccessfully ${downgrade ? "downgraded" : "upgraded"} to ${target}.\n`
465+
);
436466
},
437467
});

src/lib/binary.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,28 @@ export function isNightlyVersion(version: string): boolean {
6767
return version.includes("-dev.");
6868
}
6969

70+
/**
71+
* Compare two version strings and return their ordering.
72+
*
73+
* Uses `Bun.semver.order` which handles both stable (`X.Y.Z`) and
74+
* nightly (`X.Y.Z-dev.<unix-seconds>`) versions correctly — the numeric
75+
* pre-release identifier is compared numerically per SemVer spec.
76+
*
77+
* @returns 1 if a > b, -1 if a < b, 0 if equal
78+
*/
79+
export function compareVersions(a: string, b: string): -1 | 0 | 1 {
80+
return Bun.semver.order(a, b);
81+
}
82+
83+
/**
84+
* Check whether moving from `current` to `target` is a downgrade.
85+
*
86+
* @returns true if target is older than current
87+
*/
88+
export function isDowngrade(current: string, target: string): boolean {
89+
return compareVersions(current, target) === 1;
90+
}
91+
7092
/**
7193
* Get the binary filename for the current platform.
7294
*

src/lib/delta-upgrade.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import * as Sentry from "@sentry/bun";
2525
import {
2626
GITHUB_RELEASES_URL,
2727
getPlatformBinaryName,
28+
isDowngrade,
2829
isNightlyVersion,
2930
} from "./binary.js";
3031
import { applyPatch } from "./bspatch.js";
@@ -108,6 +109,11 @@ export function canAttemptDelta(targetVersion: string): boolean {
108109
return false;
109110
}
110111

112+
// Downgrades have no forward patch path — skip immediately
113+
if (isDowngrade(CLI_VERSION, targetVersion)) {
114+
return false;
115+
}
116+
111117
return true;
112118
}
113119

test/lib/binary.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ import {
1616
import { join } from "node:path";
1717
import {
1818
acquireLock,
19+
compareVersions,
1920
determineInstallDir,
2021
fetchWithUpgradeError,
2122
getBinaryDownloadUrl,
2223
getBinaryFilename,
2324
getBinaryPaths,
2425
installBinary,
26+
isDowngrade,
2527
releaseLock,
2628
replaceBinarySync,
2729
} from "../../src/lib/binary.js";
@@ -459,3 +461,65 @@ describe("acquireLock", () => {
459461
releaseLock(lockPath);
460462
});
461463
});
464+
465+
describe("compareVersions", () => {
466+
test("stable: newer > older", () => {
467+
expect(compareVersions("0.15.0", "0.14.0")).toBe(1);
468+
});
469+
470+
test("stable: older < newer", () => {
471+
expect(compareVersions("0.14.0", "0.15.0")).toBe(-1);
472+
});
473+
474+
test("stable: equal", () => {
475+
expect(compareVersions("0.14.0", "0.14.0")).toBe(0);
476+
});
477+
478+
test("nightly: later timestamp > earlier timestamp", () => {
479+
expect(
480+
compareVersions("0.14.0-dev.1772732047", "0.14.0-dev.1772724107")
481+
).toBe(1);
482+
});
483+
484+
test("nightly: earlier timestamp < later timestamp", () => {
485+
expect(
486+
compareVersions("0.14.0-dev.1772724107", "0.14.0-dev.1772732047")
487+
).toBe(-1);
488+
});
489+
490+
test("nightly: equal", () => {
491+
expect(
492+
compareVersions("0.14.0-dev.1772724107", "0.14.0-dev.1772724107")
493+
).toBe(0);
494+
});
495+
496+
test("stable > nightly with same base (semver: release > pre-release)", () => {
497+
expect(compareVersions("0.14.0", "0.14.0-dev.1772732047")).toBe(1);
498+
});
499+
});
500+
501+
describe("isDowngrade", () => {
502+
test("returns true when target is older stable version", () => {
503+
expect(isDowngrade("0.15.0", "0.14.0")).toBe(true);
504+
});
505+
506+
test("returns false when target is newer stable version", () => {
507+
expect(isDowngrade("0.14.0", "0.15.0")).toBe(false);
508+
});
509+
510+
test("returns false when versions are equal", () => {
511+
expect(isDowngrade("0.14.0", "0.14.0")).toBe(false);
512+
});
513+
514+
test("returns true when target is older nightly (earlier timestamp)", () => {
515+
expect(isDowngrade("0.14.0-dev.1772732047", "0.14.0-dev.1772724107")).toBe(
516+
true
517+
);
518+
});
519+
520+
test("returns false when target is newer nightly (later timestamp)", () => {
521+
expect(isDowngrade("0.14.0-dev.1772724107", "0.14.0-dev.1772732047")).toBe(
522+
false
523+
);
524+
});
525+
});

0 commit comments

Comments
 (0)