claude runtime: interrupting a session kills every process it started
hive-claude 0.1.3 spawns claude as the leader of its own process group, and its `Cancel::cancel` and idle watchdog signal that group instead of claude's pid alone. Commands claude started (a `nix build`, a test run) used to survive a subagent `interrupt` and keep running. Bump hive-claude to 0.1.3. The subagent `interrupt` call site needs no code change: `Cancel::cancel(force)` kept its signature. The harness's `/cancel` and `/logout` did not go through hive-claude: they ran `kill -INT <pid>` on the claude child they found in /proc. They now SIGINT the group that child leads, through `libc::kill`. A `claude` child that leads no group (the `claude plugin` calls) shares the harness's group and is still signalled alone. Tests: an interrupted subagent's grandchild dies, for both SIGINT and `force`, and fails on hive-claude 0.1.2; a harness cancel stops a turn's grandchild, and fails when only the pid is signalled.
This commit is contained in:
parent
63ab0bbd21
commit
028c283ff9
8 changed files with 244 additions and 24 deletions
4
Cargo.lock
generated
4
Cargo.lock
generated
|
|
@ -1835,9 +1835,9 @@ dependencies = [
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "hive-claude"
|
name = "hive-claude"
|
||||||
version = "0.1.2"
|
version = "0.1.3"
|
||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "1e43e06fd09a7b6d6cca3e70f06d00fa795ceddfe1555121ca0cf4fc578eb9c3"
|
checksum = "8c87ac627823f7bfc38478d7494e614e3f680dda6007dbcd306af51d3eed3bb3"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"libc",
|
"libc",
|
||||||
"serde",
|
"serde",
|
||||||
|
|
|
||||||
|
|
@ -97,7 +97,7 @@ hive-jobq-metrics = { path = "hive-jobq-metrics" }
|
||||||
hive-jobq-wire = { path = "hive-jobq-wire" }
|
hive-jobq-wire = { path = "hive-jobq-wire" }
|
||||||
hive-log = { path = "hive-log" }
|
hive-log = { path = "hive-log" }
|
||||||
hive-core-agent-sock = { path = "hive-core-agent-sock" }
|
hive-core-agent-sock = { path = "hive-core-agent-sock" }
|
||||||
hive-claude = "0.1.2"
|
hive-claude = "0.1.3"
|
||||||
hive-host-sock = { path = "hive-host-sock" }
|
hive-host-sock = { path = "hive-host-sock" }
|
||||||
hive-priv-sock = { path = "hive-priv-sock" }
|
hive-priv-sock = { path = "hive-priv-sock" }
|
||||||
hive-runtime = { path = "hive-runtime" }
|
hive-runtime = { path = "hive-runtime" }
|
||||||
|
|
|
||||||
|
|
@ -166,6 +166,11 @@ from how the child exited. `continue` clears it and hands the session a
|
||||||
fresh turn allowance, which is what makes an interrupt a pause you can
|
fresh turn allowance, which is what makes an interrupt a pause you can
|
||||||
undo rather than a session you have to abandon.
|
undo rather than a session you have to abandon.
|
||||||
|
|
||||||
|
On `claude`, the signal goes to claude's whole process group, so the
|
||||||
|
commands the subagent started (a `nix build`, a test run) get it too. The
|
||||||
|
signal doesn't reach a command that moved itself out of the group
|
||||||
|
(`setsid`).
|
||||||
|
|
||||||
When the session knows where its report goes — `start`'s `report_file`,
|
When the session knows where its report goes — `start`'s `report_file`,
|
||||||
or the path the subagent names when it signals — the daemon appends the
|
or the path the subagent names when it signals — the daemon appends the
|
||||||
stop reason to that file as well, so the artifact you were going to read
|
stop reason to that file as well, so the artifact you were going to read
|
||||||
|
|
|
||||||
|
|
@ -227,11 +227,11 @@ Slash commands today:
|
||||||
|
|
||||||
- `/help` — list commands locally.
|
- `/help` — list commands locally.
|
||||||
- `/clear` — wipe the local terminal view (server history kept).
|
- `/clear` — wipe the local terminal view (server history kept).
|
||||||
- `/cancel` — `POST /api/cancel` → host shellouts `pkill -INT
|
- `/cancel` — `POST /api/cancel` → SIGINT to the turn's claude and every
|
||||||
claude`, emits a Note. On an ACP agent it sends the agent
|
command it started (its process group), emits a Note. On an ACP agent
|
||||||
`session/cancel` instead, and kills it if the turn hasn't ended 10s
|
it sends the agent `session/cancel` instead, and kills it if the turn
|
||||||
later. Also surfaces as a `■ cancel turn` button in the state row
|
hasn't ended 10s later. Also surfaces as a `■ cancel turn` button in
|
||||||
while state=thinking.
|
the state row while state=thinking.
|
||||||
- `/compact` — `POST /api/compact` → sets the deferred
|
- `/compact` — `POST /api/compact` → sets the deferred
|
||||||
`Bus::request_compact()` flag and returns immediately. The harness
|
`Bus::request_compact()` flag and returns immediately. The harness
|
||||||
runs the `/compact` at the next turn boundary (end of the in-flight
|
runs the `/compact` at the next turn boundary (end of the in-flight
|
||||||
|
|
@ -272,8 +272,8 @@ shaped).
|
||||||
- `POST /login/{start,code,cancel}` — claude OAuth login flow.
|
- `POST /login/{start,code,cancel}` — claude OAuth login flow.
|
||||||
Start/cancel emit `LiveEvent::StatusChanged` to flip the
|
Start/cancel emit `LiveEvent::StatusChanged` to flip the
|
||||||
badge to/from `needs_login_in_progress`.
|
badge to/from `needs_login_in_progress`.
|
||||||
- `POST /api/cancel` — SIGINT the in-flight claude turn. Emits a
|
- `POST /api/cancel` — SIGINT the in-flight claude turn and the
|
||||||
`LiveEvent::Note`.
|
commands it started. Emits a `LiveEvent::Note`.
|
||||||
- `POST /api/compact` — run `/compact` on the persistent session
|
- `POST /api/compact` — run `/compact` on the persistent session
|
||||||
(same MCP config + system prompt + allowed tools as a normal
|
(same MCP config + system prompt + allowed tools as a normal
|
||||||
turn — only the stdin payload differs). Flips state to
|
turn — only the stdin payload differs). Flips state to
|
||||||
|
|
|
||||||
|
|
@ -69,7 +69,7 @@ pub(super) async fn post_cancel_turn(State(state): State<AppState>) -> Response
|
||||||
.emit(crate::events::LiveEvent::Note { text: note.into() });
|
.emit(crate::events::LiveEvent::Note { text: note.into() });
|
||||||
return (axum::http::StatusCode::OK, "ok").into_response();
|
return (axum::http::StatusCode::OK, "ok").into_response();
|
||||||
}
|
}
|
||||||
let out = super::sigint_claude().await;
|
let out = super::sigint_claude();
|
||||||
let note = match out {
|
let note = match out {
|
||||||
SigintOutcome::Signalled => {
|
SigintOutcome::Signalled => {
|
||||||
// A process actually got signalled — the *next* turn's wake
|
// A process actually got signalled — the *next* turn's wake
|
||||||
|
|
|
||||||
|
|
@ -84,7 +84,7 @@ pub(super) async fn post_login_cancel(State(state): State<AppState>) -> Response
|
||||||
/// preservation invariants.
|
/// preservation invariants.
|
||||||
pub(super) async fn post_logout(State(state): State<AppState>) -> Response {
|
pub(super) async fn post_logout(State(state): State<AppState>) -> Response {
|
||||||
// Step 1: SIGINT claude (best-effort, matches `post_cancel_turn`).
|
// Step 1: SIGINT claude (best-effort, matches `post_cancel_turn`).
|
||||||
let _ = super::sigint_claude().await;
|
let _ = super::sigint_claude();
|
||||||
// Step 2: delete OAuth credential files only — login::clear_session owns
|
// Step 2: delete OAuth credential files only — login::clear_session owns
|
||||||
// the file set and preserves session-history files alongside them.
|
// the file set and preserves session-history files alongside them.
|
||||||
let dir = crate::paths::claude_dir();
|
let dir = crate::paths::claude_dir();
|
||||||
|
|
|
||||||
|
|
@ -340,21 +340,40 @@ fn find_claude_child() -> Option<u32> {
|
||||||
/// (best-effort). See [`find_claude_child`] for how it's identified —
|
/// (best-effort). See [`find_claude_child`] for how it's identified —
|
||||||
/// **not** a name-based `pkill claude`, which would also hit an unrelated
|
/// **not** a name-based `pkill claude`, which would also hit an unrelated
|
||||||
/// `claude` process someone is running interactively in the container.
|
/// `claude` process someone is running interactively in the container.
|
||||||
async fn sigint_claude() -> SigintOutcome {
|
fn sigint_claude() -> SigintOutcome {
|
||||||
let Some(pid) = find_claude_child() else {
|
let Some(pid) = find_claude_child() else {
|
||||||
return SigintOutcome::NoProcess;
|
return SigintOutcome::NoProcess;
|
||||||
};
|
};
|
||||||
match tokio::process::Command::new("kill")
|
sigint_turn(pid)
|
||||||
.args(["-INT", &pid.to_string()])
|
}
|
||||||
.status()
|
|
||||||
.await
|
/// SIGINT `pid`, and its whole process group when it leads one, so the
|
||||||
{
|
/// commands a turn's claude started stop with it. `hive-claude` spawns the
|
||||||
Ok(status) if status.success() => SigintOutcome::Signalled,
|
/// turn's claude as the leader of its own group. A `claude` child that
|
||||||
// Non-zero from `kill` means "no such process" (ESRCH) — it already
|
/// leads none (`plugins.rs`'s `claude plugin` calls) shares the harness's
|
||||||
// exited between the check above and the signal. Same as never
|
/// group, so it is signalled alone.
|
||||||
// having found it.
|
fn sigint_turn(pid: u32) -> SigintOutcome {
|
||||||
Ok(_) => SigintOutcome::NoProcess,
|
// A child is never pid 0 or 1, and `-0` / `-1` would reach the
|
||||||
Err(e) => SigintOutcome::Failed(e),
|
// harness's own group or every process it may signal.
|
||||||
|
let Some(pid) = i32::try_from(pid).ok().filter(|&pid| pid > 1) else {
|
||||||
|
return SigintOutcome::NoProcess;
|
||||||
|
};
|
||||||
|
// SAFETY: `getpgid(2)` only reads; a gone pid returns -1, never `pid`.
|
||||||
|
let target = if unsafe { libc::getpgid(pid) } == pid {
|
||||||
|
-pid
|
||||||
|
} else {
|
||||||
|
pid
|
||||||
|
};
|
||||||
|
// SAFETY: plain `kill(2)` on the child or the group it leads.
|
||||||
|
if unsafe { libc::kill(target, libc::SIGINT) } == 0 {
|
||||||
|
return SigintOutcome::Signalled;
|
||||||
|
}
|
||||||
|
let err = std::io::Error::last_os_error();
|
||||||
|
if err.raw_os_error() == Some(libc::ESRCH) {
|
||||||
|
// It exited between the scan and the signal: same as never found.
|
||||||
|
SigintOutcome::NoProcess
|
||||||
|
} else {
|
||||||
|
SigintOutcome::Failed(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -419,3 +438,105 @@ fn broker_error_response(err: &BrokerError, action: &str) -> Response {
|
||||||
),
|
),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use std::os::unix::process::CommandExt as _;
|
||||||
|
use std::path::Path;
|
||||||
|
use std::process::{Command, Stdio};
|
||||||
|
use std::time::Duration;
|
||||||
|
|
||||||
|
use super::{SigintOutcome, sigint_turn};
|
||||||
|
|
||||||
|
/// Poll `path` until it holds a complete, newline-terminated pid.
|
||||||
|
fn read_pid(path: &Path) -> i32 {
|
||||||
|
for _ in 0..250 {
|
||||||
|
if let Ok(text) = std::fs::read_to_string(path)
|
||||||
|
&& text.ends_with('\n')
|
||||||
|
{
|
||||||
|
return text.trim().parse().expect("pid file holds a pid");
|
||||||
|
}
|
||||||
|
std::thread::sleep(Duration::from_millis(20));
|
||||||
|
}
|
||||||
|
unreachable!("no pid in {} within 5s", path.display());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether `pid` is still running. A zombie counts as gone: it has
|
||||||
|
/// exited, and `kill(pid, 0)` only keeps finding it until it is reaped.
|
||||||
|
fn is_running(pid: i32) -> bool {
|
||||||
|
// SAFETY: signal 0 delivers nothing; it only checks that `pid` exists.
|
||||||
|
if unsafe { libc::kill(pid, 0) } != 0 {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
!std::fs::read_to_string(format!("/proc/{pid}/stat")).is_ok_and(|stat| {
|
||||||
|
stat.rsplit_once(") ")
|
||||||
|
.is_some_and(|(_, rest)| rest.starts_with('Z'))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_cancel_stops_the_commands_a_turn_started() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let pidfile = dir.path().join("grandchild.pid");
|
||||||
|
// A turn's claude as hive-claude spawns it: leading its own group,
|
||||||
|
// with a command of its own running in the foreground (a
|
||||||
|
// non-interactive shell starts `&` jobs with SIGINT ignored). The
|
||||||
|
// trailing `exit` keeps `sh -c` from exec'ing into that command.
|
||||||
|
let mut turn = Command::new("/bin/sh")
|
||||||
|
.args([
|
||||||
|
"-c",
|
||||||
|
r#"sh -c 'echo $$ >"$1"; exec sleep 300' sh "$1"; exit"#,
|
||||||
|
"sh",
|
||||||
|
])
|
||||||
|
.arg(&pidfile)
|
||||||
|
.stdout(Stdio::null())
|
||||||
|
.stderr(Stdio::null())
|
||||||
|
.process_group(0)
|
||||||
|
.spawn()
|
||||||
|
.expect("spawn the stand-in turn");
|
||||||
|
let grandchild = read_pid(&pidfile);
|
||||||
|
assert_ne!(
|
||||||
|
i32::try_from(turn.id()).ok(),
|
||||||
|
Some(grandchild),
|
||||||
|
"the command must be a child of the turn, not the turn itself"
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(matches!(sigint_turn(turn.id()), SigintOutcome::Signalled));
|
||||||
|
|
||||||
|
let mut alive = is_running(grandchild);
|
||||||
|
for _ in 0..250 {
|
||||||
|
if !alive {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
std::thread::sleep(Duration::from_millis(20));
|
||||||
|
alive = is_running(grandchild);
|
||||||
|
}
|
||||||
|
if alive {
|
||||||
|
// SAFETY: `kill(2)` on a pid this test started; keeps a failed
|
||||||
|
// run from leaving a 300s `sleep` behind.
|
||||||
|
unsafe {
|
||||||
|
libc::kill(grandchild, libc::SIGKILL);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let _ = turn.wait();
|
||||||
|
assert!(!alive, "grandchild {grandchild} outlived the cancel");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_claude_sharing_the_harness_group_is_signalled_alone() {
|
||||||
|
// In this test process's own group, as `claude plugin` runs. A group
|
||||||
|
// signal here would SIGINT the test binary itself.
|
||||||
|
let mut child = Command::new("sleep")
|
||||||
|
.arg("300")
|
||||||
|
.spawn()
|
||||||
|
.expect("spawn sleep");
|
||||||
|
|
||||||
|
assert!(matches!(sigint_turn(child.id()), SigintOutcome::Signalled));
|
||||||
|
|
||||||
|
let status = child.wait().expect("reap sleep");
|
||||||
|
assert_eq!(
|
||||||
|
std::os::unix::process::ExitStatusExt::signal(&status),
|
||||||
|
Some(libc::SIGINT)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -3731,6 +3731,100 @@ mod tests {
|
||||||
std::fs::remove_dir_all(&dir).ok();
|
std::fs::remove_dir_all(&dir).ok();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A stand-in for claude with a command of its own still running: an
|
||||||
|
/// inner `sh` writes its pid to `pidfile` and execs `sleep`. It runs in
|
||||||
|
/// the foreground because a non-interactive shell starts `&` jobs with
|
||||||
|
/// SIGINT ignored, and its output goes to /dev/null so it cannot hold the
|
||||||
|
/// driver's pipes open.
|
||||||
|
fn busy_claude(dir: &Path, pidfile: &Path) -> PathBuf {
|
||||||
|
let script = dir.join("busy-claude");
|
||||||
|
write_executable(
|
||||||
|
&script,
|
||||||
|
&format!(
|
||||||
|
"#!/bin/sh\nsh -c 'echo $$ >\"$1\"; exec sleep 300' sh '{}' >/dev/null 2>&1\n",
|
||||||
|
pidfile.display()
|
||||||
|
),
|
||||||
|
);
|
||||||
|
script
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Poll `path` until it holds a complete, newline-terminated pid.
|
||||||
|
async fn read_pid(path: &Path) -> i32 {
|
||||||
|
for _ in 0..250 {
|
||||||
|
if let Ok(text) = std::fs::read_to_string(path)
|
||||||
|
&& text.ends_with('\n')
|
||||||
|
{
|
||||||
|
return text.trim().parse().expect("pid file holds a pid");
|
||||||
|
}
|
||||||
|
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||||
|
}
|
||||||
|
unreachable!("no pid in {} within 5s", path.display());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether `pid` is still running. A zombie counts as gone: it has
|
||||||
|
/// exited, and `kill(pid, 0)` only keeps finding it until it is reaped.
|
||||||
|
fn is_running(pid: i32) -> bool {
|
||||||
|
// SAFETY: signal 0 delivers nothing; it only checks that `pid` exists.
|
||||||
|
if unsafe { libc::kill(pid, 0) } != 0 {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
!std::fs::read_to_string(format!("/proc/{pid}/stat")).is_ok_and(|stat| {
|
||||||
|
stat.rsplit_once(") ")
|
||||||
|
.is_some_and(|(_, rest)| rest.starts_with('Z'))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn an_interrupt_stops_the_commands_the_subagent_started() {
|
||||||
|
for force in [false, true] {
|
||||||
|
let dir = std::env::temp_dir().join(format!(
|
||||||
|
"hive-subagent-group-{}-{:?}-{force}",
|
||||||
|
std::process::id(),
|
||||||
|
std::thread::current().id()
|
||||||
|
));
|
||||||
|
std::fs::create_dir_all(&dir).expect("scratch dir");
|
||||||
|
let pidfile = dir.join("grandchild.pid");
|
||||||
|
let state = Arc::new(mid_run("n", "carry on indefinitely", 5));
|
||||||
|
let config = Config {
|
||||||
|
program: Some(busy_claude(&dir, &pidfile)),
|
||||||
|
..Config::default()
|
||||||
|
};
|
||||||
|
spawn_and_track(
|
||||||
|
&state,
|
||||||
|
"n",
|
||||||
|
&config,
|
||||||
|
&Attach::Create("n".to_owned()),
|
||||||
|
"turn one".to_owned(),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.expect("the fake claude spawns");
|
||||||
|
let grandchild = read_pid(&pidfile).await;
|
||||||
|
|
||||||
|
interrupt(&state, "n", force).expect("a running turn is interruptible");
|
||||||
|
|
||||||
|
let mut alive = is_running(grandchild);
|
||||||
|
for _ in 0..250 {
|
||||||
|
if !alive {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||||
|
alive = is_running(grandchild);
|
||||||
|
}
|
||||||
|
if alive {
|
||||||
|
// SAFETY: `kill(2)` on a pid this test started; keeps a failed
|
||||||
|
// run from leaving a 300s `sleep` behind.
|
||||||
|
unsafe {
|
||||||
|
libc::kill(grandchild, libc::SIGKILL);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
std::fs::remove_dir_all(&dir).ok();
|
||||||
|
assert!(
|
||||||
|
!alive,
|
||||||
|
"grandchild {grandchild} outlived interrupt(force = {force})"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_cancelled_run_stops_with_turns_still_on_its_clock() {
|
fn a_cancelled_run_stops_with_turns_still_on_its_clock() {
|
||||||
// The loop half of the same fact, asserted directly: `Cancelled` is
|
// The loop half of the same fact, asserted directly: `Cancelled` is
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue