mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-20 22:22:57 +03:00
fix(db): migration safety abort — add bypass hint + memoize to stop cascade (#6260)
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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<SqliteAdapter, boolean>();
|
||||
|
||||
/**
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
194
tests/unit/migration-safety-abort-6260.test.ts
Normal file
194
tests/unit/migration-safety-abort-6260.test.ts
Normal file
@@ -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<T>(files: Record<string, string>, 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<T>(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<typeof Database>) {
|
||||
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<string, string> = {};
|
||||
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();
|
||||
}
|
||||
}
|
||||
);
|
||||
Reference in New Issue
Block a user