From a54f711cc5218bf341de7b34be1445658ece2c02 Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 13 Sep 2026 15:47:57 +0200 Subject: [PATCH] hive-bash-mcp: explain the status when returning it, not up front Mirrors #4333's split on the sibling hive-subagent-mcp surface (mara's ruling there, quoted on #4336: "dont explain all the possible states that can be returned in the tool description. instead explain the status when returning it"). status_explanation() carries what each TaskStatus means and what to do about it, appended to every format_task answer. The status tool's description shrinks to what it's for; a caller only ever sees one state at a time and now learns what it means without cross-referencing anything else. --- hive-bash-mcp/src/mcp.rs | 61 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/hive-bash-mcp/src/mcp.rs b/hive-bash-mcp/src/mcp.rs index 5c159409..6e840ae1 100644 --- a/hive-bash-mcp/src/mcp.rs +++ b/hive-bash-mcp/src/mcp.rs @@ -30,9 +30,35 @@ fn status_str(status: &TaskStatus) -> &'static str { } } +/// What a state means and what the caller should do about it — carried on +/// every `format_task` answer instead of the tool description enumerating +/// all six up front, so a caller who only ever sees one state at a time +/// still learns what it means without reading anything else. +fn status_explanation(status: &TaskStatus) -> &'static str { + match status { + TaskStatus::Pending => "queued, not started yet", + TaskStatus::Running => { + "still going — check back later, or pass wait_seconds to block briefly" + } + TaskStatus::Done => "finished normally", + TaskStatus::TimedOut => "exceeded timeout_secs and was killed", + TaskStatus::Interrupted => { + "the daemon restarted mid-task — the process is gone, no result to report" + } + TaskStatus::Killed => { + "killed on request via `kill` — a signal was sent to the whole process group" + } + } +} + /// Format a `TaskFile` as the human-readable status string claude sees. fn format_task(task: &TaskFile) -> String { - let mut out = format!("task `{}`: status={}", task.id, status_str(&task.status)); + let mut out = format!( + "task `{}`: status={} ({})", + task.id, + status_str(&task.status), + status_explanation(&task.status) + ); if let Some(code) = task.exit_code { let _ = write!(out, ", exit={code}"); @@ -189,9 +215,9 @@ impl BashMcp { } #[tool( - description = "Check the status of a background bash task by its ID (from `run`). \ - Returns the current status (pending/running/done/timed_out/interrupted/killed), exit \ - code if finished, and a tail of stdout/stderr. Full output lives in \ + description = "Check the status of a background bash task by its ID (from `run`). The \ + answer names the state it found and what to do about it, plus the exit code if \ + finished and a tail of stdout/stderr. Full output lives in \ `harness/bash-tasks/.out` / `.err`. \ Pass `wait_seconds` (capped at 30) to wait inline for the task to finish: when the \ task finishes within the window the full status is returned immediately. Useful to \ @@ -332,4 +358,31 @@ mod status_hint_tests { assert!(out.contains("status=done")); }); } + + #[test] + fn every_status_answer_names_its_state_and_the_next_move() { + // Pins the split this module's doc describes: the tool description + // says what the tool is for, and each individual answer — not a + // list the caller has to cross-reference — says what state it got + // and what to do about it. One state's explanation text is + // distinct enough from the others that a copy-paste regression + // (e.g. `Killed` picking up `TimedOut`'s wording) would fail this. + with_harness_dir(|| { + for status in [ + TaskStatus::Pending, + TaskStatus::Running, + TaskStatus::Done, + TaskStatus::TimedOut, + TaskStatus::Interrupted, + TaskStatus::Killed, + ] { + let explanation = super::status_explanation(&status); + let out = format_task(&task(status.clone())); + assert!( + out.contains(explanation), + "status={status:?} explanation missing from: {out}" + ); + } + }); + } }