From d2a66ee3643343a5e3adcc7e6d34d22231cbd7f2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:43:40 +0000 Subject: [PATCH 01/10] feat(cli): Back up the database on upgrade and add `cli upgrade undo` Before `sentry cli upgrade` hands the local database to another version, it now copies `cli.db` to `backups/cli-.db` under the config directory and records which version it moved from. `sentry cli upgrade undo` reinstalls that version and puts its own database back, so newer CLIs don't need to rewrite their schema for older ones. - The restored database keeps the current logins when the older version stores them in `accounts`; otherwise the command warns that the user may need to log in again. - One backup per version, at most five, never pruning the one an undo would restore. Backups are 0600 in a 0700 directory, like `cli.db`. - The upgrading process detaches from `cli.db` after restoring it, so it can't migrate the older version's database again. Closes #1623. Co-Authored-By: BYK --- apps/cli-docs/src/fragments/commands/cli.md | 6 + packages/cli/src/commands/cli/upgrade.ts | 166 ++++++-- packages/cli/src/lib/db/backup.ts | 380 ++++++++++++++++++ packages/cli/src/lib/db/index.ts | 32 +- .../cli/test/commands/cli/upgrade.test.ts | 79 +++- packages/cli/test/lib/db/backup.test.ts | 253 ++++++++++++ 6 files changed, 880 insertions(+), 36 deletions(-) create mode 100644 packages/cli/src/lib/db/backup.ts create mode 100644 packages/cli/test/lib/db/backup.test.ts diff --git a/apps/cli-docs/src/fragments/commands/cli.md b/apps/cli-docs/src/fragments/commands/cli.md index 7b13f5050..60110eb97 100644 --- a/apps/cli-docs/src/fragments/commands/cli.md +++ b/apps/cli-docs/src/fragments/commands/cli.md @@ -10,6 +10,12 @@ After switching, bare `sentry cli upgrade` will continue tracking that channel. | `stable` | Latest stable release (default) | | `nightly` | Built from `main`, updated on every commit | +## Undo an upgrade + +Before switching versions, `sentry cli upgrade` saves a copy of the local database for the version it replaces, in `backups/` under the config directory. `sentry cli upgrade undo` reinstalls that version and restores its database. The logins you have now are kept when that version stores them the same way; otherwise the command says so, and you may need to run `sentry auth login`. Other changes since the upgrade, such as defaults, are undone. + +One backup is kept per version, up to the five most recent. They contain your credentials, so only your user can read them. Undo only covers version changes made with `sentry cli upgrade`, not ones made through npm or Homebrew directly. + ## Installation Detection The CLI detects how it was installed and uses the appropriate upgrade method: diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index 735c67da0..ea5743ab8 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -31,6 +31,12 @@ import { } from "../../lib/binary.js"; import { buildCommand } from "../../lib/command.js"; import { CLI_VERSION } from "../../lib/constants.js"; +import { + backupDatabase, + type DatabaseBackup, + getUndoBackup, + restoreDatabaseBackup, +} from "../../lib/db/backup.js"; import { getDbPath, getRawDatabase } from "../../lib/db/index.js"; import { getReleaseChannel, @@ -93,9 +99,81 @@ function prepareLocalDatabaseForDowngrade(): void { } } +/** + * What happens to the local database around the new binary's setup. + * + * Every version change first backs up the current database so + * `sentry cli upgrade undo` can return to it (#1623). + */ +type DatabasePlan = { + /** Version being installed. */ + target: string; + /** Hand the database to a CLI that can't adapt a newer one (#1611). */ + handOff: boolean; + /** Undo: put back the target version's own database. */ + restore?: DatabaseBackup; + /** Set once `restore` succeeded; the hand-off is then unnecessary. */ + restored?: boolean; + /** Notes about the database for the command result. */ + warnings: string[]; +}; + +/** Back up the database as this version's before another version owns it. */ +function backupLocalDatabase(target: string): void { + try { + backupDatabase(getRawDatabase(), CLI_VERSION, target); + } catch (error) { + log.warn( + `Could not back up the local database: ${stringifyUnknown(error)}. ` + + "`sentry cli upgrade undo` won't be able to return to this version.", + ); + } +} + +/** + * Run once the new binary is downloaded, right before its setup, so a + * failed download leaves the database untouched. + */ +function prepareDatabaseForNewBinary(plan: DatabasePlan): void { + // Reinstalling this same version must not replace the record of the + // upgrade that led here. + if (plan.target !== CLI_VERSION) { + backupLocalDatabase(plan.target); + } + if (plan.restore) { + try { + plan.warnings.push( + ...restoreDatabaseBackup(getRawDatabase(), plan.restore), + ); + plan.restored = true; + return; + } catch (error) { + log.warn( + `Could not restore the database backup for ${plan.target}: ${stringifyUnknown(error)}.`, + ); + } + } + if (plan.handOff) { + prepareLocalDatabaseForDowngrade(); + } +} + +/** + * Run after setup, even when it fails: another invocation of this CLI may + * have re-migrated the database meanwhile. + */ +function finishDatabaseForNewBinary(plan: DatabasePlan): void { + if (plan.handOff && !plan.restored) { + prepareLocalDatabaseForDowngrade(); + } +} + /** Special version strings that select a channel rather than a specific release. */ const CHANNEL_VERSIONS = new Set(["nightly", "stable"]); +/** Version argument that returns to the version before the last upgrade. */ +const UNDO_VERSION = "undo"; + /** * Structured result of the upgrade command. * @@ -120,6 +198,8 @@ export type UpgradeResult = { offline?: boolean; /** Warnings to display (e.g., PATH shadowing from old package manager install) */ warnings?: string[]; + /** Set by `sentry cli upgrade undo`: the version whose database was restored. */ + restoredDatabase?: string; /** Changelog summary for the version range. Absent for offline or on fetch failure. */ changelog?: ChangelogSummary; }; @@ -581,8 +661,8 @@ type SetupOptions = { ensureAuthScopes: boolean; /** Skip agent skill installation during setup. */ noAgentSkills: boolean; - /** Hand the local database to an older CLI around setup (#1611). */ - handOffDatabase: boolean; + /** What to do with the local database around setup. */ + database: DatabasePlan; }; /** @@ -597,10 +677,8 @@ type SetupOptions = { * For package manager upgrades: the binary is already in place, so setup only * updates completions, agent skills, and records metadata. * - * With `handOffDatabase`, the database is handed off only once the older - * binary is downloaded, so a failed download keeps the current login, and - * again after setup even when it fails, in case another invocation of this - * CLI re-migrated the database meanwhile. + * The database is backed up, restored or handed off only once the new + * binary is downloaded (see {@link prepareDatabaseForNewBinary}). */ async function runSetupOnNewBinary(opts: SetupOptions): Promise { const { @@ -611,7 +689,7 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { installDir, ensureAuthScopes, noAgentSkills, - handOffDatabase, + database, } = opts; const args = [ "cli", @@ -637,16 +715,12 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { ? { ...process.env, SENTRY_INSTALL_DIR: installDir } : undefined; - if (handOffDatabase) { - prepareLocalDatabaseForDowngrade(); - } + prepareDatabaseForNewBinary(database); let exitCode: number; try { exitCode = await spawnWithRetry(binaryPath, args, env); } finally { - if (handOffDatabase) { - prepareLocalDatabaseForDowngrade(); - } + finishDatabaseForNewBinary(database); } if (exitCode !== 0) { throw new UpgradeError( @@ -715,7 +789,7 @@ async function executeStandardUpgrade(opts: { json?: boolean; noAgentSkills: boolean; source?: UpgradeSource; - handOffDatabase: boolean; + database: DatabasePlan; }): Promise { const { method, @@ -729,7 +803,7 @@ async function executeStandardUpgrade(opts: { json, noAgentSkills, source, - handOffDatabase, + database, } = opts; // Use the rolling "nightly" tag only when upgrading to latest nightly @@ -779,7 +853,7 @@ async function executeStandardUpgrade(opts: { installDir, ensureAuthScopes: !json, noAgentSkills, - handOffDatabase, + database, }); } finally { releaseLock(downloadResult.lockPath); @@ -795,7 +869,7 @@ async function executeStandardUpgrade(opts: { install: false, ensureAuthScopes: !json, noAgentSkills, - handOffDatabase, + database, }); } } @@ -825,7 +899,7 @@ async function migrateToStandaloneForNightly(opts: { json?: boolean; source?: UpgradeSource; channel: ReleaseChannel; - handOffDatabase: boolean; + database: DatabasePlan; }): Promise { const { method, @@ -835,7 +909,7 @@ async function migrateToStandaloneForNightly(opts: { json, source, channel, - handOffDatabase, + database, } = opts; log.info("Nightly builds are only available as standalone binaries."); log.info("Migrating to standalone installation..."); @@ -880,7 +954,7 @@ async function migrateToStandaloneForNightly(opts: { installDir, ensureAuthScopes: !json, noAgentSkills, - handOffDatabase, + database, }); } finally { releaseLock(downloadResult.lockPath); @@ -1004,7 +1078,10 @@ export const upgradeCommand = buildCommand({ " stable (default) Latest stable release\n" + " nightly Built from main, updated on every commit\n\n" + "The channel is persisted so that subsequent bare `sentry cli upgrade` calls\n" + - "use the same channel.", + "use the same channel.\n\n" + + "Before switching versions, the local database is backed up for the version\n" + + "being replaced. `sentry cli upgrade undo` reinstalls that version and restores\n" + + "its database, keeping your current logins when it can.", examples: [ { description: "Update to the latest version on the persisted channel", @@ -1026,6 +1103,10 @@ export const upgradeCommand = buildCommand({ description: "Install a specific stable version", command: "sentry cli upgrade 0.5.0", }, + { + description: "Return to the version before the last upgrade", + command: "sentry cli upgrade undo", + }, { description: "Force re-download even if up to date", command: "sentry cli upgrade --force", @@ -1051,7 +1132,7 @@ export const upgradeCommand = buildCommand({ parameters: [ { brief: - 'Specific version (e.g. 0.5.0), or "nightly"/"stable" to switch channel; omit to update within current channel', + 'Specific version (e.g. 0.5.0), "nightly"/"stable" to switch channel, or "undo" to return to the previous version; omit to update within current channel', parse: String, placeholder: "version", optional: true, @@ -1090,8 +1171,21 @@ export const upgradeCommand = buildCommand({ }, }, async *func(this: SentryContext, flags: UpgradeFlags, version?: string) { - const { channel, versionArg, channelChanged, method } = - await resolveContext(version, flags); + const undo = + version?.toLowerCase() === UNDO_VERSION + ? getUndoBackup(CLI_VERSION) + : undefined; + const resolvedContext = await resolveContext( + undo ? undo.version : version, + flags, + ); + const { versionArg, method } = resolvedContext; + // Undo reinstalls the earlier version on its own channel. Its restored + // database already stores that channel, so this one's is left alone. + const channel = undo + ? getArtifactChannel(undo.version) + : resolvedContext.channel; + const channelChanged = undo ? false : resolvedContext.channelChanged; log.debug(`Installation method: ${method}`); log.debug(`Current version: ${CLI_VERSION}`); @@ -1104,8 +1198,11 @@ export const upgradeCommand = buildCommand({ versionArg, offline: flags.offline, method, - persistChannelFn: () => - persistChannel(channel, channelChanged, version), + persistChannelFn: () => { + if (!undo) { + persistChannel(channel, channelChanged, version); + } + }, }), ); // Early exit for check-only (online) and up-to-date results. @@ -1168,8 +1265,13 @@ export const upgradeCommand = buildCommand({ } const downgrade = isDowngrade(CLI_VERSION, target); log.debug(`${downgrade ? "Downgrading" : "Upgrading"} to ${target}`); - const handOffDatabase = - downgrade && compareVersions(target, SELF_ADAPTING_CLI_VERSION) < 0; + const database: DatabasePlan = { + target, + handOff: + downgrade && compareVersions(target, SELF_ADAPTING_CLI_VERSION) < 0, + restore: undo, + warnings: [], + }; // Perform the actual upgrade let warnings: string[] | undefined; @@ -1184,7 +1286,7 @@ export const upgradeCommand = buildCommand({ json: flags.json, source, channel, - handOffDatabase, + database, }); } else { await executeStandardUpgrade({ @@ -1199,11 +1301,12 @@ export const upgradeCommand = buildCommand({ json: flags.json, noAgentSkills: flags["no-agent-skills"], source, - handOffDatabase, + database, }); } const changelog = await changelogPromise; + const allWarnings = [...(warnings ?? []), ...database.warnings]; yield new CommandOutput({ action: downgrade ? "downgraded" : "upgraded", currentVersion: CLI_VERSION, @@ -1212,7 +1315,8 @@ export const upgradeCommand = buildCommand({ method, forced: flags.force, offline: offline ? true : undefined, - warnings, + warnings: allWarnings.length > 0 ? allWarnings : undefined, + restoredDatabase: database.restored ? target : undefined, changelog, } satisfies UpgradeResult); return; diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts new file mode 100644 index 000000000..b8be91267 --- /dev/null +++ b/packages/cli/src/lib/db/backup.ts @@ -0,0 +1,380 @@ +/** + * Per-version backups of `cli.db` for `sentry cli upgrade undo` (#1623). + * + * Before `sentry cli upgrade` hands the database to another version, it + * copies `cli.db` to `backups/cli-.db` under the config directory + * and records which version it moved from. `sentry cli upgrade undo` + * reinstalls that version and puts its own database back, so newer CLIs + * don't have to know how to rewrite their schema for older ones. + * + * Backups hold credentials, so the directory is `0700` and files `0600`, + * like `cli.db` itself. + */ + +import { + chmodSync, + copyFileSync, + existsSync, + mkdirSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { stringifyUnknown, UpgradeError } from "../errors.js"; +import { logger } from "../logger.js"; +import { + closeDatabase, + detachDatabase, + getConfigDir, + getDbPath, +} from "./index.js"; +import { tableExists } from "./schema.js"; +import { Database, type SQLQueryBindings } from "./sqlite.js"; + +const log = logger.withTag("db.backup"); + +const BACKUP_SUBDIR = "backups"; +const MANIFEST_FILENAME = "manifest.json"; + +/** + * Backups kept at most. There is one per version, so nightly users would + * otherwise collect one per nightly they ever ran. + */ +export const MAX_DATABASE_BACKUPS = 5; + +/** A saved copy of the database as one CLI version left it. */ +export type DatabaseBackup = { + /** CLI version that last used this database. */ + version: string; + /** File name inside the backup directory. */ + file: string; + /** When the copy was made (ms since epoch). */ + createdAt: number; +}; + +/** The most recent version change made through `sentry cli upgrade`. */ +type UpgradeRecord = { + fromVersion: string; + toVersion: string; +}; + +type BackupManifest = { + backups: DatabaseBackup[]; + lastUpgrade?: UpgradeRecord; +}; + +/** Directory holding database backups and their manifest. */ +export function getBackupDir(): string { + return join(getConfigDir(), BACKUP_SUBDIR); +} + +function manifestPath(): string { + return join(getBackupDir(), MANIFEST_FILENAME); +} + +function isBackup(value: unknown): value is DatabaseBackup { + if (typeof value !== "object" || value === null) { + return false; + } + const entry = value as Record; + return ( + typeof entry.version === "string" && + typeof entry.file === "string" && + typeof entry.createdAt === "number" + ); +} + +function isUpgradeRecord(value: unknown): value is UpgradeRecord { + if (typeof value !== "object" || value === null) { + return false; + } + const record = value as Record; + return ( + typeof record.fromVersion === "string" && + typeof record.toVersion === "string" + ); +} + +/** Read the manifest, treating a missing or unreadable one as empty. */ +function readManifest(): BackupManifest { + const path = manifestPath(); + if (!existsSync(path)) { + return { backups: [] }; + } + try { + const parsed = JSON.parse(readFileSync(path, "utf-8")) as Record< + string, + unknown + >; + const backups = Array.isArray(parsed.backups) + ? parsed.backups.filter(isBackup) + : []; + const lastUpgrade = isUpgradeRecord(parsed.lastUpgrade) + ? parsed.lastUpgrade + : undefined; + return { backups, lastUpgrade }; + } catch (error) { + log.debug(`Ignoring unreadable backup manifest ${path}`, error); + return { backups: [] }; + } +} + +/** Write the manifest atomically, readable only by the user. */ +function writeManifest(manifest: BackupManifest): void { + const path = manifestPath(); + const tmp = `${path}.${process.pid}.tmp`; + writeFileSync(tmp, `${JSON.stringify(manifest, null, 2)}\n`, { + mode: 0o600, + }); + renameSync(tmp, path); +} + +function ensureBackupDir(): string { + const dir = getBackupDir(); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + try { + chmodSync(dir, 0o700); + } catch (error) { + // Windows has no POSIX modes; the directory inherits the profile's ACLs. + log.debug(`Could not restrict ${dir}`, error); + } + return dir; +} + +function restrictFile(path: string): void { + try { + chmodSync(path, 0o600); + } catch (error) { + log.debug(`Could not restrict ${path}`, error); + } +} + +/** File name for a version's backup; versions are plain semver strings. */ +function backupFileName(version: string): string { + return `cli-${version.replace(/[^A-Za-z0-9._-]/g, "_")}.db`; +} + +/** + * Keep the newest {@link MAX_DATABASE_BACKUPS} backups, plus the one an undo + * would restore, and delete the files of the rest. + */ +function pruneBackups(manifest: BackupManifest, dir: string): void { + const undoVersion = manifest.lastUpgrade?.fromVersion; + // Entries are appended as they are made, so reversing first keeps the + // newest first among backups made within the same millisecond. + const newestFirst = [...manifest.backups] + .reverse() + .sort((a, b) => b.createdAt - a.createdAt); + const kept: DatabaseBackup[] = []; + for (const entry of newestFirst) { + if (kept.length < MAX_DATABASE_BACKUPS || entry.version === undoVersion) { + kept.push(entry); + continue; + } + rmSync(join(dir, entry.file), { force: true }); + } + manifest.backups = kept; +} + +/** + * Save `db` as `fromVersion`'s backup and record the move to `toVersion`, + * so `sentry cli upgrade undo` run from `toVersion` can return to it. + * + * Replaces any earlier backup of `fromVersion`. Uses `VACUUM INTO`, which + * takes a consistent snapshot even while other CLI processes use `db`. + */ +export function backupDatabase( + db: Database, + fromVersion: string, + toVersion: string, +): DatabaseBackup { + const dir = ensureBackupDir(); + const manifest = readManifest(); + // Drop the record first: if the copy fails, an undo must not restore a + // backup that predates this upgrade. + if (manifest.lastUpgrade) { + manifest.lastUpgrade = undefined; + writeManifest(manifest); + } + + const file = backupFileName(fromVersion); + const path = join(dir, file); + const tmp = `${path}.${process.pid}.tmp`; + rmSync(tmp, { force: true }); + try { + db.query("VACUUM INTO ?").run(tmp); + restrictFile(tmp); + renameSync(tmp, path); + } finally { + rmSync(tmp, { force: true }); + } + + const entry: DatabaseBackup = { + version: fromVersion, + file, + createdAt: Date.now(), + }; + manifest.backups = [ + ...manifest.backups.filter((b) => b.version !== fromVersion), + entry, + ]; + manifest.lastUpgrade = { fromVersion, toVersion }; + pruneBackups(manifest, dir); + writeManifest(manifest); + return entry; +} + +/** + * Find the backup `sentry cli upgrade undo` should restore when run from + * `currentVersion`. + * + * @throws {UpgradeError} When no upgrade to `currentVersion` was recorded, + * or its backup is gone. + */ +export function getUndoBackup(currentVersion: string): DatabaseBackup { + const { backups, lastUpgrade } = readManifest(); + if (!lastUpgrade) { + throw new UpgradeError( + "unsupported_operation", + "Nothing to undo: no upgrade made with `sentry cli upgrade` was recorded.", + ); + } + if (lastUpgrade.toVersion !== currentVersion) { + throw new UpgradeError( + "unsupported_operation", + `Nothing to undo: the last recorded upgrade went from ${lastUpgrade.fromVersion} to ${lastUpgrade.toVersion}, ` + + `but this is ${currentVersion}. It was probably installed another way.`, + ); + } + const backup = backups.find((b) => b.version === lastUpgrade.fromVersion); + if (!(backup && existsSync(join(getBackupDir(), backup.file)))) { + throw new UpgradeError( + "unsupported_operation", + `Cannot undo: the database backup for ${lastUpgrade.fromVersion} is missing. ` + + `Run \`sentry cli upgrade ${lastUpgrade.fromVersion}\` instead.`, + ); + } + return backup; +} + +/** Columns of `table` in `db`, in declaration order. */ +function columnNames(db: Database, table: string): string[] { + const rows = db + .query(`SELECT name FROM pragma_table_info('${table}')`) + .all() as { name: string }[]; + return rows.map((row) => row.name); +} + +type AccountRows = { + columns: string[]; + rows: Record[]; +}; + +function readAccounts(db: Database): AccountRows | undefined { + if (!tableExists(db, "accounts")) { + return; + } + return { + columns: columnNames(db, "accounts"), + rows: db.query("SELECT * FROM accounts").all(), + }; +} + +/** + * Replace the accounts in `restored` with `current`'s, so the undo keeps + * the logins and refreshed tokens made since the backup. + * + * @returns A warning when they could not be carried over. + */ +function carryOverAccounts( + restored: Database, + current: AccountRows | undefined, + backup: DatabaseBackup, +): string | undefined { + if (!current || current.rows.length === 0) { + return; + } + const loginHint = `Your logins are as they were in ${backup.version}; run \`sentry auth login\` if you need another.`; + if (!tableExists(restored, "accounts")) { + return `${backup.version} stores logins in an older format, so your current logins could not be kept. ${loginHint}`; + } + const restoredColumns = new Set(columnNames(restored, "accounts")); + const shared = current.columns.filter((c) => restoredColumns.has(c)); + const placeholders = shared.map(() => "?").join(", "); + try { + restored.transaction(() => { + restored.query("DELETE FROM accounts").run(); + const insert = restored.query( + `INSERT INTO accounts (${shared.join(", ")}) VALUES (${placeholders})`, + ); + for (const row of current.rows) { + insert.run(...shared.map((c) => row[c])); + } + })(); + } catch (error) { + log.debug("Could not carry accounts into the restored database", error); + return `Could not keep your current logins (${stringifyUnknown(error)}). ${loginHint}`; + } + return; +} + +/** + * Put `backup` back as `cli.db`, keeping the current logins when the + * backup's schema allows it. + * + * Detaches the database from this process first: reopening the restored + * file here would migrate it back to this version's schema. + * + * @param db - This process's open connection to `cli.db` + * @returns What the user should know about the restored state + */ +export function restoreDatabaseBackup( + db: Database, + backup: DatabaseBackup, +): string[] { + const source = join(getBackupDir(), backup.file); + const dbPath = getDbPath(); + const tmp = `${dbPath}.restore-${process.pid}`; + const currentAccounts = readAccounts(db); + const warnings: string[] = []; + + rmSync(tmp, { force: true }); + try { + copyFileSync(source, tmp); + restrictFile(tmp); + const restored = new Database(tmp); + try { + const warning = carryOverAccounts(restored, currentAccounts, backup); + if (warning) { + warnings.push(warning); + } + } finally { + restored.close(); + } + + // Fold the write-ahead log into `cli.db`, then let it go for good. + db.exec("PRAGMA wal_checkpoint(TRUNCATE)"); + detachDatabase(); + try { + // A leftover write-ahead log belongs to the replaced file and would + // corrupt the restored one. + rmSync(`${dbPath}-wal`, { force: true }); + rmSync(`${dbPath}-shm`, { force: true }); + renameSync(tmp, dbPath); + } catch (error) { + // `cli.db` is still this version's; let callers use it again. + closeDatabase(); + throw error; + } + } finally { + rmSync(tmp, { force: true }); + } + + const savedOn = new Date(backup.createdAt).toISOString().slice(0, 10); + warnings.push( + `Restored the database ${backup.version} saved on ${savedOn}. Other changes since then, such as defaults, are undone.`, + ); + return warnings; +} diff --git a/packages/cli/src/lib/db/index.ts b/packages/cli/src/lib/db/index.ts index 185209854..be549fb1c 100644 --- a/packages/cli/src/lib/db/index.ts +++ b/packages/cli/src/lib/db/index.ts @@ -43,6 +43,11 @@ let db: Database | null = null; /** Raw database without tracing (used for repair operations) */ let rawDb: Database | null = null; let dbOpenedPath: string | null = null; +/** + * Set once {@link detachDatabase} hands `cli.db` to another CLI version. + * Later reads in this process use a throwaway in-memory database instead. + */ +let detached = false; /** * Whether the process-exit close handler has been registered. @@ -172,7 +177,7 @@ function setDbPermissions(): void { /** Get or initialize the database connection. */ export function getDatabase(): Database { - const dbPath = getDbPath(); + const dbPath = detached ? ":memory:" : getDbPath(); // Auto-invalidate if config directory changed (for tests) if (db && dbOpenedPath !== dbPath) { @@ -186,7 +191,9 @@ export function getDatabase(): Database { return db; } - ensureConfigDir(); + if (!detached) { + ensureConfigDir(); + } rawDb = new Database(dbPath); @@ -207,10 +214,14 @@ export function getDatabase(): Database { rawDb.exec("PRAGMA foreign_keys = ON"); rawDb.exec("PRAGMA synchronous = NORMAL"); - setDbPermissions(); + if (!detached) { + setDbPermissions(); + } initSchema(rawDb); runMigrations(rawDb); - migrateFromJson(rawDb); + if (!detached) { + migrateFromJson(rawDb); + } // Wrap with tracing proxy for automatic query instrumentation. // Lazy-require telemetry to avoid top-level import of @sentry/node-core (~85ms). @@ -243,6 +254,19 @@ export function closeDatabase(): void { rawDb = null; dbOpenedPath = null; } + detached = false; +} + +/** + * Close `cli.db` for good in this process, so another CLI version can own it. + * + * Used when `sentry cli upgrade undo` swaps in an older version's database: + * reopening it here would migrate it to this version's schema. Later reads + * get a fresh in-memory database, so their writes are dropped. + */ +export function detachDatabase(): void { + closeDatabase(); + detached = true; } /** diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 8662fd1ba..3052c4a56 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -39,7 +39,8 @@ import type { SentryContext } from "../../../src/context.js"; // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as binary from "../../../src/lib/binary.js"; import { CLI_VERSION } from "../../../src/lib/constants.js"; -import { getRawDatabase } from "../../../src/lib/db/index.js"; +import { backupDatabase, getUndoBackup } from "../../../src/lib/db/backup.js"; +import { getDbPath, getRawDatabase } from "../../../src/lib/db/index.js"; import { clearInstallInfo, setInstallInfo, @@ -49,6 +50,8 @@ import { setReleaseChannel, } from "../../../src/lib/db/release-channel.js"; import { CURRENT_SCHEMA_VERSION } from "../../../src/lib/db/schema.js"; +import { Database } from "../../../src/lib/db/sqlite.js"; +import { getMetadata, setMetadata } from "../../../src/lib/db/utils.js"; import { setVersionCheckInfo } from "../../../src/lib/db/version-check.js"; import { extractInputUrl } from "../../../src/lib/fetch-input.js"; import { TEST_TMP_DIR, useTestConfigDir } from "../../helpers.js"; @@ -1074,6 +1077,80 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(readSchemaVersion()).toBe(CURRENT_SCHEMA_VERSION); }); + test("backs up the database before the new version runs setup", async () => { + mockBinaryDownloadWithVersion("99.99.99"); + let backupAtSetup: string | undefined; + spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])] }); + backupAtSetup = getUndoBackup("99.99.99").version; + return fakeChildProcess(0); + }); + + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "--method", "curl"], context); + + expect(backupAtSetup).toBe(CLI_VERSION); + }); + + test("undo reinstalls the previous version with its own database", async () => { + const marker = "test.marker"; + setMetadata(getRawDatabase(), { [marker]: "saved by 0.50.0" }); + backupDatabase(getRawDatabase(), "0.50.0", CLI_VERSION); + setMetadata(getRawDatabase(), { [marker]: "changed since" }); + + const fakeContent = new Uint8Array([0x7f, 0x45, 0x4c, 0x46]); + mockFetch(async (url) => { + if (extractInputUrl(url).includes("releases/tags/cli%400.50.0")) { + return new Response(JSON.stringify({ tag_name: "cli@0.50.0" }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response(gzipSync(fakeContent), { status: 200 }); + }); + let markerAtSetup: string | undefined; + spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])] }); + const db = new Database(getDbPath()); + try { + markerAtSetup = getMetadata(db, [marker]).get(marker); + } finally { + db.close(); + } + return fakeChildProcess(0); + }); + + const { context, getOutput, restore } = createMockContext({ + homeDir: testDir, + }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); + + expect(getOutput()).toContain("Restored the database 0.50.0"); + const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); + expect(setupCall?.args).toContain("stable"); + expect(markerAtSetup).toBe("saved by 0.50.0"); + // This version's database is kept, so the undo can be undone in turn. + expect(getUndoBackup("0.50.0").version).toBe(CLI_VERSION); + }); + + test("undo explains when there is nothing to undo", async () => { + mockBinaryDownloadWithVersion("99.99.99"); + + const { context, getOutput, errors, restore } = createMockContext({ + homeDir: testDir, + }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); + + expect(getOutput() + errors.join("")).toContain("Nothing to undo"); + expect(spawnedArgs).toEqual([]); + }); + test("leaves the database alone when upgrading", async () => { mockBinaryDownloadWithVersion("99.99.99"); diff --git a/packages/cli/test/lib/db/backup.test.ts b/packages/cli/test/lib/db/backup.test.ts new file mode 100644 index 000000000..dc41cd878 --- /dev/null +++ b/packages/cli/test/lib/db/backup.test.ts @@ -0,0 +1,253 @@ +/** + * Per-version database backups for `sentry cli upgrade undo` (#1623). + */ + +import { + existsSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { + backupDatabase, + getBackupDir, + getUndoBackup, + MAX_DATABASE_BACKUPS, + restoreDatabaseBackup, +} from "../../../src/lib/db/backup.js"; +import { + closeDatabase, + getDatabase, + getDbPath, + getRawDatabase, +} from "../../../src/lib/db/index.js"; +import { __setDriverForTests, Database } from "../../../src/lib/db/sqlite.js"; +import { getMetadata, setMetadata } from "../../../src/lib/db/utils.js"; +import { UpgradeError } from "../../../src/lib/errors.js"; +import { useTestConfigDir } from "../../helpers.js"; + +useTestConfigDir("db-backup-"); + +const MARKER = "test.marker"; + +function setMarker(value: string): void { + setMetadata(getRawDatabase(), { [MARKER]: value }); +} + +function addAccount(id: string, token: string, active: boolean): void { + getRawDatabase() + .query( + "INSERT INTO accounts (account_id, host, user_id, token, is_active) VALUES (?, ?, ?, ?, ?)", + ) + .run(id, "https://sentry.io", `user-${id}`, token, active ? 1 : 0); +} + +/** Read `cli.db` directly, without migrating it to this CLI's schema. */ +function readFile(read: (db: Database) => T): T { + const db = new Database(getDbPath()); + try { + return read(db); + } finally { + db.close(); + } +} + +function permissions(path: string): number { + // oxlint-disable-next-line no-bitwise -- verifying permission bits + return statSync(path).mode & 0o777; +} + +function manifest(): { + backups: { version: string }[]; + lastUpgrade?: { fromVersion: string; toVersion: string }; +} { + return JSON.parse( + readFileSync(join(getBackupDir(), "manifest.json"), "utf-8"), + ); +} + +describe("backupDatabase", () => { + test("saves the database and records the upgrade", () => { + setMarker("before"); + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + setMarker("after"); + + const copy = new Database(join(getBackupDir(), backup.file)); + try { + expect(getMetadata(copy, [MARKER]).get(MARKER)).toBe("before"); + } finally { + copy.close(); + } + expect(manifest().lastUpgrade).toEqual({ + fromVersion: "1.0.0", + toVersion: "2.0.0", + }); + expect(getUndoBackup("2.0.0")).toEqual(backup); + }); + + test.skipIf(process.platform === "win32")( + "keeps backups readable only by the user", + () => { + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + expect(permissions(getBackupDir())).toBe(0o700); + expect(permissions(join(getBackupDir(), backup.file))).toBe(0o600); + expect(permissions(join(getBackupDir(), "manifest.json"))).toBe(0o600); + }, + ); + + test("keeps one backup per version", () => { + setMarker("first"); + backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + setMarker("second"); + const backup = backupDatabase(getRawDatabase(), "1.0.0", "3.0.0"); + + expect(manifest().backups.map((b) => b.version)).toEqual(["1.0.0"]); + const copy = new Database(join(getBackupDir(), backup.file)); + try { + expect(getMetadata(copy, [MARKER]).get(MARKER)).toBe("second"); + } finally { + copy.close(); + } + }); + + test("prunes the oldest backups", () => { + const files: string[] = []; + for (let i = 0; i <= MAX_DATABASE_BACKUPS; i++) { + files.push( + backupDatabase(getRawDatabase(), `1.0.${i}`, `1.0.${i + 1}`).file, + ); + } + + const kept = manifest().backups.map((b) => b.version); + expect(kept).toHaveLength(MAX_DATABASE_BACKUPS); + expect(kept).not.toContain("1.0.0"); + expect(existsSync(join(getBackupDir(), files[0] as string))).toBe(false); + expect( + existsSync(join(getBackupDir(), files[MAX_DATABASE_BACKUPS] as string)), + ).toBe(true); + }); +}); + +describe("getUndoBackup", () => { + function undoError(version: string): string { + try { + getUndoBackup(version); + } catch (error) { + expect(error).toBeInstanceOf(UpgradeError); + return (error as Error).message; + } + throw new Error("expected getUndoBackup to throw"); + } + + test("fails when no upgrade was recorded", () => { + expect(undoError("2.0.0")).toContain("Nothing to undo"); + }); + + test("fails when this version was installed another way", () => { + backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + expect(undoError("3.0.0")).toContain("went from 1.0.0 to 2.0.0"); + }); + + test("fails when the backup file is gone", () => { + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + rmSync(join(getBackupDir(), backup.file)); + expect(undoError("2.0.0")).toContain("backup for 1.0.0 is missing"); + }); + + test("treats an unreadable manifest as nothing to undo", () => { + backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + writeFileSync(join(getBackupDir(), "manifest.json"), "not json"); + expect(undoError("2.0.0")).toContain("Nothing to undo"); + }); +}); + +describe("restoreDatabaseBackup", () => { + test("puts the backup back and keeps the current logins", () => { + addAccount("a", "old-token", true); + setMarker("saved"); + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + + getRawDatabase() + .query("UPDATE accounts SET token = ? WHERE account_id = 'a'") + .run("refreshed-token"); + addAccount("b", "new-login", false); + setMarker("changed"); + + const warnings = restoreDatabaseBackup(getRawDatabase(), backup); + + expect(warnings.join("\n")).toContain("Restored the database 1.0.0"); + const restored = readFile((db) => ({ + marker: getMetadata(db, [MARKER]).get(MARKER), + accounts: db + .query( + "SELECT account_id, token, is_active FROM accounts ORDER BY account_id", + ) + .all(), + })); + expect(restored.marker).toBe("saved"); + expect(restored.accounts).toEqual([ + { account_id: "a", token: "refreshed-token", is_active: 1 }, + { account_id: "b", token: "new-login", is_active: 0 }, + ]); + }); + + test("leaves the restored file alone for the rest of the process", () => { + setMarker("saved"); + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + restoreDatabaseBackup(getRawDatabase(), backup); + + setMetadata(getDatabase(), { [MARKER]: "written after restore" }); + + expect(readFile((db) => getMetadata(db, [MARKER]).get(MARKER))).toBe( + "saved", + ); + closeDatabase(); + }); + + test("warns when the backup predates multi-account storage", () => { + addAccount("a", "token", true); + const backup = backupDatabase(getRawDatabase(), "0.47.0", "2.0.0"); + const copy = new Database(join(getBackupDir(), backup.file)); + try { + copy.exec("DROP TABLE accounts"); + } finally { + copy.close(); + } + + const warnings = restoreDatabaseBackup(getRawDatabase(), backup); + + expect(warnings.join("\n")).toContain( + "0.47.0 stores logins in an older format", + ); + }); +}); + +describe.each(["node", "wasm"] as const)("round trip [%s driver]", (kind) => { + beforeEach(() => { + __setDriverForTests(kind); + }); + afterEach(() => { + closeDatabase(); + __setDriverForTests(null); + }); + + test("backs up and restores the database", () => { + expect(getRawDatabase().driverKind).toBe(kind); + addAccount("a", "token", true); + setMarker("saved"); + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + setMarker("changed"); + + restoreDatabaseBackup(getRawDatabase(), backup); + + expect(readFile((db) => getMetadata(db, [MARKER]).get(MARKER))).toBe( + "saved", + ); + expect( + readFile((db) => db.query("SELECT token FROM accounts").all()), + ).toEqual([{ token: "token" }]); + }); +}); From 75d5e16dfb3e63b6ad52f869a1588aa9353e3800 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:59:11 +0000 Subject: [PATCH 02/10] fix(cli): Keep logouts across undo and explain undo on Homebrew - Restoring a backup after logging out of every account brought the backup's logins back. The restored database now ends up with no accounts (or an empty pre-v18 `auth` row) in that case. - `sentry cli upgrade undo` on a Homebrew install failed with the generic "Homebrew does not support installing a specific version" error. It now says undo can't go through Homebrew and points to `sentry cli upgrade undo --method curl`. - Commit the regenerated skill reference for the docs fragment change. Co-Authored-By: BYK --- apps/cli-docs/src/fragments/commands/cli.md | 2 +- .../skills/sentry-cli/references/cli.md | 5 ++++- packages/cli/src/commands/cli/upgrade.ts | 13 ++++++++++++- packages/cli/src/lib/db/backup.ts | 14 +++++++++++--- packages/cli/test/commands/cli/upgrade.test.ts | 16 ++++++++++++++++ packages/cli/test/lib/db/backup.test.ts | 13 +++++++++++++ 6 files changed, 57 insertions(+), 6 deletions(-) diff --git a/apps/cli-docs/src/fragments/commands/cli.md b/apps/cli-docs/src/fragments/commands/cli.md index 60110eb97..a045c52c5 100644 --- a/apps/cli-docs/src/fragments/commands/cli.md +++ b/apps/cli-docs/src/fragments/commands/cli.md @@ -14,7 +14,7 @@ After switching, bare `sentry cli upgrade` will continue tracking that channel. Before switching versions, `sentry cli upgrade` saves a copy of the local database for the version it replaces, in `backups/` under the config directory. `sentry cli upgrade undo` reinstalls that version and restores its database. The logins you have now are kept when that version stores them the same way; otherwise the command says so, and you may need to run `sentry auth login`. Other changes since the upgrade, such as defaults, are undone. -One backup is kept per version, up to the five most recent. They contain your credentials, so only your user can read them. Undo only covers version changes made with `sentry cli upgrade`, not ones made through npm or Homebrew directly. +One backup is kept per version, up to the five most recent. They contain your credentials, so only your user can read them. Undo only covers version changes made with `sentry cli upgrade`, not ones made through npm or Homebrew directly. Homebrew can't install an earlier version, so on a Homebrew install run `sentry cli upgrade undo --method curl` to return to it as a standalone binary. ## Installation Detection diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/cli.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/cli.md index f74c96834..fb924e4cf 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/cli.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/cli.md @@ -232,7 +232,7 @@ Update the Sentry CLI to the latest version | Argument | Description | |----------|-------------| -| `[]` | Specific version (e.g. 0.5.0), or "nightly"/"stable" to switch channel; omit to update within current channel | +| `[]` | Specific version (e.g. 0.5.0), "nightly"/"stable" to switch channel, or "undo" to return to the previous version; omit to update within current channel | **Flags:** - `--check - Check for updates without installing` @@ -259,6 +259,9 @@ sentry cli upgrade stable # Install a specific stable version sentry cli upgrade 0.5.0 +# Return to the version before the last upgrade +sentry cli upgrade undo + # Force re-download even if up to date sentry cli upgrade --force diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index ea5743ab8..e4fb519f5 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -353,6 +353,7 @@ function validateMethod( method: InstallationMethod, versionArg: string | undefined, offline: boolean, + undo: boolean, ): void { if (method === "unknown") { throw new UpgradeError("unknown_method"); @@ -363,6 +364,14 @@ function validateMethod( ? undefined : versionArg?.replace(VERSION_PREFIX_REGEX, ""); if (method === "brew" && pinnedVersion && !isNightlyVersion(pinnedVersion)) { + if (undo) { + throw new UpgradeError( + "unsupported_operation", + `Homebrew can't install an earlier version, so undo can't return to ${pinnedVersion} through it. ` + + "Run `sentry cli upgrade undo --method curl` to return to it as a standalone install; " + + "the Homebrew one may still come first in PATH.", + ); + } throw new UpgradeError( "unsupported_operation", "Homebrew does not support installing a specific version. Run 'brew upgrade getsentry/tools/sentry' to upgrade to the latest formula version.", @@ -989,6 +998,7 @@ async function migrateToStandaloneForNightly(opts: { async function resolveContext( version: string | undefined, flags: UpgradeFlags, + undo: boolean, ): Promise<{ channel: ReleaseChannel; versionArg: string | undefined; @@ -1000,7 +1010,7 @@ async function resolveContext( const channelChanged = channel !== currentChannel; const method = flags.method ?? (await detectInstallationMethod()); - validateMethod(method, versionArg, flags.offline); + validateMethod(method, versionArg, flags.offline, undo); return { channel, versionArg, channelChanged, method }; } @@ -1178,6 +1188,7 @@ export const upgradeCommand = buildCommand({ const resolvedContext = await resolveContext( undo ? undo.version : version, flags, + undo !== undefined, ); const { versionArg, method } = resolvedContext; // Undo reinstalls the earlier version on its own channel. Its restored diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts index b8be91267..2ebb89f9c 100644 --- a/packages/cli/src/lib/db/backup.ts +++ b/packages/cli/src/lib/db/backup.ts @@ -284,7 +284,8 @@ function readAccounts(db: Database): AccountRows | undefined { /** * Replace the accounts in `restored` with `current`'s, so the undo keeps - * the logins and refreshed tokens made since the backup. + * the logins and refreshed tokens made since the backup, and logins + * removed since then stay removed. * * @returns A warning when they could not be carried over. */ @@ -293,12 +294,19 @@ function carryOverAccounts( current: AccountRows | undefined, backup: DatabaseBackup, ): string | undefined { - if (!current || current.rows.length === 0) { + if (!current) { return; } const loginHint = `Your logins are as they were in ${backup.version}; run \`sentry auth login\` if you need another.`; if (!tableExists(restored, "accounts")) { - return `${backup.version} stores logins in an older format, so your current logins could not be kept. ${loginHint}`; + if (current.rows.length > 0) { + return `${backup.version} stores logins in an older format, so your current logins could not be kept. ${loginHint}`; + } + // Logged out since the backup: don't bring the older login back. + if (tableExists(restored, "auth")) { + restored.query("DELETE FROM auth").run(); + } + return; } const restoredColumns = new Set(columnNames(restored, "accounts")); const shared = current.columns.filter((c) => restoredColumns.has(c)); diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 3052c4a56..cf631b206 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -1137,6 +1137,22 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(getUndoBackup("0.50.0").version).toBe(CLI_VERSION); }); + test("undo explains that Homebrew can't install the earlier version", async () => { + backupDatabase(getRawDatabase(), "0.50.0", CLI_VERSION); + + const { context, getOutput, errors, restore } = createMockContext({ + homeDir: testDir, + }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "undo", "--method", "brew"], context); + + const output = getOutput() + errors.join(""); + expect(output).toContain("Homebrew can't install an earlier version"); + expect(output).toContain("sentry cli upgrade undo --method curl"); + expect(spawnedArgs).toEqual([]); + }); + test("undo explains when there is nothing to undo", async () => { mockBinaryDownloadWithVersion("99.99.99"); diff --git a/packages/cli/test/lib/db/backup.test.ts b/packages/cli/test/lib/db/backup.test.ts index dc41cd878..e7a6439a1 100644 --- a/packages/cli/test/lib/db/backup.test.ts +++ b/packages/cli/test/lib/db/backup.test.ts @@ -194,6 +194,19 @@ describe("restoreDatabaseBackup", () => { ]); }); + test("keeps the user logged out when they logged out since the backup", () => { + addAccount("a", "token", true); + const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + getRawDatabase().query("DELETE FROM accounts").run(); + + const warnings = restoreDatabaseBackup(getRawDatabase(), backup); + + expect(warnings.join("\n")).not.toContain("logins"); + expect( + readFile((db) => db.query("SELECT account_id FROM accounts").all()), + ).toEqual([]); + }); + test("leaves the restored file alone for the rest of the process", () => { setMarker("saved"); const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); From 7f946ee9deebe153ef09747a0c4f4a3ee3757d54 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:34:05 +0000 Subject: [PATCH 03/10] ref(cli): Record upgrades in backup file names instead of a manifest Each backup is now cli-_to_.db and the newest one is the last upgrade, so undo reads the backup directory instead of a manifest. Backup and restore use async fs calls, the undo argument constant is MAGIC_VERSION_FOR_UNDO, and the upgrade tests derive their versions from CLI_VERSION with semver. Co-Authored-By: BYK --- packages/cli/src/commands/cli/upgrade.ts | 18 +- packages/cli/src/lib/db/backup.ts | 277 +++++++----------- .../cli/test/commands/cli/upgrade.test.ts | 87 ++++-- packages/cli/test/lib/db/backup.test.ts | 143 ++++----- 4 files changed, 259 insertions(+), 266 deletions(-) diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index e4fb519f5..946cea6a5 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -119,9 +119,9 @@ type DatabasePlan = { }; /** Back up the database as this version's before another version owns it. */ -function backupLocalDatabase(target: string): void { +async function backupLocalDatabase(target: string): Promise { try { - backupDatabase(getRawDatabase(), CLI_VERSION, target); + await backupDatabase(getRawDatabase(), CLI_VERSION, target); } catch (error) { log.warn( `Could not back up the local database: ${stringifyUnknown(error)}. ` + @@ -134,16 +134,16 @@ function backupLocalDatabase(target: string): void { * Run once the new binary is downloaded, right before its setup, so a * failed download leaves the database untouched. */ -function prepareDatabaseForNewBinary(plan: DatabasePlan): void { +async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { // Reinstalling this same version must not replace the record of the // upgrade that led here. if (plan.target !== CLI_VERSION) { - backupLocalDatabase(plan.target); + await backupLocalDatabase(plan.target); } if (plan.restore) { try { plan.warnings.push( - ...restoreDatabaseBackup(getRawDatabase(), plan.restore), + ...(await restoreDatabaseBackup(getRawDatabase(), plan.restore)), ); plan.restored = true; return; @@ -172,7 +172,7 @@ function finishDatabaseForNewBinary(plan: DatabasePlan): void { const CHANNEL_VERSIONS = new Set(["nightly", "stable"]); /** Version argument that returns to the version before the last upgrade. */ -const UNDO_VERSION = "undo"; +const MAGIC_VERSION_FOR_UNDO = "undo"; /** * Structured result of the upgrade command. @@ -724,7 +724,7 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { ? { ...process.env, SENTRY_INSTALL_DIR: installDir } : undefined; - prepareDatabaseForNewBinary(database); + await prepareDatabaseForNewBinary(database); let exitCode: number; try { exitCode = await spawnWithRetry(binaryPath, args, env); @@ -1182,8 +1182,8 @@ export const upgradeCommand = buildCommand({ }, async *func(this: SentryContext, flags: UpgradeFlags, version?: string) { const undo = - version?.toLowerCase() === UNDO_VERSION - ? getUndoBackup(CLI_VERSION) + version?.toLowerCase() === MAGIC_VERSION_FOR_UNDO + ? await getUndoBackup(CLI_VERSION) : undefined; const resolvedContext = await resolveContext( undo ? undo.version : version, diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts index 2ebb89f9c..4f95af74f 100644 --- a/packages/cli/src/lib/db/backup.ts +++ b/packages/cli/src/lib/db/backup.ts @@ -2,25 +2,25 @@ * Per-version backups of `cli.db` for `sentry cli upgrade undo` (#1623). * * Before `sentry cli upgrade` hands the database to another version, it - * copies `cli.db` to `backups/cli-.db` under the config directory - * and records which version it moved from. `sentry cli upgrade undo` - * reinstalls that version and puts its own database back, so newer CLIs - * don't have to know how to rewrite their schema for older ones. + * copies `cli.db` to `backups/cli-_to_.db` under the config + * directory. The file name is the whole record: the newest backup is the + * last upgrade, and `sentry cli upgrade undo` run from `` reinstalls + * `` and puts that file back, so newer CLIs don't have to know how to + * rewrite their schema for older ones. * * Backups hold credentials, so the directory is `0700` and files `0600`, * like `cli.db` itself. */ import { - chmodSync, - copyFileSync, - existsSync, - mkdirSync, - readFileSync, - renameSync, - rmSync, - writeFileSync, -} from "node:fs"; + chmod, + copyFile, + mkdir, + readdir, + rename, + rm, + stat, +} from "node:fs/promises"; import { join } from "node:path"; import { stringifyUnknown, UpgradeError } from "../errors.js"; import { logger } from "../logger.js"; @@ -36,7 +36,13 @@ import { Database, type SQLQueryBindings } from "./sqlite.js"; const log = logger.withTag("db.backup"); const BACKUP_SUBDIR = "backups"; -const MANIFEST_FILENAME = "manifest.json"; + +/** Characters semver allows; `_` is not one, so it can separate versions. */ +const VERSION_PATTERN = "[0-9A-Za-z.+-]+"; +const VERSION_REGEX = new RegExp(`^${VERSION_PATTERN}$`); +const BACKUP_FILE_REGEX = new RegExp( + `^cli-(${VERSION_PATTERN})_to_(${VERSION_PATTERN})\\.db$`, +); /** * Backups kept at most. There is one per version, so nightly users would @@ -48,94 +54,65 @@ export const MAX_DATABASE_BACKUPS = 5; export type DatabaseBackup = { /** CLI version that last used this database. */ version: string; + /** Version `sentry cli upgrade` moved to after saving it. */ + toVersion: string; /** File name inside the backup directory. */ file: string; - /** When the copy was made (ms since epoch). */ + /** When the copy was made (ms since epoch), from the file's mtime. */ createdAt: number; }; -/** The most recent version change made through `sentry cli upgrade`. */ -type UpgradeRecord = { - fromVersion: string; - toVersion: string; -}; - -type BackupManifest = { - backups: DatabaseBackup[]; - lastUpgrade?: UpgradeRecord; -}; - -/** Directory holding database backups and their manifest. */ +/** Directory holding database backups. */ export function getBackupDir(): string { return join(getConfigDir(), BACKUP_SUBDIR); } -function manifestPath(): string { - return join(getBackupDir(), MANIFEST_FILENAME); -} - -function isBackup(value: unknown): value is DatabaseBackup { - if (typeof value !== "object" || value === null) { - return false; - } - const entry = value as Record; - return ( - typeof entry.version === "string" && - typeof entry.file === "string" && - typeof entry.createdAt === "number" - ); -} - -function isUpgradeRecord(value: unknown): value is UpgradeRecord { - if (typeof value !== "object" || value === null) { - return false; +function backupFileName(fromVersion: string, toVersion: string): string { + for (const version of [fromVersion, toVersion]) { + if (!VERSION_REGEX.test(version)) { + throw new Error(`Cannot name a database backup for version ${version}`); + } } - const record = value as Record; - return ( - typeof record.fromVersion === "string" && - typeof record.toVersion === "string" - ); + return `cli-${fromVersion}_to_${toVersion}.db`; } -/** Read the manifest, treating a missing or unreadable one as empty. */ -function readManifest(): BackupManifest { - const path = manifestPath(); - if (!existsSync(path)) { - return { backups: [] }; - } +/** Every backup in the backup directory, newest first. */ +async function listBackups(): Promise { + const dir = getBackupDir(); + let files: string[]; try { - const parsed = JSON.parse(readFileSync(path, "utf-8")) as Record< - string, - unknown - >; - const backups = Array.isArray(parsed.backups) - ? parsed.backups.filter(isBackup) - : []; - const lastUpgrade = isUpgradeRecord(parsed.lastUpgrade) - ? parsed.lastUpgrade - : undefined; - return { backups, lastUpgrade }; + files = await readdir(dir); } catch (error) { - log.debug(`Ignoring unreadable backup manifest ${path}`, error); - return { backups: [] }; + log.debug(`No readable backup directory ${dir}`, error); + return []; } + const backups: DatabaseBackup[] = []; + for (const file of files) { + const match = BACKUP_FILE_REGEX.exec(file); + if (!match) { + continue; + } + try { + const { mtimeMs } = await stat(join(dir, file)); + backups.push({ + version: match[1] as string, + toVersion: match[2] as string, + file, + createdAt: mtimeMs, + }); + } catch (error) { + // Removed by a concurrent prune. + log.debug(`Skipping backup ${file}`, error); + } + } + return backups.sort((a, b) => b.createdAt - a.createdAt); } -/** Write the manifest atomically, readable only by the user. */ -function writeManifest(manifest: BackupManifest): void { - const path = manifestPath(); - const tmp = `${path}.${process.pid}.tmp`; - writeFileSync(tmp, `${JSON.stringify(manifest, null, 2)}\n`, { - mode: 0o600, - }); - renameSync(tmp, path); -} - -function ensureBackupDir(): string { +async function ensureBackupDir(): Promise { const dir = getBackupDir(); - mkdirSync(dir, { recursive: true, mode: 0o700 }); + await mkdir(dir, { recursive: true, mode: 0o700 }); try { - chmodSync(dir, 0o700); + await chmod(dir, 0o700); } catch (error) { // Windows has no POSIX modes; the directory inherits the profile's ACLs. log.debug(`Could not restrict ${dir}`, error); @@ -143,120 +120,79 @@ function ensureBackupDir(): string { return dir; } -function restrictFile(path: string): void { +async function restrictFile(path: string): Promise { try { - chmodSync(path, 0o600); + await chmod(path, 0o600); } catch (error) { log.debug(`Could not restrict ${path}`, error); } } -/** File name for a version's backup; versions are plain semver strings. */ -function backupFileName(version: string): string { - return `cli-${version.replace(/[^A-Za-z0-9._-]/g, "_")}.db`; -} - /** - * Keep the newest {@link MAX_DATABASE_BACKUPS} backups, plus the one an undo - * would restore, and delete the files of the rest. - */ -function pruneBackups(manifest: BackupManifest, dir: string): void { - const undoVersion = manifest.lastUpgrade?.fromVersion; - // Entries are appended as they are made, so reversing first keeps the - // newest first among backups made within the same millisecond. - const newestFirst = [...manifest.backups] - .reverse() - .sort((a, b) => b.createdAt - a.createdAt); - const kept: DatabaseBackup[] = []; - for (const entry of newestFirst) { - if (kept.length < MAX_DATABASE_BACKUPS || entry.version === undoVersion) { - kept.push(entry); - continue; - } - rmSync(join(dir, entry.file), { force: true }); - } - manifest.backups = kept; -} - -/** - * Save `db` as `fromVersion`'s backup and record the move to `toVersion`, + * Save `db` as `fromVersion`'s backup, named for the move to `toVersion`, * so `sentry cli upgrade undo` run from `toVersion` can return to it. * - * Replaces any earlier backup of `fromVersion`. Uses `VACUUM INTO`, which - * takes a consistent snapshot even while other CLI processes use `db`. + * Replaces any earlier backup of `fromVersion` and keeps the newest + * {@link MAX_DATABASE_BACKUPS}. Uses `VACUUM INTO`, which takes a + * consistent snapshot even while other CLI processes use `db`. */ -export function backupDatabase( +export async function backupDatabase( db: Database, fromVersion: string, toVersion: string, -): DatabaseBackup { - const dir = ensureBackupDir(); - const manifest = readManifest(); - // Drop the record first: if the copy fails, an undo must not restore a - // backup that predates this upgrade. - if (manifest.lastUpgrade) { - manifest.lastUpgrade = undefined; - writeManifest(manifest); - } - - const file = backupFileName(fromVersion); +): Promise { + const file = backupFileName(fromVersion, toVersion); + const dir = await ensureBackupDir(); const path = join(dir, file); const tmp = `${path}.${process.pid}.tmp`; - rmSync(tmp, { force: true }); + await rm(tmp, { force: true }); try { db.query("VACUUM INTO ?").run(tmp); - restrictFile(tmp); - renameSync(tmp, path); + await restrictFile(tmp); + await rename(tmp, path); } finally { - rmSync(tmp, { force: true }); + await rm(tmp, { force: true }); } - const entry: DatabaseBackup = { - version: fromVersion, - file, - createdAt: Date.now(), - }; - manifest.backups = [ - ...manifest.backups.filter((b) => b.version !== fromVersion), - entry, + // Newest first, so the first ones left after this version's older copies + // are the ones to keep alongside the new backup. + const others = (await listBackups()).filter((b) => b.file !== file); + const stale = [ + ...others.filter((b) => b.version === fromVersion), + ...others + .filter((b) => b.version !== fromVersion) + .slice(MAX_DATABASE_BACKUPS - 1), ]; - manifest.lastUpgrade = { fromVersion, toVersion }; - pruneBackups(manifest, dir); - writeManifest(manifest); - return entry; + await Promise.all(stale.map((b) => rm(join(dir, b.file), { force: true }))); + const { mtimeMs } = await stat(path); + return { version: fromVersion, toVersion, file, createdAt: mtimeMs }; } /** * Find the backup `sentry cli upgrade undo` should restore when run from - * `currentVersion`. + * `currentVersion`: the newest one, if it was made upgrading to it. * - * @throws {UpgradeError} When no upgrade to `currentVersion` was recorded, - * or its backup is gone. + * @throws {UpgradeError} When there is no backup, or the newest one was + * made upgrading to another version. */ -export function getUndoBackup(currentVersion: string): DatabaseBackup { - const { backups, lastUpgrade } = readManifest(); - if (!lastUpgrade) { +export async function getUndoBackup( + currentVersion: string, +): Promise { + const [last] = await listBackups(); + if (!last) { throw new UpgradeError( "unsupported_operation", "Nothing to undo: no upgrade made with `sentry cli upgrade` was recorded.", ); } - if (lastUpgrade.toVersion !== currentVersion) { + if (last.toVersion !== currentVersion) { throw new UpgradeError( "unsupported_operation", - `Nothing to undo: the last recorded upgrade went from ${lastUpgrade.fromVersion} to ${lastUpgrade.toVersion}, ` + + `Nothing to undo: the last recorded upgrade went from ${last.version} to ${last.toVersion}, ` + `but this is ${currentVersion}. It was probably installed another way.`, ); } - const backup = backups.find((b) => b.version === lastUpgrade.fromVersion); - if (!(backup && existsSync(join(getBackupDir(), backup.file)))) { - throw new UpgradeError( - "unsupported_operation", - `Cannot undo: the database backup for ${lastUpgrade.fromVersion} is missing. ` + - `Run \`sentry cli upgrade ${lastUpgrade.fromVersion}\` instead.`, - ); - } - return backup; + return last; } /** Columns of `table` in `db`, in declaration order. */ @@ -338,23 +274,22 @@ function carryOverAccounts( * @param db - This process's open connection to `cli.db` * @returns What the user should know about the restored state */ -export function restoreDatabaseBackup( +export async function restoreDatabaseBackup( db: Database, backup: DatabaseBackup, -): string[] { +): Promise { const source = join(getBackupDir(), backup.file); const dbPath = getDbPath(); const tmp = `${dbPath}.restore-${process.pid}`; - const currentAccounts = readAccounts(db); const warnings: string[] = []; - rmSync(tmp, { force: true }); + await rm(tmp, { force: true }); try { - copyFileSync(source, tmp); - restrictFile(tmp); + await copyFile(source, tmp); + await restrictFile(tmp); const restored = new Database(tmp); try { - const warning = carryOverAccounts(restored, currentAccounts, backup); + const warning = carryOverAccounts(restored, readAccounts(db), backup); if (warning) { warnings.push(warning); } @@ -368,16 +303,16 @@ export function restoreDatabaseBackup( try { // A leftover write-ahead log belongs to the replaced file and would // corrupt the restored one. - rmSync(`${dbPath}-wal`, { force: true }); - rmSync(`${dbPath}-shm`, { force: true }); - renameSync(tmp, dbPath); + await rm(`${dbPath}-wal`, { force: true }); + await rm(`${dbPath}-shm`, { force: true }); + await rename(tmp, dbPath); } catch (error) { // `cli.db` is still this version's; let callers use it again. closeDatabase(); throw error; } } finally { - rmSync(tmp, { force: true }); + await rm(tmp, { force: true }); } const savedOn = new Date(backup.createdAt).toISOString().slice(0, 10); diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index cf631b206..775735c39 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -11,12 +11,19 @@ // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as child_process from "node:child_process"; -import { chmodSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + mkdirSync, + readdirSync, + rmSync, + writeFileSync, +} from "node:fs"; import { unlink } from "node:fs/promises"; import { homedir } from "node:os"; import { delimiter, join } from "node:path"; import { gzipSync } from "node:zlib"; import { run } from "@stricli/core"; +import { coerce as semverCoerce, inc as semverInc } from "semver"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; // Make child_process namespace mutable so vi.spyOn works on ESM exports @@ -39,7 +46,11 @@ import type { SentryContext } from "../../../src/context.js"; // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as binary from "../../../src/lib/binary.js"; import { CLI_VERSION } from "../../../src/lib/constants.js"; -import { backupDatabase, getUndoBackup } from "../../../src/lib/db/backup.js"; +import { + backupDatabase, + getBackupDir, + getUndoBackup, +} from "../../../src/lib/db/backup.js"; import { getDbPath, getRawDatabase } from "../../../src/lib/db/index.js"; import { clearInstallInfo, @@ -871,6 +882,22 @@ function fakeChildProcess(exitCode: number): child_process.ChildProcess { return emitter as unknown as child_process.ChildProcess; } +/** + * The release `steps` minor versions after this build, so tests that move + * between versions follow CLI_VERSION instead of hard-coding releases. + * CLI_VERSION is `0.0.0-dev` in tests, so there is no release before it. + */ +function releaseAfter(steps: number): string { + let version = semverCoerce(CLI_VERSION)?.version ?? "0.0.0"; + for (let i = 0; i < steps; i++) { + version = semverInc(version, "minor") ?? version; + } + return version; +} + +const NEXT_RELEASE = releaseAfter(1); +const LATER_RELEASE = releaseAfter(2); + describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy)", () => { useTestConfigDir("test-upgrade-spawn-"); @@ -1078,11 +1105,11 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy }); test("backs up the database before the new version runs setup", async () => { - mockBinaryDownloadWithVersion("99.99.99"); - let backupAtSetup: string | undefined; + mockBinaryDownloadWithVersion(NEXT_RELEASE); + let backupsAtSetup: string[] = []; spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { spawnedArgs.push({ cmd, args: [...(args ?? [])] }); - backupAtSetup = getUndoBackup("99.99.99").version; + backupsAtSetup = readdirSync(getBackupDir()); return fakeChildProcess(0); }); @@ -1091,22 +1118,30 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy await run(app, ["cli", "upgrade", "--method", "curl"], context); - expect(backupAtSetup).toBe(CLI_VERSION); + expect(backupsAtSetup).toEqual([ + `cli-${CLI_VERSION}_to_${NEXT_RELEASE}.db`, + ]); + expect((await getUndoBackup(NEXT_RELEASE)).version).toBe(CLI_VERSION); }); test("undo reinstalls the previous version with its own database", async () => { const marker = "test.marker"; - setMetadata(getRawDatabase(), { [marker]: "saved by 0.50.0" }); - backupDatabase(getRawDatabase(), "0.50.0", CLI_VERSION); + setMetadata(getRawDatabase(), { [marker]: `saved by ${NEXT_RELEASE}` }); + await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); setMetadata(getRawDatabase(), { [marker]: "changed since" }); const fakeContent = new Uint8Array([0x7f, 0x45, 0x4c, 0x46]); mockFetch(async (url) => { - if (extractInputUrl(url).includes("releases/tags/cli%400.50.0")) { - return new Response(JSON.stringify({ tag_name: "cli@0.50.0" }), { - status: 200, - headers: { "content-type": "application/json" }, - }); + if ( + extractInputUrl(url).includes(`releases/tags/cli%40${NEXT_RELEASE}`) + ) { + return new Response( + JSON.stringify({ tag_name: `cli@${NEXT_RELEASE}` }), + { + status: 200, + headers: { "content-type": "application/json" }, + }, + ); } return new Response(gzipSync(fakeContent), { status: 200 }); }); @@ -1129,16 +1164,16 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); - expect(getOutput()).toContain("Restored the database 0.50.0"); + expect(getOutput()).toContain(`Restored the database ${NEXT_RELEASE}`); const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); expect(setupCall?.args).toContain("stable"); - expect(markerAtSetup).toBe("saved by 0.50.0"); + expect(markerAtSetup).toBe(`saved by ${NEXT_RELEASE}`); // This version's database is kept, so the undo can be undone in turn. - expect(getUndoBackup("0.50.0").version).toBe(CLI_VERSION); + expect((await getUndoBackup(NEXT_RELEASE)).version).toBe(CLI_VERSION); }); test("undo explains that Homebrew can't install the earlier version", async () => { - backupDatabase(getRawDatabase(), "0.50.0", CLI_VERSION); + await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); const { context, getOutput, errors, restore } = createMockContext({ homeDir: testDir, @@ -1153,8 +1188,24 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(spawnedArgs).toEqual([]); }); + test("undo refuses when the last upgrade went to another version", async () => { + await backupDatabase(getRawDatabase(), NEXT_RELEASE, LATER_RELEASE); + + const { context, getOutput, errors, restore } = createMockContext({ + homeDir: testDir, + }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); + + expect(getOutput() + errors.join("")).toContain( + `went from ${NEXT_RELEASE} to ${LATER_RELEASE}`, + ); + expect(spawnedArgs).toEqual([]); + }); + test("undo explains when there is nothing to undo", async () => { - mockBinaryDownloadWithVersion("99.99.99"); + mockBinaryDownloadWithVersion(NEXT_RELEASE); const { context, getOutput, errors, restore } = createMockContext({ homeDir: testDir, diff --git a/packages/cli/test/lib/db/backup.test.ts b/packages/cli/test/lib/db/backup.test.ts index e7a6439a1..952558023 100644 --- a/packages/cli/test/lib/db/backup.test.ts +++ b/packages/cli/test/lib/db/backup.test.ts @@ -2,13 +2,8 @@ * Per-version database backups for `sentry cli upgrade undo` (#1623). */ -import { - existsSync, - readFileSync, - rmSync, - statSync, - writeFileSync, -} from "node:fs"; +import { statSync } from "node:fs"; +import { readdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { @@ -60,51 +55,59 @@ function permissions(path: string): number { return statSync(path).mode & 0o777; } -function manifest(): { - backups: { version: string }[]; - lastUpgrade?: { fromVersion: string; toVersion: string }; -} { - return JSON.parse( - readFileSync(join(getBackupDir(), "manifest.json"), "utf-8"), - ); +/** Backup file names in the backup directory, sorted. */ +async function backupFiles(): Promise { + return (await readdir(getBackupDir())).sort(); } describe("backupDatabase", () => { - test("saves the database and records the upgrade", () => { + test("saves the database under a name recording the upgrade", async () => { setMarker("before"); - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); setMarker("after"); + expect(backup.file).toBe("cli-1.0.0_to_2.0.0.db"); const copy = new Database(join(getBackupDir(), backup.file)); try { expect(getMetadata(copy, [MARKER]).get(MARKER)).toBe("before"); } finally { copy.close(); } - expect(manifest().lastUpgrade).toEqual({ - fromVersion: "1.0.0", - toVersion: "2.0.0", - }); - expect(getUndoBackup("2.0.0")).toEqual(backup); + expect(await getUndoBackup("2.0.0")).toEqual(backup); + }); + + test("keeps prerelease and build versions in the name", async () => { + const backup = await backupDatabase( + getRawDatabase(), + "1.0.0-dev.1700000000", + "1.0.0+build.5", + ); + + expect(await getUndoBackup("1.0.0+build.5")).toEqual(backup); + }); + + test("rejects a version that can't be part of a file name", async () => { + await expect( + backupDatabase(getRawDatabase(), "../1.0.0", "2.0.0"), + ).rejects.toThrow("Cannot name a database backup"); }); test.skipIf(process.platform === "win32")( "keeps backups readable only by the user", - () => { - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + async () => { + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); expect(permissions(getBackupDir())).toBe(0o700); expect(permissions(join(getBackupDir(), backup.file))).toBe(0o600); - expect(permissions(join(getBackupDir(), "manifest.json"))).toBe(0o600); }, ); - test("keeps one backup per version", () => { + test("keeps one backup per version", async () => { setMarker("first"); - backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); setMarker("second"); - const backup = backupDatabase(getRawDatabase(), "1.0.0", "3.0.0"); + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "3.0.0"); - expect(manifest().backups.map((b) => b.version)).toEqual(["1.0.0"]); + expect(await backupFiles()).toEqual(["cli-1.0.0_to_3.0.0.db"]); const copy = new Database(join(getBackupDir(), backup.file)); try { expect(getMetadata(copy, [MARKER]).get(MARKER)).toBe("second"); @@ -113,28 +116,25 @@ describe("backupDatabase", () => { } }); - test("prunes the oldest backups", () => { + test("prunes the oldest backups", async () => { const files: string[] = []; for (let i = 0; i <= MAX_DATABASE_BACKUPS; i++) { - files.push( - backupDatabase(getRawDatabase(), `1.0.${i}`, `1.0.${i + 1}`).file, + const { file } = await backupDatabase( + getRawDatabase(), + `1.0.${i}`, + `1.0.${i + 1}`, ); + files.push(file); } - const kept = manifest().backups.map((b) => b.version); - expect(kept).toHaveLength(MAX_DATABASE_BACKUPS); - expect(kept).not.toContain("1.0.0"); - expect(existsSync(join(getBackupDir(), files[0] as string))).toBe(false); - expect( - existsSync(join(getBackupDir(), files[MAX_DATABASE_BACKUPS] as string)), - ).toBe(true); + expect(await backupFiles()).toEqual(files.slice(1).sort()); }); }); describe("getUndoBackup", () => { - function undoError(version: string): string { + async function undoError(version: string): Promise { try { - getUndoBackup(version); + await getUndoBackup(version); } catch (error) { expect(error).toBeInstanceOf(UpgradeError); return (error as Error).message; @@ -142,33 +142,40 @@ describe("getUndoBackup", () => { throw new Error("expected getUndoBackup to throw"); } - test("fails when no upgrade was recorded", () => { - expect(undoError("2.0.0")).toContain("Nothing to undo"); + test("fails when no upgrade was recorded", async () => { + expect(await undoError("2.0.0")).toContain("Nothing to undo"); + }); + + test("fails when this version was installed another way", async () => { + await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + expect(await undoError("3.0.0")).toContain("went from 1.0.0 to 2.0.0"); }); - test("fails when this version was installed another way", () => { - backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); - expect(undoError("3.0.0")).toContain("went from 1.0.0 to 2.0.0"); + test("only undoes the newest upgrade", async () => { + await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + await backupDatabase(getRawDatabase(), "2.0.0", "3.0.0"); + expect(await undoError("2.0.0")).toContain("went from 2.0.0 to 3.0.0"); }); - test("fails when the backup file is gone", () => { - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); - rmSync(join(getBackupDir(), backup.file)); - expect(undoError("2.0.0")).toContain("backup for 1.0.0 is missing"); + test("fails when the backup file is gone", async () => { + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + await rm(join(getBackupDir(), backup.file)); + expect(await undoError("2.0.0")).toContain("Nothing to undo"); }); - test("treats an unreadable manifest as nothing to undo", () => { - backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); - writeFileSync(join(getBackupDir(), "manifest.json"), "not json"); - expect(undoError("2.0.0")).toContain("Nothing to undo"); + test("ignores other files in the backup directory", async () => { + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + await writeFile(join(getBackupDir(), "notes.txt"), "not a backup"); + await writeFile(join(getBackupDir(), "cli-9.0.0.db"), ""); + expect(await getUndoBackup("2.0.0")).toEqual(backup); }); }); describe("restoreDatabaseBackup", () => { - test("puts the backup back and keeps the current logins", () => { + test("puts the backup back and keeps the current logins", async () => { addAccount("a", "old-token", true); setMarker("saved"); - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); getRawDatabase() .query("UPDATE accounts SET token = ? WHERE account_id = 'a'") @@ -176,7 +183,7 @@ describe("restoreDatabaseBackup", () => { addAccount("b", "new-login", false); setMarker("changed"); - const warnings = restoreDatabaseBackup(getRawDatabase(), backup); + const warnings = await restoreDatabaseBackup(getRawDatabase(), backup); expect(warnings.join("\n")).toContain("Restored the database 1.0.0"); const restored = readFile((db) => ({ @@ -194,12 +201,12 @@ describe("restoreDatabaseBackup", () => { ]); }); - test("keeps the user logged out when they logged out since the backup", () => { + test("keeps the user logged out when they logged out since the backup", async () => { addAccount("a", "token", true); - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); getRawDatabase().query("DELETE FROM accounts").run(); - const warnings = restoreDatabaseBackup(getRawDatabase(), backup); + const warnings = await restoreDatabaseBackup(getRawDatabase(), backup); expect(warnings.join("\n")).not.toContain("logins"); expect( @@ -207,10 +214,10 @@ describe("restoreDatabaseBackup", () => { ).toEqual([]); }); - test("leaves the restored file alone for the rest of the process", () => { + test("leaves the restored file alone for the rest of the process", async () => { setMarker("saved"); - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); - restoreDatabaseBackup(getRawDatabase(), backup); + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + await restoreDatabaseBackup(getRawDatabase(), backup); setMetadata(getDatabase(), { [MARKER]: "written after restore" }); @@ -220,9 +227,9 @@ describe("restoreDatabaseBackup", () => { closeDatabase(); }); - test("warns when the backup predates multi-account storage", () => { + test("warns when the backup predates multi-account storage", async () => { addAccount("a", "token", true); - const backup = backupDatabase(getRawDatabase(), "0.47.0", "2.0.0"); + const backup = await backupDatabase(getRawDatabase(), "0.47.0", "2.0.0"); const copy = new Database(join(getBackupDir(), backup.file)); try { copy.exec("DROP TABLE accounts"); @@ -230,7 +237,7 @@ describe("restoreDatabaseBackup", () => { copy.close(); } - const warnings = restoreDatabaseBackup(getRawDatabase(), backup); + const warnings = await restoreDatabaseBackup(getRawDatabase(), backup); expect(warnings.join("\n")).toContain( "0.47.0 stores logins in an older format", @@ -247,14 +254,14 @@ describe.each(["node", "wasm"] as const)("round trip [%s driver]", (kind) => { __setDriverForTests(null); }); - test("backs up and restores the database", () => { + test("backs up and restores the database", async () => { expect(getRawDatabase().driverKind).toBe(kind); addAccount("a", "token", true); setMarker("saved"); - const backup = backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); + const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); setMarker("changed"); - restoreDatabaseBackup(getRawDatabase(), backup); + await restoreDatabaseBackup(getRawDatabase(), backup); expect(readFile((db) => getMetadata(db, [MARKER]).get(MARKER))).toBe( "saved", From fae6db71c617e288a96dfc21e2ef5b3eb7010014 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:43:04 +0000 Subject: [PATCH 04/10] fix(cli): Revert the backup and restore when curl setup fails The new binary installs itself during setup, so a failed setup leaves this version installed. The backup made for that upgrade then hid the earlier one from undo, and a failed undo left the older version's database in place. Both are now reverted, and this process uses its own database again. --- packages/cli/src/commands/cli/upgrade.ts | 49 +++++++++++- packages/cli/src/lib/db/backup.ts | 25 ++++++ .../cli/test/commands/cli/upgrade.test.ts | 76 +++++++++++++++---- 3 files changed, 131 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index 946cea6a5..d6f1c22f4 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -34,7 +34,9 @@ import { CLI_VERSION } from "../../lib/constants.js"; import { backupDatabase, type DatabaseBackup, + discardDatabaseBackup, getUndoBackup, + reinstateDatabaseBackup, restoreDatabaseBackup, } from "../../lib/db/backup.js"; import { getDbPath, getRawDatabase } from "../../lib/db/index.js"; @@ -114,19 +116,24 @@ type DatabasePlan = { restore?: DatabaseBackup; /** Set once `restore` succeeded; the hand-off is then unnecessary. */ restored?: boolean; + /** This version's database, saved before the new binary's setup. */ + backup?: DatabaseBackup; /** Notes about the database for the command result. */ warnings: string[]; }; /** Back up the database as this version's before another version owns it. */ -async function backupLocalDatabase(target: string): Promise { +async function backupLocalDatabase( + target: string, +): Promise { try { - await backupDatabase(getRawDatabase(), CLI_VERSION, target); + return await backupDatabase(getRawDatabase(), CLI_VERSION, target); } catch (error) { log.warn( `Could not back up the local database: ${stringifyUnknown(error)}. ` + "`sentry cli upgrade undo` won't be able to return to this version.", ); + return; } } @@ -138,7 +145,7 @@ async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { // Reinstalling this same version must not replace the record of the // upgrade that led here. if (plan.target !== CLI_VERSION) { - await backupLocalDatabase(plan.target); + plan.backup = await backupLocalDatabase(plan.target); } if (plan.restore) { try { @@ -158,6 +165,34 @@ async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { } } +/** + * Revert this run's backup and restore after a setup that installs the new + * binary itself failed, so this version, still the installed one, keeps its + * own database and `sentry cli upgrade undo` still finds the upgrade that led + * here instead of the one that failed. + */ +async function revertDatabaseForFailedInstall( + plan: DatabasePlan, +): Promise { + const { backup } = plan; + plan.backup = undefined; + if (!backup) { + return; + } + try { + if (plan.restored) { + await reinstateDatabaseBackup(backup); + plan.restored = false; + } else { + await discardDatabaseBackup(backup); + } + } catch (error) { + log.warn( + `Could not revert the local database after the failed setup: ${stringifyUnknown(error)}.`, + ); + } +} + /** * Run after setup, even when it fails: another invocation of this CLI may * have re-migrated the database meanwhile. @@ -725,10 +760,16 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { : undefined; await prepareDatabaseForNewBinary(database); - let exitCode: number; + let exitCode: number | undefined; try { exitCode = await spawnWithRetry(binaryPath, args, env); } finally { + // With --install the new binary installs itself during setup, so a + // failed setup leaves this version installed. Package managers install + // before setup, so their new version is in place either way. + if (install && exitCode !== 0) { + await revertDatabaseForFailedInstall(database); + } finishDatabaseForNewBinary(database); } if (exitCode !== 0) { diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts index 4f95af74f..6c0bd7eed 100644 --- a/packages/cli/src/lib/db/backup.ts +++ b/packages/cli/src/lib/db/backup.ts @@ -195,6 +195,31 @@ export async function getUndoBackup( return last; } +/** Delete `backup`, made for an upgrade that didn't happen. */ +export async function discardDatabaseBackup( + backup: DatabaseBackup, +): Promise { + await rm(join(getBackupDir(), backup.file), { force: true }); +} + +/** + * Move `backup` back into place as `cli.db` and let this process use it + * again, reverting a {@link restoreDatabaseBackup} whose version was never + * installed. The backup file is gone afterwards, as if the upgrade it was + * made for never happened. + */ +export async function reinstateDatabaseBackup( + backup: DatabaseBackup, +): Promise { + const dbPath = getDbPath(); + // The write-ahead log belongs to the file being replaced. + await rm(`${dbPath}-wal`, { force: true }); + await rm(`${dbPath}-shm`, { force: true }); + await rename(join(getBackupDir(), backup.file), dbPath); + // Ends the detachment from `restoreDatabaseBackup`. + closeDatabase(); +} + /** Columns of `table` in `db`, in declaration order. */ function columnNames(db: Database, table: string): string[] { const rows = db diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 775735c39..faa80f69c 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -976,6 +976,20 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy }); } + /** Mock fetch to serve a pinned release's tag and its binary. */ + function mockTaggedBinaryDownload(version: string): void { + const gzipped = gzipSync(new Uint8Array([0x7f, 0x45, 0x4c, 0x46])); + mockFetch(async (url) => { + if (extractInputUrl(url).includes(`releases/tags/cli%40${version}`)) { + return new Response(JSON.stringify({ tag_name: `cli@${version}` }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response(gzipped, { status: 200 }); + }); + } + test("runs setup on downloaded binary after curl upgrade", async () => { mockBinaryDownloadWithVersion("99.99.99"); @@ -1130,21 +1144,7 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); setMetadata(getRawDatabase(), { [marker]: "changed since" }); - const fakeContent = new Uint8Array([0x7f, 0x45, 0x4c, 0x46]); - mockFetch(async (url) => { - if ( - extractInputUrl(url).includes(`releases/tags/cli%40${NEXT_RELEASE}`) - ) { - return new Response( - JSON.stringify({ tag_name: `cli@${NEXT_RELEASE}` }), - { - status: 200, - headers: { "content-type": "application/json" }, - }, - ); - } - return new Response(gzipSync(fakeContent), { status: 200 }); - }); + mockTaggedBinaryDownload(NEXT_RELEASE); let markerAtSetup: string | undefined; spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { spawnedArgs.push({ cmd, args: [...(args ?? [])] }); @@ -1172,6 +1172,52 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect((await getUndoBackup(NEXT_RELEASE)).version).toBe(CLI_VERSION); }); + test("a failed undo keeps this version's database and its undo", async () => { + const marker = "test.marker"; + setMetadata(getRawDatabase(), { [marker]: `saved by ${NEXT_RELEASE}` }); + await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); + setMetadata(getRawDatabase(), { [marker]: "current" }); + mockTaggedBinaryDownload(NEXT_RELEASE); + spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])] }); + return fakeChildProcess(1); + }); + + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); + + expect(spawnedArgs.some((entry) => entry.args.includes("setup"))).toBe( + true, + ); + // This process uses `cli.db` again, and it is this version's. + expect(getMetadata(getRawDatabase(), [marker]).get(marker)).toBe("current"); + expect(readdirSync(getBackupDir())).toEqual([ + `cli-${NEXT_RELEASE}_to_${CLI_VERSION}.db`, + ]); + expect((await getUndoBackup(CLI_VERSION)).version).toBe(NEXT_RELEASE); + }); + + test("a failed upgrade keeps the undo of the one before", async () => { + await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); + mockBinaryDownloadWithVersion(LATER_RELEASE); + spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])] }); + return fakeChildProcess(1); + }); + + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "--method", "curl"], context); + + expect(spawnedArgs.some((entry) => entry.args.includes("setup"))).toBe( + true, + ); + expect((await getUndoBackup(CLI_VERSION)).version).toBe(NEXT_RELEASE); + }); + test("undo explains that Homebrew can't install the earlier version", async () => { await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); From 68d9123b45b7f7d4f92a429eb8ad0253fc8ce2ce Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:51:59 +0000 Subject: [PATCH 05/10] fix(cli): Keep the database as is when a hand-off setup fails A #1611 hand-off is redone after a failed setup, so reinstating this version's database first had it wiped, and discarding the backup threw away the only intact copy. A failed setup that needed the hand-off now keeps the backup and leaves a restored database in place. --- packages/cli/src/commands/cli/upgrade.ts | 8 +++- .../cli/test/commands/cli/upgrade.test.ts | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index d6f1c22f4..0cccc1fa1 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -170,15 +170,19 @@ async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { * binary itself failed, so this version, still the installed one, keeps its * own database and `sentry cli upgrade undo` still finds the upgrade that led * here instead of the one that failed. + * + * Skipped for a #1611 hand-off: it is redone after a failed setup, which + * would wipe a reinstated database, and the backup is then the only intact + * copy of this version's database, so it is kept. */ async function revertDatabaseForFailedInstall( plan: DatabasePlan, ): Promise { const { backup } = plan; - plan.backup = undefined; - if (!backup) { + if (!backup || plan.handOff) { return; } + plan.backup = undefined; try { if (plan.restored) { await reinstateDatabaseBackup(backup); diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index faa80f69c..f70513ed7 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -1100,6 +1100,49 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy true, ); expect(readSchemaVersion()).toBe(16); + // The hand-off emptied `cli.db`, so its backup is the only intact copy. + expect(readdirSync(getBackupDir())).toEqual([ + `cli-${CLI_VERSION}_to_0.47.0.db`, + ]); + }); + + test("a failed undo to a CLI that needs the hand-off keeps the logins", async () => { + getRawDatabase() + .query( + "INSERT INTO accounts (account_id, host, user_id, token, is_active) VALUES (?, ?, ?, ?, ?)", + ) + .run("a", "https://sentry.io", "user-a", "token", 1); + await backupDatabase(getRawDatabase(), "0.47.0", CLI_VERSION); + mockTaggedBinaryDownload("0.47.0"); + const downgradeSpy = vi.spyOn(binary, "isDowngrade").mockReturnValue(true); + spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])] }); + return fakeChildProcess(1); + }); + + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + try { + await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); + } finally { + downgradeSpy.mockRestore(); + } + + expect(spawnedArgs.some((entry) => entry.args.includes("setup"))).toBe( + true, + ); + const db = new Database(getDbPath()); + try { + expect(db.query("SELECT account_id FROM accounts").all()).toEqual([ + { account_id: "a" }, + ]); + } finally { + db.close(); + } + expect(readdirSync(getBackupDir())).toContain( + `cli-${CLI_VERSION}_to_0.47.0.db`, + ); }); test("leaves the database alone for an older CLI that adapts it", async () => { From 4483a903e6a4916248ed331bee211ab9288f6c25 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:00:22 +0000 Subject: [PATCH 06/10] fix(cli): Reinstate the database after a failed undo that needed a hand-off A restore makes the #1611 hand-off unnecessary, so a failed undo to a CLI below 0.48.0 can put this version's database back and skip the hand-off like any other failed undo. --- packages/cli/src/commands/cli/upgrade.ts | 9 +++---- .../cli/test/commands/cli/upgrade.test.ts | 24 +++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index 0cccc1fa1..6ac444be7 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -171,15 +171,14 @@ async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { * own database and `sentry cli upgrade undo` still finds the upgrade that led * here instead of the one that failed. * - * Skipped for a #1611 hand-off: it is redone after a failed setup, which - * would wipe a reinstated database, and the backup is then the only intact - * copy of this version's database, so it is kept. + * A #1611 hand-off without a restore is redone after a failed setup, so its + * backup, then the only intact copy of this version's database, is kept. */ async function revertDatabaseForFailedInstall( plan: DatabasePlan, ): Promise { const { backup } = plan; - if (!backup || plan.handOff) { + if (!backup || (plan.handOff && !plan.restored)) { return; } plan.backup = undefined; @@ -187,6 +186,8 @@ async function revertDatabaseForFailedInstall( if (plan.restored) { await reinstateDatabaseBackup(backup); plan.restored = false; + // This version keeps its database, so there is nothing to hand off. + plan.handOff = false; } else { await discardDatabaseBackup(backup); } diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index f70513ed7..8a824fbb0 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -1106,13 +1106,16 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy ]); }); - test("a failed undo to a CLI that needs the hand-off keeps the logins", async () => { + test("a failed undo to a CLI that needs the hand-off keeps this version's database", async () => { + const marker = "test.marker"; getRawDatabase() .query( "INSERT INTO accounts (account_id, host, user_id, token, is_active) VALUES (?, ?, ?, ?, ?)", ) .run("a", "https://sentry.io", "user-a", "token", 1); + setMetadata(getRawDatabase(), { [marker]: "saved by 0.47.0" }); await backupDatabase(getRawDatabase(), "0.47.0", CLI_VERSION); + setMetadata(getRawDatabase(), { [marker]: "current" }); mockTaggedBinaryDownload("0.47.0"); const downgradeSpy = vi.spyOn(binary, "isDowngrade").mockReturnValue(true); spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { @@ -1132,17 +1135,14 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(spawnedArgs.some((entry) => entry.args.includes("setup"))).toBe( true, ); - const db = new Database(getDbPath()); - try { - expect(db.query("SELECT account_id FROM accounts").all()).toEqual([ - { account_id: "a" }, - ]); - } finally { - db.close(); - } - expect(readdirSync(getBackupDir())).toContain( - `cli-${CLI_VERSION}_to_0.47.0.db`, - ); + // Reinstated and not handed off: the logins and schema are intact. + const db = getRawDatabase(); + expect(getMetadata(db, [marker]).get(marker)).toBe("current"); + expect(db.query("SELECT account_id FROM accounts").all()).toEqual([ + { account_id: "a" }, + ]); + expect(readSchemaVersion()).toBe(CURRENT_SCHEMA_VERSION); + expect((await getUndoBackup(CLI_VERSION)).version).toBe("0.47.0"); }); test("leaves the database alone for an older CLI that adapts it", async () => { From 07cb5e6a44ee1dc11a74f5735876e2137e06f680 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:21:08 +0000 Subject: [PATCH 07/10] fix(cli): Put this version's database back when a hand-off setup fails Warden: a failed curl downgrade to a pre-0.48 CLI kept the handed-off database, with its logins deleted, while this version stayed installed. The backup is now moved back into place and the hand-off isn't redone. Without a backup, the hand-off is still redone as before. listBackups also scans the directory with opendir, per review. --- packages/cli/src/commands/cli/upgrade.ts | 24 +++++------ packages/cli/src/lib/db/backup.ts | 37 +++++++++-------- .../cli/test/commands/cli/upgrade.test.ts | 41 ++++++++++++++++--- packages/cli/test/lib/db/backup.test.ts | 3 +- 4 files changed, 70 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index 6ac444be7..a7dc02c0d 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -166,31 +166,28 @@ async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { } /** - * Revert this run's backup and restore after a setup that installs the new - * binary itself failed, so this version, still the installed one, keeps its - * own database and `sentry cli upgrade undo` still finds the upgrade that led - * here instead of the one that failed. - * - * A #1611 hand-off without a restore is redone after a failed setup, so its - * backup, then the only intact copy of this version's database, is kept. + * Revert this run's backup, restore and hand-off after a setup that installs + * the new binary itself failed, so this version, still the installed one, + * keeps its own database and `sentry cli upgrade undo` still finds the + * upgrade that led here instead of the one that failed. */ async function revertDatabaseForFailedInstall( plan: DatabasePlan, ): Promise { const { backup } = plan; - if (!backup || (plan.handOff && !plan.restored)) { + if (!backup) { return; } plan.backup = undefined; try { - if (plan.restored) { + if (plan.restored || plan.handOff) { await reinstateDatabaseBackup(backup); - plan.restored = false; - // This version keeps its database, so there is nothing to hand off. - plan.handOff = false; } else { await discardDatabaseBackup(backup); } + // This version keeps its database, so there is nothing to hand off. + plan.restored = false; + plan.handOff = false; } catch (error) { log.warn( `Could not revert the local database after the failed setup: ${stringifyUnknown(error)}.`, @@ -200,7 +197,8 @@ async function revertDatabaseForFailedInstall( /** * Run after setup, even when it fails: another invocation of this CLI may - * have re-migrated the database meanwhile. + * have re-migrated the database meanwhile. A failure reverted by + * {@link revertDatabaseForFailedInstall} has nothing left to hand off. */ function finishDatabaseForNewBinary(plan: DatabasePlan): void { if (plan.handOff && !plan.restored) { diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts index 6c0bd7eed..bca7521bc 100644 --- a/packages/cli/src/lib/db/backup.ts +++ b/packages/cli/src/lib/db/backup.ts @@ -12,11 +12,12 @@ * like `cli.db` itself. */ +import type { Dirent } from "node:fs"; import { chmod, copyFile, mkdir, - readdir, + opendir, rename, rm, stat, @@ -79,30 +80,30 @@ function backupFileName(fromVersion: string, toVersion: string): string { /** Every backup in the backup directory, newest first. */ async function listBackups(): Promise { const dir = getBackupDir(); - let files: string[]; + let entries: AsyncIterable; try { - files = await readdir(dir); + entries = await opendir(dir); } catch (error) { - log.debug(`No readable backup directory ${dir}`, error); + log.debug(`Cannot read backup directory ${dir}`, error); return []; } const backups: DatabaseBackup[] = []; - for (const file of files) { - const match = BACKUP_FILE_REGEX.exec(file); + for await (const entry of entries) { + const match = entry.isFile() ? BACKUP_FILE_REGEX.exec(entry.name) : null; if (!match) { continue; } try { - const { mtimeMs } = await stat(join(dir, file)); + const { mtimeMs } = await stat(join(dir, entry.name)); backups.push({ version: match[1] as string, toVersion: match[2] as string, - file, + file: entry.name, createdAt: mtimeMs, }); } catch (error) { // Removed by a concurrent prune. - log.debug(`Skipping backup ${file}`, error); + log.debug(`Skipping backup ${entry.name}`, error); } } return backups.sort((a, b) => b.createdAt - a.createdAt); @@ -204,7 +205,7 @@ export async function discardDatabaseBackup( /** * Move `backup` back into place as `cli.db` and let this process use it - * again, reverting a {@link restoreDatabaseBackup} whose version was never + * again, reverting a restore or hand-off made for a version that was never * installed. The backup file is gone afterwards, as if the upgrade it was * made for never happened. */ @@ -212,12 +213,16 @@ export async function reinstateDatabaseBackup( backup: DatabaseBackup, ): Promise { const dbPath = getDbPath(); - // The write-ahead log belongs to the file being replaced. - await rm(`${dbPath}-wal`, { force: true }); - await rm(`${dbPath}-shm`, { force: true }); - await rename(join(getBackupDir(), backup.file), dbPath); - // Ends the detachment from `restoreDatabaseBackup`. - closeDatabase(); + // Nothing in this process may use the file being replaced meanwhile. + detachDatabase(); + try { + // The write-ahead log belongs to the file being replaced. + await rm(`${dbPath}-wal`, { force: true }); + await rm(`${dbPath}-shm`, { force: true }); + await rename(join(getBackupDir(), backup.file), dbPath); + } finally { + closeDatabase(); + } } /** Columns of `table` in `db`, in declaration order. */ diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 8a824fbb0..7bc1a8571 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -1075,7 +1075,9 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(readSchemaVersion()).toBe(CURRENT_SCHEMA_VERSION); }); - test("redoes the hand-off after setup fails", async () => { + test("redoes the hand-off after setup fails without a backup", async () => { + // A file where the backup directory goes makes the backup fail. + writeFileSync(getBackupDir(), ""); mockBinaryDownloadWithVersion("0.47.0"); const downgradeSpy = vi.spyOn(binary, "isDowngrade").mockReturnValue(true); spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { @@ -1094,16 +1096,45 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy await run(app, ["cli", "upgrade", "--method", "curl"], context); } finally { downgradeSpy.mockRestore(); + rmSync(getBackupDir(), { force: true }); } expect(spawnedArgs.some((entry) => entry.args.includes("setup"))).toBe( true, ); expect(readSchemaVersion()).toBe(16); - // The hand-off emptied `cli.db`, so its backup is the only intact copy. - expect(readdirSync(getBackupDir())).toEqual([ - `cli-${CLI_VERSION}_to_0.47.0.db`, - ]); + }); + + test("puts this version's database back after a hand-off setup fails", async () => { + getRawDatabase() + .query( + "INSERT INTO accounts (account_id, host, user_id, token, is_active) VALUES (?, ?, ?, ?, ?)", + ) + .run("a", "https://sentry.io", "user-a", "token", 1); + mockBinaryDownloadWithVersion("0.47.0"); + const downgradeSpy = vi.spyOn(binary, "isDowngrade").mockReturnValue(true); + let versionAtSetup: number | undefined; + spawnSpy.mockImplementation((cmd: string, args?: readonly string[]) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])] }); + versionAtSetup = readSchemaVersion(); + return fakeChildProcess(1); + }); + + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + try { + await run(app, ["cli", "upgrade", "--method", "curl"], context); + } finally { + downgradeSpy.mockRestore(); + } + + expect(versionAtSetup).toBe(16); + expect(readSchemaVersion()).toBe(CURRENT_SCHEMA_VERSION); + expect( + getRawDatabase().query("SELECT account_id FROM accounts").all(), + ).toEqual([{ account_id: "a" }]); + expect(readdirSync(getBackupDir())).toEqual([]); }); test("a failed undo to a CLI that needs the hand-off keeps this version's database", async () => { diff --git a/packages/cli/test/lib/db/backup.test.ts b/packages/cli/test/lib/db/backup.test.ts index 952558023..141cc6956 100644 --- a/packages/cli/test/lib/db/backup.test.ts +++ b/packages/cli/test/lib/db/backup.test.ts @@ -3,7 +3,7 @@ */ import { statSync } from "node:fs"; -import { readdir, rm, writeFile } from "node:fs/promises"; +import { mkdir, readdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { @@ -167,6 +167,7 @@ describe("getUndoBackup", () => { const backup = await backupDatabase(getRawDatabase(), "1.0.0", "2.0.0"); await writeFile(join(getBackupDir(), "notes.txt"), "not a backup"); await writeFile(join(getBackupDir(), "cli-9.0.0.db"), ""); + await mkdir(join(getBackupDir(), "cli-9.0.0_to_2.0.0.db")); expect(await getUndoBackup("2.0.0")).toEqual(backup); }); }); From db946bec62dfbf477bc5a5fd2e6bda0a73af53ba Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:32:53 +0000 Subject: [PATCH 08/10] fix(cli): Check backup file types with stat Some filesystems (NFS, some FUSE and bind mounts) don't report a type on directory entries, so Dirent.isFile() skipped real backups. Match the name first, then use the stat result to check the type. --- packages/cli/src/lib/db/backup.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts index bca7521bc..8380f60ec 100644 --- a/packages/cli/src/lib/db/backup.ts +++ b/packages/cli/src/lib/db/backup.ts @@ -89,12 +89,17 @@ async function listBackups(): Promise { } const backups: DatabaseBackup[] = []; for await (const entry of entries) { - const match = entry.isFile() ? BACKUP_FILE_REGEX.exec(entry.name) : null; + const match = BACKUP_FILE_REGEX.exec(entry.name); if (!match) { continue; } try { - const { mtimeMs } = await stat(join(dir, entry.name)); + // `stat`, not the entry: some filesystems don't report entry types. + const stats = await stat(join(dir, entry.name)); + if (!stats.isFile()) { + continue; + } + const { mtimeMs } = stats; backups.push({ version: match[1] as string, toVersion: match[2] as string, From 41eac94e809e9c9bc77072d4470396f126046f4e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:59:44 +0000 Subject: [PATCH 09/10] fix(cli): Keep undo from overwriting the restored channel or losing this database Undo passed the target's artifact channel to setup, which records it and overwrote the release channel stored in the restored database. Setup now gets no --channel for an undo. An undo that installs the binary through setup now stops when this version's database can't be backed up: a failed setup could otherwise not put it back, leaving this version with the restored database. --- packages/cli/src/commands/cli/upgrade.ts | 42 ++++++++++++------- .../cli/test/commands/cli/upgrade.test.ts | 36 +++++++++++++++- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index a7dc02c0d..cd640400c 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -140,13 +140,29 @@ async function backupLocalDatabase( /** * Run once the new binary is downloaded, right before its setup, so a * failed download leaves the database untouched. + * + * @param install - Whether setup installs the new binary itself, so this + * version stays installed if setup fails. + * @throws {UpgradeError} When an undo that installs the binary itself could + * not back up this version's database: a failed setup could then not put + * it back, leaving this version with the restored one. */ -async function prepareDatabaseForNewBinary(plan: DatabasePlan): Promise { +async function prepareDatabaseForNewBinary( + plan: DatabasePlan, + install: boolean, +): Promise { // Reinstalling this same version must not replace the record of the // upgrade that led here. if (plan.target !== CLI_VERSION) { plan.backup = await backupLocalDatabase(plan.target); } + if (plan.restore && install && !plan.backup) { + throw new UpgradeError( + "execution_failed", + `Undo stopped before installing ${plan.target}: this version's database could not be backed up, ` + + "so it couldn't be put back if the install failed. Nothing was changed.", + ); + } if (plan.restore) { try { plan.warnings.push( @@ -738,16 +754,13 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { noAgentSkills, database, } = opts; - const args = [ - "cli", - "setup", - "--quiet", - "--method", - method, - "--channel", - channel, - "--no-modify-path", - ]; + const args = ["cli", "setup", "--quiet", "--method", method]; + // An undo leaves the channel to the database it restores, which records + // the one that version followed. `channel` only picks its artifact. + if (!database.restore) { + args.push("--channel", channel); + } + args.push("--no-modify-path"); if (install) { args.push("--install"); } @@ -762,7 +775,7 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { ? { ...process.env, SENTRY_INSTALL_DIR: installDir } : undefined; - await prepareDatabaseForNewBinary(database); + await prepareDatabaseForNewBinary(database, install); let exitCode: number | undefined; try { exitCode = await spawnWithRetry(binaryPath, args, env); @@ -1235,8 +1248,9 @@ export const upgradeCommand = buildCommand({ undo !== undefined, ); const { versionArg, method } = resolvedContext; - // Undo reinstalls the earlier version on its own channel. Its restored - // database already stores that channel, so this one's is left alone. + // Undo downloads the earlier version's own artifact. Setup doesn't + // record this channel: the restored database already stores the one + // that version followed, and this one's is left alone. const channel = undo ? getArtifactChannel(undo.version) : resolvedContext.channel; diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 7bc1a8571..ee57b7976 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -1240,7 +1240,8 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(getOutput()).toContain(`Restored the database ${NEXT_RELEASE}`); const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); - expect(setupCall?.args).toContain("stable"); + // The restored database keeps the channel that version followed. + expect(setupCall?.args).not.toContain("--channel"); expect(markerAtSetup).toBe(`saved by ${NEXT_RELEASE}`); // This version's database is kept, so the undo can be undone in turn. expect((await getUndoBackup(NEXT_RELEASE)).version).toBe(CLI_VERSION); @@ -1273,6 +1274,39 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect((await getUndoBackup(CLI_VERSION)).version).toBe(NEXT_RELEASE); }); + test("undo stops before installing when this version's database can't be backed up", async () => { + const marker = "test.marker"; + setMetadata(getRawDatabase(), { [marker]: `saved by ${NEXT_RELEASE}` }); + await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); + setMetadata(getRawDatabase(), { [marker]: "current" }); + // A directory where this version's backup goes makes the backup fail. + const blocker = join( + getBackupDir(), + `cli-${CLI_VERSION}_to_${NEXT_RELEASE}.db`, + ); + mkdirSync(blocker); + writeFileSync(join(blocker, "file"), ""); + mockTaggedBinaryDownload(NEXT_RELEASE); + + const { context, getOutput, errors, restore } = createMockContext({ + homeDir: testDir, + }); + restoreStderr = restore; + + try { + await run(app, ["cli", "upgrade", "undo", "--method", "curl"], context); + } finally { + rmSync(blocker, { recursive: true, force: true }); + } + + expect(getOutput() + errors.join("")).toContain("Undo stopped"); + expect(spawnedArgs.some((entry) => entry.args.includes("setup"))).toBe( + false, + ); + expect(getMetadata(getRawDatabase(), [marker]).get(marker)).toBe("current"); + expect((await getUndoBackup(CLI_VERSION)).version).toBe(NEXT_RELEASE); + }); + test("a failed upgrade keeps the undo of the one before", async () => { await backupDatabase(getRawDatabase(), NEXT_RELEASE, CLI_VERSION); mockBinaryDownloadWithVersion(LATER_RELEASE); From e4586e5875887497ae85926d522d1143356a6896 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:02:45 +0000 Subject: [PATCH 10/10] ref(cli): Remove the database's write-ahead log files in parallel --- packages/cli/src/lib/db/backup.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/lib/db/backup.ts b/packages/cli/src/lib/db/backup.ts index 8380f60ec..e2a484359 100644 --- a/packages/cli/src/lib/db/backup.ts +++ b/packages/cli/src/lib/db/backup.ts @@ -208,6 +208,14 @@ export async function discardDatabaseBackup( await rm(join(getBackupDir(), backup.file), { force: true }); } +/** Remove the write-ahead log files that belong to the database at `dbPath`. */ +async function removeWriteAheadLog(dbPath: string): Promise { + await Promise.all([ + rm(`${dbPath}-wal`, { force: true }), + rm(`${dbPath}-shm`, { force: true }), + ]); +} + /** * Move `backup` back into place as `cli.db` and let this process use it * again, reverting a restore or hand-off made for a version that was never @@ -222,8 +230,7 @@ export async function reinstateDatabaseBackup( detachDatabase(); try { // The write-ahead log belongs to the file being replaced. - await rm(`${dbPath}-wal`, { force: true }); - await rm(`${dbPath}-shm`, { force: true }); + await removeWriteAheadLog(dbPath); await rename(join(getBackupDir(), backup.file), dbPath); } finally { closeDatabase(); @@ -338,8 +345,7 @@ export async function restoreDatabaseBackup( try { // A leftover write-ahead log belongs to the replaced file and would // corrupt the restored one. - await rm(`${dbPath}-wal`, { force: true }); - await rm(`${dbPath}-shm`, { force: true }); + await removeWriteAheadLog(dbPath); await rename(tmp, dbPath); } catch (error) { // `cli.db` is still this version's; let callers use it again.