diff --git a/src/commands/config/codex.rs b/src/commands/config/codex.rs index 0ce971aa31..6b89156ba0 100644 --- a/src/commands/config/codex.rs +++ b/src/commands/config/codex.rs @@ -1,8 +1,7 @@ //! Codex plugin marketplace management. -use anyhow::{Context, Result, bail}; +use anyhow::{Result, bail}; use color_print::cformat; -use worktrunk::shell_exec::Cmd; use worktrunk::styling::{eprintln, hint_message, progress_message, success_message}; use super::show::is_codex_available; @@ -29,15 +28,10 @@ pub fn handle_codex_install(yes: bool) -> Result<()> { } eprintln!("{}", progress_message("Adding Codex plugin marketplace...")); - let output = Cmd::new("codex") - .args(["plugin", "marketplace", "add", MARKETPLACE_SOURCE]) - .run() - .context("Failed to run codex CLI")?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("codex plugin marketplace add failed: {}", stderr.trim()); - } + super::run_plugin_cli( + "codex", + &["plugin", "marketplace", "add", MARKETPLACE_SOURCE], + )?; eprintln!("{}", success_message("Codex marketplace configured")); eprintln!( @@ -84,15 +78,10 @@ pub fn handle_codex_uninstall(yes: bool) -> Result<()> { "{}", progress_message("Removing Codex plugin marketplace...") ); - let output = Cmd::new("codex") - .args(["plugin", "marketplace", "remove", MARKETPLACE_NAME]) - .run() - .context("Failed to run codex CLI")?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("codex plugin marketplace remove failed: {}", stderr.trim()); - } + super::run_plugin_cli( + "codex", + &["plugin", "marketplace", "remove", MARKETPLACE_NAME], + )?; eprintln!("{}", success_message("Codex marketplace removed")); eprintln!("{}", hint_message("Installed plugins are left unchanged")); diff --git a/src/commands/config/mod.rs b/src/commands/config/mod.rs index 18ad243499..cf24b95ec2 100644 --- a/src/commands/config/mod.rs +++ b/src/commands/config/mod.rs @@ -31,6 +31,23 @@ pub use state::{ }; pub use update::handle_config_update; +/// Run a plugin-CLI command (`claude` / `codex`), surfacing a non-zero exit +/// as a typed [`worktrunk::git::CommandError`]. +fn run_plugin_cli(program: &str, args: &[&str]) -> anyhow::Result<()> { + use anyhow::Context; + + let output = worktrunk::shell_exec::Cmd::new(program) + .args(args.iter().copied()) + .run() + .with_context(|| format!("Failed to run {program} CLI"))?; + if !output.status.success() { + return Err( + worktrunk::git::CommandError::from_failed_output(program, args, &output).into(), + ); + } + Ok(()) +} + #[cfg(test)] mod tests { use insta::assert_snapshot; diff --git a/src/commands/config/plugins.rs b/src/commands/config/plugins.rs index ee5183ccc1..1d4d97e026 100644 --- a/src/commands/config/plugins.rs +++ b/src/commands/config/plugins.rs @@ -4,7 +4,6 @@ use std::path::PathBuf; use anyhow::{Context, bail}; use color_print::cformat; -use worktrunk::shell_exec::Cmd; use worktrunk::styling::{eprintln, info_message, progress_message, success_message}; use super::show::{ @@ -35,26 +34,13 @@ pub fn handle_claude_install(yes: bool) -> anyhow::Result<()> { } eprintln!("{}", progress_message("Adding plugin from marketplace...")); - let output = Cmd::new("claude") - .args(["plugin", "marketplace", "add", "max-sixty/worktrunk"]) - .run() - .context("Failed to run claude CLI")?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("claude plugin marketplace add failed: {}", stderr.trim()); - } + super::run_plugin_cli( + "claude", + &["plugin", "marketplace", "add", "max-sixty/worktrunk"], + )?; eprintln!("{}", progress_message("Installing plugin...")); - let output = Cmd::new("claude") - .args(["plugin", "install", "worktrunk@worktrunk"]) - .run() - .context("Failed to run claude CLI")?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("claude plugin install failed: {}", stderr.trim()); - } + super::run_plugin_cli("claude", &["plugin", "install", "worktrunk@worktrunk"])?; eprintln!("{}", success_message("Plugin installed")); @@ -88,15 +74,7 @@ pub fn handle_claude_uninstall(yes: bool) -> anyhow::Result<()> { } eprintln!("{}", progress_message("Uninstalling plugin...")); - let output = Cmd::new("claude") - .args(["plugin", "uninstall", "worktrunk@worktrunk"]) - .run() - .context("Failed to run claude CLI")?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("claude plugin uninstall failed: {}", stderr.trim()); - } + super::run_plugin_cli("claude", &["plugin", "uninstall", "worktrunk@worktrunk"])?; eprintln!("{}", success_message("Plugin uninstalled")); diff --git a/src/commands/step/commit.rs b/src/commands/step/commit.rs index 04438191ed..0eac03f805 100644 --- a/src/commands/step/commit.rs +++ b/src/commands/step/commit.rs @@ -142,8 +142,27 @@ fn stage_to_temp_index(repo: &Repository, add_args: &[&str]) -> anyhow::Result anyhow::Result> { + let args = [ + "ls-files", + "--ignored", + "--exclude-standard", + "-o", + "--directory", + ]; let output = Cmd::new("git") - .args([ - "ls-files", - "--ignored", - "--exclude-standard", - "-o", - "--directory", - ]) + .args(args) .current_dir(worktree_path) .context(context) .run() .context("Failed to run git ls-files")?; if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - anyhow::bail!("git ls-files failed: {}", stderr.trim()); + return Err(worktrunk::git::CommandError::from_failed_output("git", &args, &output).into()); } // Parse output: directories end with / @@ -228,3 +228,21 @@ fn list_ignored_entries( Ok(entries) } + +#[cfg(test)] +mod tests { + use worktrunk::git::CommandError; + use worktrunk::testing::TestRepo; + + /// A hard `git ls-files` failure (here: a fatal config error) must + /// surface as a typed `CommandError`. + #[test] + fn list_ignored_entries_failure_is_command_error() { + let test = TestRepo::with_initial_commit(); + std::fs::write(test.root_path().join(".git/config"), "[bad\n").unwrap(); + + let err = super::list_ignored_entries(test.root_path(), "test").unwrap_err(); + let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError"); + assert!(cmd_err.command_string().starts_with("git ls-files")); + } +} diff --git a/src/llm.rs b/src/llm.rs index 479ab2db84..935a687ee4 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use worktrunk::config::CommitGenerationConfig; -use worktrunk::git::{CommandError, CommitMessageDetail, Repository}; +use worktrunk::git::{CommandError, CommitMessageDetail, ErrorExt, Repository}; use worktrunk::path::format_path_for_display; use worktrunk::shell_exec::{Cmd, ShellConfig}; use worktrunk::styling::{eprintln, warning_message}; @@ -512,32 +512,29 @@ pub(crate) fn execute_llm_command(command: &str, prompt: &str) -> anyhow::Result // entirely. See conversation around PR #2136 for sketch. let shell = ShellConfig::get()?; + let args: Vec<&str> = shell + .args + .iter() + .map(String::as_str) + .chain([command]) + .collect(); let output = Cmd::new(shell.executable.to_string_lossy()) - .args(&shell.args) - .arg(command) + .args(args.iter().copied()) .external("commit.generation") .stdin_bytes(prompt) .run() .context("Failed to spawn LLM command")?; if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - let stderr = stderr.trim(); - if stderr.is_empty() { - // Fall back to stdout or exit code when stderr is empty - let stdout = String::from_utf8_lossy(&output.stdout); - let stdout = stdout.trim(); - if stdout.is_empty() { - anyhow::bail!( - "LLM command failed with exit code {}", - output.status.code().unwrap_or(-1) - ); - } else { - anyhow::bail!("{}", stdout); - } - } else { - anyhow::bail!("{}", stderr); - } + // Shell basename only — the full install path is noise (and on + // Windows leaks the Git Bash location); same rule as + // `render_llm_invocation`. + let shell_name = shell + .executable + .file_name() + .unwrap_or(shell.executable.as_os_str()) + .to_string_lossy(); + return Err(CommandError::from_failed_output(shell_name, &args, &output).into()); } let message = String::from_utf8_lossy(&output.stdout).trim().to_owned(); @@ -779,7 +776,7 @@ pub(crate) fn generate_commit_message( return execute_llm_command(command, &prompt).map_err(|e| { worktrunk::git::GitError::LlmCommandFailed { command: command.clone(), - error: e.to_string(), + error: e.display_message(), reproduction_command: Some(format_reproduction_command( "wt step commit --show-prompt", command, @@ -928,7 +925,7 @@ pub(crate) fn generate_squash_message( return execute_llm_command(command, &prompt).map_err(|e| { worktrunk::git::GitError::LlmCommandFailed { command: command.clone(), - error: e.to_string(), + error: e.display_message(), reproduction_command: Some(format_reproduction_command( "wt step squash --show-prompt", command, @@ -1046,7 +1043,7 @@ pub(crate) fn test_commit_generation( execute_llm_command(command, &prompt).map_err(|e| { worktrunk::git::GitError::LlmCommandFailed { command: command.clone(), - error: e.to_string(), + error: e.display_message(), reproduction_command: None, // Already a test command } .into() @@ -1091,6 +1088,23 @@ mod tests { ); } + /// A failing LLM command must surface as a typed [`CommandError`] carrying + /// the exit code and captured output — `LlmCommandFailed` and the summary + /// pane read the detail via `display_message`. + #[test] + fn test_execute_llm_command_failure_is_command_error() { + let err = execute_llm_command("printf 'oops' >&2; exit 3", "prompt").unwrap_err(); + let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError"); + assert_eq!(cmd_err.exit_code, Some(3)); + assert_eq!(err.display_message(), "oops"); + // Basename only — no install-path leakage (see render_llm_invocation). + assert!( + !cmd_err.program.contains('/') && !cmd_err.program.contains('\\'), + "shell program has directory components: {}", + cmd_err.program + ); + } + /// Single-quotes in the user's command must be escaped so the displayed string is a /// faithful, copy-pasteable shell invocation. #[test] diff --git a/src/summary.rs b/src/summary.rs index 8700eafb6a..b856171b57 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -35,7 +35,7 @@ use minijinja::Environment; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use worktrunk::cache; -use worktrunk::git::Repository; +use worktrunk::git::{CommandError, ErrorExt, Repository}; use worktrunk::path::sanitize_for_filename; use worktrunk::styling::INFO_SYMBOL; use worktrunk::sync::Semaphore; @@ -357,7 +357,22 @@ pub(crate) fn generate_summary( let reset = Reset; cformat!("{INFO_SYMBOL}{reset} {branch}{reset} has no changes to summarize\n") } - Err(e) => format!("Error: {e:#}"), + Err(e) => format_summary_error(&e), + } +} + +/// Format a summary-generation error for the preview pane. +/// +/// A typed command failure carries the LLM command's captured output — +/// `display_message` surfaces that rather than the single-line chain +/// summary. For anything else (shell spawn failure, template bug) keep +/// the full anyhow chain, which `display_message` would collapse to its +/// outermost line. +fn format_summary_error(e: &anyhow::Error) -> String { + if CommandError::find_in(e).is_some() { + format!("Error: {}", e.display_message()) + } else { + format!("Error: {e:#}") } } @@ -365,6 +380,22 @@ pub(crate) fn generate_summary( mod tests { use super::*; + /// A non-command error (spawn failure, template bug) keeps its full + /// anyhow chain in the preview pane; command failures are covered by + /// `test_generate_summary_llm_error`. + #[test] + fn format_summary_error_keeps_chain_for_non_command_errors() { + use anyhow::Context; + let err = std::io::Error::new(std::io::ErrorKind::NotFound, "No such file or directory"); + let err = anyhow::Result::<()>::Err(err.into()) + .context("Failed to spawn LLM command") + .unwrap_err(); + assert_eq!( + format_summary_error(&err), + "Error: Failed to spawn LLM command: No such file or directory" + ); + } + #[test] fn test_render_prompt_includes_diff_and_stat() { let result = render_prompt("diff content here", "stat content here").unwrap(); diff --git a/tests/integration_tests/step_copy_ignored.rs b/tests/integration_tests/step_copy_ignored.rs index 068cf4dc87..e433125fe5 100644 --- a/tests/integration_tests/step_copy_ignored.rs +++ b/tests/integration_tests/step_copy_ignored.rs @@ -624,7 +624,7 @@ fn test_copy_ignored_no_default_branch_worktree(mut repo: TestRepo) { /// Test copy-ignored in a bare repository setup /// /// This test reproduces GitHub issue #598: `wt step copy-ignored` fails in bare repo -/// with error "git ls-files failed: fatal: this operation must be run in a work tree" +/// with git's "fatal: this operation must be run in a work tree" #[test] fn test_copy_ignored_bare_repo() { use crate::common::{BareRepoTest, TestRepoBase, setup_temp_snapshot_settings, wt_command}; diff --git a/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_command_fails.snap b/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_command_fails.snap index b6a0c0845f..5daa76aa49 100644 --- a/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_command_fails.snap +++ b/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_command_fails.snap @@ -10,6 +10,7 @@ info: - "--yes" env: APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" CLICOLOR_FORCE: "1" COLUMNS: "500" GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" @@ -38,6 +39,7 @@ info: WORKTRUNK_TEST_GEMINI_INSTALLED: "0" WORKTRUNK_TEST_NUSHELL_ENV: "0" WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" @@ -50,4 +52,5 @@ exit_code: 1 ----- stderr ----- ◎ Adding plugin from marketplace... -✗ claude plugin marketplace add failed: error: network timeout +✗ claude plugin marketplace add max-sixty/worktrunk failed (exit 1) +  error: network timeout diff --git a/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_second_step_fails.snap b/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_second_step_fails.snap index 974527ebbe..341cba7fc1 100644 --- a/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_second_step_fails.snap +++ b/tests/snapshots/integration__integration_tests__config_show__plugins_claude_install_second_step_fails.snap @@ -10,6 +10,7 @@ info: - "--yes" env: APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" CLICOLOR_FORCE: "1" COLUMNS: "500" GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" @@ -38,6 +39,7 @@ info: WORKTRUNK_TEST_GEMINI_INSTALLED: "0" WORKTRUNK_TEST_NUSHELL_ENV: "0" WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" @@ -51,4 +53,5 @@ exit_code: 1 ----- stderr ----- ◎ Adding plugin from marketplace... ◎ Installing plugin... -✗ claude plugin install failed: error: install failed +✗ claude plugin install worktrunk@worktrunk failed (exit 1) +  error: install failed diff --git a/tests/snapshots/integration__integration_tests__config_show__plugins_claude_uninstall_command_fails.snap b/tests/snapshots/integration__integration_tests__config_show__plugins_claude_uninstall_command_fails.snap index 422057b81d..9a4519a1d8 100644 --- a/tests/snapshots/integration__integration_tests__config_show__plugins_claude_uninstall_command_fails.snap +++ b/tests/snapshots/integration__integration_tests__config_show__plugins_claude_uninstall_command_fails.snap @@ -10,6 +10,7 @@ info: - "--yes" env: APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" CLICOLOR_FORCE: "1" COLUMNS: "500" GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" @@ -38,6 +39,7 @@ info: WORKTRUNK_TEST_GEMINI_INSTALLED: "0" WORKTRUNK_TEST_NUSHELL_ENV: "0" WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" @@ -50,4 +52,5 @@ exit_code: 1 ----- stderr ----- ◎ Uninstalling plugin... -✗ claude plugin uninstall failed: error: uninstall failed +✗ claude plugin uninstall worktrunk@worktrunk failed (exit 1) +  error: uninstall failed diff --git a/tests/snapshots/integration__integration_tests__config_show__plugins_codex_install_command_fails.snap b/tests/snapshots/integration__integration_tests__config_show__plugins_codex_install_command_fails.snap index 9d73b50a51..2dd80cbe2e 100644 --- a/tests/snapshots/integration__integration_tests__config_show__plugins_codex_install_command_fails.snap +++ b/tests/snapshots/integration__integration_tests__config_show__plugins_codex_install_command_fails.snap @@ -10,6 +10,7 @@ info: - "--yes" env: APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" CLICOLOR_FORCE: "1" COLUMNS: "500" GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" @@ -38,6 +39,7 @@ info: WORKTRUNK_TEST_GEMINI_INSTALLED: "0" WORKTRUNK_TEST_NUSHELL_ENV: "0" WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" @@ -50,4 +52,5 @@ exit_code: 1 ----- stderr ----- ◎ Adding Codex plugin marketplace... -✗ codex plugin marketplace add failed: error: marketplace add failed +✗ codex plugin marketplace add max-sixty/worktrunk failed (exit 1) +  error: marketplace add failed