From bd04c6acd7c773058aa213fe7ca554b0d4c25f63 Mon Sep 17 00:00:00 2001
From: dave <futurechimp@users.noreply.github.com>
Date: Mon, 13 Apr 2026 16:17:06 +0000
Subject: [PATCH] fix: capture test output with background pipe draining
 instead of Stdio::inherit

Stdio::inherit sent test output to server stdout, making it invisible
to agents calling run_tests via MCP. Switch back to Stdio::piped with
background drain threads (same pattern as gates.rs) to capture output
without the pipe deadlock that caused the original switch to inherit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 server/src/http/mcp/shell_tools.rs | 56 +++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/server/src/http/mcp/shell_tools.rs b/server/src/http/mcp/shell_tools.rs
index 315a8d4d..99bafb86 100644
--- a/server/src/http/mcp/shell_tools.rs
+++ b/server/src/http/mcp/shell_tools.rs
@@ -399,12 +399,14 @@ pub(super) async fn tool_run_tests(args: &Value, ctx: &AppContext) -> Result<Str
         }
     }
 
-    // Spawn the test process.
-    let child = std::process::Command::new("bash")
+    // Spawn the test process with piped stdout/stderr so we can capture output.
+    // Pipes are drained in background threads to prevent deadlock when the
+    // child fills the 64KB OS pipe buffer.
+    let mut child = std::process::Command::new("bash")
         .arg(&script_path)
         .current_dir(&working_dir)
-        .stdout(std::process::Stdio::inherit())
-        .stderr(std::process::Stdio::inherit())
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
         .spawn()
         .map_err(|e| format!("Failed to spawn test script: {e}"))?;
 
@@ -415,6 +417,22 @@ pub(super) async fn tool_run_tests(args: &Value, ctx: &AppContext) -> Result<Str
         pid
     );
 
+    // Drain stdout/stderr in background threads so pipe buffers never fill.
+    let mut stdout_handle = child.stdout.take().map(|mut r| {
+        std::thread::spawn(move || {
+            let mut s = String::new();
+            std::io::Read::read_to_string(&mut r, &mut s).ok();
+            s
+        })
+    });
+    let mut stderr_handle = child.stderr.take().map(|mut r| {
+        std::thread::spawn(move || {
+            let mut s = String::new();
+            std::io::Read::read_to_string(&mut r, &mut s).ok();
+            s
+        })
+    });
+
     // Store the child so it can be cleaned up if the server restarts.
     {
         let mut jobs = ctx.test_jobs.lock().map_err(|e| e.to_string())?;
@@ -442,16 +460,36 @@ pub(super) async fn tool_run_tests(args: &Value, ctx: &AppContext) -> Result<Str
         if let Some(child) = job.child.as_mut() {
             match child.try_wait() {
                 Ok(Some(status)) => {
-                    // Done — collect results.
-                    let result = collect_child_result(child, status);
+                    // Done — join drain threads and collect output.
+                    jobs.remove(&working_dir);
+                    let stdout = stdout_handle
+                        .take()
+                        .and_then(|h| h.join().ok())
+                        .unwrap_or_default();
+                    let stderr = stderr_handle
+                        .take()
+                        .and_then(|h| h.join().ok())
+                        .unwrap_or_default();
+                    let combined = format!("{stdout}{stderr}");
+                    let (tests_passed, tests_failed) = parse_test_counts(&combined);
+                    let truncated = truncate_output(&combined, MAX_OUTPUT_LINES);
+                    let passed = status.success();
+                    let exit_code = status.code().unwrap_or(-1);
                     crate::slog!(
                         "[run_tests] Test job for {} finished (pid {}, passed={})",
                         working_dir.display(),
                         pid,
-                        result.passed
+                        passed
                     );
-                    jobs.remove(&working_dir);
-                    return format_test_result(&result);
+                    return serde_json::to_string_pretty(&json!({
+                        "passed": passed,
+                        "exit_code": exit_code,
+                        "timed_out": false,
+                        "tests_passed": tests_passed,
+                        "tests_failed": tests_failed,
+                        "output": truncated,
+                    }))
+                    .map_err(|e| format!("Serialization error: {e}"));
                 }
                 Ok(None) => {
                     // Still running — check timeout.