fix(git): error on missing historical mappings (#211)

Author: presempathy-awb

src/git/versioned_store/backends.rs

@@ -727,18 +727,29 @@ impl<const N: usize> VersionedKvStore<N, GitNodeStorage<N>, GitMetadataBackend>
         let config_result = self.metadata.read_file_at_commit(commit_id, &config_path);
         let mapping_result = self.metadata.read_file_at_commit(commit_id, &mapping_path);
 
-        // If files are not found, this might be an initial empty commit, return empty
-        if config_result.is_err() || mapping_result.is_err() {
-            return Ok(HashMap::new());
-        }
+        let (config_data, mapping_data) = match (config_result, mapping_result) {
+            (Ok(config_data), Ok(mapping_data)) => (config_data, mapping_data),
+            (Err(_), Err(_)) => {
+                // Both files missing can be an initial empty commit.
+                return Ok(HashMap::new());
+            }
+            (Ok(_), Err(e)) => {
+                return Err(GitKvError::GitObjectError(format!(
+                    "Historical commit {commit_id} has {config_path} but is missing {mapping_path}: {e}"
+                )));
+            }
+            (Err(e), Ok(_)) => {
+                return Err(GitKvError::GitObjectError(format!(
+                    "Historical commit {commit_id} has {mapping_path} but is missing {config_path}: {e}"
+                )));
+            }
+        };
 
-        let config_data = config_result?;
         let config: TreeConfig<N> = serde_json::from_slice(&config_data).map_err(|e| {
             GitKvError::GitObjectError(format!("Failed to deserialize config: {e}"))
         })?;
 
         // Load the hash mappings from the tree as string format and parse
-        let mapping_data = mapping_result?;
         let mapping_str = String::from_utf8(mapping_data)
             .map_err(|e| GitKvError::GitObjectError(format!("Invalid UTF-8 in mappings: {e}")))?;
 
@@ -792,6 +803,11 @@ impl<const N: usize> VersionedKvStore<N, GitNodeStorage<N>, GitMetadataBackend>
 
         // For Git storage, reconstruct the tree from hash mappings
         if hash_mappings.is_empty() {
+            if config.root_hash.is_some() {
+                return Err(GitKvError::GitObjectError(format!(
+                    "Historical commit {commit_id} has a tree root but {mapping_path} contains no hash mappings"
+                )));
+            }
             return Ok(HashMap::new());
         }
 

src/git/versioned_store/tests.rs

@@ -215,6 +215,78 @@ mod proof_tests {
             Some(&b"value3".to_vec())
         );
     }
+
+    #[test]
+    fn get_keys_at_ref_errors_when_committed_hash_mappings_are_missing() {
+        let temp_dir = TempDir::new().expect("Failed to create temp dir");
+        let repo_path = temp_dir.path().to_str().unwrap();
+
+        std::process::Command::new("git")
+            .args(["init"])
+            .current_dir(repo_path)
+            .output()
+            .expect("Failed to initialize git repo");
+        std::process::Command::new("git")
+            .args(["config", "user.name", "Test User"])
+            .current_dir(repo_path)
+            .output()
+            .expect("Failed to set git user name");
+        std::process::Command::new("git")
+            .args(["config", "user.email", "test@example.com"])
+            .current_dir(repo_path)
+            .output()
+            .expect("Failed to set git user email");
+
+        let dataset_path = temp_dir.path().join("dataset");
+        std::fs::create_dir(&dataset_path).expect("Failed to create dataset directory");
+
+        let _cwd_guard = CwdGuard::set(&dataset_path);
+        let mut store =
+            GitVersionedKvStore::<32>::init(&dataset_path).expect("Failed to initialize store");
+
+        store
+            .insert(b"key1".to_vec(), b"value1".to_vec())
+            .expect("Failed to insert key1");
+        let good_commit = store.commit("Add key1").expect("Failed to commit");
+        let good_keys = store
+            .get_keys_at_ref(&good_commit.to_hex().to_string())
+            .expect("good commit should be readable");
+        assert_eq!(good_keys.get(&b"key1".to_vec()), Some(&b"value1".to_vec()));
+
+        drop(store);
+
+        let rm_output = std::process::Command::new("git")
+            .args(["rm", "dataset/prolly_hash_mappings"])
+            .current_dir(repo_path)
+            .output()
+            .expect("git rm failed to run");
+        assert!(
+            rm_output.status.success(),
+            "git rm failed: {}",
+            String::from_utf8_lossy(&rm_output.stderr)
+        );
+        let commit_output = std::process::Command::new("git")
+            .args(["commit", "-m", "Remove hash mappings"])
+            .current_dir(repo_path)
+            .output()
+            .expect("git commit failed to run");
+        assert!(
+            commit_output.status.success(),
+            "git commit failed: {}",
+            String::from_utf8_lossy(&commit_output.stderr)
+        );
+
+        let store =
+            GitVersionedKvStore::<32>::open(&dataset_path).expect("corrupt store still opens");
+        let err = store
+            .get_keys_at_ref("HEAD")
+            .expect_err("missing committed mappings must not look like an empty store");
+
+        assert!(
+            err.to_string().contains("prolly_hash_mappings"),
+            "unexpected error: {err}"
+        );
+    }
 }
 
 #[cfg(test)]