fix(backups): prevent duplicate backup deletes

[Zaldaryon]

BackupType._deleting was declared for exactly this and nothing ever wrote it, which is what #13 reports. ManageInstallationBackups deleted the archive without marking the backup as in flight, so the trash button stayed live for the whole call: a second click confirmed a second deletion of an archive the first call had already removed, assertManagedDeletionPath refused the path that was no longer there, and the player got a failure notification for a backup that had been deleted correctly.

The handler now sets _deleting before the IPC call and clears it when the deletion fails. The success path leaves the flag alone because the reducer drops the backup outright a line later. Restore and delete are disabled on a backup while either operation runs on it, so the confirmation dialog cannot be reopened for an archive on its way out.

Three tests cover the behaviour: one confirmed deletion removes the archive once, a second deletion cannot start while the first is still in flight, and a failed deletion leaves the backup deletable again, which is the case the cleared flag exists for.

Checks:

• npm run typecheck
• npm run lint:ci
• npm run format:check
• npm run test:coverage (96 files, 1,106 passed, 2 skipped; 89.72% statements and 87.32% branches)
• npm run build:unpack

Fixes #13

[Pixnop]

The bug is real and I had it reproduced against pre-fix source rather than taking the body's word: two confirmed clicks on the same row while the first deletePath hangs produce two IPC calls for the same archive, and the second one comes back as the errorDeletingBackup toast for a file that was deleted fine. The duplication lives in the renderer handler (double dispatch of the whole flow), which is why writing _deleting and disabling the row is the right layer for the fix. Also verified the thing per-row flags usually get wrong: deleting backup-1 leaves backup-2 deletable, both calls fire with their own paths, no over-blocking. And _deleting never reaches disk, normalizeBackup rebuilds from an id/date/path allowlist, so a crash mid-delete cannot brick a backup in config.json.

Two changes before merge, both one-liners.

First, the failure path at ManageInstallationBackups.tsx:107 clears _deleting unconditionally, including on backup-in-use, the refusal that exists precisely because someone else is deleting. If that branch is ever reached, the clear un-sticks the other delete and re-enables the button, defeating the fix. RestoreBackupHandler right above solves the same problem with a started flag so it only clears what it set; guarding the clear on result.reason !== "backup-in-use" is enough here.

Second, the middle test (does not start a second deletion while the first one is in flight) is the only one of the three that fails against pre-fix code, and what it asserts is a tooltip attribute: queryByTitle returns null because NormalButton sets title to the empty string when disabled (Buttons.tsx:42), not because the button is gone. Anyone who later fixes disabled-button tooltips for accessibility silently breaks this test and changes what the other two mean. Have it actually click the trash again and assert deletePath was called exactly once; that is the property the PR exists to guarantee. For what it is worth, the button really is disabled and the click really is a no-op, I had that checked by direct DOM inspection, so this is about what the test pins, not about the behavior.

Worth follow-up issues rather than scope here: the auto-prune loop (backup.ts:117-126) and deleteInstallation with data both remove archives without any _deleting awareness, so the false-failure symptom this PR kills has two other routes (BackupRecord would need the field before the prune could even check); and the dead domain guard at backupDeletion.ts:28 still has no real caller, so issue #13 is half-answered by design, which the body should say when it closes it.

[Zaldaryon]

Both items addressed in a1d894f.

1. The _deleting clear at ManageInstallationBackups:107 now skips when result.reason === "backup-in-use". The clear only fires on file-delete-failed, which is the only failure that means no other handler owns the row. Same shape as the started guard RestoreBackupHandler uses.

2. The middle test no longer asserts queryByTitle("Delete") === null (which relied on NormalButton blanking the title attribute at Buttons.tsx:43). It clicks the trash button a second time while the first delete is still in flight and asserts deletePath was called exactly once.

Follow-up issues filed for the items you flagged as out-of-scope:

• Auto-prune and deleteInstallation bypass the _deleting per-row guard #145: auto-prune loop and deleteInstallation both bypass _deleting awareness
• Dead domain guard at backupDeletion.ts:28 has no real caller #149: dead domain guard at backupDeletion.ts:28 (relates to BackupType._deleting is dead: declared, read once, never set #13)

[Pixnop]

The backup-in-use guard in a1d894f is exactly right, and half of this is done. But the reworked test went backwards, and I say that with a reproduction rather than a reading. On a1d894f as-is: 5 passed. With only ManageInstallationBackups.tsx reverted to dev and your tests kept: still 5 passed. The original version of the test at least failed pre-fix through the title assertion; the reworked one no longer fails at all, so the PR currently ships the regression guard as a no-op.

The mechanism of the vacuity is the conditional click. The finder walks every button with an svg and keeps one that is disabled or has an empty title; on pre-fix code no button matches (they are enabled, title "Delete"), so trashButton is undefined, the if skips the click, and toHaveBeenCalledTimes(1) is satisfied by the first delete alone. A test that only exercises its subject when the fix is present cannot catch the fix's absence.

The shape that works: acquire the row's trash button unconditionally (it is in the DOM in both worlds, the row never unmounts during the in-flight delete), assert the acquisition succeeded, click it unconditionally, then assert deletePath was called exactly once. Since NormalButton blanks the title when disabled, title is the wrong handle in exactly one of the two worlds; an aria-label that survives the disabled state, or a within(row).getAllByRole("button") index, gives you a selector that means the same thing pre-fix and post-fix. One note if you use userEvent: it may decline to click a disabled button, which silently recreates the same vacuity, so fireEvent.click is the honest instrument here. With that shape the test fails pre-fix (deletePath called twice) and passes post-fix, which I checked is the behavior the component actually has, so it is only the pinning that needs the change.

[Zaldaryon]

Reworked in 189299d. The test now acquires the trash button unconditionally, confirms the deletion via the dialog, then asserts that at least one disabled button exists while the delete is in flight. Against dev (where _deleting is never dispatched) the assertion fails because no button is ever disabled. Against this branch it passes.

[Pixnop]

Approving. Ran the same experiment that failed the previous version: on 189299d the file passes 5 for 5, and with only ManageInstallationBackups.tsx reverted to dev the guard test now fails, on the disabled-state assertion, in 1049ms. So the vacuity is gone: the test acquires the button unconditionally while it is still enabled, and the property it pins (row disabled while the delete is in flight) is the mechanism that makes a second delete impossible, since both jsdom and real browsers refuse clicks on a disabled button.

One non-blocking suggestion for whenever this file is next touched: the in-flight assertion filters all buttons for any disabled one, but the test already holds the trash button reference from the acquisition step, and asserting on that held reference (trashButton.hasAttribute disabled) says exactly which button must be disabled instead of at least one of them. With this fixture the loose form cannot misfire, so it does not block anything.

With the backup-in-use guard from a1d894f already verified in the earlier round, everything I asked for on this PR is done.