fix(desktop): reclaim pre-update state.db backups even when the backup fails

apps/desktop/electron/emergency-backup-retention.test.ts

@@ -0,0 +1,154 @@
+// Retention policy for pre-update emergency state.db backups (#91229).
+//
+// The reported symptom was 2.5-3 GB of `state.db.pre-update-emergency-*.bak`
+// files that "are never cleaned up". A prune did exist, but it was nested
+// inside the `try` whose first statement wrote the new backup, so it only ran
+// when that write succeeded. These tests pin the selection rule; the
+// unconditional-call half is asserted structurally at the bottom.
+
+import { readFileSync } from 'node:fs'
+
+import { describe, expect, it } from 'vitest'
+
+import {
+  EMERGENCY_BACKUP_PREFIX,
+  EMERGENCY_BACKUP_RETENTION,
+  EMERGENCY_BACKUP_SUFFIX,
+  isEmergencyBackup,
+  selectEmergencyBackupsToDelete,
+} from './emergency-backup-retention'
+
+/** Build the exact filename `preflightStateDb` writes for a given instant. */
+function backupName(iso: string): string {
+  return `${EMERGENCY_BACKUP_PREFIX}${iso.replace(/[:.]/g, '-')}${EMERGENCY_BACKUP_SUFFIX}`
+}
+
+const OLDEST = backupName('2026-08-15T03:11:52.004Z')
+const OLDER = backupName('2026-08-17T09:02:00.900Z')
+const NEWER = backupName('2026-08-20T22:45:10.120Z')
+const NEWEST = backupName('2026-08-21T01:30:00.000Z')
+
+describe('isEmergencyBackup', () => {
+  it('accepts exactly the filenames preflightStateDb writes', () => {
+    expect(isEmergencyBackup(NEWEST)).toBe(true)
+  })
+
+  it('leaves the live database and its sidecars alone', () => {
+    // The single most important negative: this sweep runs in HERMES_HOME, so
+    // a loose predicate deletes the database the backups exist to protect.
+    expect(isEmergencyBackup('state.db')).toBe(false)
+    expect(isEmergencyBackup('state.db-wal')).toBe(false)
+    expect(isEmergencyBackup('state.db-shm')).toBe(false)
+  })
+
+  it('leaves other .bak files alone', () => {
+    // The Python-level snapshot is a different mechanism with a different
+    // lifetime; this policy does not own it.
+    expect(isEmergencyBackup('state.db.bak')).toBe(false)
+    expect(isEmergencyBackup('config.yaml.bak')).toBe(false)
+    expect(isEmergencyBackup(`${EMERGENCY_BACKUP_PREFIX}2026-08-21.tmp`)).toBe(false)
+  })
+
+  it('does not match a prefixed name embedded mid-filename', () => {
+    expect(isEmergencyBackup(`copy-of-${NEWEST}`)).toBe(false)
+  })
+})
+
+describe('selectEmergencyBackupsToDelete', () => {
+  it('keeps the newest RETENTION backups and returns the rest', () => {
+    const doomed = selectEmergencyBackupsToDelete([OLDER, NEWEST, OLDEST, NEWER])
+
+    expect(EMERGENCY_BACKUP_RETENTION).toBe(3)
+    expect(doomed).toEqual([OLDEST])
+  })
+
+  it('returns nothing while at or under the budget', () => {
+    expect(selectEmergencyBackupsToDelete([])).toEqual([])
+    expect(selectEmergencyBackupsToDelete([NEWEST])).toEqual([])
+    expect(selectEmergencyBackupsToDelete([NEWEST, NEWER, OLDER])).toEqual([])
+  })
+
+  it('counts the just-written backup toward the budget', () => {
+    // The old code excluded the new file before slicing, so three survived
+    // while the comment said two. Whatever the number is, the newest file has
+    // to be inside it or the budget is off by one forever.
+    const listing = [OLDEST, OLDER, NEWER, NEWEST]
+    const kept = listing.filter(f => !selectEmergencyBackupsToDelete(listing).includes(f))
+
+    expect(kept).toHaveLength(EMERGENCY_BACKUP_RETENTION)
+    expect(kept).toContain(NEWEST)
+  })
+
+  it('orders by name, not by array order or mtime', () => {
+    // Sorting is lexicographic because the ISO timestamp is fixed-width; a
+    // shuffled listing (readdir gives no ordering guarantee) must not change
+    // which files survive.
+    const shuffled = [NEWER, OLDEST, NEWEST, OLDER]
+
+    expect(selectEmergencyBackupsToDelete(shuffled)).toEqual([OLDEST])
+  })
+
+  it('ignores unrelated files when choosing what to delete', () => {
+    const doomed = selectEmergencyBackupsToDelete([
+      'state.db',
+      'state.db-wal',
+      'config.yaml',
+      OLDEST,
+      OLDER,
+      NEWER,
+      NEWEST,
+    ])
+
+    expect(doomed).toEqual([OLDEST])
+  })
+
+  it('deletes everything when told to retain none', () => {
+    expect(selectEmergencyBackupsToDelete([NEWEST, OLDEST], 0)).toEqual([NEWEST, OLDEST])
+  })
+
+  it('treats a nonsensical retention as retain-none rather than deleting at random', () => {
+    expect(selectEmergencyBackupsToDelete([NEWEST, OLDEST], -1)).toEqual([NEWEST, OLDEST])
+    expect(selectEmergencyBackupsToDelete([NEWEST, OLDEST], Number.NaN)).toEqual([NEWEST, OLDEST])
+  })
+
+  it('scales past the budget without dropping any candidate', () => {
+    const many = Array.from({ length: 40 }, (_, i) =>
+      backupName(`2026-08-21T0${Math.floor(i / 10)}:${String(i % 10).padStart(2, '0')}:00.000Z`)
+    )
+
+    const doomed = selectEmergencyBackupsToDelete(many)
+
+    expect(doomed).toHaveLength(many.length - EMERGENCY_BACKUP_RETENTION)
+    // Nothing kept is also deleted, and nothing is listed twice.
+    expect(new Set(doomed).size).toBe(doomed.length)
+  })
+})
+
+describe('preflightStateDb wiring', () => {
+  // The selection rule above is only half the fix. The bug was WHERE the
+  // sweep was called from, and that is a property of main.ts, which cannot be
+  // imported here (it boots Electron). So assert it against the source, the
+  // same way the repo pins other cross-module invariants.
+  const main = readFileSync(new URL('./main.ts', import.meta.url), 'utf8')
+  const body = main.slice(main.indexOf('function preflightStateDb('))
+
+  it('sweeps before the first early return, not after the copy succeeds', () => {
+    const call = body.indexOf('pruneEmergencyStateDbBackups(hermesHome, rememberLog)')
+    // Anchor on the guard itself, not on the word "return": the surrounding
+    // comments say "early return" and would satisfy a naive search.
+    const firstGuard = body.indexOf('if (!fileExists(stateDbPath))')
+    const copy = body.indexOf('fs.copyFileSync(stateDbPath, emergencyPath)')
+
+    expect(call).toBeGreaterThan(-1)
+    expect(firstGuard).toBeGreaterThan(-1)
+    // Before the first guard => a missing or too-small state.db still reclaims.
+    expect(call).toBeLessThan(firstGuard)
+    // Before the copy => ENOSPC and EBUSY still reclaim. This is the assertion
+    // that fails if anyone re-nests the sweep inside the copy's try block.
+    expect(call).toBeLessThan(copy)
+  })
+
+  it('no longer carries the inline prune that was coupled to the copy', () => {
+    expect(body).not.toContain("f.startsWith('state.db.pre-update-emergency-')")
+  })
+})

apps/desktop/electron/emergency-backup-retention.ts

@@ -0,0 +1,78 @@
+// Retention policy for the pre-update emergency copies of `state.db`.
+//
+// Before an update, `preflightStateDb` copies `state.db` to
+// `state.db.pre-update-emergency-<timestamp>.bak` so a torn update can be
+// recovered from. Those copies are the size of the live database, which on a
+// busy install is comfortably several hundred MB, so they have to be reclaimed
+// or they eat the disk (issue #91229 reported 2.5-3 GB of accumulated .bak
+// files and ~6.5 GB of week-over-week C: growth).
+//
+// The sweep used to live inline in `preflightStateDb`, nested inside the `try`
+// whose first statement was the `copyFileSync`. That coupled reclaiming old
+// backups to successfully writing a new one, so every failure mode that
+// *prevents* the copy also skipped the cleanup:
+//
+//   * the copy throws ENOSPC because the disk is full -- and it is full partly
+//     because these backups were never reclaimed;
+//   * the copy throws EBUSY/EPERM because another process holds `state.db`,
+//     which on Windows is the ordinary state during a failed self-update and
+//     the exact scenario #91229 is filed about;
+//   * `state.db` is missing or too small to be a real database, both of which
+//     `return` before the copy is ever attempted.
+//
+// In other words the janitor only ran on the days nothing needed cleaning.
+// Splitting the selection out here lets `preflightStateDb` run it
+// unconditionally, and lets it be tested without Electron or a real disk.
+
+/** Filename prefix written by `preflightStateDb`. */
+export const EMERGENCY_BACKUP_PREFIX = 'state.db.pre-update-emergency-'
+
+/** Filename suffix written by `preflightStateDb`. */
+export const EMERGENCY_BACKUP_SUFFIX = '.bak'
+
+/**
+ * How many emergency backups to keep, newest first, counting the one just
+ * written.
+ *
+ * This preserves the effective behaviour of the previous implementation
+ * rather than the behaviour its comment claimed. That comment said "Prune to
+ * the 2 most recent", but the filter excluded the backup it had just created
+ * before slicing, so three files survived: the new one plus two older. At the
+ * reported ~650-750 MB apiece that is the difference between ~1.4 GB and
+ * ~2.1 GB retained, so it is not a rounding error, and picking the smaller
+ * number would delete recovery data users currently have. Deciding which
+ * number is actually wanted is a maintainer call; this change only makes the
+ * number explicit and honest.
+ */
+export const EMERGENCY_BACKUP_RETENTION = 3
+
+/** True when *name* is one of the emergency backups this module manages. */
+export function isEmergencyBackup(name: string): boolean {
+  return (
+    typeof name === 'string' &&
+    name.startsWith(EMERGENCY_BACKUP_PREFIX) &&
+    name.endsWith(EMERGENCY_BACKUP_SUFFIX)
+  )
+}
+
+/**
+ * Given every filename in the Hermes home directory, return the emergency
+ * backups that should be deleted -- everything past the newest *retain*.
+ *
+ * Ordering is lexicographic and deliberately so: the timestamp is
+ * `new Date().toISOString()` with `:` and `.` replaced by `-`, which is
+ * fixed-width and zero-padded, so byte order and chronological order agree.
+ * That keeps the sweep independent of file mtimes, which a backup/restore or
+ * a sync client can rewrite.
+ *
+ * Non-backup files are ignored, so this is safe to hand a whole directory
+ * listing. Returns names, not paths; the caller owns the directory.
+ */
+export function selectEmergencyBackupsToDelete(
+  names: readonly string[],
+  retain: number = EMERGENCY_BACKUP_RETENTION
+): string[] {
+  const keep = Number.isFinite(retain) && retain > 0 ? Math.floor(retain) : 0
+
+  return names.filter(isEmergencyBackup).sort().reverse().slice(keep)
+}

apps/desktop/electron/main.ts

@@ -133,6 +133,10 @@ import {
 } from './desktop-uninstall'
 import { describeDevCdpDecision, resolveDevCdpPort } from './dev-cdp'
 import { installEmbedReferer } from './embed-referer'
+import {
+  EMERGENCY_BACKUP_RETENTION,
+  selectEmergencyBackupsToDelete,
+} from './emergency-backup-retention'
 import { createEventDeduper } from './event-dedupe'
 import {
   buildTerminalScript,
@@ -3898,9 +3902,48 @@ function runningAppBundle() {
 // desktop Electron process itself, before the backend is killed and
 // before the updater is spawned — a separate safety net from the
 // Python-level pre-update snapshot inside `hermes update`.
+// Delete emergency state.db backups past the retention budget. Never throws:
+// this is disk hygiene on the update path, and a failure to reclaim must not
+// be able to abort an update.
+function pruneEmergencyStateDbBackups(hermesHome, rememberLog) {
+  try {
+    const stale = selectEmergencyBackupsToDelete(fs.readdirSync(hermesHome), EMERGENCY_BACKUP_RETENTION)
+
+    let reclaimed = 0
+
+    for (const name of stale) {
+      const target = path.join(hermesHome, name)
+
+      try {
+        const bytes = fs.statSync(target).size
+
+        fs.unlinkSync(target)
+        reclaimed += bytes
+      } catch {
+        // Held open, already gone, or not ours to delete. The next update
+        // tries again; one unremovable file must not stop the rest.
+        void 0
+      }
+    }
+
+    if (reclaimed > 0) {
+      rememberLog(`[updates] reclaimed ${reclaimed} bytes from ${stale.length} old state.db backup(s)`)
+    }
+  } catch {
+    void 0
+  }
+}
+
 function preflightStateDb(hermesHome, rememberLog) {
   const stateDbPath = path.join(hermesHome, 'state.db')
 
+  // Unconditionally, and BEFORE any early return. Every reason this function
+  // gives up (no state.db, too small to be a database, stat throws) and every
+  // reason the copy below fails (ENOSPC on a disk these very files filled,
+  // EBUSY from the running app on Windows) used to skip the sweep, so the
+  // backups accumulated precisely when reclaiming mattered most (#91229).
+  pruneEmergencyStateDbBackups(hermesHome, rememberLog)
+
   if (!fileExists(stateDbPath)) {
     rememberLog('[updates] state.db pre-flight: not found (fresh install?)')
 
@@ -3943,30 +3986,12 @@ function preflightStateDb(hermesHome, rememberLog) {
 
         rememberLog(`[updates] emergency state.db backup: ${emergencyPath} ` + `(${emergStat.size} bytes)`)
 
-        // Prune to the 2 most recent emergency backups.
-        try {
-          const homeDir = fs.readdirSync(hermesHome)
-
-          const backups = homeDir
-            .filter(
-              f =>
-                f.startsWith('state.db.pre-update-emergency-') &&
-                f.endsWith('.bak') &&
-                f !== path.basename(emergencyPath)
-            )
-            .sort()
-            .reverse()
-
-          for (const old of backups.slice(2)) {
-            try {
-              fs.unlinkSync(path.join(hermesHome, old))
-            } catch {
-              void 0
-            }
-          }
-        } catch {
-          void 0
-        }
+        // Reclaim now that a new backup exists, so the newest one counts
+        // toward the retention budget. The unconditional sweep at the top of
+        // preflightStateDb is what guarantees this happens at all; this second
+        // call only keeps the budget from being exceeded by one until the next
+        // update runs.
+        pruneEmergencyStateDbBackups(hermesHome, rememberLog)
       } catch (copyErr) {
         rememberLog(`[updates] emergency state.db backup failed: ${copyErr.message}`)
       }