fix(shell): continued kill after target failures

Recorded per-target PID and jobspec errors while continuing through every remaining operand, then returned a non-zero aggregate status.

Added a regression with a stale PID between two live processes.

Fixes #6779
This commit is contained in:
roboomp
2026-07-27 09:32:21 +00:00
parent fb6a62c227
commit 0e0fa77a51
3 changed files with 72 additions and 8 deletions
+48
View File
@@ -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,
&params,
)
.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")]
+23 -7
View File
@@ -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())
}
}
}
+1 -1
View File
@@ -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