tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

feat: return structured {stdout, stderr} from shell tool with output schema (#7604)

Browse Source
This commit is contained in:
jh-block
2026-03-02 21:29:05 +01:00
committed by GitHub
parent 858c1b6919
commit 670293631e
2 changed files with 153 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose/src/agents/platform_extensions/developer/mod.rs
+4 -6
View File
@@ -14,7 +14,7 @@ use rmcp::model::{
};
use schemars::{schema_for, JsonSchema};
use serde_json::Value;
use shell::{ShellParams, ShellTool};
use shell::{ShellOutput, ShellParams, ShellTool};
use std::path::Path;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
@@ -121,9 +121,10 @@ impl DeveloperClient {
}),
Tool::new(
"shell".to_string(),
"Execute a shell command in the user's default shell in the current dir and return both stdout/stderr. The output is limited to up to 2000 lines, and longer outputs will be saved to a temporary file.".to_string(),
"Execute a shell command in the user's default shell in the current dir. Returns an object with stdout and stderr as separate fields. The output of each stream is limited to up to 2000 lines, and longer outputs will be saved to a temporary file.".to_string(),
Self::schema::<ShellParams>(),
)
.with_output_schema::<ShellOutput>()
.annotate(ToolAnnotations {
title: Some("Shell".to_string()),
read_only_hint: Some(false),
@@ -174,10 +175,7 @@ impl McpClientTrait for DeveloperClient {
match name {
"shell" => match Self::parse_args::<ShellParams>(arguments) {
Ok(params) => Ok(self.shell_tool.shell_with_cwd(params, working_dir).await),
Err(error) => Ok(CallToolResult::error(vec![Content::text(format!(
"Error: {error}"
))
.with_priority(0.0)])),
Err(error) => Ok(ShellTool::error_result(&format!("Error: {error}"), None)),
},
"write" => match Self::parse_args::<FileWriteParams>(arguments) {
Ok(params) => Ok(self.edit_tools.file_write_with_cwd(params, working_dir)),
crates/goose/src/agents/platform_extensions/developer/shell.rs
+149 -59
View File
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::path::PathBuf;
use std::process::Stdio;
use std::sync::{Mutex, OnceLock};
@@ -5,7 +6,7 @@ use std::time::Duration;
use rmcp::model::{CallToolResult, Content};
use schemars::JsonSchema;
use serde::Deserialize;
use serde::{Deserialize, Serialize};
use tokio::io::{AsyncBufReadExt, BufReader};
use tokio_stream::{wrappers::SplitStream, StreamExt};
@@ -22,6 +23,19 @@ pub struct ShellParams {
pub timeout_secs: Option<u64>,
}
#[derive(Debug, Serialize, JsonSchema)]
pub struct ShellOutput {
pub stdout: String,
pub stderr: String,
/// Process exit code. 0 indicates success, non-zero indicates failure.
/// Absent if the process was killed (e.g. timeout).
#[serde(skip_serializing_if = "Option::is_none")]
pub exit_code: Option<i32>,
/// True if the command was killed because it exceeded the timeout.
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub timed_out: bool,
}
/// Resolve the user's full PATH by running a login shell.
///
/// When goosed is launched from a desktop app (e.g. Electron), it may inherit
@@ -81,27 +95,47 @@ impl ShellTool {
working_dir: Option<&std::path::Path>,
) -> CallToolResult {
if params.command.trim().is_empty() {
return CallToolResult::error(vec![Content::text(
"Command cannot be empty.".to_string(),
)
.with_priority(0.0)]);
return Self::error_result("Command cannot be empty.", None);
}
let execution = match run_command(&params.command, params.timeout_secs, working_dir).await {
Ok(execution) => execution,
Err(error) => {
return CallToolResult::error(vec![Content::text(error).with_priority(0.0)])
Err(error) => return Self::error_result(&error, None),
};
// Derive stdout, stderr, and interleaved display from the single tagged-line buffer
let (raw_stdout, raw_stderr, interleaved) = split_lines(&execution.lines);
let truncated_stdout = if raw_stdout.is_empty() {
String::new()
} else {
match truncate_output(&raw_stdout, "stdout") {
Ok(t) => t,
Err(error) => return Self::error_result(&error, None),
}
};
let truncated_stderr = if raw_stderr.is_empty() {
String::new()
} else {
match truncate_output(&raw_stderr, "stderr") {
Ok(t) => t,
Err(error) => return Self::error_result(&error, None),
}
};
let mut rendered = match render_output(&execution.output) {
let shell_output = ShellOutput {
stdout: truncated_stdout,
stderr: truncated_stderr,
exit_code: execution.exit_code,
timed_out: execution.timed_out,
};
let structured_content = serde_json::to_value(&shell_output).ok();
let mut rendered = match render_output(&interleaved, "output") {
Ok(rendered) => rendered,
Err(error) => {
return CallToolResult::error(vec![Content::text(error).with_priority(0.0)])
}
Err(error) => return Self::error_result(&error, None),
};
if execution.timed_out {
let is_error = if execution.timed_out {
if let Some(timeout_secs) = params.timeout_secs {
rendered.push_str(&format!(
"\n\nCommand timed out after {} seconds",
@@ -110,18 +144,36 @@ impl ShellTool {
} else {
rendered.push_str("\n\nCommand timed out");
}
return CallToolResult::error(vec![Content::text(rendered).with_priority(0.0)]);
true
} else {
execution.exit_code.unwrap_or(1) != 0
};
if is_error {
if let Some(code) = execution.exit_code.filter(|c| *c != 0) {
rendered.push_str(&format!("\n\nCommand exited with code {code}"));
}
let mut result =
CallToolResult::error(vec![Content::text(rendered).with_priority(0.0)]);
result.structured_content = structured_content;
return result;
}
if execution.exit_code.unwrap_or(1) != 0 {
rendered.push_str(&format!(
"\n\nCommand exited with code {}",
execution.exit_code.unwrap_or(1)
));
return CallToolResult::error(vec![Content::text(rendered).with_priority(0.0)]);
}
let mut result = CallToolResult::success(vec![Content::text(rendered).with_priority(0.0)]);
result.structured_content = structured_content;
result
}
CallToolResult::success(vec![Content::text(rendered).with_priority(0.0)])
pub fn error_result(message: &str, exit_code: Option<i32>) -> CallToolResult {
let shell_output = ShellOutput {
stdout: String::new(),
stderr: message.to_string(),
exit_code,
timed_out: false,
};
let mut result = CallToolResult::error(vec![Content::text(message).with_priority(0.0)]);
result.structured_content = serde_json::to_value(&shell_output).ok();
result
}
}
@@ -132,7 +184,8 @@ impl Default for ShellTool {
}
struct ExecutionOutput {
output: String,
/// Lines in arrival order, tagged by source: (is_stderr, text)
lines: Vec<(bool, String)>,
exit_code: Option<i32>,
timed_out: bool,
}
@@ -160,16 +213,16 @@ async fn run_command(
.spawn()
.map_err(|error| format!("Failed to spawn shell command: {}", error))?;
let stdout = child
let child_stdout = child
.stdout
.take()
.ok_or_else(|| "Failed to capture stdout".to_string())?;
let stderr = child
let child_stderr = child
.stderr
.take()
.ok_or_else(|| "Failed to capture stderr".to_string())?;
let output_task = tokio::spawn(async move { collect_merged_output(stdout, stderr).await });
let output_task = tokio::spawn(collect_tagged_lines(child_stdout, child_stderr));
let mut timed_out = false;
let exit_code = if let Some(timeout_secs) = timeout_secs.filter(|value| *value > 0) {
@@ -192,13 +245,13 @@ async fn run_command(
.code()
};
let output = output_task
let lines = output_task
.await
.map_err(|error| format!("Failed to collect shell output: {}", error))?
.map_err(|error| format!("Failed to collect shell output: {}", error))?;
Ok(ExecutionOutput {
output,
lines,
exit_code,
timed_out,
})
@@ -228,31 +281,58 @@ fn build_shell_command(command_line: &str) -> tokio::process::Command {
command
}
async fn collect_merged_output(
stdout: tokio::process::ChildStdout,
stderr: tokio::process::ChildStderr,
) -> Result<String, std::io::Error> {
let stdout = BufReader::new(stdout);
let stderr = BufReader::new(stderr);
let stdout = SplitStream::new(stdout.split(b'\n')).map(|line| ("stdout", line));
let stderr = SplitStream::new(stderr.split(b'\n')).map(|line| ("stderr", line));
let mut merged = stdout.merge(stderr);
let mut output = String::new();
while let Some((_stream, line)) = merged.next().await {
let mut line = line?;
line.push(b'\n');
output.push_str(&String::from_utf8_lossy(&line));
/// Split tagged lines into (stdout, stderr, interleaved) strings.
fn split_lines(lines: &[(bool, String)]) -> (String, String, String) {
let mut stdout = String::new();
let mut stderr = String::new();
let mut interleaved = String::new();
let mut stdout_started = false;
let mut stderr_started = false;
for (i, (is_stderr, text)) in lines.iter().enumerate() {
if i > 0 {
interleaved.push('\n');
}
interleaved.push_str(text);
let (target, started) = if *is_stderr {
(&mut stderr, &mut stderr_started)
} else {
(&mut stdout, &mut stdout_started)
};
if *started {
target.push('\n');
}
*started = true;
target.push_str(text);
}
Ok(output.trim_end_matches('\n').to_string())
(stdout, stderr, interleaved)
}
fn render_output(full_output: &str) -> Result<String, String> {
/// Collect lines from stdout and stderr in arrival order, tagging each with its source.
/// Returns a vec of (is_stderr, line_text) preserving interleaved ordering.
async fn collect_tagged_lines(
stdout: tokio::process::ChildStdout,
stderr: tokio::process::ChildStderr,
) -> Result<Vec<(bool, String)>, std::io::Error> {
let stdout_lines = SplitStream::new(BufReader::new(stdout).split(b'\n')).map(|l| (false, l));
let stderr_lines = SplitStream::new(BufReader::new(stderr).split(b'\n')).map(|l| (true, l));
let mut merged = stdout_lines.merge(stderr_lines);
let mut lines = Vec::new();
while let Some((is_stderr, line)) = merged.next().await {
let line = line?;
lines.push((is_stderr, String::from_utf8_lossy(&line).into_owned()));
}
Ok(lines)
}
fn render_output(full_output: &str, label: &str) -> Result<String, String> {
if full_output.is_empty() {
return Ok("(no output)".to_string());
}
truncate_output(full_output, label)
}
fn truncate_output(full_output: &str, label: &str) -> Result<String, String> {
let lines: Vec<&str> = full_output.split('\n').collect();
let total_lines = lines.len();
let total_bytes = full_output.len();
@@ -264,7 +344,7 @@ fn render_output(full_output: &str) -> Result<String, String> {
return Ok(full_output.to_string());
}
let output_path = save_full_output(full_output)?;
let output_path = save_full_output(full_output, label)?;
let preview_start = total_lines.saturating_sub(OUTPUT_PREVIEW_LINES);
let preview = lines[preview_start..].join("\n");
@@ -286,10 +366,11 @@ fn render_output(full_output: &str) -> Result<String, String> {
))
}
fn output_buffer_path() -> Result<PathBuf, String> {
static PATH: Mutex<Option<PathBuf>> = Mutex::new(None);
let mut guard = PATH.lock().map_err(|e| format!("Lock poisoned: {e}"))?;
if let Some(path) = guard.as_ref() {
fn output_buffer_path(label: &str) -> Result<PathBuf, String> {
static PATHS: Mutex<Option<HashMap<String, PathBuf>>> = Mutex::new(None);
let mut guard = PATHS.lock().map_err(|e| format!("Lock poisoned: {e}"))?;
let map = guard.get_or_insert_with(HashMap::new);
if let Some(path) = map.get(label) {
return Ok(path.clone());
}
let temp_file =
@@ -297,12 +378,12 @@ fn output_buffer_path() -> Result<PathBuf, String> {
let (_, path) = temp_file
.keep()
.map_err(|e| format!("Failed to persist temp file: {}", e.error))?;
*guard = Some(path.clone());
map.insert(label.to_string(), path.clone());
Ok(path)
}
fn save_full_output(output: &str) -> Result<PathBuf, String> {
let path = output_buffer_path()?;
fn save_full_output(output: &str, label: &str) -> Result<PathBuf, String> {
let path = output_buffer_path(label)?;
std::fs::write(&path, output).map_err(|e| format!("Failed to write output buffer: {e}"))?;
Ok(path)
}
@@ -376,13 +457,13 @@ mod tests {
.collect::<Vec<_>>()
.join("\n");
let rendered = render_output(&input).unwrap();
let rendered = render_output(&input, "test").unwrap();
assert_eq!(rendered, input);
}
#[test]
fn render_output_shows_empty_message() {
let rendered = render_output("").unwrap();
let rendered = render_output("", "test").unwrap();
assert_eq!(rendered, "(no output)");
}
@@ -393,7 +474,7 @@ mod tests {
.collect::<Vec<_>>()
.join("\n");
let rendered = render_output(&input).unwrap();
let rendered = render_output(&input, "test_lines").unwrap();
let (preview, metadata) = rendered.split_once("\n\n[").unwrap();
assert_eq!(preview.lines().count(), OUTPUT_PREVIEW_LINES);
@@ -416,7 +497,7 @@ mod tests {
assert!(input.len() > OUTPUT_LIMIT_BYTES);
assert!(input.lines().count() <= OUTPUT_LIMIT_LINES);
let rendered = render_output(&input).unwrap();
let rendered = render_output(&input, "test_bytes").unwrap();
let (_preview, metadata) = rendered.split_once("\n\n[").unwrap();
assert!(metadata.contains("byte limit"));
@@ -426,9 +507,18 @@ mod tests {
#[test]
fn save_full_output_reuses_same_path() {
let path1 = save_full_output("first").unwrap();
let path2 = save_full_output("second").unwrap();
let path1 = save_full_output("first", "test_reuse").unwrap();
let path2 = save_full_output("second", "test_reuse").unwrap();
assert_eq!(path1, path2);
assert_eq!(std::fs::read_to_string(&path2).unwrap(), "second");
}
#[test]
fn save_full_output_uses_separate_files_per_label() {
let path_a = save_full_output("aaa", "label_a").unwrap();
let path_b = save_full_output("bbb", "label_b").unwrap();
assert_ne!(path_a, path_b);
assert_eq!(std::fs::read_to_string(&path_a).unwrap(), "aaa");
assert_eq!(std::fs::read_to_string(&path_b).unwrap(), "bbb");
}
}