Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions src/commands/config/codex.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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!(
Expand Down Expand Up @@ -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"));
Expand Down
17 changes: 17 additions & 0 deletions src/commands/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 6 additions & 28 deletions src/commands/config/plugins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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"));

Expand Down Expand Up @@ -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"));

Expand Down
23 changes: 21 additions & 2 deletions src/commands/step/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,27 @@ fn stage_to_temp_index(repo: &Repository, add_args: &[&str]) -> anyhow::Result<t
.run()
.context("Failed to stage changes into temp index")?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git {} failed: {}", add_args.join(" "), stderr.trim());
return Err(
worktrunk::git::CommandError::from_failed_output("git", add_args, &output).into(),
);
}
Ok(temp)
}

#[cfg(test)]
mod tests {
use worktrunk::git::{CommandError, Repository};
use worktrunk::testing::TestRepo;

/// A failing `git add` into the temp index (here: an invalid pathspec)
/// must surface as a typed `CommandError`.
#[test]
fn stage_to_temp_index_failure_is_command_error() {
let test = TestRepo::with_initial_commit();
let repo = Repository::at(test.root_path()).unwrap();

let err = super::stage_to_temp_index(&repo, &["add", "--", ":(bad"]).unwrap_err();
let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
assert_eq!(cmd_err.command_string(), "git add -- :(bad");
}
}
36 changes: 27 additions & 9 deletions src/commands/step/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,22 +198,22 @@ fn list_ignored_entries(
worktree_path: &Path,
context: &str,
) -> anyhow::Result<Vec<(std::path::PathBuf, bool)>> {
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 /
Expand All @@ -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"));
}
}
60 changes: 37 additions & 23 deletions src/llm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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]
Expand Down
35 changes: 33 additions & 2 deletions src/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -357,14 +357,45 @@ pub(crate) fn generate_summary(
let reset = Reset;
cformat!("{INFO_SYMBOL}{reset} <bold>{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:#}")
}
}

#[cfg(test)]
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();
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/step_copy_ignored.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Loading
Loading