From a2fd26274373d9c5dee4e1ea258ae7ebdfab9961 Mon Sep 17 00:00:00 2001 From: gsc Date: Mon, 26 Oct 2026 15:55:02 -0700 Subject: [PATCH 1/4] fix(exec): propagate command stdout errors without panicking Keep stdout write failures distinct from child stdin errors so callers can handle a closed output pipe quietly. [[tasks/harmony-2793]] --- src/lib.rs | 83 +++++++++++++++++++++++++++++++++------------------- src/tests.rs | 9 ++++-- 2 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9436067..31e7622 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -315,25 +315,44 @@ fn dry_run_line(dir: &Path, command: &str) -> String { format!("Would execute in {dir_display}:\n {command}") } +/// A write to command-output stdout failed. Kept distinct from child stdin errors. +#[derive(Debug)] +pub struct StdoutError(pub io::Error); + +impl std::fmt::Display for StdoutError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Failed to write command output: {}", self.0) + } +} + +impl std::error::Error for StdoutError {} + +macro_rules! command_print { + ($($arg:tt)*) => { write!(io::stdout().lock(), $($arg)*).map_err(StdoutError)? }; +} +macro_rules! command_println { + ($($arg:tt)*) => { writeln!(io::stdout().lock(), $($arg)*).map_err(StdoutError)? }; +} + pub fn execute_command_in_directory( dir: &Path, argv: &[String], config: &LoopConfig, aliases: &HashMap, extra_env: Option<&HashMap>, -) -> CommandResult { +) -> Result { if !dir.exists() { - println!("\nNo directory found for {}", escape_path(dir)); - println!( + command_println!("\nNo directory found for {}", escape_path(dir)); + command_println!( "\x1b[31m\n✗ {}: No directory found. Command: {} (Exit code: {})\x1b[0m", display_name(dir, config.root_dir.as_deref()), display_command(argv), 1 ); - return CommandResult { + return Ok(CommandResult { exit_code: 1, ..failed_result(dir, display_command(argv), String::new()) - }; + }); } let argv = match resolve_aliases(argv, aliases) { @@ -342,31 +361,35 @@ pub fn execute_command_in_directory( if !config.silent { eprintln!("{e:#}"); } - return failed_result(dir, display_command(argv), format!("{e:#}\n")); + return Ok(failed_result( + dir, + display_command(argv), + format!("{e:#}\n"), + )); } }; let command = display_command(&argv); // Dry run mode: print what would be executed without running it if config.dry_run { - println!("{} {}", "[DRY RUN]".cyan(), dry_run_line(dir, &command)); - return CommandResult { + command_println!("{} {}", "[DRY RUN]".cyan(), dry_run_line(dir, &command)); + return Ok(CommandResult { success: true, exit_code: 0, directory: dir.to_path_buf(), command, stdout: String::new(), stderr: String::new(), - }; + }); } if config.verbose { - println!("Executing in directory: {}", escape_path(dir)); + command_println!("Executing in directory: {}", escape_path(dir)); } if !config.silent { - println!(); - io::stdout().flush().ok(); + command_println!(); + io::stdout().flush().map_err(StdoutError)?; } let status = build_command(&argv, dir, extra_env).and_then(|mut cmd| { @@ -396,21 +419,21 @@ pub fn execute_command_in_directory( if !config.silent { let name = result_name(dir, config.root_dir.as_deref()); if success { - println!("\x1b[32m\n✓ {name}\x1b[0m"); + command_println!("\x1b[32m\n✓ {name}\x1b[0m"); } else { - println!("\x1b[31m\n✗ {name}: exited code {exit_code}\x1b[0m"); + command_println!("\x1b[31m\n✗ {name}: exited code {exit_code}\x1b[0m"); } - io::stdout().flush().ok(); + io::stdout().flush().map_err(StdoutError)?; } - CommandResult { + Ok(CommandResult { success, exit_code, directory: dir.to_path_buf(), command, stdout: String::new(), // Sequential mode uses Stdio::inherit(), so no capture stderr: String::new(), - } + }) } /// Capturing version for parallel execution - captures stdout/stderr for display after completion @@ -703,7 +726,7 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re .any(|r| !r.stdout.trim().is_empty() || !r.stderr.trim().is_empty()); if has_any_output { - println!(); + command_println!(); } for result in results.iter() { @@ -713,17 +736,17 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re !result.stdout.trim().is_empty() || !result.stderr.trim().is_empty(); if has_output { if result.success { - println!("{} {}:", "✓".green(), dir_name.green()); + command_println!("{} {}:", "✓".green(), dir_name.green()); } else { - println!("{} {}:", "✗".red(), dir_name.red()); + command_println!("{} {}:", "✗".red(), dir_name.red()); } if !result.stdout.trim().is_empty() { - print!("{}", result.stdout); + command_print!("{}", result.stdout); } if !result.stderr.trim().is_empty() { - print!("{}", result.stderr); + command_print!("{}", result.stderr); } - println!(); // Blank line after each repo's output + command_println!(); // Blank line after each repo's output } } } @@ -747,7 +770,7 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re config, &aliases, dir_cmd.env.as_ref(), - ) + )? }; results .lock() @@ -788,20 +811,20 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re }, }; - println!("{}", serde_json::to_string_pretty(&output)?); + command_println!("{}", serde_json::to_string_pretty(&output)?); } else if !config.silent { // Text output mode if config.dry_run { - println!( + command_println!( "\n{} Would run {} command(s) across {} directories", "[DRY RUN]".cyan(), total.to_string().yellow(), total.to_string().yellow() ); } else if failed_count == 0 { - println!("{} commands complete", total.to_string().green()); + command_println!("{} commands complete", total.to_string().green()); } else { - println!( + command_println!( "\nSummary: {} {} out of {} commands failed", "✗".red(), failed_count.to_string().red(), @@ -813,7 +836,7 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re Some(root) => result_name(&result.directory, Some(root)), None => escape_path(&result.directory), }; - println!( + command_println!( "\n{} {}: {} (Exit code {}) ", "✗".red(), name, @@ -821,7 +844,7 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re result.exit_code ); } - println!(); + command_println!(); } } diff --git a/src/tests.rs b/src/tests.rs index df4762b..507208b 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -177,12 +177,14 @@ fn test_execute_command_in_directory() { let temp_dir = TempDir::new().unwrap(); let result = - execute_command_in_directory(temp_dir.path(), &sh("echo test"), &config, &aliases, None); + execute_command_in_directory(temp_dir.path(), &sh("echo test"), &config, &aliases, None) + .unwrap(); assert!(result.success); assert_eq!(result.exit_code, 0); let result = - execute_command_in_directory(temp_dir.path(), &fail_cmd(), &config, &aliases, None); + execute_command_in_directory(temp_dir.path(), &fail_cmd(), &config, &aliases, None) + .unwrap(); assert!(!result.success); assert_eq!(result.exit_code, 1); } @@ -477,7 +479,8 @@ fn test_execute_command_in_directory_dry_run() { let temp_dir = TempDir::new().unwrap(); let result = - execute_command_in_directory(temp_dir.path(), &fail_cmd(), &config, &aliases, None); + execute_command_in_directory(temp_dir.path(), &fail_cmd(), &config, &aliases, None) + .unwrap(); assert!(result.success, "dry_run should return success"); assert_eq!(result.exit_code, 0); } From dcff393655908d9034b5b0cabd9b3f0a3f269506 Mon Sep 17 00:00:00 2001 From: gsc Date: Tue, 27 Oct 2026 14:24:17 -0700 Subject: [PATCH 2/4] fix(exec): escape the command in Failed to run [[tasks/harmony-2793]] --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 31e7622..6005321 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -404,7 +404,7 @@ pub fn execute_command_in_directory( Stdio::inherit() }) .status() - .with_context(|| format!("Failed to run {command}")) + .with_context(|| format!("Failed to run {}", escape(&command))) }); let (success, exit_code) = match status { Ok(status) => (status.success(), status.code().unwrap_or(-1)), From 585379a9fbf5044b2fb0b37899ee6a97e3725b61 Mon Sep 17 00:00:00 2001 From: gsc Date: Tue, 27 Oct 2026 15:10:21 -0700 Subject: [PATCH 3/4] fix(exec): escape commands in loop's own lines [[tasks/harmony-2793]] --- src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6005321..50e35ec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -312,7 +312,7 @@ fn dry_run_line(dir: &Path, command: &str) -> String { } else { escape_path(dir) }; - format!("Would execute in {dir_display}:\n {command}") + format!("Would execute in {dir_display}:\n {}", escape(command)) } /// A write to command-output stdout failed. Kept distinct from child stdin errors. @@ -346,7 +346,7 @@ pub fn execute_command_in_directory( command_println!( "\x1b[31m\n✗ {}: No directory found. Command: {} (Exit code: {})\x1b[0m", display_name(dir, config.root_dir.as_deref()), - display_command(argv), + escape(&display_command(argv)), 1 ); return Ok(CommandResult { @@ -514,7 +514,7 @@ pub fn execute_command_in_directory_capturing( Err(e) => failed_result( dir, command.clone(), - format!("Failed to run {command}: {e}\n"), + format!("Failed to run {}: {e}\n", escape(&command)), ), } } @@ -840,7 +840,7 @@ fn execute_commands_internal(config: &LoopConfig, commands: &[DirCommand]) -> Re "\n{} {}: {} (Exit code {}) ", "✗".red(), name, - result.command, + escape(&result.command), result.exit_code ); } From 83e9eb12f5f4d9b951aa2163ca7256ed4af9abf7 Mon Sep 17 00:00:00 2001 From: gsc Date: Tue, 27 Oct 2026 15:29:56 -0700 Subject: [PATCH 4/4] fix(exec): escape malformed alias errors [[tasks/harmony-2793]] --- src/lib.rs | 8 +++++++- src/tests.rs | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 50e35ec..dfd2c9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,7 +263,13 @@ fn resolve_aliases(argv: &[String], aliases: &HashMap) -> Result }; let mut words = shlex::split(alias) .filter(|words| !words.is_empty()) - .with_context(|| format!("Alias '{program}' is not a command: {alias}"))?; + .with_context(|| { + format!( + "Alias '{}' is not a command: {}", + escape(program), + escape(alias) + ) + })?; words.extend_from_slice(args); Ok(words) } diff --git a/src/tests.rs b/src/tests.rs index 507208b..010cc56 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -837,3 +837,17 @@ fn test_display_names_escape_control_characters() { ); assert_eq!(display_name(Path::new("b\u{9b}31m"), None), "b\\u{9b}31m"); } + +#[test] +fn a_malformed_alias_is_named_escaped() { + let aliases = HashMap::from([( + "bad\u{1b}[2J".to_string(), + "'unclosed\u{1b}[2J\nFORGED".to_string(), + )]); + let err = resolve_aliases(&["bad\u{1b}[2J".to_string()], &aliases).unwrap_err(); + let message = format!("{err:#}"); + assert!( + !message.contains('\u{1b}') && !message.contains('\n'), + "{message}" + ); +}