fix(chunker): remove empty parent dirs on download delete (#21)

client/src/chunker.rs

@@ -148,11 +148,36 @@ impl Chunker {
         trace!("deleting {:?}", path);
         let full_path = self.full_path(path);
 
-        // TODO delete folders up too if empty
-        fs::remove_file(full_path)
+        fs::remove_file(&full_path)
             .await
             .map_err(|e| SyncError::from_io_error(path, e))?;
 
+        // Walk parents upward and remove empty directories. `remove_dir`
+        // only succeeds on empty directories, which gives us strict-empty
+        // semantics without manually counting entries. Stop at the storage
+        // root (never remove it) and on any error (most commonly ENOTEMPTY
+        // when a sibling file is present — that's the normal terminating
+        // condition, not a failure to propagate).
+        //
+        // SAFETY: `dir == self.base_path` is a lexical comparison. It is
+        // sound here because `full_path` is built by `base_path.clone()`
+        // + `push(path)` with no canonicalization on either side, and the
+        // server-provided `path` is trusted to not contain `..` segments
+        // (the indexer produces forward-slash relative paths via WalkDir,
+        // which never emits parent components). If that contract ever
+        // weakens, harden this guard with `starts_with` or a depth check.
+        let mut parent = full_path.parent();
+        while let Some(dir) = parent {
+            if dir == self.base_path {
+                break;
+            }
+            if let Err(e) = fs::remove_dir(dir).await {
+                trace!("stopping empty-dir cleanup at {:?}: {}", dir, e);
+                break;
+            }
+            parent = dir.parent();
+        }
+
         Ok(())
     }
 
@@ -646,4 +671,152 @@ mod tests {
             "three newline-terminated lines should yield three text chunks; got {hashes:?}"
         );
     }
+
+    #[tokio::test]
+    async fn delete_removes_empty_parent_directories() {
+        // Issue #21: when the syncer deletes the only file inside a
+        // nested directory, those now-empty parents must also be removed.
+        // Otherwise users see ghost folders accumulate on receiving devices.
+        let temp = tempfile::TempDir::new().unwrap();
+        let cache = InMemoryCache::new(100, 10_000);
+        let mut chunker = Chunker::new(cache, temp.path().to_path_buf());
+
+        let nested_dir = temp.path().join("a").join("b");
+        tokio::fs::create_dir_all(&nested_dir).await.unwrap();
+        tokio::fs::write(nested_dir.join("c.cook"), b"eggs\n")
+            .await
+            .unwrap();
+
+        chunker
+            .delete("a/b/c.cook")
+            .await
+            .expect("delete should succeed");
+
+        assert!(
+            !temp.path().join("a/b/c.cook").exists(),
+            "file should be removed"
+        );
+        assert!(
+            !temp.path().join("a/b").exists(),
+            "empty intermediate directory should be removed"
+        );
+        assert!(
+            !temp.path().join("a").exists(),
+            "empty grandparent directory should be removed"
+        );
+        assert!(
+            temp.path().exists(),
+            "storage root must never be removed"
+        );
+    }
+
+    #[tokio::test]
+    async fn delete_stops_at_first_non_empty_ancestor() {
+        // If `a/` still has a sibling file after we delete `a/b/c.cook`,
+        // we must remove `a/b/` (now empty) but leave `a/` alone.
+        // remove_dir naturally enforces this via ENOTEMPTY; this test
+        // pins that behavior so a future refactor can't accidentally
+        // implement recursive deletion.
+        let temp = tempfile::TempDir::new().unwrap();
+        let cache = InMemoryCache::new(100, 10_000);
+        let mut chunker = Chunker::new(cache, temp.path().to_path_buf());
+
+        let nested_dir = temp.path().join("a").join("b");
+        tokio::fs::create_dir_all(&nested_dir).await.unwrap();
+        tokio::fs::write(nested_dir.join("c.cook"), b"eggs\n")
+            .await
+            .unwrap();
+        tokio::fs::write(temp.path().join("a").join("sibling.cook"), b"flour\n")
+            .await
+            .unwrap();
+
+        chunker
+            .delete("a/b/c.cook")
+            .await
+            .expect("delete should succeed");
+
+        assert!(
+            !temp.path().join("a/b/c.cook").exists(),
+            "target file should be removed"
+        );
+        assert!(
+            !temp.path().join("a/b").exists(),
+            "empty intermediate directory should be removed"
+        );
+        assert!(
+            temp.path().join("a").exists(),
+            "non-empty ancestor must be preserved"
+        );
+        assert!(
+            temp.path().join("a/sibling.cook").exists(),
+            "sibling file must be preserved"
+        );
+    }
+
+    #[tokio::test]
+    async fn delete_does_not_remove_storage_root() {
+        // Even when the very last file in the storage root is deleted,
+        // the root itself must survive — otherwise the next download
+        // cycle has nowhere to write to, and the indexer would crash
+        // walking a missing directory.
+        let temp = tempfile::TempDir::new().unwrap();
+        let cache = InMemoryCache::new(100, 10_000);
+        let mut chunker = Chunker::new(cache, temp.path().to_path_buf());
+
+        tokio::fs::write(temp.path().join("only.cook"), b"sugar\n")
+            .await
+            .unwrap();
+
+        chunker
+            .delete("only.cook")
+            .await
+            .expect("delete should succeed");
+
+        assert!(
+            !temp.path().join("only.cook").exists(),
+            "file should be removed"
+        );
+        assert!(
+            temp.path().exists(),
+            "storage root must never be removed"
+        );
+        assert!(
+            temp.path().is_dir(),
+            "storage root must still be a directory"
+        );
+    }
+
+    #[tokio::test]
+    async fn delete_leaves_sibling_files_in_same_directory() {
+        // Deleting `a/x.cook` when `a/y.cook` also exists must leave
+        // both `a/` and `a/y.cook` intact. This is the "obvious" case
+        // but worth pinning — a naive implementation that always
+        // removes the immediate parent would break it.
+        let temp = tempfile::TempDir::new().unwrap();
+        let cache = InMemoryCache::new(100, 10_000);
+        let mut chunker = Chunker::new(cache, temp.path().to_path_buf());
+
+        let dir = temp.path().join("a");
+        tokio::fs::create_dir_all(&dir).await.unwrap();
+        tokio::fs::write(dir.join("x.cook"), b"salt\n").await.unwrap();
+        tokio::fs::write(dir.join("y.cook"), b"pepper\n").await.unwrap();
+
+        chunker
+            .delete("a/x.cook")
+            .await
+            .expect("delete should succeed");
+
+        assert!(
+            !temp.path().join("a/x.cook").exists(),
+            "target file should be removed"
+        );
+        assert!(
+            temp.path().join("a").exists(),
+            "directory with remaining files must be preserved"
+        );
+        assert!(
+            temp.path().join("a/y.cook").exists(),
+            "sibling file must be preserved"
+        );
+    }
 }