fix(backup): persist last-success/error status; wall-clock-based schedule

BackupService tracked last-success/last-error as plain in-process fields and scheduled the daily backup via setInterval measured from process start — so any server restart (deploy/crash/OOM/reboot, routine under `restart: always`) silently reset the admin UI to "last successful backup: Never" and drifted the actual cadence, independent of whether backups were writing correctly to disk (they were — a real field incident at park-buzi showed 7 valid rotating backups on disk with the status stuck on "Never"). Persist last-success/error to new site_config columns (migration 0025) and add BackupService.isDue(), computed from the persisted timestamp instead of process uptime; server.ts now polls every 15 min and lets isDue() gate the actual run. No API/UI contract change. Claude-Session: https://claude.ai/code/session_01FWncR69HgGPuei1dLrW3cU
2026-08-30 18:11:23 +02:00
parent 3a176c5cc8
commit 2910672b5a
7 changed files with 317 additions and 28 deletions
@@ -11,6 +11,12 @@ import { DEFAULT_BACKUP_RETENTION, runBackup, type BackupResult, type BackupRete
 // a key must never live in the DB it backs up. Remembers the last outcome so the route + UI can
 // show last-success / last-error, and serializes concurrent runs (manual + timer). See
 // wiki/concepts/backup-recovery.md.
+//
+// Last-success/last-error are PERSISTED to site_config (backup_last_*), not just held in
+// memory — an earlier version tracked these as plain in-process fields only, so every server
+// restart (deploy, crash, OOM, host reboot — all routine under `restart: always`) silently
+// reset the admin UI to "last successful backup: Never", even with valid, correctly-rotating
+// backups already on disk (2026-08-30 field incident, park-buzi). See wiki/concepts/backup-recovery.md.

 /** The dedicated backup-encryption key, from env (NOT the DB). Separate from EVENT_SIGNING_KEY. */
 export function backupKeyFromEnv(): string {
@@ -65,16 +71,33 @@ export class BackupService {
  readonly #logger?: FastifyBaseLogger;

  #running = false;
-  #lastSuccessAt: string | null = null;
-  #lastResult: BackupResult | null = null;
-  #lastErrorAt: string | null = null;
-  #lastError: string | null = null;

  constructor(db: Db, logger?: FastifyBaseLogger) {
    this.#db = db;
    this.#logger = logger;
  }

+  /** Fresh read of the persisted row (single source of truth — no in-memory cache to go stale
+   *  or reset on restart). */
+  #row(): { backupLastSuccessAt: string | null; backupLastResultJson: string | null; backupLastErrorAt: string | null; backupLastError: string | null } | undefined {
+    return this.#db.select().from(siteConfig).where(eq(siteConfig.id, 1)).get();
+  }
+
+  #persist(patch: {
+    backupLastSuccessAt?: string | null;
+    backupLastResultJson?: string | null;
+    backupLastErrorAt?: string | null;
+    backupLastError?: string | null;
+  }): void {
+    const updatedAt = new Date().toISOString();
+    const existing = this.#row();
+    if (existing) {
+      this.#db.update(siteConfig).set({ ...patch, updatedAt }).where(eq(siteConfig.id, 1)).run();
+    } else {
+      this.#db.insert(siteConfig).values({ id: 1, ...patch, updatedAt }).run();
+    }
+  }
+
  /** The admin-chosen target dir from site_config (null/empty = unset). Read fresh each call. */
  targetDir(): string | null {
    const row = this.#db.select().from(siteConfig).where(eq(siteConfig.id, 1)).get();
@@ -104,6 +127,15 @@ export class BackupService {

  status(): BackupStatus {
    const r = this.retention();
+    const row = this.#row();
+    let lastResult: BackupStatus["lastResult"] = null;
+    if (row?.backupLastResultJson) {
+      try {
+        lastResult = JSON.parse(row.backupLastResultJson) as BackupStatus["lastResult"];
+      } catch {
+        lastResult = null; // corrupt/foreign value in the column — don't let it crash status()
+      }
+    }
    return {
      configured: this.configured,
      targetDir: this.targetDir(),
@@ -111,12 +143,10 @@ export class BackupService {
      keepDailyDays: r.keepDailyDays,
      keyPresent: this.keyPresent,
      running: this.#running,
-      lastSuccessAt: this.#lastSuccessAt,
-      lastResult: this.#lastResult
-        ? { path: this.#lastResult.path, bytes: this.#lastResult.bytes, prunedFiles: this.#lastResult.prunedFiles }
-        : null,
-      lastErrorAt: this.#lastErrorAt,
-      lastError: this.#lastError,
+      lastSuccessAt: row?.backupLastSuccessAt ?? null,
+      lastResult,
+      lastErrorAt: row?.backupLastErrorAt ?? null,
+      lastError: row?.backupLastError ?? null,
    };
  }

@@ -139,14 +169,17 @@ export class BackupService {
      try {
        this.#logger?.info(`backup: starting (${trigger}) → ${targetDir}`);
        const res = await runBackup(this.#db, { targetDir, key, retention: this.retention() }, this.#logger);
-        this.#lastResult = res;
-        this.#lastSuccessAt = new Date().toISOString();
-        this.#lastError = null;
+        this.#persist({
+          backupLastSuccessAt: new Date().toISOString(),
+          backupLastResultJson: JSON.stringify({ path: res.path, bytes: res.bytes, prunedFiles: res.prunedFiles }),
+          backupLastErrorAt: null,
+          backupLastError: null,
+        });
        return res;
      } catch (err) {
-        this.#lastError = (err as Error).message;
-        this.#lastErrorAt = new Date().toISOString();
-        this.#logger?.error(`backup: failed (${trigger}): ${this.#lastError}`);
+        const message = (err as Error).message;
+        this.#persist({ backupLastErrorAt: new Date().toISOString(), backupLastError: message });
+        this.#logger?.error(`backup: failed (${trigger}): ${message}`);
        throw err;
      } finally {
        this.#running = false;
@@ -156,13 +189,34 @@ export class BackupService {
    return this.#inflight;
  }

-  /** Scheduled-run wrapper: never throws (a timer must not crash the process). */
+  /**
+   * Scheduled-run wrapper: never throws (a timer must not crash the process). Safe to call on
+   * a short, frequent poll (see server.ts) — it's a no-op unless `isDue()` says a full interval
+   * has actually elapsed since the last recorded success, so frequent polling doesn't cause
+   * frequent backups.
+   */
  async runScheduled(): Promise<void> {
    if (!this.configured) return; // silent no-op when backups aren't set up
+    if (!this.isDue()) return;
    try {
      await this.run("scheduled");
    } catch {
      /* recorded in last-error; already logged */
    }
  }
+
+  /**
+   * Wall-clock check: has enough time elapsed since the last successful backup for a new one
+   * to be due? Deliberately based on the PERSISTED last-success instant, not "time since this
+   * process started" — a `setInterval(..., 24h)` measured from process start silently drifts
+   * (or skips a whole day) across every restart, since the countdown restarts from zero each
+   * time regardless of when the last real backup happened. See wiki/concepts/backup-recovery.md.
+   */
+  isDue(now: Date = new Date(), intervalMs = 24 * 60 * 60 * 1000): boolean {
+    const lastSuccessAt = this.#row()?.backupLastSuccessAt;
+    if (!lastSuccessAt) return true; // never recorded a success → due immediately once configured
+    const last = new Date(lastSuccessAt).getTime();
+    if (Number.isNaN(last)) return true;
+    return now.getTime() - last >= intervalMs;
+  }
 }