diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index 011b95dfd..df419ceef 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -2357,6 +2357,54 @@ mod tests { assert_eq!(plain_status.expect("plain wait").code(), Some(42)); } + /// A failed target makes `kill` return non-zero without preventing later + /// process operands from receiving the selected signal. + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn kill_builtin_continues_after_target_failure() { + let mut first = Command::new("sleep") + .arg("30") + .spawn() + .expect("first sleep"); + let mut second = Command::new("sleep") + .arg("30") + .spawn() + .expect("second sleep"); + let mut stale = Command::new("true").spawn().expect("stale process"); + let first_pid = first.id().expect("first pid"); + let second_pid = second.id().expect("second pid"); + let stale_pid = stale.id().expect("stale pid"); + stale.wait().await.expect("stale wait"); + + let (mut session, params) = kill_test_context().await; + let source_info = SourceInfo::from("pi-natives:test"); + let result = session + .shell + .run_string( + format!("kill -KILL {first_pid} {stale_pid} {second_pid}"), + &source_info, + ¶ms, + ) + .await + .expect("kill command"); + let code = exit_code(&result); + + let statuses = + time::timeout(Duration::from_secs(5), async { tokio::join!(first.wait(), second.wait()) }) + .await; + if statuses.is_err() { + let _ = first.start_kill(); + let _ = second.start_kill(); + let _ = first.wait().await; + let _ = second.wait().await; + panic!("kill must continue signaling after an intermediate target fails"); + } + let (first_status, second_status) = statuses.expect("checked timeout"); + assert_ne!(code, 0, "a failed target should make kill return non-zero"); + assert_eq!(first_status.expect("first wait").signal(), Some(libc::SIGKILL)); + assert_eq!(second_status.expect("second wait").signal(), Some(libc::SIGKILL)); + } + /// `cmp` remains available with no executable search path, proving the shell /// dispatches the in-process builtin rather than a platform binary. #[tokio::test(flavor = "multi_thread")] diff --git a/crates/vendor/brush-builtins/src/kill.rs b/crates/vendor/brush-builtins/src/kill.rs index 24e4f9625..0b03d4c11 100644 --- a/crates/vendor/brush-builtins/src/kill.rs +++ b/crates/vendor/brush-builtins/src/kill.rs @@ -115,11 +115,12 @@ impl builtins::Command for KillCommand { return Ok(ExecutionExitCode::InvalidUsage.into()); } + let mut had_failure = false; for pid_or_job_spec in operands { - if pid_or_job_spec.starts_with('%') { + let signal_result = if pid_or_job_spec.starts_with('%') { // It's a job spec. if let Some(job) = context.shell.jobs_mut().resolve_job_spec(pid_or_job_spec) { - job.kill(trap_signal)?; + job.kill(trap_signal) } else { writeln!( context.stderr(), @@ -127,16 +128,31 @@ impl builtins::Command for KillCommand { context.command_name, pid_or_job_spec )?; - return Ok(ExecutionResult::general_error()); + had_failure = true; + continue; } } else { - let pid = brush_core::int_utils::parse(pid_or_job_spec.as_str(), 10)?; + brush_core::int_utils::parse(pid_or_job_spec.as_str(), 10) + .and_then(|pid| sys::signal::kill_process(pid, trap_signal)) + }; - // It's a pid. - sys::signal::kill_process(pid, trap_signal)?; + if let Err(err) = signal_result { + writeln!( + context.stderr(), + "{}: {}: {}", + context.command_name, + pid_or_job_spec, + err + )?; + had_failure = true; } } - Ok(ExecutionResult::success()) + + if had_failure { + Ok(ExecutionResult::general_error()) + } else { + Ok(ExecutionResult::success()) + } } } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9d3771346..92c7a4124 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed the bash tool's `kill` builtin rejecting numeric signals and multiple process operands, and changed its default signal from `SIGKILL` to the standard `SIGTERM`. Negative PID operands (process groups per `kill(2)`) and the `--` end-of-options marker are now handled instead of being misparsed as signals ([#6779](https://github.com/can1357/oh-my-pi/issues/6779)). +- Fixed the bash tool's `kill` builtin rejecting numeric signals and multiple process operands, stopping after the first failed target, and defaulting to `SIGKILL` instead of the standard `SIGTERM`. Negative PID operands (process groups per `kill(2)`) and the `--` end-of-options marker are now handled instead of being misparsed as signals ([#6779](https://github.com/can1357/oh-my-pi/issues/6779)). ## [17.1.5] - 2026-07-27