fix(db): prune old backups after health-check-repair snapshots

src/lib/db/backup.ts

@@ -17,9 +17,12 @@ import { resetAllDbModuleState } from "./stateReset";
 import {
   MAX_DB_BACKUPS,
   DEFAULT_DB_BACKUP_RETENTION_DAYS,
+  DB_BACKUP_MAX_FILES_KEY,
+  DB_BACKUP_RETENTION_DAYS_KEY,
   parsePositiveInt,
   parseNonNegativeInt,
   pruneBackupDirectory,
+  readStoredDbBackupSetting,
 } from "./backupRetention";
 import { isAutomatedTestProcess } from "@/shared/utils/testProcess";
 
@@ -38,22 +41,6 @@ const TRUE_ENV_VALUES = new Set(["1", "true", "yes", "on"]);
 // `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";
-const DB_BACKUP_RETENTION_DAYS_KEY = "retentionDays";
-
-function getStoredDbBackupInteger(key: string, options: { min: number }): number | undefined {
-  try {
-    const db = getDbInstance();
-    const row = db
-      .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
-      .get(DB_BACKUP_SETTINGS_NAMESPACE, key) as { value?: string } | undefined;
-    if (!row?.value) return undefined;
-    const parsed = JSON.parse(row.value);
-    return Number.isInteger(parsed) && parsed >= options.min ? parsed : undefined;
-  } catch {
-    return undefined;
-  }
-}
 
 function setStoredDbBackupInteger(key: string, value: number, options: { min: number }): void {
   if (!Number.isInteger(value) || value < options.min) return;
@@ -75,7 +62,7 @@ export function getDbBackupMaxFiles() {
   if (process.env.DB_BACKUP_MAX_FILES) {
     return parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS);
   }
-  return getStoredDbBackupInteger(DB_BACKUP_MAX_FILES_KEY, { min: 1 }) ?? MAX_DB_BACKUPS;
+  return readStoredDbBackupSetting(getDbInstance(), DB_BACKUP_MAX_FILES_KEY, 1) ?? MAX_DB_BACKUPS;
 }
 
 /** Persist the operator-chosen age-based backup retention window. */
@@ -92,7 +79,7 @@ export function getDbBackupRetentionDays() {
     );
   }
   return (
-    getStoredDbBackupInteger(DB_BACKUP_RETENTION_DAYS_KEY, { min: 0 }) ??
+    readStoredDbBackupSetting(getDbInstance(), DB_BACKUP_RETENTION_DAYS_KEY, 0) ??
     DEFAULT_DB_BACKUP_RETENTION_DAYS
   );
 }

src/lib/db/backupRetention.ts

@@ -11,10 +11,55 @@
 
 import fs from "fs";
 import path from "path";
+import type { SqliteAdapter } from "./adapters/types";
 
 export const MAX_DB_BACKUPS = 20;
 export const DEFAULT_DB_BACKUP_RETENTION_DAYS = 0;
 
+const DB_BACKUP_SETTINGS_NAMESPACE = "dbBackup";
+export const DB_BACKUP_MAX_FILES_KEY = "maxFiles";
+export const DB_BACKUP_RETENTION_DAYS_KEY = "retentionDays";
+
+/**
+ * Reads a persisted `dbBackup` retention setting through the caller's own open adapter.
+ *
+ * Takes `db` explicitly rather than resolving the singleton itself: `core.ts`'s
+ * health-check backup path runs from inside database initialization/repair, where
+ * asking for the singleton via `getDbInstance()` would re-enter it. A DB too old to
+ * have `key_value` yet simply falls back to the default.
+ */
+export function readStoredDbBackupSetting(
+  db: SqliteAdapter,
+  key: string,
+  min: number
+): number | undefined {
+  try {
+    const row = db
+      .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
+      .get(DB_BACKUP_SETTINGS_NAMESPACE, key) as { value?: string } | undefined;
+    if (!row?.value) return undefined;
+    const parsed = JSON.parse(row.value);
+    return Number.isInteger(parsed) && parsed >= min ? parsed : undefined;
+  } catch {
+    return undefined;
+  }
+}
+
+/** Shared maxFiles/retentionDays precedence: env override → persisted operator setting → default. */
+export function resolveDbBackupRetentionSettings(db: SqliteAdapter): {
+  maxFiles: number;
+  retentionDays: number;
+} {
+  const maxFiles = process.env.DB_BACKUP_MAX_FILES
+    ? parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS)
+    : (readStoredDbBackupSetting(db, DB_BACKUP_MAX_FILES_KEY, 1) ?? MAX_DB_BACKUPS);
+  const retentionDays = process.env.DB_BACKUP_RETENTION_DAYS
+    ? parseNonNegativeInt(process.env.DB_BACKUP_RETENTION_DAYS, DEFAULT_DB_BACKUP_RETENTION_DAYS)
+    : (readStoredDbBackupSetting(db, DB_BACKUP_RETENTION_DAYS_KEY, 0) ??
+      DEFAULT_DB_BACKUP_RETENTION_DAYS);
+  return { maxFiles, retentionDays };
+}
+
 export function parsePositiveInt(value: string | undefined, fallback: number) {
   if (!value) return fallback;
   const parsed = Number.parseInt(value, 10);
@@ -151,3 +196,29 @@ export function pruneBackupDirectory(options: {
     retentionDays,
   };
 }
+
+/**
+ * Resolve settings, prune, and log — the exact sequence every backup call site needs
+ * right after writing a new snapshot. Never throws: a backup must not fail because
+ * housekeeping did. Not used by the pre-migration path: retention there deliberately
+ * stays outside the migration window (see db-pre-migration-backup-retention-10421.test.ts).
+ */
+export function pruneManagedDbBackups(
+  db: SqliteAdapter,
+  backupDir: string,
+  logPrefix: string
+): void {
+  try {
+    const { maxFiles, retentionDays } = resolveDbBackupRetentionSettings(db);
+    const result = pruneBackupDirectory({ backupDir, maxFiles, retentionDays });
+    if (result.deletedFiles > 0) {
+      console.log(
+        `${logPrefix} Pruned ${result.deletedFiles} old backup file(s) ` +
+          `(${result.keptBackupFamilies} kept, maxFiles=${maxFiles}, retentionDays=${retentionDays}).`
+      );
+    }
+  } catch (err: unknown) {
+    const message = err instanceof Error ? err.message : String(err);
+    console.warn(`${logPrefix} Failed to prune old backups: ${message}`);
+  }
+}

src/lib/db/core.ts

@@ -18,6 +18,7 @@ import fs from "fs";
 import { resolveWritableDataDir, getLegacyDotDataDir } from "../dataPaths";
 import { isNextBuildPhase } from "../buildPhase";
 import { runMigrations } from "./migrationRunner";
+import { pruneManagedDbBackups } from "./backupRetention";
 import { runDbHealthCheck } from "./healthCheck";
 import { resetAllDbModuleState } from "./stateReset";
 import { parseStoredPayload } from "../logPayloads";
@@ -857,6 +858,10 @@ function createManagedDbBackup(db: SqliteDatabase, reason: string): boolean {
 
     db.exec(`VACUUM INTO '${escapedBackupPath}'`);
     console.log(`[DB] Backup created (${reason}): ${backupPath}`);
+    // Unlike backup.ts's backupDbFile(), this path had no retention call at all: a
+    // periodic health-check backup running every few minutes with no pruning grew
+    // db_backups/ unbounded (observed: 570 GB / 60 files against a small live database).
+    pruneManagedDbBackups(db, backupDir, `[DB (${reason})]`);
     return true;
   } catch (error: unknown) {
     const message = error instanceof Error ? error.message : String(error);

tests/unit/db-backup-retention-shared.test.ts

@@ -0,0 +1,148 @@
+// Health-check-repair backups (core.ts's createManagedDbBackup) never called into the
+// shared retention policy at all, so db_backups/ grew without bound (observed live:
+// 570 GB / 60 files against a small live database). backup.ts and migrationRunner.ts
+// both already resolved settings + pruned after writing; core.ts's health-check path
+// is the one call site that reached VACUUM INTO with no retention step whatsoever.
+//
+// core.ts's real writer (createManagedDbBackup) is gated behind isAutomatedTestProcess()
+// and cannot be exercised end-to-end from this test runner (that gate is an intentional,
+// pre-existing production-safety check, not something this fix should bypass) — which is
+// also exactly why a missing prune call there could go unnoticed by the existing suite.
+// These tests instead pin the shared helper (pruneManagedDbBackups /
+// resolveDbBackupRetentionSettings) that core.ts's fix now calls, the same way
+// db-pre-migration-backup-retention-10421.test.ts pins migrationRunner.ts's call site.
+
+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 Database from "better-sqlite3";
+import {
+  pruneManagedDbBackups,
+  resolveDbBackupRetentionSettings,
+} from "../../src/lib/db/backupRetention.ts";
+
+const serial = { concurrency: false };
+
+/** Minimal adapter: pruneManagedDbBackups only ever calls db.prepare(...).get(...). */
+function createSettingsDb(sqlitePath: string) {
+  const db = new Database(sqlitePath);
+  db.exec(
+    "CREATE TABLE IF NOT EXISTS key_value (namespace TEXT, key TEXT, value TEXT, PRIMARY KEY (namespace, key))"
+  );
+  return {
+    prepare: (sql: string) => db.prepare(sql),
+    close: () => db.close(),
+  };
+}
+
+function storeSetting(db: ReturnType<typeof createSettingsDb>, key: string, value: number) {
+  db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
+    "dbBackup",
+    key,
+    JSON.stringify(value)
+  );
+}
+
+function makeTempDir() {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-backup-retention-shared-"));
+  fs.mkdirSync(path.join(dir, "db_backups"), { recursive: true });
+  return dir;
+}
+
+function seedBackups(backupDir: string, count: number, reason: string) {
+  for (let i = 0; i < count; i++) {
+    const name = `db_2026-08-${String(i + 1).padStart(2, "0")}T00-00-00-000Z_${reason}.sqlite`;
+    const filePath = path.join(backupDir, name);
+    fs.writeFileSync(filePath, "x");
+    const t = new Date(2026, 7, i + 1).getTime() / 1000;
+    fs.utimesSync(filePath, t, t);
+  }
+}
+
+function countBackups(backupDir: string) {
+  return fs.readdirSync(backupDir).filter((n) => n.startsWith("db_")).length;
+}
+
+function withEnv(vars: Record<string, string | undefined>, fn: () => void) {
+  const saved: Record<string, string | undefined> = {};
+  for (const [k, v] of Object.entries(vars)) {
+    saved[k] = process.env[k];
+    if (v === undefined) delete process.env[k];
+    else process.env[k] = v;
+  }
+  try {
+    return fn();
+  } finally {
+    for (const [k, v] of Object.entries(saved)) {
+      if (v === undefined) delete process.env[k];
+      else process.env[k] = v;
+    }
+  }
+}
+
+test(
+  "pruneManagedDbBackups caps health-check-repair backups at the persisted maxFiles",
+  serial,
+  () => {
+    const dataDir = makeTempDir();
+    const backupDir = path.join(dataDir, "db_backups");
+    const db = createSettingsDb(path.join(dataDir, "settings.sqlite"));
+
+    try {
+      seedBackups(backupDir, 30, "health-check-repair");
+      assert.equal(countBackups(backupDir), 30, "precondition: 30 stale backups on disk");
+      storeSetting(db, "maxFiles", 5);
+      storeSetting(db, "retentionDays", 0);
+
+      withEnv({ DB_BACKUP_MAX_FILES: undefined, DB_BACKUP_RETENTION_DAYS: undefined }, () => {
+        pruneManagedDbBackups(db as never, backupDir, "[DB (health-check-repair)]");
+      });
+
+      const remaining = countBackups(backupDir);
+      assert.ok(
+        remaining <= 5,
+        `expected the operator's persisted maxFiles=5 to cap db_backups, found ${remaining}`
+      );
+    } finally {
+      db.close();
+      fs.rmSync(dataDir, { recursive: true, force: true });
+    }
+  }
+);
+
+test("pruneManagedDbBackups never throws when pruning itself fails", serial, () => {
+  const dataDir = makeTempDir();
+  const db = createSettingsDb(path.join(dataDir, "settings.sqlite"));
+
+  try {
+    // A backup dir that cannot exist as a directory (it's a file) makes pruning fail;
+    // callers (health-check-repair, in production) must not see that as a backup failure.
+    const notADir = path.join(dataDir, "not-a-directory");
+    fs.writeFileSync(notADir, "x");
+
+    assert.doesNotThrow(() => {
+      pruneManagedDbBackups(db as never, path.join(notADir, "db_backups"), "[DB (test)]");
+    });
+  } finally {
+    db.close();
+    fs.rmSync(dataDir, { recursive: true, force: true });
+  }
+});
+
+test("resolveDbBackupRetentionSettings: env override wins over the persisted value", serial, () => {
+  const dataDir = makeTempDir();
+  const db = createSettingsDb(path.join(dataDir, "settings.sqlite"));
+
+  try {
+    storeSetting(db, "maxFiles", 5);
+    withEnv({ DB_BACKUP_MAX_FILES: "12" }, () => {
+      const { maxFiles } = resolveDbBackupRetentionSettings(db as never);
+      assert.equal(maxFiles, 12);
+    });
+  } finally {
+    db.close();
+    fs.rmSync(dataDir, { recursive: true, force: true });
+  }
+});