From 64456d75a46b102d41045868969e8cdc89fe7782 Mon Sep 17 00:00:00 2001 From: ishaanko Date: Thu, 10 Sep 2026 14:51:55 -0700 Subject: [PATCH 1/4] fix(storage): keep remote copies out of cleanup 0.3.7.1 made cleanup delete the remote copy of an unprotected backup. Cleanup now frees space on this computer only. A synchronized backup keeps its catalog record as a remote-only entry; the Delete button removes it from the remote. --- CHANGELOG.md | 11 ++++ README.md | 12 ++-- gradle.properties | 2 +- .../ui/CleanupConfirmationScreen.java | 2 +- .../worldarchive/ui/CleanupPreviewScreen.java | 2 +- .../storage/management/CleanupExecutor.java | 45 +++++--------- .../storage/management/CleanupItem.java | 7 +-- .../storage/management/CleanupPlanner.java | 26 +++++---- .../management/ManagedStorageSupport.java | 9 +++ .../management/CleanupIntegrationTest.java | 58 ++++++++++++++----- 10 files changed, 104 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a099a2..8bc81d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 0.3.7.2 (2026-09-10) + +### Changed + +- Storage cleanup no longer deletes anything from the configured remote. + 0.3.7.1 removed the remote copy of an unprotected backup together with its + local copies. Cleanup now frees space on this computer only. A backup whose + snapshot is on the remote stays listed as a remote-only entry, and the + Delete button is the way to remove it from the remote. A backup with no copy + left anywhere still leaves the catalog. + ## 0.3.7.1 (2026-09-10) ### Fixed diff --git a/README.md b/README.md index 4760526..2bd2082 100644 --- a/README.md +++ b/README.md @@ -89,11 +89,13 @@ Guided cleanup keeps: Within a period, manual backups and backups with more changed files win. Cleanup always shows a preview first. Nothing is deleted until you confirm. -Cleanup deletes the backups it lists the same way the Delete button does: the -Git snapshot, its copy on the configured remote, the ZIP, and the catalog -entry. A protected backup is never deleted. When space is still short after -that, cleanup can drop the local Git copies of protected backups, but only when -each of them keeps a ZIP or a verified remote copy. +Cleanup frees space on this computer only. For a backup it lists, it removes +the ZIP and the local Git copy. Copies on the configured remote are never +touched: a backup that is synchronized stays listed as a remote-only entry, and +the Delete button is the way to remove it from the remote. A backup with no copy +left anywhere leaves the catalog. A protected backup is never deleted; when +space is still short, cleanup can drop the local Git copies of protected +backups, but only when each of them keeps a ZIP or a verified remote copy. ## Minecraft versions diff --git a/gradle.properties b/gradle.properties index 8ac9949..b3dfe11 100644 --- a/gradle.properties +++ b/gradle.properties @@ -9,6 +9,6 @@ loom_version=1.17.19 fabric_api_version=0.157.0+26.2 modmenu_version=20.0.1 -mod_version=0.3.7.1 +mod_version=0.3.7.2 maven_group=dev.ishaanko automatic_release_channel=alpha diff --git a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java index 1fe3ab3..502736c 100644 --- a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java +++ b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java @@ -76,7 +76,7 @@ protected void init() { contentWidth, 18, Component.literal( - "These backups will be deleted. This cannot be undone.") + "These backups will be deleted from this computer. This cannot be undone.") .withStyle(ChatFormatting.RED), font)); int pageSize = Math.max(1, Math.min(6, (height - 142) / 24)); diff --git a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java index 75d2e07..9a7a951 100644 --- a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java +++ b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java @@ -62,7 +62,7 @@ protected void init() { addRenderableOnly(Widgets.title(font, x, 9, contentWidth, 20, title)); String summary = plan.items().isEmpty() ? "Nothing to clean up right now. Your keep settings protect every backup." - : "Backups your keep settings do not protect are deleted everywhere, including GitHub. Protected backups only lose their local Git copy."; + : "Choose which backups to delete from this computer. Copies on GitHub are kept and stay listed; use Delete to remove one from GitHub."; addRenderableOnly(new MultiLineTextWidget( x, 31, diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java index 73e0911..f74cfc6 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java @@ -26,9 +26,10 @@ import java.util.Set; /** - * Applies a confirmed {@link CleanupPlan}. A backup the keep settings do not protect is - * deleted the same way the Delete button deletes it: Git snapshot (local and remote), - * ZIP, and catalog record. A protected backup only loses its local Git copy. + * Applies a confirmed {@link CleanupPlan}. Cleanup frees space on this computer only: + * it never contacts the configured remote. A backup whose snapshot is synchronized to + * the remote keeps its catalog record as a remote-only entry; a backup with no copy + * left anywhere leaves the catalog. */ final class CleanupExecutor { private final BackupCatalog catalog; @@ -123,10 +124,6 @@ private static void validateSelection( } } - /** - * Git goes first: a remote that refuses the deletion fails the item before its ZIP - * is touched, so the backup stays whole and restorable. - */ private boolean applyItems( CleanupPlan plan, CleanupRequest request, @@ -139,22 +136,16 @@ private boolean applyItems( } try { if (item.removeGit()) { - if (plan.protectedBackups().contains(item.backupId())) { - ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot( - plan.worldId(), - item.backupId())); - // The catalog keeps pointing at a synchronized remote copy, so - // the backup stays visible, verifiable, and deletable later. - removeDestination( - item.backupId(), - DestinationType.GIT, - synchronizedRemoteCopy(current, item.backupId())); - } else { - ManagedStorageSupport.await(git.deleteSnapshot( - plan.worldId(), - item.backupId())); - removeDestination(item.backupId(), DestinationType.GIT, false); - } + ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot( + plan.worldId(), + item.backupId())); + // The catalog keeps pointing at a synchronized remote copy, so the + // backup stays visible, verifiable, and deletable with Delete. + removeDestination( + item.backupId(), + DestinationType.GIT, + ManagedStorageSupport.synchronizedRemoteCopy( + ManagedStorageSupport.record(current, item.backupId()))); removedGit = true; } if (item.removeZip()) { @@ -266,14 +257,6 @@ private boolean currentRemoteContainsSnapshot(Snapshot snapshot, BackupId backup } } - private static boolean synchronizedRemoteCopy(Snapshot snapshot, BackupId backupId) { - return ManagedStorageSupport.destination( - ManagedStorageSupport.record(snapshot, backupId), DestinationType.GIT) - .filter(result -> result.ownership() == ArtifactOwnership.MANAGED - && result.syncStatus() == SyncStatus.SYNCED) - .isPresent(); - } - /** A protected backup may lose its last local copy only if the remote provably has it. */ private void requireCurrentRemoteCopies( CleanupPlan plan, diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java index 324b265..b1a5321 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java @@ -6,10 +6,9 @@ import java.util.Optional; /** - * One cleanup action shown before confirmation. For a backup the keep settings do not - * protect, {@code removeGit} deletes the Git snapshot everywhere, including the - * configured remote, and the backup leaves the catalog. For a protected backup it drops - * only the local Git copy; the ZIP or the verified remote copy stays. + * One local cleanup action shown before confirmation. {@code removeGit} drops the Git + * copy on this computer only; a copy on the configured remote is never touched and + * keeps the backup listed. A backup with no copy left anywhere leaves the catalog. */ public record CleanupItem( BackupId backupId, diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java index e3bb7c2..433a00e 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java @@ -45,10 +45,11 @@ final class CleanupPlanner { } /** - * Picks the lightest backups the keep settings do not protect and deletes each one - * completely. Only when that still leaves the world over its target does the plan - * drop the local Git copies of protected backups, and only when every one of them - * keeps a ZIP or a verified remote copy. + * Picks the lightest backups the keep settings do not protect and removes every + * local copy of each one. Only when that still leaves the world over its target + * does the plan drop the local Git copies of protected backups, and only when every + * one of them keeps a ZIP or a verified remote copy. Copies on the remote are never + * touched. */ CleanupPlan prepare(WorldId worldId, Snapshot snapshot) throws Exception { BackupId safetyFloor = verifiedSafetyFloor(snapshot) @@ -96,7 +97,7 @@ CleanupPlan prepare(WorldId worldId, Snapshot snapshot) throws Exception { snapshot.fingerprint()); } - /** Adds whole-backup deletions, lightest first, until the target is met. */ + /** Removes the local copies of unprotected backups, lightest first, until the target is met. */ private static long deleteUnprotected( Snapshot snapshot, List cleanupOrder, @@ -112,18 +113,21 @@ private static long deleteUnprotected( ZipBackupArtifact zip = snapshot.zipArtifacts().get(backupId); boolean removeZip = zip != null && ManagedStorageSupport.managedDestination(record, DestinationType.ZIP); - boolean removeGit = ManagedStorageSupport.managedDestination( - record, DestinationType.GIT); + boolean removeGit = snapshot.localGitSnapshots().containsKey(backupId) + && ManagedStorageSupport.managedDestination(record, DestinationType.GIT); if (!removeZip && !removeGit) { continue; } long zipBytes = removeZip ? ManagedStorageSupport.artifactBytes(zip) : 0; - long gitBytes = removeGit && snapshot.localGitSnapshots().containsKey(backupId) - ? gitShare - : 0; + long gitBytes = removeGit ? gitShare : 0; selected.put(backupId, item( record, - new CleanupItemFlags(removeGit, removeZip, false, gitBytes, zipBytes))); + new CleanupItemFlags( + removeGit, + removeZip, + ManagedStorageSupport.synchronizedRemoteCopy(record), + gitBytes, + zipBytes))); projected = Math.max(0, projected - zipBytes - gitBytes); } return projected; diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java index dfb7782..131fa76 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java @@ -5,6 +5,7 @@ import dev.ishaanko.worldarchive.model.BackupRecord; import dev.ishaanko.worldarchive.model.DestinationResult; import dev.ishaanko.worldarchive.model.DestinationType; +import dev.ishaanko.worldarchive.model.SyncStatus; import dev.ishaanko.worldarchive.storage.zip.ZipBackupArtifact; import java.io.IOException; import java.nio.file.Files; @@ -25,6 +26,14 @@ static boolean managedDestination( .isPresent(); } + /** True when the catalog says this backup's Git snapshot is on the configured remote. */ + static boolean synchronizedRemoteCopy(BackupRecord record) { + return destination(record, DestinationType.GIT) + .filter(result -> result.ownership() != ArtifactOwnership.EXTERNAL + && result.syncStatus() == SyncStatus.SYNCED) + .isPresent(); + } + static Optional destination( BackupRecord record, DestinationType type) { diff --git a/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java b/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java index 782758c..3eac364 100644 --- a/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java +++ b/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java @@ -117,7 +117,7 @@ void tearDown() { } @Test - void unprotectedBackupsAreDeletedEverywhereAndTheirLfsObjectsFreed() throws Exception { + void unprotectedBackupsLoseLocalCopiesAndStayListedOnTheRemote() throws Exception { BackupId oldest = backup(3, true); BackupId middle = backup(2, true); BackupId newest = backup(1, true); @@ -128,9 +128,10 @@ void unprotectedBackupsAreDeletedEverywhereAndTheirLfsObjectsFreed() throws Exce assertEquals(Set.of(newest), plan.protectedBackups()); Map items = plan.items().stream() .collect(Collectors.toMap(CleanupItem::backupId, item -> item)); - for (BackupId deleted : List.of(oldest, middle)) { - assertTrue(items.get(deleted).removeGit() && items.get(deleted).removeZip()); - assertTrue(items.get(deleted).removesRestorePoint()); + for (BackupId removed : List.of(oldest, middle)) { + assertTrue(items.get(removed).removeGit() && items.get(removed).removeZip()); + assertFalse(items.get(removed).removesRestorePoint(), + "the synchronized remote copy remains a restore point"); } assertFalse(items.get(newest).removesRestorePoint(), "the protected backup is only offered as a local Git eviction"); @@ -139,26 +140,40 @@ void unprotectedBackupsAreDeletedEverywhereAndTheirLfsObjectsFreed() throws Exce new CleanupRequest(plan.confirmationToken(), Set.of(oldest, middle)))); assertEquals(Map.of(), result.failures()); - assertEquals(List.of(newest), catalog.list(worldId).stream() - .map(record -> record.manifest().backupId()).toList()); + assertEquals(Set.of(oldest, middle, newest), remoteBackupIds(), "cleanup never touches the remote"); + for (BackupId removed : List.of(oldest, middle)) { + BackupRecord record = catalog.find(removed).orElseThrow(); + assertEquals(List.of(DestinationType.GIT), record.result().destinations().stream() + .map(DestinationResult::destination).toList()); + assertFalse(deletions.contains(removed)); + } assertEquals(Set.of(newest), await(git.listCurrentSnapshots(worldId)).stream() .map(snapshot -> snapshot.backupId()).collect(Collectors.toSet())); - assertEquals(Set.of(newest), remoteBackupIds()); assertEquals(1, zipStore.listArchives().size()); - assertTrue(deletions.contains(oldest) && deletions.contains(middle)); assertEquals(2, lfsObjectCount(), "only the newest snapshot's objects remain"); assertTrue(await(git.verifyCurrentSnapshot(worldId, newest)).valid()); assertTrue(result.reclaimedBytes() > 0); imports().rebuildLocal().toCompletableFuture().get(30, TimeUnit.SECONDS); - assertEquals(List.of(newest), catalog.list(worldId).stream() - .map(record -> record.manifest().backupId()).toList()); + assertEquals(3, catalog.list(worldId).size()); + assertEquals(1, await(git.listCurrentSnapshots(worldId)).size()); } @Test - void protectedBackupKeepsItsRemoteCopyWhenLocalGitIsEvicted() throws Exception { + void backupWithNoCopyLeftAnywhereLeavesTheCatalog() throws Exception { BackupId older = backup(2, true); BackupId newest = backup(1, false); + nativeGit("--git-dir=" + remote, "update-ref", "-d", + remoteRef(newest)); + catalog.update(newest, record -> new BackupRecord( + record.manifest(), + BackupResult.aggregate( + newest, + worldId, + record.result().destinations().stream() + .map(destination -> destination.withSync(SyncStatus.NOT_CONFIGURED)) + .toList(), + record.result().completedAt()))); ManagedStorageService service = service(KEEP_ONLY_SAFETY_FLOOR); CleanupPlan plan = await(service.prepareCleanup(worldId)); @@ -175,15 +190,17 @@ void protectedBackupKeepsItsRemoteCopyWhenLocalGitIsEvicted() throws Exception { new CleanupRequest(plan.confirmationToken(), items.keySet()))); assertEquals(Map.of(), result.failures()); - assertEquals(Set.of(older), remoteBackupIds(), "the unprotected backup left the remote"); - assertTrue(await(git.listCurrentSnapshots(worldId)).isEmpty()); - assertEquals(0, lfsObjectCount()); + assertFalse(catalog.find(newest).isPresent()); + assertTrue(deletions.contains(newest)); BackupRecord kept = catalog.find(older).orElseThrow(); assertEquals( List.of(DestinationType.GIT, DestinationType.ZIP), kept.result().destinations().stream() - .map(DestinationResult::destination).toList()); - assertFalse(catalog.find(newest).isPresent()); + .map(DestinationResult::destination).toList(), + "the synchronized remote copy stays in the catalog"); + assertEquals(Set.of(older), remoteBackupIds()); + assertTrue(await(git.listCurrentSnapshots(worldId)).isEmpty()); + assertEquals(0, lfsObjectCount()); } private BackupId backup(int daysAgo, boolean withZip) throws Exception { @@ -276,6 +293,15 @@ public ZipBackupStore defaultStore() { executor); } + private String remoteRef(BackupId backupId) throws Exception { + return nativeGit("--git-dir=" + remote, "for-each-ref", "--format=%(refname)", + "refs/heads/backups/") + .lines() + .filter(line -> line.endsWith(backupId.toString())) + .findFirst() + .orElseThrow(); + } + private Set remoteBackupIds() throws Exception { return nativeGit("--git-dir=" + remote, "for-each-ref", "--format=%(refname)", "refs/heads/backups/") From e8b71eb1436f6bfad05db79a40a193b681d5c90d Mon Sep 17 00:00:00 2001 From: ishaanko Date: Thu, 10 Sep 2026 14:57:47 -0700 Subject: [PATCH 2/4] address greptile review feedback --- .../storage/management/CleanupExecutor.java | 18 +++++++++++------- .../storage/management/CleanupPlanner.java | 2 +- .../management/ManagedStorageSupport.java | 15 +++++++++++++-- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java index f74cfc6..39271eb 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java @@ -136,16 +136,20 @@ private boolean applyItems( } try { if (item.removeGit()) { + // Checked before the local ref goes, because the check compares the + // remote commit with the local one. A remote that cannot be reached + // fails the item with nothing deleted, so the catalog never loses + // its pointer to a copy that may still exist. + boolean remoteCopy = ManagedStorageSupport.synchronizedRemoteCopy( + ManagedStorageSupport.record(current, item.backupId())) + && ManagedStorageSupport.await(git.currentRemoteContainsSnapshot( + plan.worldId(), item.backupId())); ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot( plan.worldId(), item.backupId())); - // The catalog keeps pointing at a synchronized remote copy, so the - // backup stays visible, verifiable, and deletable with Delete. - removeDestination( - item.backupId(), - DestinationType.GIT, - ManagedStorageSupport.synchronizedRemoteCopy( - ManagedStorageSupport.record(current, item.backupId()))); + // A proven remote copy keeps the catalog record, so the backup stays + // visible, verifiable, and deletable with Delete. + removeDestination(item.backupId(), DestinationType.GIT, remoteCopy); removedGit = true; } if (item.removeZip()) { diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java index 433a00e..704ecf7 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java @@ -114,7 +114,7 @@ private static long deleteUnprotected( boolean removeZip = zip != null && ManagedStorageSupport.managedDestination(record, DestinationType.ZIP); boolean removeGit = snapshot.localGitSnapshots().containsKey(backupId) - && ManagedStorageSupport.managedDestination(record, DestinationType.GIT); + && ManagedStorageSupport.ownGitSnapshot(record); if (!removeZip && !removeGit) { continue; } diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java index 131fa76..c0cdd63 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageSupport.java @@ -26,14 +26,25 @@ static boolean managedDestination( .isPresent(); } - /** True when the catalog says this backup's Git snapshot is on the configured remote. */ + /** + * True when the catalog says this backup's own Git snapshot is on the configured + * remote. Imported snapshots are excluded: their sync status refers to the import + * source, which restore and verification do not search. + */ static boolean synchronizedRemoteCopy(BackupRecord record) { return destination(record, DestinationType.GIT) - .filter(result -> result.ownership() != ArtifactOwnership.EXTERNAL + .filter(result -> result.ownership() == ArtifactOwnership.MANAGED && result.syncStatus() == SyncStatus.SYNCED) .isPresent(); } + /** True when this backup's Git snapshot is WorldArchive's own, not imported or linked. */ + static boolean ownGitSnapshot(BackupRecord record) { + return destination(record, DestinationType.GIT) + .filter(result -> result.ownership() == ArtifactOwnership.MANAGED) + .isPresent(); + } + static Optional destination( BackupRecord record, DestinationType type) { From 71ce3e3a43a8403ec57153a2af59f9f6749cfdc1 Mon Sep 17 00:00:00 2001 From: ishaanko Date: Thu, 10 Sep 2026 15:01:36 -0700 Subject: [PATCH 3/4] address greptile review feedback --- .../storage/management/CleanupExecutor.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java index 39271eb..9d723ab 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java @@ -136,14 +136,8 @@ private boolean applyItems( } try { if (item.removeGit()) { - // Checked before the local ref goes, because the check compares the - // remote commit with the local one. A remote that cannot be reached - // fails the item with nothing deleted, so the catalog never loses - // its pointer to a copy that may still exist. - boolean remoteCopy = ManagedStorageSupport.synchronizedRemoteCopy( - ManagedStorageSupport.record(current, item.backupId())) - && ManagedStorageSupport.await(git.currentRemoteContainsSnapshot( - plan.worldId(), item.backupId())); + boolean remoteCopy = requireRemoteCopyPromisedByPreview( + plan, current, item.backupId()); ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot( plan.worldId(), item.backupId())); @@ -261,6 +255,29 @@ private boolean currentRemoteContainsSnapshot(Snapshot snapshot, BackupId backup } } + /** + * The preview promised "another copy still exists" for a synchronized backup. This + * proves it against the configured remote before the local ref goes, because the + * check compares the remote commit with the local one. A remote that has lost the + * snapshot, or cannot be reached, fails the item with nothing deleted: cleanup is + * never more destructive than what the user confirmed. + */ + private boolean requireRemoteCopyPromisedByPreview( + CleanupPlan plan, + Snapshot snapshot, + BackupId backupId) throws Exception { + if (!ManagedStorageSupport.synchronizedRemoteCopy( + ManagedStorageSupport.record(snapshot, backupId))) { + return false; + } + if (!ManagedStorageSupport.await(git.currentRemoteContainsSnapshot( + plan.worldId(), backupId))) { + throw new IOException( + "The configured remote no longer has this backup; review cleanup again"); + } + return true; + } + /** A protected backup may lose its last local copy only if the remote provably has it. */ private void requireCurrentRemoteCopies( CleanupPlan plan, From af794318351b899125ba89e16a9e2cf4afef79c6 Mon Sep 17 00:00:00 2001 From: ishaanko Date: Thu, 10 Sep 2026 15:25:18 -0700 Subject: [PATCH 4/4] docs(storage): say cleanup reads the remote but never changes it --- .../worldarchive/storage/management/CleanupExecutor.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java index 9d723ab..8c38165 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java @@ -26,10 +26,11 @@ import java.util.Set; /** - * Applies a confirmed {@link CleanupPlan}. Cleanup frees space on this computer only: - * it never contacts the configured remote. A backup whose snapshot is synchronized to - * the remote keeps its catalog record as a remote-only entry; a backup with no copy - * left anywhere leaves the catalog. + * Applies a confirmed {@link CleanupPlan}. Cleanup frees space on this computer only. + * It never changes the configured remote, but it does ask the remote whether a + * synchronized snapshot is still there before the local copy goes. Such a backup keeps + * its catalog record as a remote-only entry; a backup with no copy left anywhere + * leaves the catalog. */ final class CleanupExecutor { private final BackupCatalog catalog;