From fa32a3cb5a28c155e66526ca3caeee48d63bb127 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Sun, 14 Jun 2026 21:34:14 -0300 Subject: [PATCH] fix(db): persist the Keep-latest-backups retention setting (#3834) --- CHANGELOG.md | 1 + src/app/api/db-backups/route.ts | 6 +++++ src/lib/db/backup.ts | 38 ++++++++++++++++++++++++++- src/lib/localDb.ts | 1 + tests/unit/db-backup-extended.test.ts | 30 +++++++++++++++++++++ 5 files changed, 75 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbe445328a4e..71f8d9264c32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### πŸ› Fixed +- **fix(db): persist the "Keep latest backups" retention setting** β€” changing the backup-retention count in Settings β†’ Database backup retention had no effect: it always snapped back to 20 on refresh (and editing `.env` post-start was ignored too, since `process.env` isn't reloaded). `getDbBackupMaxFiles()` only read the `DB_BACKUP_MAX_FILES` env var β€” there was no setter and no persisted value. The value now round-trips through a dedicated `key_value` store (`getDbBackupMaxFiles` precedence: env override β†’ persisted UI value β†’ default 20), and the "Clean old backups" action persists the chosen count. Existing installs keep the historical default of 20 until explicitly changed. ([#3834](https://github.com/diegosouzapw/OmniRoute/issues/3834) β€” thanks @netstratego) - **fix(sse): clamp Gemini thinking budget to the model's real cap (`reasoning_effort`/`effort=high` 400)** β€” translating OpenAI `reasoning_effort=high` (and Claude-Code `output_config.effort=high`) to a Gemini target sent a hardcoded `thinkingBudget: 32768`, which exceeds Flash-tier Gemini's real max of 24576 β†’ upstream HTTP 400 (the `thinkingLevel=high` path already used 24576 and worked on the same model). `gemini-2.5-flash` now declares its real `thinkingBudgetCap` (24576) so the existing `capThinkingBudget()` chokepoint actually clamps, and the Claudeβ†’Gemini `output_config.effort` path β€” which previously sent the raw value with no cap at all β€” now routes through the same clamp (pro-tier, real cap 32768, is left untouched). ([#3842](https://github.com/diegosouzapw/OmniRoute/issues/3842) β€” thanks @andrea-kingautomation) - **fix(intelligence): run pricing + models.dev sync from the live startup path** β€” like the Arena ELO sync (v3.8.24), the external **pricing sync** (`PRICING_SYNC_ENABLED`) and the **models.dev capability sync** (Settings β†’ AI toggle) were only initialized from `server-init.ts`, which the Next standalone runtime never executes β€” and models.dev had no caller at all. Their toggles were inert in production. Both are now initialized from `instrumentation-node.ts` (self-gated, opt-in preserved, non-blocking, never fatal). (thanks @diegosouzapw) - **test(proxy): guard the per-connection 'direct' bypass over a global proxy + clearer label** β€” the per-connection "Proxy Off" toggle (`proxyEnabled: false`) already overrides a configured **global** proxy (`resolveProxyForConnection` short-circuits to `level: "direct"` before the global step). Added an explicit regression test proving the bypass beats a global assignment (and round-trips on re-enable), and relabeled the UI to "Direct (bypass proxy)" so operators recognize it. Closes the verification gap in [#2996](https://github.com/diegosouzapw/OmniRoute/issues/2996). (thanks @diegosouzapw) diff --git a/src/app/api/db-backups/route.ts b/src/app/api/db-backups/route.ts index de499739cce9..135811bcc5f4 100644 --- a/src/app/api/db-backups/route.ts +++ b/src/app/api/db-backups/route.ts @@ -5,6 +5,7 @@ import { backupDbFile, cleanupDbBackups, getDbBackupMaxFiles, + setDbBackupMaxFiles, getDbBackupRetentionDays, } from "@/lib/localDb"; import { dbBackupCleanupSchema, dbBackupRestoreSchema } from "@/shared/validation/schemas"; @@ -124,6 +125,11 @@ export async function DELETE(request) { const keepLatest = validation.data.keepLatest ?? getDbBackupMaxFiles(); const retentionDays = validation.data.retentionDays ?? getDbBackupRetentionDays(); + // #3834: persist the operator's chosen retention so it survives the page refresh + // and the subsequent loadStorageHealth() refetch (it previously snapped back to 20). + if (validation.data.keepLatest !== undefined) { + setDbBackupMaxFiles(validation.data.keepLatest); + } const result = cleanupDbBackups({ maxFiles: keepLatest, retentionDays }); return NextResponse.json({ cleaned: true, diff --git a/src/lib/db/backup.ts b/src/lib/db/backup.ts index ac9373083191..87421ca0c3b6 100644 --- a/src/lib/db/backup.ts +++ b/src/lib/db/backup.ts @@ -37,8 +37,44 @@ function parseNonNegativeInt(value: string | undefined, fallback: number) { return Number.isInteger(parsed) && parsed >= 0 ? parsed : fallback; } +// #3834: the "Keep latest backups" UI value is persisted here so it survives a page +// refresh / the loadStorageHealth() refetch. A dedicated namespace avoids any +// cross-talk with the databaseSettings key_value store (which rewrites all of its own +// keys on every update). It is intentionally separate from the orphan +// `databaseSettings.backup.keepLastNBackups` (default 5) so existing installs keep the +// historical default of 20 until an operator explicitly changes it here. +const DB_BACKUP_SETTINGS_NAMESPACE = "dbBackup"; +const DB_BACKUP_MAX_FILES_KEY = "maxFiles"; + +function getStoredDbBackupMaxFiles(): number | undefined { + try { + const db = getDbInstance(); + const row = db + .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?") + .get(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY) as { value?: string } | undefined; + if (!row?.value) return undefined; + const parsed = Number.parseInt(JSON.parse(row.value), 10); + return Number.isInteger(parsed) && parsed > 0 ? parsed : undefined; + } catch { + return undefined; + } +} + +/** Persist the operator-chosen "keep latest backups" retention count (#3834). */ +export function setDbBackupMaxFiles(value: number): void { + if (!Number.isInteger(value) || value <= 0) return; + const db = getDbInstance(); + db.prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" + ).run(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY, JSON.stringify(value)); +} + export function getDbBackupMaxFiles() { - return parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS); + // Precedence: DB_BACKUP_MAX_FILES env override (ops) β†’ persisted UI value β†’ default. + if (process.env.DB_BACKUP_MAX_FILES) { + return parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS); + } + return getStoredDbBackupMaxFiles() ?? MAX_DB_BACKUPS; } export function getDbBackupRetentionDays() { diff --git a/src/lib/localDb.ts b/src/lib/localDb.ts index 97079d4a2b47..9901dd7b849b 100755 --- a/src/lib/localDb.ts +++ b/src/lib/localDb.ts @@ -200,6 +200,7 @@ export { backupDbFile, cleanupDbBackups, getDbBackupMaxFiles, + setDbBackupMaxFiles, getDbBackupRetentionDays, listDbBackups, restoreDbBackup, diff --git a/tests/unit/db-backup-extended.test.ts b/tests/unit/db-backup-extended.test.ts index 127eb97999dc..a0b6acb935b5 100644 --- a/tests/unit/db-backup-extended.test.ts +++ b/tests/unit/db-backup-extended.test.ts @@ -192,3 +192,33 @@ test("cleanupDbBackups honors retentionDays for older backups", async () => { assert.equal(fs.existsSync(oldBackup), false); assert.equal(fs.existsSync(freshBackup), true); }); + +// Regression for #3834: the "Keep latest backups" value did not persist β€” it always +// snapped back to 20 because getDbBackupMaxFiles() only read the env var (no setter, +// no stored value). It now round-trips through a dedicated key_value store. +test("getDbBackupMaxFiles defaults to 20 when nothing is stored (#3834)", () => { + delete process.env.DB_BACKUP_MAX_FILES; + core.getDbInstance(); // ensure the DB + key_value table exist + assert.equal(backupDb.getDbBackupMaxFiles(), 20); +}); + +test("setDbBackupMaxFiles persists and getDbBackupMaxFiles reflects it (#3834)", () => { + delete process.env.DB_BACKUP_MAX_FILES; + core.getDbInstance(); + backupDb.setDbBackupMaxFiles(5); + assert.equal(backupDb.getDbBackupMaxFiles(), 5); + // A second value overwrites the first (operator changes the setting again). + backupDb.setDbBackupMaxFiles(12); + assert.equal(backupDb.getDbBackupMaxFiles(), 12); +}); + +test("DB_BACKUP_MAX_FILES env override wins over the persisted value (#3834)", () => { + core.getDbInstance(); + backupDb.setDbBackupMaxFiles(5); + process.env.DB_BACKUP_MAX_FILES = "7"; + try { + assert.equal(backupDb.getDbBackupMaxFiles(), 7); + } finally { + delete process.env.DB_BACKUP_MAX_FILES; + } +});