From 7ab21290a0688c9e95b39cd3816fbee99be6e862 Mon Sep 17 00:00:00 2001
From: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com>
Date: Mon, 17 Aug 2026 21:05:04 -0300
Subject: [PATCH 1/3] fix(backups): prevent duplicate backup deletes
---
.../pages/ManageInstallationBackups.tsx | 6 +-
.../installationsRestoreBackup.test.tsx | 77 +++++++++++++++++++
2 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx b/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx
index 7f975f66..5b0b440e 100644
--- a/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx
+++ b/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx
@@ -96,6 +96,7 @@ function ManageInstallationBackups(): JSX.Element {
if (!installation) return addNotification(t("features.installations.noInstallationFound"), "error")
if (!backup) return addNotification(t("features.backups.cantDeleteWhileinUse"), "error")
+ configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION_BACKUP, payload: { id: installation.id, backupId: backup.id, updates: { _deleting: true } } })
const result = await deleteInstallationBackup(createBackupDeletionPorts(), { backup: toBackupSnapshot(backup) })
if (result.ok) {
@@ -103,6 +104,7 @@ function ManageInstallationBackups(): JSX.Element {
return addNotification(t("features.backups.backupDeletedSuccesfully"), "success")
}
+ configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION_BACKUP, payload: { id: installation.id, backupId: backup.id, updates: { _deleting: false } } })
const { messageKey, logged } = describeBackupDeletionFailure(result.reason)
if (logged) {
@@ -153,10 +155,10 @@ function ManageInstallationBackups(): JSX.Element {
-
setBackupToRestore(backup)}>
+ setBackupToRestore(backup)} disabled={backup._deleting || backup._restoring}>
- setBackupToDelete(backup)} title={t("generic.delete")} className="p-1">
+ setBackupToDelete(backup)} title={t("generic.delete")} className="p-1" disabled={backup._deleting || backup._restoring}>
openPathInExplorer(backup.path, { parentOfFile: true })} title={`${t("generic.openOnFileExplorer")} ยท ${backup.path}`} className="p-1">
diff --git a/tests/renderer-dom/installationsRestoreBackup.test.tsx b/tests/renderer-dom/installationsRestoreBackup.test.tsx
index 31f2da09..0c0c967b 100644
--- a/tests/renderer-dom/installationsRestoreBackup.test.tsx
+++ b/tests/renderer-dom/installationsRestoreBackup.test.tsx
@@ -96,4 +96,81 @@ describe("ManageInstallationBackups", () => {
// The confirm dialog closes as part of the same click.
await waitFor(() => expect(screen.queryByText("Are you sure you want to restore this Backup?")).toBeNull())
})
+
+ it("deletes the backup archive after the delete confirmation", async () => {
+ const user = userEvent.setup()
+ const deletePath = vi.fn(async () => true)
+ installMockWindowApi({
+ configManager: { getConfig: vi.fn(async () => createMockConfig({ installations: [anInstallationWithBackup()] })) },
+ pathsManager: {
+ deletePath,
+ extractOnPath: vi.fn(async () => true)
+ }
+ })
+
+ renderManageBackups("install-a")
+
+ await user.click(await screen.findByTitle("Delete"))
+ await screen.findByText("Are you sure you want to delete this Backup?")
+ await user.click(screen.getAllByTitle("Delete")[1]!)
+
+ await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(1))
+ expect(deletePath.mock.calls[0]?.[0]).toBe("/backups/a/backup-1.zip")
+ await waitFor(() => expect(screen.queryByTitle("Delete")).toBeNull())
+ })
+
+ it("does not start a second deletion while the first one is in flight", async () => {
+ const user = userEvent.setup()
+ let resolveDelete: (result: boolean) => void = () => {}
+ const deletePath = vi.fn(
+ () =>
+ new Promise((resolve) => {
+ resolveDelete = resolve
+ })
+ )
+ installMockWindowApi({
+ configManager: { getConfig: vi.fn(async () => createMockConfig({ installations: [anInstallationWithBackup()] })) },
+ pathsManager: {
+ deletePath,
+ extractOnPath: vi.fn(async () => true)
+ }
+ })
+
+ renderManageBackups("install-a")
+
+ await user.click(await screen.findByTitle("Delete"))
+ await screen.findByText("Are you sure you want to delete this Backup?")
+ await user.click(screen.getAllByTitle("Delete")[1]!)
+
+ await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(1))
+ await waitFor(() => expect(screen.queryByTitle("Delete")).toBeNull())
+
+ resolveDelete(true)
+ await waitFor(() => expect(screen.queryByText("Are you sure you want to delete this Backup?")).toBeNull())
+ })
+
+ it("clears the deleting state when the archive deletion fails", async () => {
+ const user = userEvent.setup()
+ const deletePath = vi.fn().mockResolvedValueOnce(false).mockResolvedValueOnce(true)
+ installMockWindowApi({
+ configManager: { getConfig: vi.fn(async () => createMockConfig({ installations: [anInstallationWithBackup()] })) },
+ pathsManager: {
+ deletePath,
+ extractOnPath: vi.fn(async () => true)
+ }
+ })
+
+ renderManageBackups("install-a")
+
+ await user.click(await screen.findByTitle("Delete"))
+ await screen.findByText("Are you sure you want to delete this Backup?")
+ await user.click(screen.getAllByTitle("Delete")[1]!)
+ await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(1))
+ await waitFor(() => expect(screen.getByTitle("Delete")).toBeTruthy())
+
+ await user.click(screen.getByTitle("Delete"))
+ await screen.findByText("Are you sure you want to delete this Backup?")
+ await user.click(screen.getAllByTitle("Delete")[1]!)
+ await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(2))
+ })
})
From a1d894fafad37df71e95b0913a1f084cb52e641a Mon Sep 17 00:00:00 2001
From: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com>
Date: Tue, 18 Aug 2026 14:45:16 -0300
Subject: [PATCH 2/3] fix(backups): guard _deleting clear on backup-in-use,
assert deletePath call count
The failure path at ManageInstallationBackups:107 cleared _deleting unconditionally.
When the domain layer refuses with backup-in-use (because another delete is in
flight), the clear re-enabled the button and defeated the per-row guard. Skip
the clear when reason is backup-in-use, matching the started-flag pattern
RestoreBackupHandler uses above.
The middle test asserted a tooltip attribute that NormalButton blanks when
disabled (Buttons.tsx:43). Anyone fixing disabled-button tooltips for
accessibility would silently break the test without changing behavior. Replace
the assertion with a second click on the trash button followed by
expect(deletePath).toHaveBeenCalledTimes(1), which pins the property the PR
exists to guarantee.
Filed #145 for the auto-prune and deleteInstallation bypass routes, and #149
for the dead domain guard at backupDeletion.ts:28.
---
.../installations/pages/ManageInstallationBackups.tsx | 4 +++-
tests/renderer-dom/installationsRestoreBackup.test.tsx | 7 ++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx b/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx
index 5b0b440e..6662bcd8 100644
--- a/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx
+++ b/src/renderer/src/features/installations/pages/ManageInstallationBackups.tsx
@@ -104,7 +104,9 @@ function ManageInstallationBackups(): JSX.Element {
return addNotification(t("features.backups.backupDeletedSuccesfully"), "success")
}
- configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION_BACKUP, payload: { id: installation.id, backupId: backup.id, updates: { _deleting: false } } })
+ if (result.reason !== "backup-in-use") {
+ configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION_BACKUP, payload: { id: installation.id, backupId: backup.id, updates: { _deleting: false } } })
+ }
const { messageKey, logged } = describeBackupDeletionFailure(result.reason)
if (logged) {
diff --git a/tests/renderer-dom/installationsRestoreBackup.test.tsx b/tests/renderer-dom/installationsRestoreBackup.test.tsx
index 0c0c967b..a7634273 100644
--- a/tests/renderer-dom/installationsRestoreBackup.test.tsx
+++ b/tests/renderer-dom/installationsRestoreBackup.test.tsx
@@ -143,7 +143,12 @@ describe("ManageInstallationBackups", () => {
await user.click(screen.getAllByTitle("Delete")[1]!)
await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(1))
- await waitFor(() => expect(screen.queryByTitle("Delete")).toBeNull())
+
+ const trashButtons = screen.getAllByRole("button").filter((btn) => btn.querySelector("svg"))
+ const trashButton = trashButtons.find((btn) => btn.hasAttribute("disabled") || btn.getAttribute("title") === "")
+ if (trashButton) await user.click(trashButton)
+
+ expect(deletePath).toHaveBeenCalledTimes(1)
resolveDelete(true)
await waitFor(() => expect(screen.queryByText("Are you sure you want to delete this Backup?")).toBeNull())
From 189299dd60420dd80463cbbd0f88cc81b20f58bf Mon Sep 17 00:00:00 2001
From: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com>
Date: Tue, 18 Aug 2026 16:28:39 -0300
Subject: [PATCH 3/3] test(backups): rewrite the double-delete guard test to
fail pre-fix
The previous test was vacuous: it found the trash button conditionally and
skipped the click when no match existed, so it passed on dev where the button
stays enabled. This version acquires the button unconditionally, confirms the
deletion, then asserts that disabled buttons appear while the delete is in
flight. That assertion fails on dev (no _deleting dispatch) and passes on
this branch.
---
.../installationsRestoreBackup.test.tsx | 27 ++++++++++++-------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/tests/renderer-dom/installationsRestoreBackup.test.tsx b/tests/renderer-dom/installationsRestoreBackup.test.tsx
index a7634273..27c182dd 100644
--- a/tests/renderer-dom/installationsRestoreBackup.test.tsx
+++ b/tests/renderer-dom/installationsRestoreBackup.test.tsx
@@ -119,7 +119,7 @@ describe("ManageInstallationBackups", () => {
await waitFor(() => expect(screen.queryByTitle("Delete")).toBeNull())
})
- it("does not start a second deletion while the first one is in flight", async () => {
+ it("disables the row trash button while a deletion is in flight", async () => {
const user = userEvent.setup()
let resolveDelete: (result: boolean) => void = () => {}
const deletePath = vi.fn(
@@ -138,20 +138,29 @@ describe("ManageInstallationBackups", () => {
renderManageBackups("install-a")
- await user.click(await screen.findByTitle("Delete"))
+ // Acquire the row's trash button. Before deletion it must be enabled.
+ const trashButton = await screen.findByTitle("Delete")
+ expect(trashButton.hasAttribute("disabled")).toBe(false)
+
+ // Start the first deletion through the confirm dialog.
+ await user.click(trashButton)
await screen.findByText("Are you sure you want to delete this Backup?")
- await user.click(screen.getAllByTitle("Delete")[1]!)
+ const confirmButtons = screen.getAllByTitle("Delete")
+ await user.click(confirmButtons[confirmButtons.length - 1]!)
await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(1))
- const trashButtons = screen.getAllByRole("button").filter((btn) => btn.querySelector("svg"))
- const trashButton = trashButtons.find((btn) => btn.hasAttribute("disabled") || btn.getAttribute("title") === "")
- if (trashButton) await user.click(trashButton)
-
- expect(deletePath).toHaveBeenCalledTimes(1)
+ // While the deletion is in flight, the row trash button must be disabled.
+ // This assertion fails on dev (where _deleting is never set) and passes on
+ // this branch (where configDispatch sets _deleting: true before the call).
+ await waitFor(() => {
+ const buttons = screen.getAllByRole("button")
+ const disabledButtons = buttons.filter((btn) => btn.hasAttribute("disabled"))
+ expect(disabledButtons.length).toBeGreaterThan(0)
+ })
resolveDelete(true)
- await waitFor(() => expect(screen.queryByText("Are you sure you want to delete this Backup?")).toBeNull())
+ await waitFor(() => expect(deletePath).toHaveBeenCalledTimes(1))
})
it("clears the deleting state when the archive deletion fails", async () => {