diff --git a/hive-bash-mcp/src/bin/mcp.rs b/hive-bash-mcp/src/bin/mcp.rs index 90c675c6..e3bbc427 100644 --- a/hive-bash-mcp/src/bin/mcp.rs +++ b/hive-bash-mcp/src/bin/mcp.rs @@ -220,10 +220,11 @@ struct BashStatusArgs { struct BashKillArgs { /// Task ID (from `run`) to kill. id: String, - /// `false` (default) sends SIGINT to the task's process group — graceful, - /// the process can clean up. `true` sends SIGKILL — immediate. Either way - /// the whole process group is signalled, so children of the shell (e.g. a - /// `cargo`/`nix` invocation) are stopped too, not just the shell itself. + /// `false` (default): SIGINT to the task's process group — graceful, the + /// process can clean up — escalating to SIGKILL after a grace period if it + /// doesn't exit. `true`: SIGKILL immediately. Either way the whole process + /// group is signalled, so children of the shell (e.g. a `cargo`/`nix` + /// invocation) are stopped too, not just the shell itself. #[serde(default)] force: bool, } @@ -289,12 +290,12 @@ impl BashMcp { #[tool( description = "Kill a background bash task you started (by its ID from `run`). \ - `force: false` (default) sends SIGINT — graceful, lets the process clean up; \ - `force: true` sends SIGKILL — immediate. Either way the task's whole process \ - group is signalled, so a runaway child (cargo/nix/etc.) is stopped too, not just \ - the shell. A still-pending task is cancelled before it starts. The task ends as \ - `killed` and fires the usual completion wake. SIGINT relies on the process \ - honouring it — pass `force` if it won't stop." + `force: false` (default) sends SIGINT — graceful, lets the process clean up — \ + then escalates to SIGKILL after a grace period if it doesn't exit; `force: true` \ + sends SIGKILL immediately. Either way the task's whole process group is signalled, \ + so a runaway child (cargo/nix/etc.) is stopped too, not just the shell. A \ + still-pending task is cancelled before it starts. The task ends as `killed` and \ + fires the usual completion wake." )] async fn kill(&self, Parameters(args): Parameters) -> String { let req = DaemonRequest::BashKill { diff --git a/hive-bash-mcp/src/runner.rs b/hive-bash-mcp/src/runner.rs index 585ead59..c18494dc 100644 --- a/hive-bash-mcp/src/runner.rs +++ b/hive-bash-mcp/src/runner.rs @@ -40,6 +40,10 @@ pub const MAX_WAIT_SECS: u64 = 30; /// Poll interval used by the inline-wait loops. const POLL_MS: u64 = 100; +/// Grace window after a graceful (`SIGINT`) kill before escalating to +/// `SIGKILL` — bounds how long a SIGINT-ignoring process can linger. +const KILL_GRACE: Duration = Duration::from_secs(10); + static TASK_SEQ: AtomicU64 = AtomicU64::new(0); // --------------------------------------------------------------------------- @@ -297,16 +301,23 @@ pub async fn wait_for_task(id: &str, wait_secs: u64) -> Option { /// Kill a running or still-pending task. /// /// - **Running** (in the registry): signals its process group via the -/// `exec_cmd` cancel path — `SIGKILL` if `force`, else `SIGINT`. The task +/// `exec_cmd` cancel path. `force` → `SIGKILL`. Otherwise `SIGINT`, then +/// `SIGKILL` if the process hasn't exited within [`KILL_GRACE`] — so a +/// SIGINT-ignoring process is still stopped rather than lingering. The task /// transitions to [`TaskStatus::Killed`] once the process exits and a -/// completion wake fires as usual. A `SIGINT` relies on the process -/// honouring it; pass `force` for a guaranteed stop. +/// completion wake fires as usual. /// - **Pending** (queued, not yet started): marked `Killed` directly so the /// runner loop never starts it. No process exists yet, so `force` is moot. /// - **Terminal or unknown id**: no-op. /// /// Returns `(killed, was_running)`: `killed` = a kill was issued (signal sent /// or pending task cancelled); `was_running` = the task was actively running. +/// +/// # Panics +/// +/// Panics if the running-task registry mutex is poisoned (a prior holder +/// panicked) — unrecoverable, consistent with the rest of the daemon's +/// `Mutex` usage. pub fn kill_task(id: &str, force: bool) -> (bool, bool) { // Running task: signal via the registry. { @@ -591,10 +602,30 @@ async fn exec_cmd( (&mut wait).await } () = cancel.notified() => { - let forced = force.load(Ordering::SeqCst); - signal_group(pgid, if forced { libc::SIGKILL } else { libc::SIGINT }); - killed = Some(forced); - (&mut wait).await + if force.load(Ordering::SeqCst) { + // Forced: SIGKILL can't be caught/ignored. + signal_group(pgid, libc::SIGKILL); + killed = Some(true); + (&mut wait).await + } else { + // Graceful: SIGINT first, then escalate to SIGKILL if the + // process hasn't exited within the grace window — so a + // SIGINT-ignoring process is still stopped rather than + // leaving the task hung "running" (silent no-op). The + // reported signal reflects whichever actually ended it. + signal_group(pgid, libc::SIGINT); + match tokio::time::timeout(KILL_GRACE, &mut wait).await { + Ok(res) => { + killed = Some(false); + res + } + Err(_elapsed) => { + signal_group(pgid, libc::SIGKILL); + killed = Some(true); + (&mut wait).await + } + } + } } } };