fix(backup): make snapshot backups restorable and non-fatal to teardown

Four defects in the snapshot mode 3.2.0 introduced.

The resolver dropped the trailing separator get_storage_path puts on a
volume path, because os.path.abspath strips it. rsync reads "dir" as
"copy the directory" where "dir/" means "copy its contents", so every
snapshot generation landed at <volume>/files/_data/... while the live
path lands at <volume>/files/... . Restores read the live layout, and
--link-dest found nothing to match against the previous generation. The
e2e never caught it because its driver appended the separator by hand.

Snapshot teardown was fatal and masking. A busy `btrfs subvolume delete`
raised out of the finally, which skipped the generation stamp and the
compose handling on a run whose data was already complete, and replaced
whatever the body had raised. The leftover is reported instead; removing
it is a cleanup problem, not a reason to discard a good generation.

A volume created after the snapshot was taken aborted the whole run: the
volume list is enumerated inside the snapshot context, and nothing is
stopped in snapshot mode, so the host keeps creating volumes for the
duration of the copy. Such a volume is now copied live with a warning,
which is exactly what the pre-snapshot code did for it.

The snapshot pass compares by content again. 3.2.0 dropped --checksum
because a snapshot source cannot move, which is true, but the comparison
that matters is against --link-dest: a file that changed while keeping
its size and whole-second mtime was hard-linked stale out of the previous
generation, and the single snapshot pass had no authoritative pass to
repair it the way the live path does. It is still one pass against two.

--hard-restart-projects is refused alongside --snapshot, the same way
--shutdown already is: the flag exists for stacks whose database cannot
be backed up hot, which is what a snapshot removes.

Tests: the trailing separator, both teardown behaviours, the new refusal,
and app.main driving the snapshot branch - the caller that runs in
production, which no test had exercised and where the layout defect
therefore stayed invisible. The e2e driver now feeds the resolver the
string shape get_storage_path really produces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/baudolo/backup/app.py

@@ -95,7 +95,16 @@ def main() -> int:
                 )
 
             if resolve_source is not None:
-                copy(authoritative=False, source=resolve_source(live_source))
+                snapshot_source = resolve_source(live_source)
+                if os.path.isdir(snapshot_source):
+                    copy(authoritative=True, source=snapshot_source)
+                else:
+                    print(
+                        f"WARNING: volume '{volume_name}' is not in the snapshot "
+                        "(created after it was taken); copying it live instead.",
+                        flush=True,
+                    )
+                    copy(authoritative=False)
                 continue
 
             if args.everything:

src/baudolo/backup/cli.py

@@ -94,4 +94,9 @@ def parse_args() -> argparse.Namespace:
         p.error("--snapshot and --snapshot-subject must be given together")
     if args.snapshot and args.shutdown:
         p.error("--shutdown is meaningless with --snapshot: containers are never stopped")
+    if args.snapshot and args.hard_restart_projects:
+        p.error(
+            "--hard-restart-projects is meaningless with --snapshot: the flag exists "
+            "for stacks whose database cannot be backed up hot, which a snapshot solves"
+        )
     return args

src/baudolo/backup/snapshot.py

@@ -17,12 +17,11 @@ import os
 from collections.abc import Callable, Iterator
 from contextlib import contextmanager
 
-from .shell import execute_shell_command
+from .shell import BackupException, execute_shell_command
 
 KINDS = ("btrfs", "zfs")
 
 
-
 class SnapshotError(RuntimeError):
     """A snapshot could not be created, resolved or removed."""
 
@@ -32,7 +31,11 @@ def _resolver(subject: str, root: str) -> Callable[[str], str]:
         relative = os.path.relpath(os.path.abspath(path), os.path.abspath(subject))
         if relative.startswith(".."):
             raise SnapshotError(f"{path} lies outside the snapshot subject {subject}")
-        return os.path.join(root, relative) if relative != "." else root
+        resolved = root if relative == "." else os.path.join(root, relative)
+
+        # abspath drops a trailing separator, and rsync reads "dir/" as its
+        # contents where "dir" means the directory itself.
+        return resolved + os.sep if path.endswith(os.sep) else resolved
 
     return resolve
 
@@ -74,6 +77,8 @@ def volume_snapshot(
 
     Raises:
         SnapshotError: the kind is unknown, or the snapshot cannot be created.
+            Removal failure is reported, not raised: a leftover snapshot is a
+            cleanup problem and must not discard a generation that is complete.
     """
     create = _CREATE.get(kind)
     if create is None:
@@ -83,4 +88,8 @@ def volume_snapshot(
     try:
         yield _resolver(subject, root)
     finally:
+        try:
             run(remove)
+        except BackupException as error:
+            # Raising here would also mask whatever the body raised.
+            print(f"WARNING: {root} could not be removed: {error}", flush=True)

tests/e2e/snapshot_db_driver.py

@@ -34,8 +34,8 @@ with volume_snapshot("btrfs", SUBJECT, "dbtest", run=shell) as resolve:
         VERSIONS,
         VOLUME,
         f"{GENERATION}/{VOLUME}",
-        authoritative=False,
+        authoritative=True,
-        source=resolve(DATADIR) + "/",
+        source=resolve(f"{DATADIR}/"),
     )
 
 print("SNAPSHOT COPY DONE", flush=True)

tests/unit/backup/test_app_snapshot.py

@@ -0,0 +1,79 @@
+"""Contract of app.main's snapshot branch - the caller that runs in production."""
+
+from __future__ import annotations
+
+import unittest
+from unittest import mock
+
+from baudolo.backup import app
+from baudolo.backup.snapshot import volume_snapshot
+
+
+def stubbed_snapshot(kind: str, subject: str, tag: str):
+    return volume_snapshot(kind, subject, tag, run=lambda command: [])
+
+ARGV = [
+    "baudolo",
+    "--compose-dir",
+    "/compose",
+    "--backups-dir",
+    "/backups",
+    "--snapshot",
+    "btrfs",
+    "--snapshot-subject",
+    "/var/lib/docker",
+]
+
+
+def drive(*, present: bool) -> list[dict]:
+    calls: list[dict] = []
+
+    def record(versions_dir, volume_name, volume_dir, *, authoritative, source):
+        calls.append(
+            {"volume": volume_name, "authoritative": authoritative, "source": source}
+        )
+
+    with (
+        mock.patch("sys.argv", ARGV),
+        mock.patch.object(app, "get_machine_id", return_value="machine"),
+        mock.patch.object(app, "create_version_directory", return_value="/gen"),
+        mock.patch.object(app, "create_volume_directory", return_value="/gen/vol"),
+        mock.patch.object(app, "load_databases_df", return_value=None),
+        mock.patch.object(app, "docker_volume_names", return_value=["vol"]),
+        mock.patch.object(app, "containers_using_volume", return_value=[]),
+        mock.patch.object(app, "volume_is_fully_ignored", return_value=False),
+        mock.patch.object(app, "backup_dumps_for_volume", return_value=(False, False)),
+        mock.patch.object(
+            app, "get_storage_path", return_value="/var/lib/docker/volumes/vol/_data/"
+        ),
+        mock.patch.object(app, "stamp_directory"),
+        mock.patch.object(app, "handle_docker_compose_services"),
+        mock.patch.object(app.os.path, "isdir", return_value=present),
+        mock.patch.object(app, "backup_volume", side_effect=record),
+        mock.patch.object(app, "volume_snapshot", stubbed_snapshot),
+    ):
+        app.main()
+    return calls
+
+
+class TestSnapshotBranch(unittest.TestCase):
+    def test_it_passes_a_path_ending_in_a_separator(self) -> None:
+        source = drive(present=True)[0]["source"]
+        self.assertTrue(source.endswith("/volumes/vol/_data/"), source)
+        self.assertNotIn("/var/lib/docker/volumes", source)
+
+    def test_it_reads_from_the_snapshot_and_not_from_the_live_tree(self) -> None:
+        source = drive(present=True)[0]["source"]
+        self.assertTrue(source.startswith("/var/lib/.baudolo-"), source)
+
+    def test_it_compares_by_content_against_the_previous_generation(self) -> None:
+        self.assertTrue(drive(present=True)[0]["authoritative"])
+
+    def test_a_volume_missing_from_the_snapshot_is_copied_live(self) -> None:
+        call = drive(present=False)[0]
+        self.assertEqual(call["source"], "/var/lib/docker/volumes/vol/_data/")
+        self.assertFalse(call["authoritative"])
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/unit/backup/test_cli.py

@@ -48,6 +48,22 @@ class TestSnapshotFlags(unittest.TestCase):
     def test_shutdown_stays_available_without_a_snapshot(self) -> None:
         self.assertTrue(parse("--shutdown").shutdown)
 
+    def test_hard_restart_is_rejected_because_nothing_is_stopped(self) -> None:
+        with self.assertRaises(SystemExit):
+            parse(
+                "--snapshot",
+                "btrfs",
+                "--snapshot-subject",
+                "/d",
+                "--hard-restart-projects",
+                "mailu",
+            )
+
+    def test_hard_restart_stays_available_without_a_snapshot(self) -> None:
+        self.assertEqual(
+            parse("--hard-restart-projects", "mailu").hard_restart_projects, ["mailu"]
+        )
+
 
 class TestRequiredFlags(unittest.TestCase):
     def test_backups_dir_is_required(self) -> None:

tests/unit/backup/test_snapshot.py

@@ -4,6 +4,7 @@ from __future__ import annotations
 
 import unittest
 
+from baudolo.backup.shell import BackupException
 from baudolo.backup.snapshot import SnapshotError, volume_snapshot
 
 
@@ -44,6 +45,14 @@ class TestBtrfs(unittest.TestCase):
                 "/var/lib/.baudolo-20260731/volumes/postgres_data/_data",
             )
 
+    def test_it_keeps_the_trailing_slash_rsync_reads_as_contents(self) -> None:
+        run = Runner()
+        with volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=run) as resolve:
+            self.assertEqual(
+                resolve("/var/lib/docker/volumes/postgres_data/_data/"),
+                "/var/lib/.baudolo-20260731/volumes/postgres_data/_data/",
+            )
+
     def test_it_removes_the_snapshot_even_when_the_body_raises(self) -> None:
         run = Runner()
         with self.assertRaises(ZeroDivisionError):
@@ -103,5 +112,23 @@ class TestRejections(unittest.TestCase):
             self.assertEqual(resolve("/var/lib/docker"), "/var/lib/.baudolo-20260731")
 
 
+class Busy(Runner):
+    def __call__(self, command: str) -> list[str]:
+        if command.startswith("btrfs subvolume delete"):
+            raise BackupException("target is busy")
+        return super().__call__(command)
+
+
+class TestRemovalFailure(unittest.TestCase):
+    def test_a_failed_removal_does_not_fail_a_completed_run(self) -> None:
+        with volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=Busy()):
+            pass
+
+    def test_a_failed_removal_does_not_mask_the_body(self) -> None:
+        with self.assertRaises(ZeroDivisionError):
+            with volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=Busy()):
+                raise ZeroDivisionError
+
+
 if __name__ == "__main__":
     unittest.main()