From 04bea603036e60e78047c88f48bf25e1c4eb6831 Mon Sep 17 00:00:00 2001 From: Koosha Paridehpour <42529354+KooshaPari@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:17:11 -0700 Subject: [PATCH] fix(backend): prune old backups after health-check-repair snapshots (#13308) (#13404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prunes `db_backups/` after the health-check-repair `VACUUM INTO` snapshot (#13308). That path never ran retention, so every restart of a healthy database added another full-size copy. It now uses the same `DB_BACKUP_MAX_FILES` / `DB_BACKUP_RETENTION_DAYS` limits as `backup.ts`, importing `backupRetention` directly to avoid the import cycle. Maintainer additions: merged the current release branch and rebaselined `src/lib/db/core.ts` 1745→1767 in `file-size-baseline.json` with a dated annotation (legitimate growth). Chosen over #13540, which did the same with a fire-and-forget dynamic import. Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green. Thanks @KooshaPari! --- config/quality/file-size-baseline.json | 3 +- src/lib/db/core.ts | 23 ++++++ .../db-backup-healthcheck-prune-13308.test.ts | 73 +++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 tests/unit/db-backup-healthcheck-prune-13308.test.ts diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index e5ac089c0e..57e30418c8 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -1,4 +1,5 @@ { + "_rebaseline_2026_09_14_13404_healthcheck_backup_prune": "PR #13404 own growth: src/lib/db/core.ts 1745->1767 (+22). createManagedDbBackup (the health-check-repair snapshot path) never ran retention, so every restart of a healthy DB added a full-size copy to db_backups/; it now prunes with the same env-driven limits as backup.ts, importing backupRetention directly to avoid the backup.ts cycle. Covered by tests/unit/db-backup-healthcheck-prune-13308.test.ts.", "_rebaseline_2026_09_14_13349_virtualfactory_custom_models_guard": "PR #13349 own growth: open-sse/services/autoCombo/virtualFactory.ts 1219->1230 (+11). The customModels key_value blob is operator-writable raw JSON, so a null or non-object row null-derefed every read and no auto/* pool could materialize; the builder now filters rows the same way catalog.ts already does. Irreducible at the read site. Covered by tests/unit/combo-auto-pool-visible-only.test.ts.", "_rebaseline_2026_09_11_12732_catalog_timeout_pin": "+1 in tests/unit/models-catalog-route.test.ts (1652->1653) for a single line: process.env.CATALOG_BUILD_TIMEOUT_MS. #12627 bounds a cold catalog build at 8s; beforeEach resets the catalog cache so every case in this file pays a cold build, and a tsx runner needs 10-13s under load — the file returned catalog_build_timeout instead of rows and oscillated between 1 and 10 failures per run, reddening the whole PR queue (base-red #12732). The bound itself stays covered by tests/unit/12627-catalog-inflight-timeout.test.ts. The file is already at its frozen ceiling, so the pin cannot be absorbed; structural shrink tracked in #3501.", "_rebaseline_2026_09_11_12945_image_only_model_guard": "PR #12945 own growth: open-sse/handlers/imageGeneration.ts 3259->3293 (+35/-1). The image-only-model guard the PR adds to clear its base-red: the handler now recognises a model that only serves image generation and answers before the chat path can mis-route it. Irreducible at this call site; the predicate itself lives outside the file. Landed as its own PR rather than on #12945 because that branch has a live worktree in another session and pushing to it would pull the branch out from under whoever is working it. Covered by the batch run: 203/208 with the 5 remaining failures reproducing on the pure tip.", @@ -468,7 +469,7 @@ "src/app/api/v1/models/catalog.ts": 2075, "src/app/docs/lib/openapi.generated.ts": 1347, "src/lib/db/apiKeys.ts": 1625, - "src/lib/db/core.ts": 1745, + "src/lib/db/core.ts": 1767, "src/lib/db/migrationRunner.ts": 1206, "src/lib/tailscaleTunnel.ts": 1208, "src/lib/tokenHealthCheck.ts": 1218, diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index 9d1a93a502..7ccacdffbe 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -16,6 +16,13 @@ import path from "path"; import { retryProbeIfTransient } from "./probeUtils"; import fs from "fs"; import { resolveWritableDataDir, getLegacyDotDataDir } from "../dataPaths"; +import { + MAX_DB_BACKUPS, + DEFAULT_DB_BACKUP_RETENTION_DAYS, + parsePositiveInt, + parseNonNegativeInt, + pruneBackupDirectory, +} from "./backupRetention"; import { isNextBuildPhase } from "../buildPhase"; import { runMigrations } from "./migrationRunner"; import { runDbHealthCheck } from "./healthCheck"; @@ -888,6 +895,22 @@ function createManagedDbBackup(db: SqliteDatabase, reason: string): boolean { db.exec(`VACUUM INTO '${escapedBackupPath}'`); console.log(`[DB] Backup created (${reason}): ${backupPath}`); + + // Prune old backups to prevent the directory from growing without bound. + // This mirrors the post-backup pruning in backup.ts but avoids a circular + // dependency by importing directly from backupRetention.ts. + try { + const maxFiles = process.env.DB_BACKUP_MAX_FILES + ? parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS) + : MAX_DB_BACKUPS; + const retentionDays = process.env.DB_BACKUP_RETENTION_DAYS + ? parseNonNegativeInt(process.env.DB_BACKUP_RETENTION_DAYS, DEFAULT_DB_BACKUP_RETENTION_DAYS) + : DEFAULT_DB_BACKUP_RETENTION_DAYS; + pruneBackupDirectory({ backupDir, maxFiles, retentionDays }); + } catch { + // Retention is best-effort; never let a pruning failure obscure the backup result. + } + return true; } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); diff --git a/tests/unit/db-backup-healthcheck-prune-13308.test.ts b/tests/unit/db-backup-healthcheck-prune-13308.test.ts new file mode 100644 index 0000000000..74889c2dd8 --- /dev/null +++ b/tests/unit/db-backup-healthcheck-prune-13308.test.ts @@ -0,0 +1,73 @@ +// #13308 — health-check-repair backups were never pruned because the retention +// call was missing from the VACUUM INTO path in core.ts. This test seeds a +// backup directory with more families than MAX_DB_BACKUPS, then runs the same +// pruneBackupDirectory call that createManagedDbBackup now executes after each +// health-check snapshot, and asserts that overflow families are deleted. + +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 { + pruneBackupDirectory, + MAX_DB_BACKUPS, +} from "../../src/lib/db/backupRetention.ts"; + +const serial = { concurrency: false }; + +function makeBackupDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-backup-prune-")); +} + +function seedFamilies(dir: string, count: number) { + for (let i = 0; i < count; i++) { + const ts = new Date(Date.now() - i * 1000).toISOString().replace(/[:.]/g, "-"); + const name = `db_${ts}_health-check-repair.sqlite`; + fs.writeFileSync(path.join(dir, name), Buffer.from(`fake-snapshot-${i}`)); + } +} + +test("#13308 — pruneBackupDirectory removes overflow from health-check-repair path", serial, () => { + const dir = makeBackupDir(); + try { + const extra = 5; + seedFamilies(dir, MAX_DB_BACKUPS + extra); + + const before = fs.readdirSync(dir).filter((f) => f.endsWith(".sqlite")).length; + assert.ok(before >= MAX_DB_BACKUPS + extra, `seeded ${before} families`); + + const result = pruneBackupDirectory({ + backupDir: dir, + maxFiles: MAX_DB_BACKUPS, + retentionDays: 0, + }); + + const after = fs.readdirSync(dir).filter((f) => f.endsWith(".sqlite")).length; + assert.equal(after, MAX_DB_BACKUPS, `pruned to MAX_DB_BACKUPS (${MAX_DB_BACKUPS}), got ${after}`); + assert.equal(result.deletedBackupFamilies, extra, `deleted ${extra} overflow families`); + assert.equal(result.keptBackupFamilies, MAX_DB_BACKUPS); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test("#13308 — pruneBackupDirectory is a no-op when under limit", serial, () => { + const dir = makeBackupDir(); + try { + seedFamilies(dir, 3); + + const result = pruneBackupDirectory({ + backupDir: dir, + maxFiles: MAX_DB_BACKUPS, + retentionDays: 0, + }); + + const after = fs.readdirSync(dir).filter((f) => f.endsWith(".sqlite")).length; + assert.equal(after, 3, "no files removed when under limit"); + assert.equal(result.deletedBackupFamilies, 0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +});