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>
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user