Count an archive that is already gone as deleted

src/domain/installations/backupDeletion.ts

@@ -7,7 +7,7 @@ export type DeleteInstallationBackupFailure = "backup-in-use" | "file-delete-fai
 export type DeleteInstallationBackupResult = { ok: true } | { ok: false; reason: DeleteInstallationBackupFailure }
 
 export interface DeleteInstallationBackupPorts {
-  fileSystem: Pick<FileSystem, "remove">
+  fileSystem: Pick<FileSystem, "exists" | "remove">
 }
 
 export interface DeleteInstallationBackupInput {
@@ -18,6 +18,15 @@ export interface DeleteInstallationBackupInput {
  * Deletes one archive off disk. Dropping it from the installation record is the
  * caller's job, and only once this says the file is gone.
  *
+ * A record whose archive is not on disk any more counts as deleted. The host
+ * refuses to delete a path it cannot find (assertManagedDeletionPath runs with
+ * `allowMissing: false`, see ipc/pathPolicy.ts), so an archive the player
+ * removed from the Backups folder by hand used to answer "file-delete-failed"
+ * for good: the record could never be dropped, the manual delete kept failing
+ * and, once enough records piled up, the prune in makeInstallationBackup
+ * refused every new backup (#507). The file is gone either way, which is what
+ * the caller asked for.
+ *
  * @param ports Host capabilities the work runs on.
  * @param input The archive to delete.
  * @returns Success, or the reason the file is still there.
@@ -26,7 +35,10 @@ export async function deleteInstallationBackup(ports: DeleteInstallationBackupPo
   const { backup } = input
 
   if (backup.isRestoring || backup.isDeleting) return { ok: false, reason: "backup-in-use" }
-  if (!(await ports.fileSystem.remove(backup.path))) return { ok: false, reason: "file-delete-failed" }
+
+  if (!(await ports.fileSystem.remove(backup.path))) {
+    if (await ports.fileSystem.exists(backup.path)) return { ok: false, reason: "file-delete-failed" }
+  }
 
   return { ok: true }
 }

src/domain/installations/delete.ts

@@ -23,7 +23,7 @@ export type DeleteInstallationResult =
   | { ok: false; reason: DeleteInstallationFailure }
 
 export interface DeleteInstallationPorts {
-  fileSystem: Pick<FileSystem, "remove">
+  fileSystem: Pick<FileSystem, "exists" | "remove">
 }
 
 export interface DeleteInstallationInput {

src/renderer/src/locales/en-US.json

@@ -724,7 +724,7 @@
       "compressUnsafeEntry": "No backup made: this Installation's folder contains a symlink or a special file that cannot be archived.",
       "compressTooManyFiles": "No backup made: this Installation's folder has too many files to archive.",
       "compressWriteFailed": "No backup made: the backup archive could not be written. Check that the Backups folder is on a writable drive with free space.",
-      "pruneFailed": "No backup made: an old backup could not be removed to make room for the new one.",
+      "pruneFailed": "No backup made: an old backup file could not be deleted to make room for the new one. Check that it is not open in another program, then try again.",
       "errorRestoringBackup": "The Backup could not be restored, so your Installation was left as it was.",
       "restoreLeftDataAside": "The Backup could not be restored and your old Installation data is now in {{path}}. Move that folder back yourself before playing.",
       "backupsAmount": "Backups limit",

src/renderer/src/locales/fr-FR.json

@@ -724,7 +724,7 @@
       "compressUnsafeEntry": "Aucune sauvegarde faite : le dossier de cette installation contient un lien symbolique ou un fichier spécial qui ne peut pas être archivé.",
       "compressTooManyFiles": "Aucune sauvegarde faite : le dossier de cette installation contient trop de fichiers à archiver.",
       "compressWriteFailed": "Aucune sauvegarde faite : l'archive de sauvegarde n'a pas pu être écrite. Vérifiez que le dossier des sauvegardes est sur un disque accessible en écriture et qu'il y reste de la place.",
-      "pruneFailed": "Aucune sauvegarde faite : une ancienne sauvegarde n'a pas pu être retirée pour faire de la place à la nouvelle.",
+      "pruneFailed": "Aucune sauvegarde faite : un ancien fichier de sauvegarde n'a pas pu être supprimé pour faire de la place à la nouvelle. Vérifiez qu'il n'est pas ouvert dans un autre programme, puis réessayez.",
       "errorRestoringBackup": "La sauvegarde n'a pas pu être restaurée, votre installation est restée telle quelle.",
       "restoreLeftDataAside": "La sauvegarde n'a pas pu être restaurée et vos anciennes données d'installation se trouvent maintenant dans {{path}}. Remettez ce dossier en place vous-même avant de jouer.",
       "backupsAmount": "Nombre maximum",

tests/domain/installations/backup.test.ts

@@ -13,15 +13,23 @@ const FIXED_NOW = new Date(2025, 7, 16, 1, 20, 0).getTime()
 /** Everything the fakes wrote down, in the order it happened. */
 let trace: string[] = []
 
-function fakeFileSystem(options: { exists?: boolean; removals?: Record<string, boolean> } = {}): FileSystem {
+/**
+ * `missing` holds archive paths that are no longer on disk: the host refuses to
+ * delete a path it cannot find (assertManagedDeletionPath in ipc/pathPolicy.ts
+ * runs with allowMissing false), so those answer false to both calls.
+ */
+function fakeFileSystem(options: { exists?: boolean; removals?: Record<string, boolean>; missing?: readonly string[] } = {}): FileSystem {
   const removals = options.removals ?? {}
+  const missing = new Set(options.missing ?? [])
   return {
     exists: async (path: string): Promise<boolean> => {
       trace.push(`exists:${path}`)
+      if (missing.has(path)) return false
       return options.exists ?? true
     },
     remove: async (path: string): Promise<boolean> => {
       trace.push(`remove:${path}`)
+      if (missing.has(path)) return false
       return removals[path] ?? true
     },
     move: async (from: string, to: string): Promise<boolean> => {
@@ -237,6 +245,25 @@ describe("makeInstallationBackup pruning", () => {
     )
   })
 
+  it("makes the backup when the oldest record's archive is already gone from disk", async () => {
+    // Reported on Discord: six records, the two oldest deleted from the Backups
+    // folder by hand, so the player counted four archives. The prune could not
+    // remove a file that was not there and the whole backup was refused.
+    const installation = snapshot({ backupsLimit: 6, backups: [backup("b1"), backup("b2"), backup("b3"), backup("b4"), backup("b5"), backup("b6")] })
+    const ports = fakePorts({ fileSystem: fakeFileSystem({ missing: ["/backups/b5.tar.gz", "/backups/b6.tar.gz"] }) })
+
+    const result = await makeInstallationBackup(ports, { installation, backupsFolder: "/backups" }, recordingEvents())
+
+    assert.equal(result.ok, true)
+    // The stale record comes off with the archives: reporting it as deleted is
+    // what drops it from the installation, so it stops taking a slot.
+    assert.deepEqual(result.deletedBackupIds, ["b6"])
+    assert.equal(
+      trace.some((entry) => entry.startsWith("compress:")),
+      true
+    )
+  })
+
   it("skips the oldest backup when it is being restored, without touching its file", async () => {
     const installation = snapshot({ backupsLimit: 2, backups: [backup("b1"), backup("b2"), backup("b3", { isRestoring: true })] })
 

tests/domain/installations/backupDeletion.test.ts

@@ -8,10 +8,11 @@ function backup(overrides: Partial<BackupSnapshot> = {}): BackupSnapshot {
   return { id: "backup-1", path: "/backups/my-install.zip", isRestoring: false, isDeleting: false, ...overrides }
 }
 
-function ports(options: { removed?: boolean } = {}): { fileSystem: { remove: (path: string) => Promise<boolean> }; removals: string[] } {
+function ports(options: { removed?: boolean; onDisk?: boolean } = {}): { fileSystem: { exists: (path: string) => Promise<boolean>; remove: (path: string) => Promise<boolean> }; removals: string[] } {
   const removals: string[] = []
   return {
     fileSystem: {
+      exists: async (): Promise<boolean> => options.onDisk ?? true,
       remove: async (path: string): Promise<boolean> => {
         removals.push(path)
         return options.removed ?? true
@@ -51,4 +52,10 @@ describe("deleteInstallationBackup", () => {
 
     assert.deepEqual(result, { ok: false, reason: "file-delete-failed" })
   })
+
+  it("treats an archive that is no longer on disk as deleted, so its record can go", async () => {
+    const result = await deleteInstallationBackup(ports({ removed: false, onDisk: false }), { backup: backup() })
+
+    assert.deepEqual(result, { ok: true })
+  })
 })

tests/domain/installations/delete.test.ts

@@ -9,12 +9,15 @@ type BackupFixture = InstallationDeleteSnapshot["backups"][number]
 /** Everything the fakes wrote down, in the order it happened. */
 let trace: string[] = []
 
-function fakePorts(options: { removals?: Record<string, boolean> } = {}): DeleteInstallationPorts {
+function fakePorts(options: { removals?: Record<string, boolean>; missing?: readonly string[] } = {}): DeleteInstallationPorts {
   const removals = options.removals ?? {}
+  const missing = new Set(options.missing ?? [])
   return {
     fileSystem: {
+      exists: async (path: string): Promise<boolean> => !missing.has(path),
       remove: async (path: string): Promise<boolean> => {
         trace.push(`remove:${path}`)
+        if (missing.has(path)) return false
         return removals[path] ?? true
       }
     }

tests/renderer-dom/installationsRestoreBackup.test.tsx

@@ -170,6 +170,9 @@ describe("ManageInstallationBackups", () => {
     installMockWindowApi({
       configManager: { getConfig: vi.fn(async () => createMockConfig({ installations: [anInstallationWithBackup()] })) },
       pathsManager: {
+        // The archive is still on disk, so the refusal is a real one: a path
+        // the host cannot find counts as deleted (see backupDeletion.ts).
+        checkPathExists: vi.fn(async () => true),
         deletePath,
         extractOnPath: vi.fn(async () => true)
       }

tests/renderer-dom/launchPlayGame.test.tsx

@@ -100,7 +100,7 @@ async function clickPlay(user: ReturnType<typeof userEvent.setup>): Promise<void
 }
 
 const BACKUP_WRITE_FAILED = "No backup made: the backup archive could not be written. Check that the Backups folder is on a writable drive with free space."
-const BACKUP_PRUNE_FAILED = "No backup made: an old backup could not be removed to make room for the new one."
+const BACKUP_PRUNE_FAILED = "No backup made: an old backup file could not be deleted to make room for the new one. Check that it is not open in another program, then try again."
 const SKIP_PROMPT = "The backup failed. Launch without a backup this time?"
 
 /**

tests/renderer-dom/useMakeInstallationBackup.test.tsx

@@ -98,3 +98,45 @@ describe("useMakeInstallationBackup failure notification", () => {
     expect(result.current.notifications.history.map((notification) => notification.body)).not.toContain(GENERIC_WRITE_FAILURE)
   })
 })
+
+/**
+ * Issue #507, reported on Discord. An archive removed from the Backups folder
+ * outside the launcher leaves its record behind, and the host refuses to delete
+ * a path it cannot find (assertManagedDeletionPath in ipc/pathPolicy.ts). Once
+ * the records reach the Installation's limit the prune has to remove one, so
+ * every backup was refused with a sentence about making room, which reads as a
+ * limit the player has to raise.
+ */
+describe("useMakeInstallationBackup with an archive missing from the Backups folder", () => {
+  const PRUNE_FAILED = "No backup made: an old backup file could not be deleted to make room for the new one. Check that it is not open in another program, then try again."
+  const MISSING_ARCHIVE = "/backups/a/backup-6.tar.gz"
+
+  function anInstallationAtItsLimit(): InstallationType {
+    return {
+      ...anInstallation(),
+      backupsLimit: 6,
+      // Newest first, the order the config keeps them in.
+      backups: Array.from({ length: 6 }, (_, index) => ({ id: `backup-${index + 1}`, date: 1_700_000_000_000 - index, path: `/backups/a/backup-${index + 1}.tar.gz` }))
+    }
+  }
+
+  it("makes the backup and drops the record whose archive is gone", async () => {
+    installMockWindowApi({
+      configManager: { getConfig: vi.fn(async () => createMockConfig({ backupsFolder: "/backups", installations: [anInstallationAtItsLimit()] })) },
+      pathsManager: {
+        checkPathExists: vi.fn(async (path: string) => path !== MISSING_ARCHIVE),
+        deletePath: vi.fn(async (path: string) => path !== MISSING_ARCHIVE),
+        compressOnPath: vi.fn(async () => true)
+      }
+    })
+
+    const { result } = renderHook(() => ({ makeBackup: useMakeInstallationBackup(), installations: useInstallations(), notifications: useNotificationsContext() }), { wrapper })
+    await waitFor(() => expect(result.current.installations).toHaveLength(1))
+
+    const outcome = await result.current.makeBackup("install-a")
+
+    expect(outcome).toEqual({ ok: true })
+    expect(result.current.notifications.history.map((notification) => notification.body)).not.toContain(PRUNE_FAILED)
+    await waitFor(() => expect(result.current.installations[0]?.backups.map((backup) => backup.id)).not.toContain("backup-6"))
+  })
+})