From 8fc4023f94bdb42225bf2aa4e81c0558d903a954 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Mon, 10 Aug 2026 11:18:19 -0300 Subject: [PATCH] fix(migrations): allow fresh install past mass-migration guard (#9934) (#10022) Co-authored-by: diegosouzapw --- .../fixes/9934-migration-fresh-setup.md | 1 + src/lib/db/core.ts | 24 ++- src/lib/db/migrationRunner.ts | 17 +++ tests/unit/db-fresh-setup-9934.test.ts | 138 ++++++++++++++++++ 4 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/9934-migration-fresh-setup.md create mode 100644 tests/unit/db-fresh-setup-9934.test.ts diff --git a/changelog.d/fixes/9934-migration-fresh-setup.md b/changelog.d/fixes/9934-migration-fresh-setup.md new file mode 100644 index 0000000000..b794540c58 --- /dev/null +++ b/changelog.d/fixes/9934-migration-fresh-setup.md @@ -0,0 +1 @@ +- fix(migrations): don't abort on fresh install with only the 001 seed (#9934) \ No newline at end of file diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index 50d874a3ba..e2c29be2dd 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -1045,13 +1045,35 @@ export function getDbInstance(): SqliteDatabase { // This is needed so the migration runner skips the mass-migration safety abort // that would otherwise trigger because heuristic seeding marks some migrations // as applied, making the fresh DB look like a wiped existing DB (#1328). - const isNewDb = !fs.existsSync(sqliteFile); + // #9934: also classify as fresh a file that `omniroute setup` created with + // only the clipped skeleton schema (see the probe below) — even though the + // file exists, it has never had migrations run. + let isNewDb = !fs.existsSync(sqliteFile); // Detect and handle old schema format — preserve data when possible (#146) // Uses a single probe connection that becomes the real connection when possible. if (fs.existsSync(sqliteFile)) { try { const probe = openSqliteDatabase(sqliteFile, { readonly: true }); + // #9934: init asymmetry — bin/cli/sqlite.mjs::openOmniRouteDb (used by + // `omniroute setup`) creates storage.sqlite with only the partial inline + // schema (key_value + provider_connections) and never runs migrations. + // Purely file-existence-based freshness made that file look like an + // existing DB, so the first `serve` auto-seeded only the 001 marker and + // tripped the mass-migration safety abort on a brand-new install. A + // skeleton file has provider_connections but none of the tables the 001 + // migration creates (combos) — treat it as fresh, not as a wiped DB. + const probeHasProviderConnections = !!probe + .prepare( + "SELECT name FROM sqlite_master WHERE type='table' AND name='provider_connections'" + ) + .get(); + const probeHasCombos = !!probe + .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='combos'") + .get(); + if (probeHasProviderConnections && !probeHasCombos) { + isNewDb = true; + } const hasOldSchema = probe .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='schema_migrations'") .get(); diff --git a/src/lib/db/migrationRunner.ts b/src/lib/db/migrationRunner.ts index fabcd06e6d..2214ce08f0 100644 --- a/src/lib/db/migrationRunner.ts +++ b/src/lib/db/migrationRunner.ts @@ -922,9 +922,26 @@ export function runMigrations(db: SqliteAdapter, options?: { isNewDb?: boolean } // interpolates this resolved value, so it auto-reflects any override. const maxPendingMigrations = resolveMaxPendingMigrations(); + // #9934: `omniroute setup`'s openOmniRouteDb writes a partial skeleton file + // (provider_connections + key_value) that has never had migrations run. When + // the first `serve` opens it and auto-seeds only the 001 marker, the applied + // set is exactly {001} — which would otherwise look like a wiped existing DB + // and trip this abort on a brand-new install. This is distinct from a real + // wiped/backup-restored database: that case has a non-trivial physical schema + // (baseline inference is non-null) and full data tables, so it still aborts. + // The 001-marker-only state on a provider_connections skeleton is the fresh + // auto-seed — let it through. A genuinely empty table is already exempt via + // `applied.size > 0`, and an upgraded DB has a non-trivial applied set. + const isFreshSeedOnly = + applied.size === 1 && + applied.has("001") && + inferPhysicalSchemaBaseline(db) === null && + hasTable(db, "provider_connections"); + if ( !isTestEnvironment && !isNewDb && + !isFreshSeedOnly && process.env.DISABLE_SQLITE_AUTO_BACKUP !== "true" && maxPendingMigrations > 0 && applied.size > 0 && diff --git a/tests/unit/db-fresh-setup-9934.test.ts b/tests/unit/db-fresh-setup-9934.test.ts new file mode 100644 index 0000000000..715035d1c6 --- /dev/null +++ b/tests/unit/db-fresh-setup-9934.test.ts @@ -0,0 +1,138 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import Database from "better-sqlite3"; +import { resetDbInstance } from "../../src/lib/db/core.ts"; + +// Regression guard for #9934 — init asymmetry breaks a fresh install. +// +// `omniroute setup` (bin/cli/sqlite.mjs::openOmniRouteDb) creates +// storage.sqlite with the *partial* inline schema (key_value + +// provider_connections) but NEVER creates _omniroute_migrations and never runs +// migrations. That file flips the server's new-DB heuristic +// (src/lib/db/core.ts uses `!fs.existsSync(sqliteFile)`), so the first +// `omniroute serve` believes it is an existing DB, auto-seeds only the 001 +// marker, and then trips the mass-migration safety abort because 139 pending +// migrations exceed the default threshold of 50 (#6260 gate). +// +// A DB whose ONLY applied migration is the 001 initial-schema auto-seed is a +// fresh install, not a wiped/backup-restored database — it must NOT abort. + +const serial = { concurrency: false }; + +// Re-import a module so module-level env-derived constants (DATA_DIR, +// SQLITE_FILE) re-resolve after we set DATA_DIR. Static import cannot work +// here: the whole point is exercising the module-loading boundary. +async function importFresh(modulePath: string) { + const url = pathToFileURL(path.resolve(modulePath)).href; + return import(`${url}?test=${Date.now()}-${Math.random().toString(16).slice(2)}`); +} + +// Simulate a production (non-test) process so the #6260 mass-migration safety +// gate is actually LIVE: under `node --test` the runner would be detected and +// the gate skipped, making the bug invisible. +function withNonTestEnvironment(fn: () => R): R { + const originalNodeEnv = process.env.NODE_ENV; + const originalVitest = process.env.VITEST; + const originalDisableAutoBackup = process.env.DISABLE_SQLITE_AUTO_BACKUP; + const originalArgv = [...process.argv]; + const originalExecArgv = [...process.execArgv]; + + delete process.env.NODE_ENV; + delete process.env.VITEST; + delete process.env.DISABLE_SQLITE_AUTO_BACKUP; + process.argv = process.argv.filter((arg) => !arg.includes("test")); + process.execArgv = process.execArgv.filter((arg) => !arg.includes("test")); + + try { + return fn(); + } finally { + process.argv = originalArgv; + process.execArgv = originalExecArgv; + if (originalNodeEnv === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = originalNodeEnv; + if (originalVitest === undefined) delete process.env.VITEST; + else process.env.VITEST = originalVitest; + if (originalDisableAutoBackup === undefined) delete process.env.DISABLE_SQLITE_AUTO_BACKUP; + else process.env.DISABLE_SQLITE_AUTO_BACKUP = originalDisableAutoBackup; + } +} + +function cleanupGlobalDb() { + try { + const g = globalThis as Record void }>; + if (g.__omnirouteDb?.open) g.__omnirouteDb.close?.(); + } catch { + /* ignore */ + } + delete (globalThis as Record).__omnirouteDb; +} + +test.after(() => { + cleanupGlobalDb(); + resetDbInstance(); +}); + +test( + "fresh `omniroute setup` DB (only the 001 seed) survives first serve without mass-migration abort (#9934)", + serial, + async () => { + const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-9934-")); + const originalDataDir = process.env.DATA_DIR; + process.env.DATA_DIR = dataDir; + + try { + // Step 1 — mimic `omniroute setup`: the CLI opens the DB, writes the + // partial inline schema (key_value + provider_connections) and closes it, + // WITHOUT running migrations or creating _omniroute_migrations. + const cli = await importFresh("bin/cli/sqlite.mjs"); + const setup = await cli.openOmniRouteDb(); + assert.ok(fs.existsSync(setup.dbPath), "setup created storage.sqlite"); + setup.db.close(); + + const onDisk = new Database(setup.dbPath, { readonly: true }); + try { + const hasMigrationTable = !!onDisk + .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name=?") + .get("_omniroute_migrations"); + assert.equal( + hasMigrationTable, + false, + "setup must NOT pre-create the migrations tracking table (bug premise)" + ); + } finally { + onDisk.close(); + } + + // Step 2 — mimic the first `omniroute serve`: the real server opens the + // same DB, auto-seeds only the 001 marker and runs migrations. Under a + // live (non-test) safety gate this must NOT throw. + const core = await importFresh("src/lib/db/core.ts"); + cleanupGlobalDb(); + resetDbInstance(); + + let db: { prepare?: (sql: string) => { get: () => { maxV: number } | undefined } }; + assert.doesNotThrow(() => { + withNonTestEnvironment(() => { + db = core.getDbInstance(); + }); + }, "first serve must not abort on a fresh setup DB that only has the 001 seed (#9934)"); + + // Prove the fresh DB actually got migrated past 001 to the latest version. + const maxRow = db.prepare( + "SELECT MAX(CAST(version AS INTEGER)) AS maxV FROM _omniroute_migrations" + ).get(); + assert.ok( + (maxRow?.maxV ?? 0) > 1, + `expected migrations beyond 001 to run, got max=${maxRow?.maxV}` + ); + } finally { + if (originalDataDir === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = originalDataDir; + fs.rmSync(dataDir, { recursive: true, force: true }); + } + } +);