From 3059523172442a051df82efbb795763e8939552f Mon Sep 17 00:00:00 2001
From: damocles <damocles@hyperhive.local>
Date: Fri, 31 Jul 2026 15:48:50 +0200
Subject: [PATCH] hive-bash-mcp: flag bash-task completions with stderr instead
 of exit code

done_summary previously only pointed at .out/.err without any visual
distinction, so a completed task with clean-looking stdout and a
nonzero exit still read as routine bookkeeping in the todo queue.

Rather than gating a flag on the exit code, key it on has_stderr - a
failed command mid-chain (cd bad-path && rm ...) can exit 0 while the
real evidence sits in stderr, so an exit-code trigger would filter out
precisely the cases where nothing looks wrong. .err's presence is
already the scarce, meaningful signal the Read() pointer is built on;
keying the flag on the same condition costs nothing on the common
quiet-success path (no stderr, no pointer, unchanged) and fires on
every case where something was written to stderr, including the ones
the exit code can't be trusted to reveal.

When stderr is present: header reads as a flag instead of neutral
bookkeeping, and the .err pointer is listed before .out so it's not
the last thing skimmed past on a long completion.
---
 hive-bash-mcp/src/runner.rs | 75 +++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 4 deletions(-)

diff --git a/hive-bash-mcp/src/runner.rs b/hive-bash-mcp/src/runner.rs
index 21527da5..0ddb8874 100644
--- a/hive-bash-mcp/src/runner.rs
+++ b/hive-bash-mcp/src/runner.rs
@@ -757,16 +757,29 @@ fn done_summary(id: &str, summary: &str, output: Option<(bool, bool)>) -> String
     if let Some((has_stdout, has_stderr)) = output
         && (has_stdout || has_stderr)
     {
-        body.push_str("\n\noutput captured — read the full text with:");
-        if has_stdout {
-            let _ = write!(body, "\n  Read({})", crate::paths::task_out(id).display());
-        }
+        // Header + pointer order are keyed on `has_stderr`, not on the exit
+        // code: a failed command mid-chain (`cd bad-path && rm ...`) can
+        // exit 0 while the real evidence sits in `.err` — an exit-code
+        // trigger would filter out precisely the cases where nothing looks
+        // wrong. `.err`'s presence is already the scarce, meaningful signal
+        // the pointer is built on; keying the flag on the same condition
+        // costs nothing on the common quiet-success path (no stderr, no
+        // pointer, exactly like before) and fires on every case where
+        // something was written to stderr, including the ones the exit
+        // code can't be trusted to reveal.
         if has_stderr {
+            body.push_str("\n\n⚠️ stderr present — read the full output:");
             let _ = write!(
                 body,
                 "\n  Read({})  # stderr",
                 crate::paths::task_err(id).display()
             );
+            if has_stdout {
+                let _ = write!(body, "\n  Read({})", crate::paths::task_out(id).display());
+            }
+        } else {
+            body.push_str("\n\noutput captured — read the full text with:");
+            let _ = write!(body, "\n  Read({})", crate::paths::task_out(id).display());
         }
     }
     body
@@ -837,4 +850,58 @@ mod tests {
         assert!(take_wake_suppressed(id));
         assert!(!take_wake_suppressed(id));
     }
+
+    // done_summary: the stderr-present branch must trigger on has_stderr
+    // alone, never on the exit code — see the comment on the function. A
+    // completed task with exit=0 and stderr present (a swallowed failure
+    // mid-chain) still needs to read as a flag, not as routine bookkeeping.
+
+    #[test]
+    fn done_summary_no_output_is_bare() {
+        use super::done_summary;
+        assert_eq!(
+            done_summary("t1", "exit=0", None),
+            "bash task `t1` finished: exit=0"
+        );
+        assert_eq!(
+            done_summary("t1", "exit=0", Some((false, false))),
+            "bash task `t1` finished: exit=0",
+            "no captured output on either stream ⇒ no pointer block at all"
+        );
+    }
+
+    #[test]
+    fn done_summary_stdout_only_is_unflagged() {
+        use super::done_summary;
+        let body = done_summary("t1", "exit=0", Some((true, false)));
+        assert!(body.contains("output captured — read the full text with:"));
+        assert!(body.contains("Read("));
+        assert!(body.contains(".out"));
+        assert!(!body.contains(".err"));
+        assert!(!body.contains('\u{26a0}'), "no flag without stderr");
+    }
+
+    #[test]
+    fn done_summary_stderr_present_is_flagged_regardless_of_summary_text() {
+        use super::done_summary;
+        // exit=0 in the summary — a swallowed mid-chain failure that
+        // still exits clean. The flag must fire anyway, keyed on
+        // has_stderr, not on the summary text.
+        let body = done_summary("t1", "exit=0", Some((true, true)));
+        assert!(
+            body.contains("⚠️ stderr present"),
+            "stderr present must flag even on a clean exit code"
+        );
+        let err_pos = body.find(".err").expect("err pointer present");
+        let out_pos = body.find(".out").expect("out pointer present");
+        assert!(err_pos < out_pos, ".err must be listed before .out");
+    }
+
+    #[test]
+    fn done_summary_stderr_only_omits_out_pointer() {
+        use super::done_summary;
+        let body = done_summary("t1", "exit=1", Some((false, true)));
+        assert!(body.contains(".err"));
+        assert!(!body.contains(".out"), "no stdout ⇒ no stdout pointer");
+    }
 }