fix(storage): keep remote copies out of cleanup

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

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
 

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

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));

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,

src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java

@@ -26,9 +26,11 @@
 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 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;
@@ -123,10 +125,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 +137,14 @@ 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);
-                    }
+                    boolean remoteCopy = requireRemoteCopyPromisedByPreview(
+                            plan, current, item.backupId());
+                    ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot(
+                            plan.worldId(),
+                            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()) {
@@ -266,12 +256,27 @@ 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();
+    /**
+     * 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. */

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,

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<BackupRecord> 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.ownGitSnapshot(record);
             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;

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,25 @@ static boolean managedDestination(
                 .isPresent();
     }
 
+    /**
+     * 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.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<DestinationResult> destination(
             BackupRecord record,
             DestinationType type) {

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<BackupId, CleanupItem> 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<BackupId> remoteBackupIds() throws Exception {
         return nativeGit("--git-dir=" + remote, "for-each-ref", "--format=%(refname)",
                         "refs/heads/backups/")