Implement robust check for uncommitted changes in submodules

src/git_ops/gix_ops.rs

@@ -589,22 +589,31 @@ impl GitOperations for GixOperations {
             // 2. Check if submodule has uncommitted changes (unless force is true)
             if !force && submodule_path.exists() && submodule_path.join(".git").exists() {
                 if let Ok(submodule_repo) = gix::open(&submodule_path) {
-                    // TODO: properly implement this
-                    // Check for uncommitted changes
-                    // Note: gix status API is complex, for now we'll do a simple check
-                    // by looking at the index vs HEAD
-                    let head = submodule_repo.head_commit().ok();
-                    let index = submodule_repo.index_or_empty().ok();
-
-                    // Simple check: if we can't get head or index, assume there might be changes
-                    if head.is_none() || index.is_none() {
-                        if !force {
+                    // Check for uncommitted changes using gix.
+                    // The is_dirty() method will return true if there are uncommitted changes,
+                    // including untracked files and modifications to tracked files.
+                    match submodule_repo.is_dirty() {
+                        Ok(is_dirty) => {
+                            if is_dirty {
+                                return Err(anyhow::anyhow!(
+                                    "Submodule '{}' has uncommitted changes. Use force=true to override.",
+                                    path
+                                ));
+                            }
+                        }
+                        Err(err) => {
+                            // If we can't determine dirty status reliably, assume it might have changes
                             return Err(anyhow::anyhow!(
-                                "Submodule '{}' might have uncommitted changes. Use force=true to override.",
-                                path
+                                "Submodule '{}' might have uncommitted changes. Use force=true to override.\nError: {}",
+                                path, err
                             ));
                         }
                     }
+                } else {
+                    return Err(anyhow::anyhow!(
+                        "Submodule '{}' might have uncommitted changes. Use force=true to override.",
+                        path
+                    ));
                 }
             }
 

src/git_ops/mod.rs

@@ -262,9 +262,59 @@ impl GitOperations for GitOpsManager {
             |gix| gix.add_submodule(opts),
             |git2| git2.add_submodule(opts),
         )
-        .or_else(|_| {
+        .or_else(|git2_err| {
             let workdir = self.git2_ops.workdir()
                 .ok_or_else(|| anyhow::anyhow!("Repository has no working directory"))?;
+
+            // Clean up potentially partially initialized submodule path before fallback
+            let sub_path = workdir.join(&opts.path);
+            if sub_path.exists() {
+                let _ = std::fs::remove_dir_all(&sub_path);
+            }
+
+            // git2 also adds the submodule to .gitmodules, which will cause CLI to fail
+            // if we don't clean it up.
+            let gitmodules_path = workdir.join(".gitmodules");
+            if gitmodules_path.exists() {
+                // If it fails to read or write we just ignore it as it's a fallback cleanup
+                if let Ok(content) = std::fs::read_to_string(&gitmodules_path) {
+                    let mut new_content = String::new();
+                    let mut in_target_section = false;
+                    for line in content.lines() {
+                        if line.starts_with("[submodule \"") {
+                            in_target_section = line.contains(&format!("\"{}\"", opts.name));
+                        }
+                        if !in_target_section {
+                            new_content.push_str(line);
+                            new_content.push('\n');
+                        }
+                    }
+                    let _ = std::fs::write(&gitmodules_path, new_content);
+                }
+            }
+
+            // Also git2 might have added it to .git/config
+            let gitconfig_path = workdir.join(".git").join("config");
+            if gitconfig_path.exists() {
+                let _ = std::process::Command::new("git")
+                    .args(["config", "--remove-section", &format!("submodule.{}", opts.name)])
+                    .current_dir(workdir)
+                    .output();
+            }
+
+            // Also git2 might have created the internal git directory
+            let internal_git_dir = workdir.join(".git").join("modules").join(&opts.name);
+            if internal_git_dir.exists() {
+                let _ = std::fs::remove_dir_all(&internal_git_dir);
+            }
+
+            // And removed from index
+            let _ = std::process::Command::new("git")
+                .args(["rm", "--cached", "-r", "--ignore-unmatch"])
+                .arg(&opts.path)
+                .current_dir(workdir)
+                .output();
+
             let mut cmd = std::process::Command::new("git");
             cmd.current_dir(workdir)
                 .arg("submodule")
@@ -283,7 +333,8 @@ impl GitOperations for GitOpsManager {
                 Ok(())
             } else {
                 Err(anyhow::anyhow!(
-                    "Failed to add submodule: {}",
+                    "Failed to add submodule (git2 failed with: {}). CLI output: {}",
+                    git2_err,
                     String::from_utf8_lossy(&output.stderr).trim()
                 ))
             }

tests/integration_tests.rs

@@ -760,9 +760,32 @@ active = true
         let remote_repo = harness
             .create_test_remote("shallow_lib")
             .expect("Failed to create remote");
+
+        // We use a file URL since that works locally for Git.
+        // Note: Git locally defaults to turning off full file-based shallow clone protocols,
+        // so we need to enable it for testing.
+        std::process::Command::new("git")
+            .args(["config", "protocol.file.allow", "always"])
+            .current_dir(&harness.work_dir)
+            .output()
+            .expect("Failed to configure git protocol");
+
+        // Also enable `uploadpack.allowFilter` to let git clone shallowly from file URL
+        std::process::Command::new("git")
+            .args(["config", "uploadpack.allowFilter", "true"])
+            .current_dir(&remote_repo)
+            .output()
+            .expect("Failed to configure git uploadpack");
+
+        std::process::Command::new("git")
+            .args(["config", "uploadpack.allowAnySHA1InWant", "true"])
+            .current_dir(&remote_repo)
+            .output()
+            .expect("Failed to configure git uploadpack");
+
         let remote_url = format!("file://{}", remote_repo.display());
 
-        // Add submodule with shallow flag
+        // Add submodule with shallow flag (add branch argument to explicitly point to main)
         let stdout = harness
             .run_submod_success(&[
                 "add",