fix(git): preserve staging on failed commit (#197)

Isolated fix off current `main`; includes a regression test.

Author: presempathy-awb

src/git/versioned_store/core.rs

@@ -278,40 +278,67 @@ where
 
     /// Commit staged changes
     pub fn commit(&mut self, message: &str) -> Result<gix::ObjectId, GitKvError> {
+        let original_tree = self.tree.clone();
+        let original_staging_area = self.staging_area.clone();
+
         // Apply staged changes in a single batch so we run the streaming
         // canonical chunker once per commit instead of once per staged item.
-        let changes: Vec<(Vec<u8>, Option<Vec<u8>>)> = self.staging_area.drain().collect();
+        let changes: Vec<(Vec<u8>, Option<Vec<u8>>)> =
+            original_staging_area.clone().into_iter().collect();
         self.tree.apply_changes(changes);
 
-        // Persist the tree state (including updating root hash and saving config)
-        self.tree.persist_root();
-
-        // For all storage types, also save the tree config to git for historical access
-        self.save_tree_config_to_git_internal()?;
-
-        // Get the git root directory using work_dir() for worktree/submodule compatibility
-        let dataset_dir = self
-            .dataset_dir
-            .as_ref()
-            .ok_or_else(|| GitKvError::GitObjectError("Dataset directory not set".into()))?;
-        let git_root = self
-            .metadata
-            .work_dir()
-            .or_else(|| Self::find_git_root(dataset_dir))
-            .ok_or_else(|| GitKvError::GitObjectError("Could not find git root".into()))?;
-
-        // Stage and write tree via metadata backend
-        let tree_id = self.metadata.stage_and_write_tree(&git_root)?;
-
-        // Create commit via metadata backend
-        let commit_id = self.metadata.write_commit(tree_id, message)?;
+        let commit_result = (|| {
+            // Persist the tree state (including updating root hash and saving config)
+            self.tree.persist_root();
+
+            // For all storage types, also save the tree config to git for historical access
+            self.save_tree_config_to_git_internal()?;
+
+            // Get the git root directory using work_dir() for worktree/submodule compatibility
+            let dataset_dir = self
+                .dataset_dir
+                .as_ref()
+                .ok_or_else(|| GitKvError::GitObjectError("Dataset directory not set".into()))?;
+            let git_root = self
+                .metadata
+                .work_dir()
+                .or_else(|| Self::find_git_root(dataset_dir))
+                .ok_or_else(|| GitKvError::GitObjectError("Could not find git root".into()))?;
+
+            // Stage and write tree via metadata backend
+            let tree_id = self.metadata.stage_and_write_tree(&git_root)?;
+
+            // Create commit via metadata backend
+            let commit_id = self.metadata.write_commit(tree_id, message)?;
+
+            // Update branch ref and HEAD
+            self.metadata
+                .update_branch(&self.current_branch, commit_id)?;
+            self.metadata.update_head(&self.current_branch)?;
+
+            Ok(commit_id)
+        })();
+
+        let commit_id = match commit_result {
+            Ok(commit_id) => commit_id,
+            Err(err) => {
+                self.tree = original_tree;
+                self.staging_area = original_staging_area;
+                let staging_restore = self.save_staging_area();
+                let _ = self.save_tree_config_to_git_internal();
+
+                if let Err(restore_err) = staging_restore {
+                    return Err(GitKvError::GitObjectError(format!(
+                        "{err}; additionally failed to restore staging area after aborted commit: {restore_err}"
+                    )));
+                }
 
-        // Update branch ref and HEAD
-        self.metadata
-            .update_branch(&self.current_branch, commit_id)?;
-        self.metadata.update_head(&self.current_branch)?;
+                return Err(err);
+            }
+        };
 
         // Clear staging area file since we've committed
+        self.staging_area.clear();
         self.save_staging_area()?;
 
         Ok(commit_id)

src/git/versioned_store/tests.rs

@@ -323,6 +323,56 @@ mod tests {
         assert_eq!(status.len(), 0);
     }
 
+    #[test]
+    fn failed_commit_preserves_staging_and_committed_head() {
+        let temp_dir = TempDir::new().unwrap();
+
+        gix::init(temp_dir.path()).unwrap();
+
+        std::process::Command::new("git")
+            .args(["config", "user.name", "Test User"])
+            .current_dir(temp_dir.path())
+            .output()
+            .expect("git config name failed");
+        std::process::Command::new("git")
+            .args(["config", "user.email", "test@example.com"])
+            .current_dir(temp_dir.path())
+            .output()
+            .expect("git config email failed");
+
+        let dataset_dir = temp_dir.path().join("dataset");
+        std::fs::create_dir_all(&dataset_dir).unwrap();
+        let _cwd = CwdGuard::set(&dataset_dir);
+        let mut store = GitVersionedKvStore::<32>::init(&dataset_dir).unwrap();
+
+        store.insert(b"key1".to_vec(), b"value1".to_vec()).unwrap();
+
+        let config_path = dataset_dir.join("prolly_config_tree_config");
+        std::fs::remove_file(&config_path).unwrap();
+        std::fs::create_dir(&config_path).unwrap();
+
+        let err = store
+            .commit("commit should fail")
+            .expect_err("config write failure should abort commit");
+        assert!(
+            err.to_string().contains("Failed to write config file"),
+            "unexpected error: {err}"
+        );
+
+        let status = store.status();
+        assert_eq!(
+            status,
+            vec![(b"key1".to_vec(), "added".to_string())],
+            "failed commit must leave the staged change available to retry"
+        );
+
+        let head_keys = store.get_keys_at_ref("HEAD").unwrap();
+        assert!(
+            !head_keys.contains_key(&b"key1".to_vec()),
+            "failed commit must not advance committed history"
+        );
+    }
+
     #[test]
     fn test_single_commit_behavior() {
         let temp_dir = TempDir::new().unwrap();

src/tree.rs

@@ -312,7 +312,7 @@ impl Default for TreeStats {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct ProllyTree<const N: usize, S: NodeStorage<N>> {
     pub root: ProllyNode<N>,
     pub storage: S,