fix(backups): skip archives already being deleted in prune and delete-installation

src/domain/installations/backup.ts

@@ -29,7 +29,7 @@ export interface InstallationSnapshot {
   path: string
   backupsLimit: number
   compressionLevel: number
-  backups: readonly BackupRecord[]
+  backups: readonly (BackupRecord & { isDeleting?: boolean })[]
   isBackingUp: boolean
   isPlaying: boolean
   isRestoringBackup: boolean
@@ -117,12 +117,18 @@ export async function makeInstallationBackup(ports: MakeInstallationBackupPorts,
     while (remaining > 0 && remaining >= installation.backupsLimit) {
       const oldest = installation.backups[remaining - 1]
       if (!oldest) break
+      remaining--
+
+      // Already on its way out through another operation (a manual delete in
+      // flight). Removing it here too would race the same file; counting it
+      // toward `remaining` without touching it is correct either way, since
+      // it will not be there once that other operation finishes.
+      if (oldest.isDeleting) continue
 
       if (!(await ports.fileSystem.remove(oldest.path))) return refuse("prune-failed", deletedBackupIds)
 
       deletedBackupIds.push(oldest.id)
       events.onBackupDeleted?.(oldest)
-      remaining--
     }
 
     const date = ports.clock.now()

src/domain/installations/delete.ts

@@ -4,7 +4,7 @@ import type { BackupRecord } from "./backup"
 /** The installation state a delete decision needs, copied out of wherever it lives. */
 export interface InstallationDeleteSnapshot {
   path: string
-  backups: readonly Pick<BackupRecord, "path">[]
+  backups: readonly (Pick<BackupRecord, "path"> & { isDeleting?: boolean })[]
   isPlaying: boolean
   isBackingUp: boolean
   isRestoringBackup: boolean
@@ -80,6 +80,13 @@ export async function deleteInstallation(ports: DeleteInstallationPorts, input:
   const failedBackupPaths: string[] = []
 
   for (const backup of installation.backups) {
+    // Already on its way out through another operation (a manual delete in
+    // flight). Neither this operation's success nor its failure, so it is
+    // reported as neither: removing it here would race the same file, and
+    // counting it as a failure would be misleading when the other operation
+    // is the one that actually succeeds.
+    if (backup.isDeleting) continue
+
     if (await ports.fileSystem.remove(backup.path)) {
       events.onBackupDeleted?.(backup.path)
     } else {

src/renderer/src/features/installations/adapters/backup.ts

@@ -58,7 +58,7 @@ export function toInstallationSnapshot(installation: InstallationType): Installa
     path: installation.path,
     backupsLimit: installation.backupsLimit,
     compressionLevel: installation.compressionLevel,
-    backups: installation.backups,
+    backups: installation.backups.map((backup) => ({ id: backup.id, date: backup.date, path: backup.path, isDeleting: backup._deleting ?? false })),
     isBackingUp: installation._backuping ?? false,
     isPlaying: installation._playing ?? false,
     isRestoringBackup: installation._restoringBackup ?? false

src/renderer/src/features/installations/adapters/delete.ts

@@ -10,7 +10,7 @@ export function createDeleteInstallationPorts(): DeleteInstallationPorts {
 export function toInstallationDeleteSnapshot(installation: InstallationType): InstallationDeleteSnapshot {
   return {
     path: installation.path,
-    backups: installation.backups.map((backup) => ({ path: backup.path })),
+    backups: installation.backups.map((backup) => ({ path: backup.path, isDeleting: backup._deleting ?? false })),
     isPlaying: installation._playing ?? false,
     isBackingUp: installation._backuping ?? false,
     isRestoringBackup: installation._restoringBackup ?? false

tests/domain/installations/backup.test.ts

@@ -65,8 +65,8 @@ function fakePorts(overrides: Partial<MakeInstallationBackupPorts> = {}): MakeIn
   }
 }
 
-function backup(id: string): BackupRecord {
-  return { id, date: 1, path: `/backups/${id}.zip` }
+function backup(id: string, overrides: { isDeleting?: boolean } = {}): BackupRecord & { isDeleting?: boolean } {
+  return { id, date: 1, path: `/backups/${id}.zip`, ...overrides }
 }
 
 function snapshot(overrides: Partial<InstallationSnapshot> = {}): InstallationSnapshot {
@@ -193,6 +193,45 @@ describe("makeInstallationBackup pruning", () => {
     assert.equal(trace.at(-2), "guard-release")
     assert.equal(trace.at(-1), "finished")
   })
+
+  it("skips the oldest backup when it is already being deleted elsewhere, without touching its file", async () => {
+    const installation = snapshot({ backupsLimit: 2, backups: [backup("b1"), backup("b2"), backup("b3", { isDeleting: true })] })
+
+    const result = await makeInstallationBackup(fakePorts(), { installation, backupsFolder: "/backups" }, recordingEvents())
+
+    assert.equal(result.ok, true)
+    assert.deepEqual(result.deletedBackupIds, ["b2"])
+    assert.deepEqual(
+      trace.filter((entry) => entry.startsWith("remove:") || entry.startsWith("deleted:")),
+      ["remove:/backups/b2.zip", "deleted:b2"]
+    )
+  })
+
+  it("counts a skipped in-flight deletion toward the limit instead of also removing a newer backup", async () => {
+    const installation = snapshot({ backupsLimit: 2, backups: [backup("b1"), backup("b2", { isDeleting: true })] })
+
+    const result = await makeInstallationBackup(fakePorts(), { installation, backupsFolder: "/backups" }, recordingEvents())
+
+    assert.equal(result.ok, true)
+    assert.deepEqual(result.deletedBackupIds, [])
+    assert.equal(
+      trace.some((entry) => entry.startsWith("remove:")),
+      false
+    )
+  })
+
+  it("keeps pruning past a skipped in-flight deletion to reach the limit", async () => {
+    const installation = snapshot({ backupsLimit: 2, backups: [backup("b1"), backup("b2"), backup("b3", { isDeleting: true }), backup("b4")] })
+
+    const result = await makeInstallationBackup(fakePorts(), { installation, backupsFolder: "/backups" }, recordingEvents())
+
+    assert.equal(result.ok, true)
+    assert.deepEqual(result.deletedBackupIds, ["b4", "b2"])
+    assert.deepEqual(
+      trace.filter((entry) => entry.startsWith("remove:") || entry.startsWith("deleted:")),
+      ["remove:/backups/b4.zip", "deleted:b4", "remove:/backups/b2.zip", "deleted:b2"]
+    )
+  })
 })
 
 describe("makeInstallationBackup archiving", () => {

tests/domain/installations/delete.test.ts

@@ -151,4 +151,36 @@ describe("deleteInstallation with data deletion", () => {
 
     assert.deepEqual(result, { ok: true, failedBackupPaths: [] })
   })
+
+  it("skips a backup already being deleted elsewhere, without touching its file or reporting it as failed", async () => {
+    const installation = snapshot({
+      backups: [{ path: "/backups/b1.zip" }, { path: "/backups/b2.zip", isDeleting: true }, { path: "/backups/b3.zip" }]
+    })
+
+    const result = await deleteInstallation(fakePorts(), { installation, deleteData: true }, recordingEvents())
+
+    assert.deepEqual(result, { ok: true, failedBackupPaths: [] })
+    assert.deepEqual(trace, [
+      "remove:/installations/my-install",
+      "data-deleted",
+      "remove:/backups/b1.zip",
+      "backup-deleted:/backups/b1.zip",
+      "remove:/backups/b3.zip",
+      "backup-deleted:/backups/b3.zip"
+    ])
+  })
+
+  it("treats every backup as in flight the same way, reporting a clean success with none removed here", async () => {
+    const installation = snapshot({
+      backups: [
+        { path: "/backups/b1.zip", isDeleting: true },
+        { path: "/backups/b2.zip", isDeleting: true }
+      ]
+    })
+
+    const result = await deleteInstallation(fakePorts(), { installation, deleteData: true }, recordingEvents())
+
+    assert.deepEqual(result, { ok: true, failedBackupPaths: [] })
+    assert.deepEqual(trace, ["remove:/installations/my-install", "data-deleted"])
+  })
 })