From 56d705e2bbdf24f903eb2b2e2491a613ec8139fd Mon Sep 17 00:00:00 2001
From: DanConwayDev <DanConwayDev@protonmail.com>
Date: Wed, 19 Aug 2026 07:46:49 +0000
Subject: [PATCH] fix(storage): persist backup retirement before journal
 removal

Motivation: writing Retired before recursive deletion makes retries safe, but removing the journal durably without fsyncing the backup directory changes could leave a resurrected backup and no journal after power loss.

Approach: fsync the immediate backup parent after remove_dir_all, then fsync each parent after pruning an empty child directory. The journal is removed only after those directory-entry changes are durable. Non-empty or already-absent ancestors stop optional pruning; unexpected pruning errors are logged without undoing the already-durable backup removal.

Correctness assumptions: the migration root and journals reside on the same durable Git-data filesystem, and directory fsync provides the persistence boundary for entry removal. A failure before journal removal retains Retired state for an idempotent restart.

Deliberately excluded: this does not change which backups qualify for retirement or the content-addressed quarantine layout.

Validation: all migration unit tests pass, including complete and Retired restart states, successful parent pruning, retained backups, and content quarantine. Formatting and diff checks pass.
---
 CHANGELOG.md                                  |  3 +-
 docs/explanation/git-family-object-storage.md |  5 +++
 src/git/migration.rs                          | 35 ++++++++++++++++---
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31ab8fa..b07afcb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -81,7 +81,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   family migrates, bounding peak migration disk overhead to the family in
   flight. Unindexed legacy packs are quarantined by content under
   `.grasp/migration/unindexed-packs/`, unverifiable backups are retained with
-  a warning, and completed installations whose backups were already removed
+  a warning, backup-directory removals are fsynced before their journals are
+  removed, and completed installations whose backups were already removed
   manually start unchanged.
 - Stop treating server-side shallow repositories as valid family storage.
   Migration preserves a legacy `shallow` marker on the installed thin view
diff --git a/docs/explanation/git-family-object-storage.md b/docs/explanation/git-family-object-storage.md
index dcf92c7..e0f7fb9 100644
--- a/docs/explanation/git-family-object-storage.md
+++ b/docs/explanation/git-family-object-storage.md
@@ -291,6 +291,11 @@ before the next family is migrated. Peak migration overhead is therefore
 bounded by the family currently in flight except for the small set of families
 still awaiting repair.
 
+Backup deletion is itself durable: every directory entry removed while
+deleting and pruning a backup is fsynced before its journal is removed. A
+power loss therefore leaves either a discoverable `retired` journal that
+finishes deletion or no backup entry to rediscover.
+
 Packs without an index are Git-invisible, so the superset proof cannot vouch
 for them; they are moved into content-addressed directories beneath
 `.grasp/migration/unindexed-packs/` before their backup is deleted. Repeated
diff --git a/src/git/migration.rs b/src/git/migration.rs
index a6477f4..970de59 100644
--- a/src/git/migration.rs
+++ b/src/git/migration.rs
@@ -979,6 +979,7 @@ fn remove_retired_backup(storage: &LocalGitStorage, backup: &Path) -> Result<boo
             backup.display()
         ));
     }
+    let backup_parent = backup.parent().context("migration backup has no parent")?;
     match std::fs::remove_dir_all(backup) {
         Ok(()) => {}
         Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false),
@@ -987,12 +988,38 @@ fn remove_retired_backup(storage: &LocalGitStorage, backup: &Path) -> Result<boo
                 .with_context(|| format!("remove retired backup {}", backup.display()));
         }
     }
-    let mut parent = backup.parent();
-    while let Some(directory) = parent {
-        if !directory.starts_with(&root) || std::fs::remove_dir(directory).is_err() {
+    // Commit removal of the backup entry before the durable journal is
+    // removed. Otherwise a power loss could restore the backup without its
+    // journal, leaving an orphan that no later launch discovers.
+    fsync_directory(backup_parent)?;
+
+    let mut directory = backup_parent;
+    while directory.starts_with(&root) {
+        let Some(parent) = directory.parent() else {
             break;
+        };
+        match std::fs::remove_dir(directory) {
+            Ok(()) => {
+                fsync_directory(parent)?;
+                directory = parent;
+            }
+            Err(error)
+                if matches!(
+                    error.kind(),
+                    std::io::ErrorKind::DirectoryNotEmpty | std::io::ErrorKind::NotFound
+                ) =>
+            {
+                break;
+            }
+            Err(error) => {
+                warn!(
+                    directory = %directory.display(),
+                    %error,
+                    "Could not prune empty migration backup directory"
+                );
+                break;
+            }
         }
-        parent = directory.parent();
     }
     Ok(true)
 }