From a83dc96a2263134a3a1f09578e15398e17bc9918 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Mon, 6 Jul 2026 14:55:00 -0300 Subject: [PATCH] =?UTF-8?q?fix(db):=20migration=20safety=20abort=20?= =?UTF-8?q?=E2=80=94=20add=20bypass=20hint=20+=20memoize=20to=20stop=20cas?= =?UTF-8?q?cade=20(#6260)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 + src/lib/db/migrationRunner.ts | 44 +++- .../unit/migration-safety-abort-6260.test.ts | 194 ++++++++++++++++++ 3 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 tests/unit/migration-safety-abort-6260.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 33a8a33ffb..41625e6242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ ### 🐛 Bug Fixes +- **fix(db):** the mass-migration **safety abort** now tells the operator how to bypass it and stops flooding the log ([#6260](https://github.com/diegosouzapw/OmniRoute/issues/6260)) — after restoring a backup that wiped the migration tracking table, `runMigrations()` threw the abort on every downstream `ensureDbInitialized()`, re-logging the full banner 11+ times, and the message never mentioned the existing `OMNIROUTE_MAX_PENDING_MIGRATIONS` escape hatch. The abort text now appends a bypass hint (set `OMNIROUTE_MAX_PENDING_MIGRATIONS=0` in `server.env` / `DATA_DIR/.env`), and a new `MigrationSafetyAbortError` is memoized so repeated calls in the same process throw the same instance and emit a single concise line instead of the full cascade. Regression guard: `tests/unit/migration-safety-abort-6260.test.ts`. (thanks @chirag127) + - **fix(auth):** importing a **distinct** Codex/ChatGPT OAuth `auth.json` is no longer falsely rejected as "already exists" when it belongs to a different user in the same workspace ([#6301](https://github.com/diegosouzapw/OmniRoute/issues/6301)). `findExistingCodexConnection` (in `src/lib/oauth/utils/codexAuthImport.ts`) deduped **only** on `providerSpecificData.workspaceId === accountId`, where `accountId` is the shared `chatgpt_account_id`/`tokens.account_id` — so two members of the same ChatGPT Team collapsed onto a single connection (409 `duplicate_account`). The id_token's `https://api.openai.com/auth` claim carries a per-user `chatgpt_user_id` alongside the workspace id (the device-flow path already persisted it as `chatgptUserId`, but the import path did not). Now `parseAndValidateCodexAuth` extracts `userId` (`chatgpt_user_id` → `user_id` → JWT `sub`) into `ParsedCodexAuth`, the create/update paths persist `chatgptUserId` in `providerSpecificData` (mirroring `codex.ts`), and dedup keys on `workspaceId` **AND** `chatgptUserId` — with a backward-compat fallback to legacy accountId-only matching when no stored connection for that workspace records a `chatgptUserId`, so genuinely-same accounts still dedup. Regression guard: `tests/unit/codex-auth-import-userid-dedup-6301.test.ts` (4). (thanks @anungma) - **fix(providers):** importing models for the **venice-web** provider no longer fails with a red "Provider venice-web does not support models listing" ([#6269](https://github.com/diegosouzapw/OmniRoute/issues/6269)). `venice-web` is a web-cookie provider with an executor but no upstream `/v1/models` endpoint and no registry `models`, so the models route fell through to the tail `400`. Mirroring the `jules`/`linkup-search`/`ollama-search` fix (#5569), it now ships a static local catalog entry in `src/lib/providers/staticModels.ts` — seeding the current Venice lineup (`venice-uncensored`, `llama-3.3-70b`, `qwen3-235b`, `qwen3-4b`, `deepseek-r1-671b`; Venice rotates its catalog, see docs.venice.ai/models/overview) — so the route returns `200` with `source:"local_catalog"`, `intentional:true`. Regression guard: `tests/unit/static-models-venice-web-6269.test.ts`. (thanks @chirag127) diff --git a/src/lib/db/migrationRunner.ts b/src/lib/db/migrationRunner.ts index 1f4467a622..0b2e414326 100644 --- a/src/lib/db/migrationRunner.ts +++ b/src/lib/db/migrationRunner.ts @@ -138,6 +138,29 @@ function resolveMaxPendingMigrations(): number { return DEFAULT_MAX_PENDING_MIGRATIONS_ON_EXISTING_DB; } +/** + * Raised by the mass-migration safety check when far more migrations are pending + * than the resolved threshold — a strong signal the migration tracking table was + * wiped (e.g. a restored backup). Given its own type so callers/loggers can + * recognize the memoized cascade and keep repeated logs concise (#6260). + */ +export class MigrationSafetyAbortError extends Error { + constructor(message: string) { + super(message); + this.name = "MigrationSafetyAbortError"; + } +} + +/** + * Memoized mass-migration abort (#6260). After a backup restore wipes the + * migration tracking table, EVERY downstream `ensureDbInitialized()` re-opens + * the DB and re-calls `runMigrations()`, which used to recompute the abort and + * re-`console.error` the full banner 11+ times. Caching the thrown instance + * (keyed by the exact message it would compute) lets repeated calls in the same + * process throw the SAME instance and log a single concise line instead. + */ +let memoizedSafetyAbort: MigrationSafetyAbortError | null = null; + const fts5SupportCache = new WeakMap(); /** @@ -890,14 +913,31 @@ export function runMigrations(db: SqliteAdapter, options?: { isNewDb?: boolean } `(${physicalBaseline.description}), so at most ${plausiblePendingCount} pending ` + `migration(s) are expected from a legitimate upgrade.` : ""; + const bypassHint = + ` To bypass this check (e.g. after restoring a backup where the migration ` + + `tracking table was wiped), set OMNIROUTE_MAX_PENDING_MIGRATIONS=0 in your ` + + `server.env or DATA_DIR/.env and restart.`; const msg = `[Migration] 🛑 ABORT: Detected ${actionablePending.length} pending migrations on an existing database ` + `(threshold is ${maxPendingMigrations}). ` + `This usually means the migration tracking table was accidentally wiped. ` + `Running all migrations from scratch will cause data loss or schema errors.` + - schemaHint; + schemaHint + + bypassHint; + + // #6260: memoize so the cascade of downstream ensureDbInitialized() calls + // that re-open the DB throw the SAME instance and only log once. + if (memoizedSafetyAbort && memoizedSafetyAbort.message === msg) { + console.error( + `[Migration] 🛑 ABORT (repeat — see earlier detail): ` + + `${actionablePending.length} pending > threshold ${maxPendingMigrations}. ` + + `Set OMNIROUTE_MAX_PENDING_MIGRATIONS=0 to bypass.` + ); + throw memoizedSafetyAbort; + } console.error(msg); - throw new Error(msg); + memoizedSafetyAbort = new MigrationSafetyAbortError(msg); + throw memoizedSafetyAbort; } } diff --git a/tests/unit/migration-safety-abort-6260.test.ts b/tests/unit/migration-safety-abort-6260.test.ts new file mode 100644 index 0000000000..ae649116e0 --- /dev/null +++ b/tests/unit/migration-safety-abort-6260.test.ts @@ -0,0 +1,194 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +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 #6260: +// 1. The mass-migration safety-abort message must tell the operator how to +// bypass the check (OMNIROUTE_MAX_PENDING_MIGRATIONS=0) — e.g. after +// restoring a backup where the migration tracking table was wiped. +// 2. Repeated runMigrations() calls on the same over-threshold DB must throw +// the SAME memoized MigrationSafetyAbortError instance, so downstream +// subsystems re-opening the DB do not re-compute + re-log the full abort +// banner 11+ times (the cascade described in the issue). + +const serial = { concurrency: false }; + +async function importFresh(modulePath: string) { + const url = pathToFileURL(path.resolve(modulePath)).href; + return import(`${url}?test=${Date.now()}-${Math.random().toString(16).slice(2)}`); +} + +function withMockedMigrationFs(files: Record, fn: () => T): T { + const originalExistsSync = fs.existsSync; + const originalReaddirSync = fs.readdirSync; + const originalReadFileSync = fs.readFileSync; + + const isMigrationDir = (target: unknown) => + String(target).replaceAll("\\", "/").endsWith("/src/lib/db/migrations") || + String(target).replaceAll("\\", "/").endsWith("/migrations"); + + fs.existsSync = ((target: fs.PathLike) => { + if (isMigrationDir(target)) return true; + const fileName = path.basename(String(target)); + if (Object.hasOwn(files, fileName)) return true; + return originalExistsSync(target); + }) as typeof fs.existsSync; + + fs.readdirSync = ((target: fs.PathLike, options?: unknown) => { + if (isMigrationDir(target)) return Object.keys(files); + return (originalReaddirSync as (t: fs.PathLike, o?: unknown) => unknown)(target, options); + }) as typeof fs.readdirSync; + + fs.readFileSync = ((target: fs.PathOrFileDescriptor, options?: unknown) => { + const fileName = path.basename(String(target)); + if (Object.hasOwn(files, fileName)) return files[fileName]; + return (originalReadFileSync as (t: fs.PathOrFileDescriptor, o?: unknown) => unknown)( + target, + options + ); + }) as typeof fs.readFileSync; + + try { + return fn(); + } finally { + fs.existsSync = originalExistsSync; + fs.readdirSync = originalReaddirSync; + fs.readFileSync = originalReadFileSync; + } +} + +function withNonTestEnvironment(fn: () => T): T { + const originalNodeEnv = process.env.NODE_ENV; + const originalVitest = process.env.VITEST; + const originalDisableAutoBackup = process.env.DISABLE_SQLITE_AUTO_BACKUP; + const originalArgv = [...process.argv]; + + 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")); + + try { + return fn(); + } finally { + process.argv = originalArgv; + 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; + } +} + +// Existing DB with only the migrations table + one applied row and no physical +// schema sentinel tables, so inferPhysicalSchemaBaseline() returns null and the +// abort decision depends purely on the resolved threshold. +function seedExistingDbWithoutPhysicalBaseline(db: InstanceType) { + db.exec(` + CREATE TABLE _omniroute_migrations ( + version TEXT PRIMARY KEY, + name TEXT NOT NULL, + applied_at TEXT NOT NULL DEFAULT (datetime('now')) + ); + `); + db.prepare("INSERT INTO _omniroute_migrations (version, name) VALUES (?, ?)").run( + "001", + "initial_schema" + ); +} + +function buildMockMigrationFiles(startVersion: number, endVersion: number, prefix: string) { + const files: Record = {}; + for (let version = startVersion; version <= endVersion; version++) { + const padded = String(version).padStart(3, "0"); + const fileName = version === 1 ? "001_initial_schema.sql" : `${padded}_${prefix}_${padded}.sql`; + files[fileName] = `CREATE TABLE ${prefix}_${padded} (id INTEGER);`; + } + return files; +} + +function createDb() { + return new Database(":memory:"); +} + +test.after(() => { + resetDbInstance(); +}); + +test( + "abort message tells the operator to set OMNIROUTE_MAX_PENDING_MIGRATIONS=0 to bypass (#6260)", + serial, + async () => { + const runner = await importFresh("src/lib/db/migrationRunner.ts"); + const db = createDb(); + try { + seedExistingDbWithoutPhysicalBaseline(db); + let thrown: unknown; + assert.throws(() => { + try { + withNonTestEnvironment(() => + withMockedMigrationFs(buildMockMigrationFiles(1, 60, "bypass_hint"), () => + runner.runMigrations(db) + ) + ); + } catch (err) { + thrown = err; + throw err; + } + }); + const message = thrown instanceof Error ? thrown.message : String(thrown); + assert.match(message, /OMNIROUTE_MAX_PENDING_MIGRATIONS=0/); + } finally { + db.close(); + } + } +); + +test( + "two consecutive aborts on the same over-threshold DB throw the SAME memoized instance (#6260)", + serial, + async () => { + const runner = await importFresh("src/lib/db/migrationRunner.ts"); + const db = createDb(); + try { + seedExistingDbWithoutPhysicalBaseline(db); + + const runOnce = () => + withNonTestEnvironment(() => + withMockedMigrationFs(buildMockMigrationFiles(1, 60, "cascade"), () => + runner.runMigrations(db) + ) + ); + + let first: unknown; + let second: unknown; + assert.throws(() => { + try { + runOnce(); + } catch (err) { + first = err; + throw err; + } + }); + assert.throws(() => { + try { + runOnce(); + } catch (err) { + second = err; + throw err; + } + }); + + assert.ok(first instanceof runner.MigrationSafetyAbortError); + assert.ok(second instanceof runner.MigrationSafetyAbortError); + assert.strictEqual(first, second, "cascade re-triggers must reuse the memoized instance"); + } finally { + db.close(); + } + } +);