From ae84174e3a29f5e730d1f78ac37200cfcd39e5dc Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 13 Sep 2026 15:23:25 +0200 Subject: [PATCH] subagent daemon: explain the status when returning it, not up front MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `status` tool description enumerated all five states it can report, so every caller paid for four answers it didn't get and read the explanation in the wrong place. The description now states only what the tool is for and that it costs nothing to call; each answer it returns carries its own meaning and the caller's next move instead — running, starting and idle were terse, the not-found error terser still, and they had been leaning on the enumeration to be legible. The killed answer and the end-of-turn todo are unchanged. `continue`'s description keeps its killed-resume sentence: that describes what the tool does, not a state it might hand back. Refs #4326 --- hive-subagent-mcp/src/mcp.rs | 8 ++--- hive-subagent-mcp/src/session.rs | 62 +++++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/hive-subagent-mcp/src/mcp.rs b/hive-subagent-mcp/src/mcp.rs index 0d59958b..f147c2c4 100644 --- a/hive-subagent-mcp/src/mcp.rs +++ b/hive-subagent-mcp/src/mcp.rs @@ -164,12 +164,8 @@ impl SubagentMcp { #[tool( description = "Report whether a subagent is currently running — a zero-cost check that \ - never launches a process, unlike `continue`. Distinguishes running, starting (a \ - `start`/`continue` is in flight but not yet a confirmed spawn — this is normally \ - over in well under a second), killed (its last turn died on a signal — the kernel's \ - OOM killer, a stopped unit, an `interrupt` — so its work was cut off mid-turn rather \ - than finished), idle (a session exists, its last turn finished, nothing is in flight \ - — `continue` to give it another turn), and no such session at all." + never launches a process, unlike `continue`. The answer says what state it found \ + and what to do about it." )] fn status(&self, Parameters(args): Parameters) -> String { match session::status(&self.state, &args.name, args.dir.as_deref()) { diff --git a/hive-subagent-mcp/src/session.rs b/hive-subagent-mcp/src/session.rs index bc6bc9bf..8d907fde 100644 --- a/hive-subagent-mcp/src/session.rs +++ b/hive-subagent-mcp/src/session.rs @@ -576,6 +576,12 @@ pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result, @@ -583,10 +589,18 @@ fn describe_status( session_exists: bool, ) -> anyhow::Result { match occupancy { - Some(true) => return Ok(format!("subagent `{name}` is running")), + Some(true) => { + return Ok(format!( + "subagent `{name}` is running — its turn is still in flight, so there's nothing \ + to do but let it work: the daemon pushes a todo when the turn ends, or \ + `interrupt` it if you want it stopped early." + )); + } Some(false) => { return Ok(format!( - "subagent `{name}` is starting — not yet confirmed running" + "subagent `{name}` is starting — a `start`/`continue` has claimed the name but \ + its process isn't confirmed spawned yet, which is normally over in well under a \ + second: check again shortly rather than starting anything else under this name." )); } None => {} @@ -602,10 +616,16 @@ fn describe_status( } if session_exists { Ok(format!( - "subagent `{name}` is idle — its last turn finished; `continue` to give it another" + "subagent `{name}` is idle — its session exists, its last turn ended on its own \ + rather than being cut off, and nothing is in flight: that turn's own todo says how \ + it went, and `continue` gives it another." )) } else { - anyhow::bail!("no subagent named `{name}` exists — `start` creates one") + anyhow::bail!( + "no subagent named `{name}` exists — nothing is running under that name and there's \ + no session on disk to resume, so `start` is what creates one (check the name if you \ + expected something here)." + ) } } @@ -913,6 +933,40 @@ mod tests { ); } + #[test] + fn every_status_answer_names_its_state_and_the_next_move() { + // The tool description no longer lists the states, so each answer + // has to carry its own explanation — checked one state at a time, + // which is all a caller ever gets back. + let running = describe_status("n", Some(true), None, false).expect("running status"); + assert!( + running.contains("running") && running.contains("`interrupt`"), + "a running answer must say the turn is in flight and how to stop it: {running}" + ); + let starting = describe_status("n", Some(false), None, false).expect("starting status"); + assert!( + starting.contains("starting") && starting.contains("check again"), + "a starting answer must say the spawn isn't confirmed yet and to retry: {starting}" + ); + let idle = describe_status("n", None, None, true).expect("idle status"); + assert!( + idle.contains("idle") && idle.contains("`continue`"), + "an idle answer must say the last turn ended on its own and how to give it another: \ + {idle}" + ); + let killed = describe_status("n", None, Some(libc::SIGKILL), true).expect("killed status"); + assert!( + killed.contains("killed") && killed.contains("`continue`"), + "a killed answer must name the kill and say resuming is still possible: {killed}" + ); + let missing = describe_status("n", None, None, false) + .expect_err("nothing tracked and nothing on disk is an error, not a state"); + assert!( + missing.to_string().contains("`start`"), + "the no-such-session answer must point at what creates one: {missing}" + ); + } + #[test] fn a_killed_turn_and_a_clean_one_do_not_land_in_the_same_state() { // The whole path a real turn takes, minus the process: what the