vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

hive-bash-mcp: kill is fire-and-forget; retry with force to escalate; must_use (review)

Browse source
This commit is contained in:
damocles 2026-06-19 08:20:22 +02:00 • committed by mara
commit 3d15fcb409
2 changed files with 36 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

20
hive-bash-mcp/src/bin/mcp.rs
View file

@ -221,10 +221,11 @@ struct BashKillArgs {
/// Task ID (from `run`) to kill. /// Task ID (from `run`) to kill.
id: String, id: String,
/// `false` (default): SIGINT to the task's process group — graceful, the /// `false` (default): SIGINT to the task's process group — graceful, the
/// process can clean up — escalating to SIGKILL after a grace period if it /// process can clean up. `true`: SIGKILL. Either way the whole process
/// doesn't exit. `true`: SIGKILL immediately. Either way the whole process
/// group is signalled, so children of the shell (e.g. a `cargo`/`nix` /// group is signalled, so children of the shell (e.g. a `cargo`/`nix`
/// invocation) are stopped too, not just the shell itself. /// invocation) are stopped too, not just the shell. Fire-and-forget: the
/// call sends the signal and returns without waiting. If a SIGINT'd task
/// doesn't exit, call `kill` again with `force: true` to send SIGKILL.
#[serde(default)] #[serde(default)]
force: bool, force: bool,
} }
@ -290,12 +291,13 @@ impl BashMcp {
#[tool( #[tool(
description = "Kill a background bash task you started (by its ID from `run`). \ 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; \
then escalates to SIGKILL after a grace period if it doesn't exit; `force: true` \ `force: true` sends SIGKILL. Either way the task's whole process group is \
sends SIGKILL immediately. Either way the task's whole process group is signalled, \ signalled, so a runaway child (cargo/nix/etc.) is stopped too, not just the shell. \
so a runaway child (cargo/nix/etc.) is stopped too, not just the shell. A \ Fire-and-forget: sends the signal and returns without waiting. If a SIGINT'd task \
still-pending task is cancelled before it starts. The task ends as `killed` and \ doesn't exit, call kill again with `force: true` to SIGKILL. A still-pending task \
fires the usual completion wake." is cancelled before it starts. The task ends as `killed` and fires the usual \
completion wake."
)] )]
async fn kill(&self, Parameters(args): Parameters<BashKillArgs>) -> String { async fn kill(&self, Parameters(args): Parameters<BashKillArgs>) -> String {
let req = DaemonRequest::BashKill { let req = DaemonRequest::BashKill {

64
hive-bash-mcp/src/runner.rs
View file

@ -40,10 +40,6 @@ pub const MAX_WAIT_SECS: u64 = 30;
/// Poll interval used by the inline-wait loops. /// Poll interval used by the inline-wait loops.
const POLL_MS: u64 = 100; 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); static TASK_SEQ: AtomicU64 = AtomicU64::new(0);
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -300,12 +296,14 @@ pub async fn wait_for_task(id: &str, wait_secs: u64) -> Option<TaskFile> {
/// Kill a running or still-pending task. /// Kill a running or still-pending task.
/// ///
/// Fire-and-forget: this **sends a signal and returns** — it does not wait
/// for the process to actually exit.
///
/// - **Running** (in the registry): signals its process group via the /// - **Running** (in the registry): signals its process group via the
/// `exec_cmd` cancel path. `force` → `SIGKILL`. Otherwise `SIGINT`, then /// `exec_cmd` cancel path — `SIGKILL` if `force`, else `SIGINT`. If the
/// `SIGKILL` if the process hasn't exited within [`KILL_GRACE`] — so a /// process ignores `SIGINT`, the caller re-invokes with `force` to send
/// SIGINT-ignoring process is still stopped rather than lingering. The task /// `SIGKILL` (no daemon-side auto-escalation). The task transitions to
/// transitions to [`TaskStatus::Killed`] once the process exits and a /// [`TaskStatus::Killed`] once the process exits and a completion wake fires.
/// completion wake fires as usual.
/// - **Pending** (queued, not yet started): marked `Killed` directly so the /// - **Pending** (queued, not yet started): marked `Killed` directly so the
/// runner loop never starts it. No process exists yet, so `force` is moot. /// runner loop never starts it. No process exists yet, so `force` is moot.
/// - **Terminal or unknown id**: no-op. /// - **Terminal or unknown id**: no-op.
@ -318,6 +316,7 @@ pub async fn wait_for_task(id: &str, wait_secs: u64) -> Option<TaskFile> {
/// Panics if the running-task registry mutex is poisoned (a prior holder /// Panics if the running-task registry mutex is poisoned (a prior holder
/// panicked) — unrecoverable, consistent with the rest of the daemon's /// panicked) — unrecoverable, consistent with the rest of the daemon's
/// `Mutex` usage. /// `Mutex` usage.
#[must_use]
pub fn kill_task(id: &str, force: bool) -> (bool, bool) { pub fn kill_task(id: &str, force: bool) -> (bool, bool) {
// Running task: signal via the registry. // Running task: signal via the registry.
{ {
@ -594,37 +593,24 @@ async fn exec_cmd(
let wait = child.wait(); let wait = child.wait();
tokio::pin!(wait); tokio::pin!(wait);
tokio::pin!(timeout_fut); tokio::pin!(timeout_fut);
tokio::select! { loop {
res = &mut wait => res, tokio::select! {
() = &mut timeout_fut => { res = &mut wait => break res,
signal_group(pgid, libc::SIGKILL); () = &mut timeout_fut => {
timed_out = true; // Per-task timeout: hard-kill the whole group and reap.
(&mut wait).await
}
() = cancel.notified() => {
if force.load(Ordering::SeqCst) {
// Forced: SIGKILL can't be caught/ignored.
signal_group(pgid, libc::SIGKILL); signal_group(pgid, libc::SIGKILL);
killed = Some(true); timed_out = true;
(&mut wait).await break (&mut wait).await;
} else { }
// Graceful: SIGINT first, then escalate to SIGKILL if the () = cancel.notified() => {
// process hasn't exited within the grace window — so a // A kill was requested: send the signal and keep waiting.
// SIGINT-ignoring process is still stopped rather than // `kill` is fire-and-forget — we do NOT block-for-exit or
// leaving the task hung "running" (silent no-op). The // auto-escalate. If the process ignores SIGINT the caller
// reported signal reflects whichever actually ended it. // re-invokes kill with `force`, which re-fires this arm
signal_group(pgid, libc::SIGINT); // with SIGKILL. `killed` tracks the last signal sent.
match tokio::time::timeout(KILL_GRACE, &mut wait).await { let forced = force.load(Ordering::SeqCst);
Ok(res) => { signal_group(pgid, if forced { libc::SIGKILL } else { libc::SIGINT });
killed = Some(false); killed = Some(forced);
res
}
Err(_elapsed) => {
signal_group(pgid, libc::SIGKILL);
killed = Some(true);
(&mut wait).await
}
}
} }
} }
} }