fix: record delete operations

bundler/src/main/java/io/ib67/sfcraft/bundler/BundleEntry.java

@@ -16,5 +16,7 @@ public record BundleEntry(
 ) {
     public static short ATTR_REASSEMBLE = 1;
     public static short ATTR_SYMLINK = 2;
+    /** Tombstone: the file was deleted since the base backup; the reader removes it on restore. */
+    public static short ATTR_DELETE = 4;
     public static short MAGIC = (short) 0xA2E3;
 }

bundler/src/main/java/io/ib67/sfcraft/bundler/BundleReader.java

@@ -10,6 +10,7 @@ import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.channels.FileChannel;
+import java.nio.file.DirectoryNotEmptyException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
@@ -21,15 +22,17 @@ public class BundleReader {
     protected final boolean linkSymbol;
     private final byte[] buffer = new byte[4096];
     protected final boolean skipOldEntry;
+    protected final boolean ignoreDeletions;
     protected final ByteBufAllocator allocator;
     protected final byte[] dict;
 
     @Builder
-    public BundleReader(long maxTime, long minTime, boolean linkSymbol, boolean skipOldEntry, ByteBufAllocator allocator, byte[] dict) {
+    public BundleReader(long maxTime, long minTime, boolean linkSymbol, boolean skipOldEntry, boolean ignoreDeletions, ByteBufAllocator allocator, byte[] dict) {
         this.maxTime = maxTime == 0 ? Long.MAX_VALUE : maxTime;
         this.minTime = minTime;
         this.linkSymbol = linkSymbol;
         this.skipOldEntry = skipOldEntry;
+        this.ignoreDeletions = ignoreDeletions;
         this.allocator = allocator == null ? ByteBufAllocator.DEFAULT : allocator;
         this.dict = dict;
     }
@@ -62,6 +65,22 @@ public class BundleReader {
                 readMaps.put(entry.name(), time);
 
                 var path = resolveSafely(destRoot, entry.name());
+                if ((entry.attribute() & BundleEntry.ATTR_DELETE) != 0) {
+                    // tombstone from an incremental: mirror the deletion instead of leaving a
+                    // resurrected file from the base backup. Deleting a path not present in the
+                    // base is a harmless no-op. When ignoreDeletions is set the tombstone is
+                    // skipped, so files deleted after the base are kept on restore.
+                    if (!ignoreDeletions) {
+                        try {
+                            Files.deleteIfExists(path);
+                        } catch (DirectoryNotEmptyException e) {
+                            // a directory tombstone; its contained files are removed by their own
+                            // tombstones and an empty leftover directory is harmless
+                        }
+                    }
+                    skipExactly(in, entry.length());
+                    continue;
+                }
                 var parentDir = path.getParent();
                 if (Files.notExists(parentDir)) Files.createDirectories(parentDir);
                 if ((entry.attribute() & BundleEntry.ATTR_SYMLINK) != 0) {

bundler/src/main/java/io/ib67/sfcraft/bundler/BundleWriter.java

@@ -133,6 +133,20 @@ public class BundleWriter implements Closeable {
             Path pathToBundle,
             Iterable<Path> path,
             UnaryOperator<BundleWriterBuilder> config
+    ) throws IOException {
+        createBundle(pathToBundle, path, List.of(), config);
+    }
+
+    /**
+     * @param deletedRelPaths bundle-relative paths of files deleted since the base backup;
+     *                        written as tombstones so a chained restore mirrors deletions
+     *                        instead of resurrecting files. Empty for full backups.
+     */
+    public static void createBundle(
+            Path pathToBundle,
+            Iterable<Path> path,
+            Iterable<String> deletedRelPaths,
+            UnaryOperator<BundleWriterBuilder> config
     ) throws IOException {
         try (var channel = FileChannel.open(pathToBundle,
                 StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) {
@@ -144,6 +158,9 @@ public class BundleWriter implements Closeable {
                     var s = p.toString();
                     writer.write(p, (s.contains("region") && s.endsWith(".mca")) || s.endsWith(".dat"));
                 }
+                for (var rel : deletedRelPaths) {
+                    writer.writeTombstone(rel);
+                }
             }
             // the zstd epilogue is flushed by writer.close(); fsync before closing so a crash
             // right after "backup success" cannot leave a hollow bundle behind
@@ -185,6 +202,14 @@ public class BundleWriter implements Closeable {
         writePlain(path);
     }
 
+    /**
+     * Records that {@code relPath} was deleted since the base backup. Carries no payload; on
+     * restore the reader removes the file so a chained restore mirrors the deletion.
+     */
+    public void writeTombstone(String relPath) throws IOException {
+        outputStream.writeEntryHeader(relPath, BundleEntry.ATTR_DELETE, 0, System.currentTimeMillis());
+    }
+
     private void writeSymbol(Path path) throws IOException {
         if (!Files.isSymbolicLink(path)) return;
         var target = Files.readSymbolicLink(path);

bundler/src/test/java/io/ib67/bundler/TestBundleReader.java

@@ -41,6 +41,80 @@ public class TestBundleReader {
         assertEquals("untouched", Files.readString(dest.resolve("b.txt")));
     }
 
+    @Test
+    public void testTombstoneRemovesFileOnRestore(@TempDir Path tmp) throws IOException {
+        var src = Files.createDirectories(tmp.resolve("src"));
+        Files.writeString(src.resolve("keep.txt"), "keep");
+        Files.writeString(src.resolve("gone.txt"), "gone");
+        var full = tmp.resolve("full.swb.zst");
+        BundleWriter.createBundle(full, List.of(src.resolve("keep.txt"), src.resolve("gone.txt")),
+                c -> c.relativeRoot(src));
+
+        // gone.txt was deleted; the incremental carries only a tombstone for it
+        var incr = tmp.resolve("incr.swb.zst");
+        BundleWriter.createBundle(incr, List.of(), List.of("gone.txt"), c -> c.relativeRoot(src));
+
+        var dest = tmp.resolve("dest");
+        var reader = BundleReader.builder().build();
+        reader.extract(full, dest);
+        assertTrue(Files.exists(dest.resolve("gone.txt")), "base restore should lay down the file first");
+
+        reader.extract(incr, dest);
+        assertTrue(Files.exists(dest.resolve("keep.txt")), "unrelated files must survive the tombstone");
+        assertFalse(Files.exists(dest.resolve("gone.txt")),
+                "a tombstoned file must be removed on chained restore, not resurrected");
+    }
+
+    @Test
+    public void testIgnoreDeletionsKeepsTombstonedFile(@TempDir Path tmp) throws IOException {
+        var src = Files.createDirectories(tmp.resolve("src"));
+        Files.writeString(src.resolve("keep.txt"), "keep");
+        Files.writeString(src.resolve("gone.txt"), "gone");
+        var full = tmp.resolve("full.swb.zst");
+        BundleWriter.createBundle(full, List.of(src.resolve("keep.txt"), src.resolve("gone.txt")),
+                c -> c.relativeRoot(src));
+        var incr = tmp.resolve("incr.swb.zst");
+        BundleWriter.createBundle(incr, List.of(), List.of("gone.txt"), c -> c.relativeRoot(src));
+
+        var dest = tmp.resolve("dest");
+        var reader = BundleReader.builder().ignoreDeletions(true).build();
+        reader.extract(full, dest);
+        reader.extract(incr, dest);
+
+        assertTrue(Files.exists(dest.resolve("gone.txt")),
+                "with ignoreDeletions the tombstone is skipped and the file is kept");
+        assertEquals("gone", Files.readString(dest.resolve("gone.txt")));
+        assertEquals("keep", Files.readString(dest.resolve("keep.txt")));
+    }
+
+    @Test
+    public void testTombstoneForUnknownPathIsHarmless(@TempDir Path tmp) throws IOException {
+        var src = Files.createDirectories(tmp.resolve("src"));
+        Files.writeString(src.resolve("a.txt"), "a");
+        var bundle = tmp.resolve("b.swb.zst");
+        // tombstone a nested path that was never in the base backup
+        BundleWriter.createBundle(bundle, List.of(src.resolve("a.txt")),
+                List.of("region/r.0.0.mca"), c -> c.relativeRoot(src));
+
+        var dest = tmp.resolve("dest");
+        BundleReader.builder().build().extract(bundle, dest);
+        assertEquals("a", Files.readString(dest.resolve("a.txt")));
+    }
+
+    @Test
+    public void testTombstoneCannotEscapeDestination(@TempDir Path tmp) throws IOException {
+        var outside = Files.writeString(tmp.resolve("outside.txt"), "precious");
+        var bundle = tmp.resolve("evil.swb.zst");
+        try (var zstd = new ZstdOutputStreamNoFinalizer(Files.newOutputStream(bundle), 3);
+             var out = new EntryOutputStream(zstd)) {
+            out.writeEntryHeader("../outside.txt", io.ib67.sfcraft.bundler.BundleEntry.ATTR_DELETE, 0,
+                    System.currentTimeMillis());
+        }
+        var dest = tmp.resolve("dest");
+        assertThrows(IOException.class, () -> BundleReader.builder().build().extract(bundle, dest));
+        assertTrue(Files.exists(outside), "a tombstone must not delete files outside the destination");
+    }
+
     @Test
     public void testZipSlipIsRejected(@TempDir Path tmp) throws IOException {
         var bundle = tmp.resolve("evil.swb.zst");

src/main/java/io/ib67/sfcraft/BackupCommands.java

@@ -36,10 +36,11 @@ public class BackupCommands {
             "    -- Show this message",
             " /backup list ",
             "    -- List backups",
-            " /backup restore <backupKey>",
+            " /backup restore [keepdeleted] <backupKey>",
             "    -- Restore the backup on next server restart.",
             "    Restoring an incremental backup automatically applies its full base",
             "    and every incremental in between, in order.",
+            "    keepdeleted: skip deletions, keeping files removed after the base backup.",
             "    Can be cancelled by /backup cancelrestore",
             " /backup delete <backupKey>",
             "    -- Delete a backup. This does not ask for a confirmation.",
@@ -71,9 +72,13 @@ public class BackupCommands {
                                 .suggests(this::suggestBackups)
                                 .executes(this::cmdDelete)))
                 .then(literal("restore")
+                        .then(literal("keepdeleted")
                                 .then(argument("backupKey", StringArgumentType.greedyString())
                                         .suggests(this::suggestBackups)
-                                .executes(this::cmdRestore)))
+                                        .executes(ctx -> cmdRestore(ctx, true))))
+                        .then(argument("backupKey", StringArgumentType.greedyString())
+                                .suggests(this::suggestBackups)
+                                .executes(ctx -> cmdRestore(ctx, false))))
                 .then(literal("create")
                         .then(argument("incremental", BoolArgumentType.bool())
                                 .then(argument("subject", StringArgumentType.greedyString())
@@ -130,7 +135,7 @@ public class BackupCommands {
         return Command.SINGLE_SUCCESS;
     }
 
-    private int cmdRestore(CommandContext<CommandSourceStack> context) {
+    private int cmdRestore(CommandContext<CommandSourceStack> context, boolean ignoreDeletions) {
         var s = context.getSource();
         if (restoreIssued != null) {
             s.sendSystemMessage(Component.literal("Another restore request is already present.").withColor(Color.RED.getRGB()));
@@ -155,6 +160,10 @@ public class BackupCommands {
             s.sendSystemMessage(Component.literal("  -> [" + (b.incremental() ? "INCR" : "FULL") + "] " + b.backupKey())
                     .withColor(Color.GRAY.getRGB()));
         }
+        if (ignoreDeletions) {
+            s.sendSystemMessage(Component.literal("Deletions will be ignored: files removed after the base backup are kept.")
+                    .withColor(Color.YELLOW.getRGB()));
+        }
         restoreIssued = () -> {
             var destination = Path.of(backup.from());
             var movedDst = destination.resolveSibling(destination.getFileName().toString() + "_bak_" + System.currentTimeMillis());
@@ -180,8 +189,9 @@ public class BackupCommands {
             try {
                 for (int i = 0; i < chain.size(); i++) {
                     var b = chain.get(i);
-                    log.info("Applying {} ({}/{})", b.backupKey(), i + 1, chain.size());
-                    mod.tracker.recoverBackup(b, destination);
+                    log.info("Applying {} ({}/{}){}", b.backupKey(), i + 1, chain.size(),
+                            ignoreDeletions ? " [ignoring deletions]" : "");
+                    mod.tracker.recoverBackup(b, destination, ignoreDeletions);
                 }
                 log.info("Backup has been recovered.");
             } catch (Exception e) {

src/main/java/io/ib67/sfcraft/BackupSubject.java

@@ -5,15 +5,20 @@ import java.util.HashSet;
 import java.util.Set;
 
 /**
- * A directory tree being backed up, plus the set of files the watcher saw change since the
- * last incremental backup. All access to the change set goes through synchronized methods so
- * the watcher's adds and the worker's drain can never interleave badly (no lost events, no
- * ConcurrentModificationException while bundling).
+ * A directory tree being backed up, plus the changes the watcher saw since the last
+ * incremental backup: files that were created/modified, and files that were deleted. All
+ * access goes through synchronized methods so the watcher's events and the worker's drain can
+ * never interleave badly (no lost events, no ConcurrentModificationException while bundling).
  */
 public final class BackupSubject {
+    /** A drained snapshot of changes: touched files to bundle, deleted files to tombstone. */
+    public record Changes(Set<Path> changed, Set<Path> deleted) {
+    }
+
     private final String name;
     private final Path root;
     private final Set<Path> changedFiles = new HashSet<>();
+    private final Set<Path> deletedFiles = new HashSet<>();
     private boolean forceFullNext;
 
     public BackupSubject(String name, Path root) {
@@ -29,22 +34,41 @@ public final class BackupSubject {
         return root;
     }
 
+    /** A create/modify supersedes any pending deletion of the same path. */
     public synchronized void addChanged(Path file) {
+        deletedFiles.remove(file);
         changedFiles.add(file);
     }
 
-    /**
-     * Atomically takes ownership of the accumulated change set, leaving an empty one behind.
-     * If the backup fails, give the files back via {@link #mergeBack(Set)}.
-     */
-    public synchronized Set<Path> drainChanged() {
-        var drained = new HashSet<>(changedFiles);
-        changedFiles.clear();
-        return drained;
+    /** A deletion supersedes any pending create/modify of the same path. */
+    public synchronized void addDeleted(Path file) {
+        changedFiles.remove(file);
+        deletedFiles.add(file);
     }
 
-    public synchronized void mergeBack(Set<Path> files) {
-        changedFiles.addAll(files);
+    /**
+     * Atomically takes ownership of the accumulated changes, leaving empty sets behind.
+     * If the backup fails, give them back via {@link #mergeBack(Changes)}.
+     */
+    public synchronized Changes drain() {
+        var changed = new HashSet<>(changedFiles);
+        var deleted = new HashSet<>(deletedFiles);
+        changedFiles.clear();
+        deletedFiles.clear();
+        return new Changes(changed, deleted);
+    }
+
+    /**
+     * Restores drained changes after a failed backup, without clobbering newer events: a path
+     * the watcher has since seen change is not re-marked deleted, and vice versa.
+     */
+    public synchronized void mergeBack(Changes changes) {
+        for (var f : changes.changed()) {
+            if (!deletedFiles.contains(f)) changedFiles.add(f);
+        }
+        for (var f : changes.deleted()) {
+            if (!changedFiles.contains(f)) deletedFiles.add(f);
+        }
     }
 
     /**
@@ -58,7 +82,11 @@ public final class BackupSubject {
     public synchronized boolean consumeForceFull() {
         var v = forceFullNext;
         forceFullNext = false;
-        if (v) changedFiles.clear();
+        if (v) {
+            // a full backup snapshots the live filesystem, so pending deltas are moot
+            changedFiles.clear();
+            deletedFiles.clear();
+        }
         return v;
     }
 }

src/main/java/io/ib67/sfcraft/BackupWorker.java

@@ -9,6 +9,7 @@ import net.minecraft.server.MinecraftServer;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
@@ -69,7 +70,8 @@ public record BackupWorker(
         }
 
         var tmpBundle = tmpDir.resolve("backup_" + subject.name() + "_" + createdAt + ".swb.zst");
-        Set<Path> drained = null;
+        BackupSubject.Changes changes = null;
+        List<String> tombstones = List.of();
         Path stagingRoot = null;
         try {
             // Barrier: a main-thread save that started before we locked has finished by now.
@@ -80,8 +82,9 @@ public record BackupWorker(
             if (full) {
                 files = collectFullFileList();
             } else {
-                drained = subject.drainChanged();
-                files = drained.stream().filter(this::eligible).toList();
+                changes = subject.drain();
+                files = changes.changed().stream().filter(this::eligible).toList();
+                tombstones = resolveTombstones(changes.deleted());
             }
 
             var packRoot = subject.root();
@@ -99,11 +102,12 @@ public record BackupWorker(
                 log.info("Reflinked {} files to staging; world IO resumed, packing without the lock.", files.size());
             }
 
-            log.info("Creating {} bundle {} ({} files)", full ? "full" : "incremental", tmpBundle, files.size());
+            log.info("Creating {} bundle {} ({} files, {} deletions)",
+                    full ? "full" : "incremental", tmpBundle, files.size(), tombstones.size());
             var finalPackRoot = packRoot;
-            BundleWriter.createBundle(tmpBundle, files, c -> config.apply(c).relativeRoot(finalPackRoot));
+            BundleWriter.createBundle(tmpBundle, files, tombstones, c -> config.apply(c).relativeRoot(finalPackRoot));
         } catch (Exception e) {
-            if (drained != null) subject.mergeBack(drained);
+            if (changes != null) subject.mergeBack(changes);
             Files.deleteIfExists(tmpBundle);
             throw e;
         } finally {
@@ -120,13 +124,32 @@ public record BackupWorker(
         try {
             return strategy.createBackup(subject.name(), subject.root(), tmpBundle, !full, createdAt);
         } catch (Exception e) {
-            if (drained != null) subject.mergeBack(drained);
+            if (changes != null) subject.mergeBack(changes);
             throw e;
         } finally {
             Files.deleteIfExists(tmpBundle);
         }
     }
 
+    /**
+     * Turns absolute deleted paths into bundle-relative tombstone names. A path that exists
+     * again at snapshot time (deleted then re-created) is dropped — it is bundled as content
+     * instead — as is anything we would not have backed up (session.lock, excluded roots, or
+     * a path outside the subject).
+     */
+    private List<String> resolveTombstones(Set<Path> deleted) {
+        var absRoot = subject.root().toAbsolutePath();
+        var result = new ArrayList<String>();
+        for (Path p : deleted) {
+            var abs = p.toAbsolutePath();
+            if (Files.exists(abs)) continue;
+            if (!abs.startsWith(absRoot)) continue;
+            if (!eligible(abs)) continue;
+            result.add(absRoot.relativize(abs).toString());
+        }
+        return result;
+    }
+
     /**
      * Runs a task on the main thread, but never hangs: once the server leaves its tick loop
      * (shutdown) submitted tasks are no longer processed, so we poll and abort the backup

src/main/java/io/ib67/sfcraft/FileWatcherThread.java

@@ -18,6 +18,13 @@ public class FileWatcherThread extends Thread {
     public interface Listener {
         void onChange(String subject, Path file);
 
+        /**
+         * A path under this subject was deleted. The path is already gone, so it may have been
+         * a file or a directory; the backup layer reconciles against the live filesystem and
+         * only tombstones paths that are still absent at snapshot time.
+         */
+        void onDelete(String subject, Path file);
+
         /**
          * Events for this subject were lost (OS queue overflow, or its directory tree was
          * replaced underneath us); the accumulated change set can no longer be trusted and
@@ -87,7 +94,9 @@ public class FileWatcherThread extends Thread {
                     }
                     var path = dir.resolve((Path) ev.context());
                     log.debug("event {} on {} ({})", kind, path, subject);
-                    if (Files.isRegularFile(path)) {
+                    if (kind == StandardWatchEventKinds.ENTRY_DELETE) {
+                        listener.onDelete(subject, path);
+                    } else if (Files.isRegularFile(path)) {
                         listener.onChange(subject, path);
                     } else if (Files.isDirectory(path) && kind == StandardWatchEventKinds.ENTRY_CREATE) {
                         // report contained files too: they may have been created before we
@@ -168,7 +177,8 @@ public class FileWatcherThread extends Thread {
                     var abs = p.toAbsolutePath();
                     var key = abs.register(watcher,
                             StandardWatchEventKinds.ENTRY_CREATE,
-                            StandardWatchEventKinds.ENTRY_MODIFY);
+                            StandardWatchEventKinds.ENTRY_MODIFY,
+                            StandardWatchEventKinds.ENTRY_DELETE);
                     watchedDirs.put(abs, subject);
                     keyToDir.put(key, abs);
                 } else if (reportFiles && Files.isRegularFile(p)) {

src/main/java/io/ib67/sfcraft/JustBackupMod.java

@@ -129,6 +129,12 @@ public class JustBackupMod implements ModInitializer, FileWatcherThread.Listener
         if (subject != null) subject.addChanged(file);
     }
 
+    @Override
+    public void onDelete(String subjectName, Path file) {
+        var subject = backupSubjects.get(subjectName);
+        if (subject != null) subject.addDeleted(file);
+    }
+
     @Override
     public void onOverflow(String subjectName) {
         var subject = backupSubjects.get(subjectName);

src/main/java/io/ib67/sfcraft/strategy/BackupStrategy.java

@@ -17,7 +17,13 @@ public interface BackupStrategy {
      */
     Backup createBackup(String subject, Path source, Path bundle, boolean incremental, long createdAt);
 
-    void recoverBackup(Backup backup, Path restorePath);
+    /**
+     * Extracts {@code backup} into {@code restorePath}.
+     *
+     * @param ignoreDeletions when true, tombstone entries are skipped, so files deleted since
+     *                        the base backup are kept instead of removed on restore
+     */
+    void recoverBackup(Backup backup, Path restorePath, boolean ignoreDeletions);
 
     void deleteBackup(Backup backup);
 

src/main/java/io/ib67/sfcraft/strategy/BackupTracker.java

@@ -45,8 +45,8 @@ public class BackupTracker implements BackupStrategy {
     }
 
     @Override
-    public void recoverBackup(Backup backup, Path restorePath) {
-        underlying.recoverBackup(backup, restorePath);
+    public void recoverBackup(Backup backup, Path restorePath, boolean ignoreDeletions) {
+        underlying.recoverBackup(backup, restorePath, ignoreDeletions);
     }
 
     @Override

src/main/java/io/ib67/sfcraft/strategy/LocalBackupStrategy.java

@@ -41,10 +41,10 @@ public class LocalBackupStrategy implements BackupStrategy {
 
     @Override
     @SneakyThrows
-    public void recoverBackup(Backup backup, Path restorePath) {
+    public void recoverBackup(Backup backup, Path restorePath, boolean ignoreDeletions) {
         var backupFile = backupParentRoot.resolve(backup.backupKey());
         if (Files.notExists(backupFile)) throw new IllegalArgumentException("Backup " + backupFile + " does not exist");
-        BundleReader.builder().build().extract(backupFile, restorePath);
+        BundleReader.builder().ignoreDeletions(ignoreDeletions).build().extract(backupFile, restorePath);
     }
 
     @SneakyThrows

src/main/java/io/ib67/sfcraft/strategy/S3BackupStrategy.java

@@ -72,14 +72,14 @@ public class S3BackupStrategy implements BackupStrategy {
 
     @Override
     @SneakyThrows
-    public void recoverBackup(Backup backup, Path restorePath) {
+    public void recoverBackup(Backup backup, Path restorePath, boolean ignoreDeletions) {
         var target = temporaryDownloadPath.resolve("s3_" + System.currentTimeMillis());
         try {
             try (var resp = s3.getObject(b -> b.bucket(option.bucket()).key(backup.backupKey()));
                  var fs = Files.newOutputStream(target, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) {
                 resp.transferTo(fs);
             }
-            BundleReader.builder().build().extract(target, restorePath);
+            BundleReader.builder().ignoreDeletions(ignoreDeletions).build().extract(target, restorePath);
         } finally {
             Files.deleteIfExists(target);
         }