refactor(errors): route git-plumbing failure bails through CommandError (#3474)

Follow-up to #3459. Eight git-plumbing sites turned a captured non-zero
exit into `bail!("…{stderr}")`, bypassing
`CommandError::from_failed_output` — the one constructor that normalizes
`\r` and gives renderers the typed command/output split. This converts
them (`resolve_git_common_dir`, the bulk `config --list` read, `config
--unset`, `config --get-regexp`, `merge-tree`, `merge-base`, the
untracked-diff numstat, and `llm.rs::run_git_capture`), so
failed-command display routes through one constructor and the `main.rs`
multiline-error tripwire has nothing left to catch on these paths.
`from_failed_output` now takes `&[impl AsRef<str>]` so `Vec<String>` arg
lists work.

One structural fix rode along: `generate_commit_message` now wraps only
the `execute_llm_command` failure in `GitError::LlmCommandFailed`,
mirroring `generate_squash_message` — a git failure while building the
prompt is not an LLM-command failure, and flattening it into
`LlmCommandFailed.error` dropped the typed stderr.

Rendering: context-wrapped failures render byte-identically (the
outside-repo snapshots pass untouched); bare ones gain the standard `git
… failed (exit N)` header + gutter. The only snapshot changes are two
`wt list` task-failure lines where merge-base errors drop the
hand-rolled `git merge-base failed for <sha> <sha>:` prefix and now
match their sibling lines.

Every converted error path has a direct test — five new ones using
empirically-probed failure modes (invalid regexp pattern → exit 6,
invalid key → exit 1, corrupt config → 128 for the lazily-populated bulk
read and merge-tree, unborn HEAD for the numstat), plus the updated
`run_git_capture` unit test; `resolve_git_common_dir` and `merge_base`
were already covered by existing snapshots.

Two same-pattern sites remain in the commands layer (`step/shared.rs`
ls-files, `step/commit.rs` stage-to-temp-index) — deliberately left for
a follow-up along with the non-git CLIs (gh/glab, claude/codex).

> _This was written by Claude Code on behalf of max_

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: max-sixty

src/git/error.rs

@@ -219,14 +219,14 @@ impl CommandError {
     /// Build from the captured `Output` of a non-zero exit.
     pub fn from_failed_output(
         program: impl Into<String>,
-        args: &[&str],
+        args: &[impl AsRef<str>],
         output: &std::process::Output,
     ) -> Self {
         let stderr = String::from_utf8_lossy(&output.stderr).replace('\r', "\n");
         let stdout = String::from_utf8_lossy(&output.stdout).replace('\r', "\n");
         Self {
             program: program.into(),
-            args: args.iter().map(|&s| s.to_string()).collect(),
+            args: args.iter().map(|s| s.as_ref().to_string()).collect(),
             stderr,
             stdout,
             exit_code: output.status.code(),

src/git/repository/config.rs

@@ -7,6 +7,8 @@ use color_print::cformat;
 
 use crate::config::ProjectConfig;
 
+use crate::git::CommandError;
+
 use super::{DefaultBranchName, GitError, Repository};
 
 impl Repository {
@@ -67,15 +69,15 @@ impl Repository {
     /// Removes the canonical form (matching what git emits) to stay in
     /// sync with `set_config_value` and `config_last`.
     pub(super) fn unset_config_value(&self, key: &str) -> anyhow::Result<bool> {
-        let output = self.run_command_output(&["config", "--unset", key])?;
+        let args = ["config", "--unset", key];
+        let output = self.run_command_output(&args)?;
         let existed = if output.status.success() {
             true
         } else if output.status.code() == Some(5) {
             // --unset exit code 5 = key didn't exist
             false
         } else {
-            let stderr = String::from_utf8_lossy(&output.stderr);
-            anyhow::bail!("git config --unset {}: {}", key, stderr.trim());
+            return Err(CommandError::from_failed_output("git", &args, &output).into());
         };
         if let Some(lock) = self.cache.all_config.get() {
             // `shift_remove` preserves remaining order (swap_remove would
@@ -94,15 +96,15 @@ impl Repository {
     /// surfaced as `Err`). Use this instead of `run_command` + `.unwrap_or_default()`,
     /// which conflates the two.
     pub fn get_config_regexp(&self, pattern: &str) -> anyhow::Result<String> {
-        let output = self.run_command_output(&["config", "--get-regexp", pattern])?;
+        let args = ["config", "--get-regexp", pattern];
+        let output = self.run_command_output(&args)?;
         if output.status.success() {
             Ok(String::from_utf8_lossy(&output.stdout).into_owned())
         } else if output.status.code() == Some(1) {
             // Exit 1 = no keys matched the pattern
             Ok(String::new())
         } else {
-            let stderr = String::from_utf8_lossy(&output.stderr);
-            anyhow::bail!("git config --get-regexp {}: {}", pattern, stderr.trim());
+            Err(CommandError::from_failed_output("git", &args, &output).into())
         }
     }
 
@@ -871,6 +873,44 @@ mod tests {
         assert_eq!(output, "");
     }
 
+    #[test]
+    fn test_get_config_regexp_failure_is_command_error() {
+        // A real failure (invalid pattern, exit 6) must surface as a typed
+        // `CommandError` — unlike exit 1, which means "no keys matched".
+        let test = TestRepo::with_initial_commit();
+        let repo = Repository::at(test.root_path()).unwrap();
+
+        let err = repo.get_config_regexp("(").unwrap_err();
+        let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
+        assert_eq!(cmd_err.command_string(), "git config --get-regexp (");
+    }
+
+    #[test]
+    fn test_unset_config_failure_is_command_error() {
+        // A real failure (invalid key, exit 1) must surface as a typed
+        // `CommandError` — unlike exit 5, which means "key didn't exist".
+        let test = TestRepo::with_initial_commit();
+        let repo = Repository::at(test.root_path()).unwrap();
+
+        let err = repo.unset_config("inva lid.key").unwrap_err();
+        let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
+        assert_eq!(cmd_err.command_string(), "git config --unset inva lid.key");
+    }
+
+    #[test]
+    fn test_config_read_failure_is_command_error() {
+        // Corrupting the config after the repository is open (the bulk map
+        // populates lazily) makes `git config --list -z` fail — the failure
+        // must surface as a typed `CommandError`.
+        let test = TestRepo::with_initial_commit();
+        let repo = Repository::at(test.root_path()).unwrap();
+        std::fs::write(test.root_path().join(".git/config"), "[bad\n").unwrap();
+
+        let err = repo.config_value("user.name").unwrap_err();
+        let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
+        assert_eq!(cmd_err.command_string(), "git config --list -z");
+    }
+
     #[test]
     fn test_get_config_regexp_returns_matches() {
         let test = TestRepo::with_initial_commit();

src/git/repository/diff.rs

@@ -270,15 +270,18 @@ impl Repository {
                 }
 
                 // Exit codes: 0 = found, 1 = no common ancestor, 128+ = invalid ref
-                let output = self.run_command_output(&["merge-base", sha1, sha2])?;
+                let args = ["merge-base", sha1, sha2];
+                let output = self.run_command_output(&args)?;
 
                 let result = if output.status.success() {
                     Some(String::from_utf8_lossy(&output.stdout).trim().to_owned())
                 } else if output.status.code() == Some(1) {
                     None
                 } else {
-                    let stderr = String::from_utf8_lossy(&output.stderr);
-                    bail!("git merge-base failed for {sha1} {sha2}: {}", stderr.trim());
+                    return Err(crate::git::CommandError::from_failed_output(
+                        "git", &args, &output,
+                    )
+                    .into());
                 };
 
                 super::sha_cache::put_merge_base(self, sha1, sha2, &result);

src/git/repository/integration.rs

@@ -291,15 +291,17 @@ impl Repository {
     /// than success is an error; exit 0 → [`MergeTreeOutcome::Clean`] with the
     /// resulting tree SHA (first line of stdout).
     fn compute_merge_tree_outcome(&self, a: &str, b: &str) -> anyhow::Result<MergeTreeOutcome> {
-        // Exit codes: 0 = clean merge, 1 = conflicts, 128+ = error (invalid ref, corrupt repo)
-        let output = self.run_command_output(&["merge-tree", "--write-tree", a, b])?;
+        // Exit codes: 0 = clean merge, 1 = conflicts (git also uses 1 for
+        // unresolvable args — callers pass pre-resolved commit SHAs, see
+        // `run_merge_tree`), anything else = error (corrupt repo, bad usage)
+        let args = ["merge-tree", "--write-tree", a, b];
+        let output = self.run_command_output(&args)?;
 
         if output.status.code() == Some(1) {
             return Ok(MergeTreeOutcome::Conflict);
         }
         if !output.status.success() {
-            let stderr = String::from_utf8_lossy(&output.stderr);
-            anyhow::bail!("git merge-tree failed for {a} {b}: {}", stderr.trim());
+            return Err(crate::git::CommandError::from_failed_output("git", &args, &output).into());
         }
 
         // Clean merge — first line of stdout is the resulting tree SHA.
@@ -1129,6 +1131,30 @@ mod patch_id_tests {
     }
 }
 
+#[cfg(test)]
+mod merge_tree_error_tests {
+    use super::*;
+    use crate::testing::TestRepo;
+
+    /// A hard `git merge-tree` failure (here: a fatal config error, exit
+    /// 128) must surface as a typed `CommandError`, distinct from exit 1,
+    /// which means "merged with conflicts".
+    #[test]
+    fn hard_failure_is_command_error() {
+        let test = TestRepo::with_initial_commit();
+        let repo = Repository::at(test.root_path()).unwrap();
+        std::fs::write(test.root_path().join(".git/config"), "[bad\n").unwrap();
+
+        let err = repo.compute_merge_tree_outcome("HEAD", "HEAD").unwrap_err();
+        let cmd_err =
+            crate::git::CommandError::find_in(&err).expect("error should carry a CommandError");
+        assert_eq!(
+            cmd_err.command_string(),
+            "git merge-tree --write-tree HEAD HEAD"
+        );
+    }
+}
+
 #[cfg(test)]
 mod merge_tree_cache_tests {
     use super::*;

src/git/repository/mod.rs

@@ -118,7 +118,7 @@ use color_print::cformat;
 use dashmap::DashMap;
 use once_cell::sync::OnceCell;
 
-use anyhow::{Context, bail};
+use anyhow::Context;
 use dunce::canonicalize;
 
 use crate::config::{LoadError, ProjectConfig, ResolvedConfig, UserConfig};
@@ -891,16 +891,18 @@ impl Repository {
         // captures the surrounding canonicalize + cache-insert work too.
         let _span = crate::trace::Span::new("resolve_git_common_dir");
 
+        let args = ["rev-parse", "--git-common-dir"];
         let output = Cmd::new("git")
-            .args(["rev-parse", "--git-common-dir"])
+            .args(args)
             .current_dir(discovery_path)
             .context(path_to_logging_context(discovery_path))
             .run()
             .context("Failed to execute: git rev-parse --git-common-dir")?;
 
         if !output.status.success() {
-            let stderr = String::from_utf8_lossy(&output.stderr);
-            bail!("{}", stderr.trim());
+            return Err(
+                super::error::CommandError::from_failed_output("git", &args, &output).into(),
+            );
         }
 
         let stdout = String::from_utf8_lossy(&output.stdout);
@@ -1189,15 +1191,17 @@ impl Repository {
         &self,
     ) -> anyhow::Result<&std::sync::RwLock<indexmap::IndexMap<String, Vec<String>>>> {
         self.cache.all_config.get_or_try_init(|| {
+            let args = ["config", "--list", "-z"];
             let output = Cmd::new("git")
-                .args(["config", "--list", "-z"])
+                .args(args)
                 .current_dir(&self.git_common_dir)
                 .context(path_to_logging_context(&self.git_common_dir))
                 .run()
                 .context("failed to read git config")?;
             if !output.status.success() {
-                let stderr = String::from_utf8_lossy(&output.stderr);
-                bail!("git config --list failed: {}", stderr.trim());
+                return Err(
+                    super::error::CommandError::from_failed_output("git", &args, &output).into(),
+                );
             }
             Ok(std::sync::RwLock::new(parse_config_list_z(&output.stdout)))
         })

src/git/repository/working_tree.rs

@@ -518,14 +518,12 @@ impl<'a> WorkingTree<'a> {
             "--".to_string(),
         ];
         args.extend(paths);
-        let command = args.join(" ");
         let output = idx
-            .git(args)
+            .git(&args)
             .run()
             .context("Failed to compute untracked diff stats")?;
         if !output.status.success() {
-            let stderr = String::from_utf8_lossy(&output.stderr);
-            anyhow::bail!("git {} failed: {}", command, stderr.trim());
+            return Err(CommandError::from_failed_output("git", &args, &output).into());
         }
 
         let mut stats = LineDiff::default();
@@ -932,6 +930,25 @@ mod tests {
         );
     }
 
+    #[test]
+    fn untracked_diff_stats_unborn_head_is_command_error() {
+        // With an unborn HEAD the untracked files stage fine into the temp
+        // index, but `git diff --cached --numstat HEAD` cannot resolve HEAD —
+        // the failure must surface as a typed `CommandError`.
+        let test = TestRepo::new();
+        std::fs::write(test.root_path().join("new.txt"), "hello\n").unwrap();
+        let repo = Repository::at(test.root_path()).unwrap();
+
+        let err = repo.current_worktree().untracked_diff_stats().unwrap_err();
+        let cmd_err =
+            crate::git::CommandError::find_in(&err).expect("error should carry a CommandError");
+        assert!(
+            cmd_err
+                .command_string()
+                .starts_with("git diff --cached --numstat HEAD")
+        );
+    }
+
     #[test]
     fn temp_index_tolerates_missing_real_index() {
         // A worktree whose `<gitdir>/index` file is absent must not error