hive-bash-mcp: escalate sigint→sigkill on graceful kill + document panics (review)

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,
+    /// `false` (default): SIGINT to the task's process group — graceful, the
-    /// the process can clean up. `true` sends SIGKILL — immediate. Either way
+    /// process can clean up — escalating to SIGKILL after a grace period if it
-    /// the whole process group is signalled, so children of the shell (e.g. a
+    /// doesn't exit. `true`: SIGKILL immediately. Either way the whole process
-    /// `cargo`/`nix` invocation) are stopped too, not just the shell itself.
+    /// 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: false` (default) sends SIGINT — graceful, lets the process clean up — \
-            `force: true` sends SIGKILL — immediate. Either way the task's whole process \
+            then escalates to SIGKILL after a grace period if it doesn't exit; `force: true` \
-            group is signalled, so a runaway child (cargo/nix/etc.) is stopped too, not just \
+            sends SIGKILL immediately. Either way the task's whole process group is signalled, \
-            the shell. A still-pending task is cancelled before it starts. The task ends as \
+            so a runaway child (cargo/nix/etc.) is stopped too, not just the shell. A \
-            `killed` and fires the usual completion wake. SIGINT relies on the process \
+            still-pending task is cancelled before it starts. The task ends as `killed` and \
-            honouring it — pass `force` if it won't stop."
+            fires the usual completion wake."
     )]
     async fn kill(&self, Parameters(args): Parameters<BashKillArgs>) -> String {
         let req = DaemonRequest::BashKill {

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<TaskFile> {
 /// 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
+///   completion wake fires as usual.
-///   honouring it; pass `force` for a guaranteed stop.
 /// - **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);
+                if force.load(Ordering::SeqCst) {
-                signal_group(pgid, if forced { libc::SIGKILL } else { libc::SIGINT });
+                    // Forced: SIGKILL can't be caught/ignored.
-                killed = Some(forced);
+                    signal_group(pgid, libc::SIGKILL);
-                (&mut wait).await
+                    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
+                        }
+                    }
+                }
             }
         }
     };