Skip to content

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

Open
Zaldaryon wants to merge 1 commit into
devfrom
fix/backup-deletion-guard-all-paths
Open

fix(backups): skip archives already being deleted in prune and delete-installation#158
Zaldaryon wants to merge 1 commit into
devfrom
fix/backup-deletion-guard-all-paths

Conversation

@Zaldaryon

Copy link
Copy Markdown
Collaborator

PR #136 wired the per-backup _deleting flag into the manual-delete button's own guard, but two other paths that remove backup archives never learned about it. The auto-prune loop in makeInstallationBackup (backupsLimit enforcement, runs before every new backup) removes the oldest archive directly and structurally could not check the flag, since InstallationSnapshot.backups carried only id, date and path. deleteInstallation's backup cleanup had the same gap: InstallationDeleteSnapshot.backups was narrowed to just path.

Starting a manual delete on an archive, then triggering either of those before it finishes, races the same file: the prune loop can report prune-failed for a file another operation already removed correctly, and deleteInstallation can land a bogus entry in failedBackupPaths for the same reason, producing the misleading "some backups survived" warning even when nothing actually survived.

Both InstallationSnapshot.backups and InstallationDeleteSnapshot.backups now carry an optional isDeleting flag, filled in by the adapters from the config-owned _deleting field. The prune loop skips a candidate that is isDeleting without touching its file, still counting it toward the target since it is on its way out through the other operation either way. deleteInstallation does the same: skips it, reports it as neither removed nor failed, since its actual fate belongs to whichever operation is handling it.

Six tests added: three for the prune loop (skips the oldest when it is in flight, counts a skip toward the limit instead of reaching for a newer backup, keeps pruning past a skip to reach the limit) and two for deleteInstallation (skips one in-flight backup among others, treats every backup being in flight as a clean success with none removed here), plus the existing suites verified unaffected.

Checks:

  • npm run typecheck
  • npm run lint:ci
  • npm run format:check
  • npm run test:coverage (98 files, 1140 passed, 2 skipped; 90.31% statements, 87.47% branches, both domain files this PR touches at 100% and 97.82% statement coverage)
  • npm run build:unpack

Fixes #142

…-installation

PR #136 wired the per-backup _deleting flag into the manual-delete
button's own guard, but two other paths that remove backup archives
never learned about it: the auto-prune loop in makeInstallationBackup
(backupsLimit enforcement, runs before every new backup) and the
backup cleanup inside deleteInstallation. Starting a manual delete
on an archive, then triggering either of those before it finishes,
raced the same file: the prune loop could report prune-failed for a
file another operation had already removed correctly, and
deleteInstallation could land a bogus entry in failedBackupPaths for
the same reason.

Both InstallationSnapshot.backups and InstallationDeleteSnapshot.backups
now carry an optional isDeleting flag, filled in by the adapters from
the config-owned _deleting field. The prune loop skips a candidate
that is isDeleting without touching its file, still counting it
toward the target since it is on its way out through the other
operation either way. deleteInstallation does the same: skips it,
reports it as neither removed nor failed, since its actual fate
belongs to whichever operation is handling it.

Fixes #142
@Zaldaryon
Zaldaryon requested a review from Pixnop August 19, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant