Revert "Internal MCP Crate Cleanup (#4800)" (#4883)

This commit is contained in:
Alex Hancock
2025-09-29 14:21:30 -04:00
committed by GitHub
parent 2cfef016e2
commit b9ba8dca29
78 changed files with 1090 additions and 844 deletions
+12 -18
View File
@@ -285,30 +285,24 @@ impl GooseAcpAgent {
// Extract tool name and parameters from the ToolCall if successful
let (tool_name, locations) = match &tool_request.tool_call {
Ok(tool_call) => {
let name = tool_call.name.clone();
// Extract file locations from certain tools for client tracking
let mut locs = Vec::new();
if tool_call.name == "developer__text_editor" {
if name == "developer__text_editor" {
// Try to extract the path from the arguments
if let Some(path_str) = tool_call
.arguments
.as_ref()
.and_then(|args_map| args_map.get("path"))
.and_then(|p| p.as_str())
{
let path = std::path::PathBuf::from(path_str);
if path.exists() && path.is_file() {
locs.push(acp::ToolCallLocation {
path: path_str.into(),
line: Some(1),
meta: None,
});
}
let args = &tool_call.arguments;
if let Some(path_str) = args.get("path").and_then(|p| p.as_str()) {
locs.push(acp::ToolCallLocation {
path: path_str.into(),
line: Some(1),
meta: None,
});
}
}
(tool_call.name.to_string(), locs)
(name, locs)
}
Err(_) => ("error".to_string(), vec![]),
Err(_) => ("unknown".to_string(), Vec::new()),
};
// Send tool call notification
+4 -12
View File
@@ -17,7 +17,6 @@ use goose::conversation::message::Message as GooseMessage;
use axum::response::Redirect;
use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::{net::SocketAddr, sync::Arc};
use tokio::sync::{Mutex, RwLock};
use tower_http::cors::{Any, CorsLayer};
@@ -460,10 +459,8 @@ async fn process_message_streaming(
serde_json::to_string(
&WebSocketMessage::ToolRequest {
id: req.id.clone(),
tool_name: tool_call.name.to_string(),
arguments: Value::from(
tool_call.arguments.clone(),
),
tool_name: tool_call.name.clone(),
arguments: tool_call.arguments.clone(),
},
)
.unwrap()
@@ -480,13 +477,8 @@ async fn process_message_streaming(
serde_json::to_string(
&WebSocketMessage::ToolConfirmation {
id: confirmation.id.clone(),
tool_name: confirmation
.tool_name
.to_string()
.clone(),
arguments: Value::from(
confirmation.arguments.clone(),
),
tool_name: confirmation.tool_name.clone(),
arguments: confirmation.arguments.clone(),
needs_confirmation: true,
},
)
@@ -1,7 +1,7 @@
//! MockClient is a mock implementation of the McpClientTrait for testing purposes.
//! add a tool you want to have around and then add the client to the extension router
use goose::agents::mcp_client::{Error, McpClientTrait};
use mcp_client::client::{Error, McpClientTrait};
use rmcp::{
model::{
CallToolResult, Content, ErrorData, GetPromptResult, ListPromptsResult,
@@ -91,11 +91,11 @@ impl McpClientTrait for MockClient {
async fn call_tool(
&self,
name: &str,
arguments: Option<serde_json::Map<String, Value>>,
arguments: Value,
_cancel_token: CancellationToken,
) -> Result<CallToolResult, Error> {
if let Some(handler) = self.handlers.get(name) {
match handler(&Value::Object(arguments.unwrap_or_default())) {
match handler(&arguments) {
Ok(content) => Ok(CallToolResult {
content,
is_error: None,
+68 -84
View File
@@ -127,11 +127,9 @@ pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) ->
));
md.push_str("**Arguments:**\n");
match call.name.as_ref() {
match call.name.as_str() {
"developer__shell" => {
if let Some(Value::String(command)) =
call.arguments.as_ref().and_then(|args| args.get("command"))
{
if let Some(Value::String(command)) = call.arguments.get("command") {
md.push_str(&format!(
"* **command**:\n ```sh\n {}\n ```\n",
command.trim()
@@ -139,7 +137,7 @@ pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) ->
}
let other_args: serde_json::Map<String, Value> = call
.arguments
.as_ref()
.as_object()
.map(|obj| {
obj.iter()
.filter(|(k, _)| k.as_str() != "command")
@@ -156,16 +154,10 @@ pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) ->
}
}
"developer__text_editor" => {
if let Some(Value::String(path)) =
call.arguments.as_ref().and_then(|args| args.get("path"))
{
if let Some(Value::String(path)) = call.arguments.get("path") {
md.push_str(&format!("* **path**: `{}`\n", path));
}
if let Some(Value::String(code_edit)) = call
.arguments
.as_ref()
.and_then(|args| args.get("code_edit"))
{
if let Some(Value::String(code_edit)) = call.arguments.get("code_edit") {
md.push_str(&format!(
"* **code_edit**:\n ```\n{}\n ```\n",
code_edit
@@ -174,7 +166,7 @@ pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) ->
let other_args: serde_json::Map<String, Value> = call
.arguments
.as_ref()
.as_object()
.map(|obj| {
obj.iter()
.filter(|(k, _)| k.as_str() != "path" && k.as_str() != "code_edit")
@@ -191,15 +183,7 @@ pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) ->
}
}
_ => {
if let Some(args) = &call.arguments {
md.push_str(&value_to_markdown(
&Value::Object(args.clone()),
0,
export_all_content,
));
} else {
md.push_str("*No arguments*\n");
}
md.push_str(&value_to_markdown(&call.arguments, 0, export_all_content));
}
}
}
@@ -386,8 +370,8 @@ pub fn message_to_markdown(message: &Message, export_all_content: bool) -> Strin
mod tests {
use super::*;
use goose::conversation::message::{Message, ToolRequest, ToolResponse};
use rmcp::model::{CallToolRequestParam, Content, RawTextContent, TextContent};
use rmcp::object;
use mcp_core::tool::ToolCall;
use rmcp::model::{Content, RawTextContent, TextContent};
use serde_json::json;
#[test]
@@ -502,12 +486,12 @@ mod tests {
#[test]
fn test_tool_request_to_markdown_shell() {
let tool_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let tool_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "ls -la",
"working_dir": "/home/user"
})),
}),
};
let tool_request = ToolRequest {
id: "test-id".to_string(),
@@ -525,12 +509,12 @@ mod tests {
#[test]
fn test_tool_request_to_markdown_text_editor() {
let tool_call = CallToolRequestParam {
name: "developer__text_editor".into(),
arguments: Some(object!({
let tool_call = ToolCall {
name: "developer__text_editor".to_string(),
arguments: json!({
"path": "/path/to/file.txt",
"code_edit": "print('Hello World')"
})),
}),
};
let tool_request = ToolRequest {
id: "test-id".to_string(),
@@ -594,9 +578,9 @@ mod tests {
#[test]
fn test_message_to_markdown_with_tool_request() {
let tool_call = CallToolRequestParam {
name: "test_tool".into(),
arguments: Some(object!({"param": "value"})),
let tool_call = ToolCall {
name: "test_tool".to_string(),
arguments: json!({"param": "value"}),
};
let message = Message::assistant().with_tool_request("test-id", Ok(tool_call));
@@ -653,11 +637,11 @@ mod tests {
#[test]
fn test_shell_tool_with_code_output() {
let tool_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let tool_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "cat main.py"
})),
}),
};
let tool_request = ToolRequest {
id: "shell-cat".to_string(),
@@ -699,11 +683,11 @@ if __name__ == "__main__":
#[test]
fn test_shell_tool_with_git_commands() {
let git_status_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let git_status_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "git status --porcelain"
})),
}),
};
let tool_request = ToolRequest {
id: "git-status".to_string(),
@@ -737,11 +721,11 @@ if __name__ == "__main__":
#[test]
fn test_shell_tool_with_build_output() {
let cargo_build_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let cargo_build_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "cargo build"
})),
}),
};
let _tool_request = ToolRequest {
id: "cargo-build".to_string(),
@@ -781,11 +765,11 @@ warning: unused variable `x`
#[test]
fn test_shell_tool_with_json_api_response() {
let curl_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let curl_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "curl -s https://api.github.com/repos/microsoft/vscode/releases/latest"
})),
}),
};
let _tool_request = ToolRequest {
id: "curl-api".to_string(),
@@ -827,13 +811,13 @@ warning: unused variable `x`
#[test]
fn test_text_editor_tool_with_code_creation() {
let editor_call = CallToolRequestParam {
name: "developer__text_editor".into(),
arguments: Some(object!({
let editor_call = ToolCall {
name: "developer__text_editor".to_string(),
arguments: json!({
"command": "write",
"path": "/tmp/fibonacci.js",
"file_text": "function fibonacci(n) {\n if (n <= 1) return n;\n return fibonacci(n - 1) + fibonacci(n - 2);\n}\n\nconsole.log(fibonacci(10));"
})),
}),
};
let tool_request = ToolRequest {
id: "editor-write".to_string(),
@@ -868,12 +852,12 @@ warning: unused variable `x`
#[test]
fn test_text_editor_tool_view_code() {
let editor_call = CallToolRequestParam {
name: "developer__text_editor".into(),
arguments: Some(object!({
let editor_call = ToolCall {
name: "developer__text_editor".to_string(),
arguments: json!({
"command": "view",
"path": "/src/utils.py"
})),
}),
};
let _tool_request = ToolRequest {
id: "editor-view".to_string(),
@@ -918,11 +902,11 @@ def process_data(data: List[Dict]) -> List[Dict]:
#[test]
fn test_shell_tool_with_error_output() {
let error_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let error_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "python nonexistent_script.py"
})),
}),
};
let _tool_request = ToolRequest {
id: "shell-error".to_string(),
@@ -953,11 +937,11 @@ Command failed with exit code 2"#;
#[test]
fn test_shell_tool_complex_script_execution() {
let script_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let script_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "python -c \"import sys; print(f'Python {sys.version}'); [print(f'{i}^2 = {i**2}') for i in range(1, 6)]\""
})),
}),
};
let tool_request = ToolRequest {
id: "script-exec".to_string(),
@@ -999,11 +983,11 @@ Command failed with exit code 2"#;
#[test]
fn test_shell_tool_with_multi_command() {
let multi_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let multi_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "cd /tmp && ls -la | head -5 && pwd"
})),
}),
};
let _tool_request = ToolRequest {
id: "multi-cmd".to_string(),
@@ -1043,11 +1027,11 @@ drwx------ 3 user staff 96 Dec 6 16:20 com.apple.launchd.abc
#[test]
fn test_developer_tool_grep_code_search() {
let grep_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let grep_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "rg 'async fn' --type rust -n"
})),
}),
};
let tool_request = ToolRequest {
id: "grep-search".to_string(),
@@ -1086,11 +1070,11 @@ src/middleware.rs:12:async fn auth_middleware(req: Request, next: Next) -> Resul
#[test]
fn test_shell_tool_json_detection_works() {
// This test shows that JSON detection in tool responses DOES work
let tool_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let tool_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "echo '{\"test\": \"json\"}'"
})),
}),
};
let _tool_request = ToolRequest {
id: "json-test".to_string(),
@@ -1120,11 +1104,11 @@ src/middleware.rs:12:async fn auth_middleware(req: Request, next: Next) -> Resul
#[test]
fn test_shell_tool_with_package_management() {
let npm_call = CallToolRequestParam {
name: "developer__shell".into(),
arguments: Some(object!({
let npm_call = ToolCall {
name: "developer__shell".to_string(),
arguments: json!({
"command": "npm install express typescript @types/node --save-dev"
})),
}),
};
let tool_request = ToolRequest {
id: "npm-install".to_string(),
+3 -7
View File
@@ -1050,10 +1050,11 @@ impl CliSession {
}
})
})
.unwrap_or_else(|| "unknown".to_string().into());
.unwrap_or_else(|| "unknown".to_string());
let success = tool_response.tool_result.is_ok();
let result_status = if success { "success" } else { "error" };
tracing::info!(
counter.goose.tool_completions = 1,
tool_name = %tool_name,
@@ -1327,12 +1328,7 @@ impl CliSession {
let mut response_message = Message::user();
let last_tool_name = tool_requests
.last()
.and_then(|(_, tool_call)| {
tool_call
.as_ref()
.ok()
.map(|tool| tool.name.to_string().clone())
})
.and_then(|(_, tool_call)| tool_call.as_ref().ok().map(|tool| tool.name.clone()))
.unwrap_or_else(|| "tool".to_string());
let notification = if interrupt {
+86 -97
View File
@@ -7,8 +7,9 @@ use goose::providers::pricing::get_model_pricing;
use goose::providers::pricing::parse_model_id;
use goose::utils::safe_truncate;
use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
use mcp_core::tool::ToolCall;
use regex::Regex;
use rmcp::model::{CallToolRequestParam, JsonObject, PromptArgument};
use rmcp::model::PromptArgument;
use serde_json::Value;
use std::cell::RefCell;
use std::collections::HashMap;
@@ -247,7 +248,7 @@ pub fn goose_mode_message(text: &str) {
fn render_tool_request(req: &ToolRequest, theme: Theme, debug: bool) {
match &req.tool_call {
Ok(call) => match call.name.to_string().as_str() {
Ok(call) => match call.name.as_str() {
"developer__text_editor" => render_text_editor_request(call, debug),
"developer__shell" => render_shell_request(call, debug),
"dynamic_task__create_task" => render_dynamic_task_request(call, debug),
@@ -389,55 +390,44 @@ pub fn render_builtin_error(names: &str, error: &str) {
println!();
}
fn render_text_editor_request(call: &CallToolRequestParam, debug: bool) {
fn render_text_editor_request(call: &ToolCall, debug: bool) {
print_tool_header(call);
// Print path first with special formatting
if let Some(args) = &call.arguments {
if let Some(Value::String(path)) = args.get("path") {
println!(
"{}: {}",
style("path").dim(),
style(shorten_path(path, debug)).green()
);
}
if let Some(Value::String(path)) = call.arguments.get("path") {
println!(
"{}: {}",
style("path").dim(),
style(shorten_path(path, debug)).green()
);
}
// Print other arguments normally, excluding path
if let Some(args) = &call.arguments {
let mut other_args = serde_json::Map::new();
for (k, v) in args {
if k != "path" {
other_args.insert(k.clone(), v.clone());
}
}
if !other_args.is_empty() {
print_params(&Some(other_args), 0, debug);
// Print other arguments normally, excluding path
if let Some(args) = call.arguments.as_object() {
let mut other_args = serde_json::Map::new();
for (k, v) in args {
if k != "path" {
other_args.insert(k.clone(), v.clone());
}
}
print_params(&Value::Object(other_args), 0, debug);
}
println!();
}
fn render_shell_request(call: &CallToolRequestParam, debug: bool) {
fn render_shell_request(call: &ToolCall, debug: bool) {
print_tool_header(call);
print_params(&call.arguments, 0, debug);
println!();
}
fn render_dynamic_task_request(call: &CallToolRequestParam, debug: bool) {
fn render_dynamic_task_request(call: &ToolCall, debug: bool) {
print_tool_header(call);
// Print task_parameters array
if let Some(task_parameters) = call
.arguments
.as_ref()
.and_then(|args| args.get("task_parameters"))
.and_then(|v| match v {
Value::Array(arr) => Some(arr),
_ => None,
})
{
if let Some(Value::Array(task_parameters)) = call.arguments.get("task_parameters") {
println!("{}:", style("task_parameters").dim());
for task_param in task_parameters.iter() {
println!(" -");
@@ -457,9 +447,7 @@ fn render_dynamic_task_request(call: &CallToolRequestParam, debug: bool) {
} else if let Value::Object(_) = item {
// For objects in arrays, print them with indentation
print!(" - ");
if let Value::Object(obj) = item {
print_params(&Some(obj.clone()), 3, debug);
}
print_params(item, 3, debug);
} else {
println!(
" - {}",
@@ -471,9 +459,7 @@ fn render_dynamic_task_request(call: &CallToolRequestParam, debug: bool) {
Value::Object(_) => {
// For objects, print them with proper indentation
println!(" {}:", style(key).dim());
if let Value::Object(obj) = value {
print_params(&Some(obj.clone()), 2, debug);
}
print_params(value, 2, debug);
}
_ => {
// For other types (numbers, booleans, null)
@@ -492,22 +478,20 @@ fn render_dynamic_task_request(call: &CallToolRequestParam, debug: bool) {
println!();
}
fn render_todo_request(call: &CallToolRequestParam, _debug: bool) {
fn render_todo_request(call: &ToolCall, _debug: bool) {
print_tool_header(call);
// For todo tools, always show the full content without redaction
if let Some(args) = &call.arguments {
if let Some(Value::String(content)) = args.get("content") {
println!("{}: {}", style("content").dim(), style(content).green());
} else {
// For todo__read, there are no arguments
// Just print an empty line for consistency
}
if let Some(Value::String(content)) = call.arguments.get("content") {
println!("{}: {}", style("content").dim(), style(content).green());
} else {
// For todo__read, there are no arguments
// Just print an empty line for consistency
}
println!();
}
fn render_default_request(call: &CallToolRequestParam, debug: bool) {
fn render_default_request(call: &ToolCall, debug: bool) {
print_tool_header(call);
print_params(&call.arguments, 0, debug);
println!();
@@ -515,7 +499,7 @@ fn render_default_request(call: &CallToolRequestParam, debug: bool) {
// Helper functions
fn print_tool_header(call: &CallToolRequestParam) {
fn print_tool_header(call: &ToolCall) {
let parts: Vec<_> = call.name.rsplit("__").collect();
let tool_header = format!(
"─── {} | {} ──────────────────────────",
@@ -580,65 +564,70 @@ fn print_value(value: &Value, debug: bool, reserve_width: usize) {
println!("{}", formatted);
}
fn print_params(value: &Option<JsonObject>, depth: usize, debug: bool) {
fn print_params(value: &Value, depth: usize, debug: bool) {
let indent = INDENT.repeat(depth);
if let Some(json_object) = value {
for (key, val) in json_object.iter() {
match val {
Value::Object(obj) => {
println!("{}{}:", indent, style(key).dim());
print_params(&Some(obj.clone()), depth + 1, debug);
}
Value::Array(arr) => {
// Check if all items are simple values (not objects or arrays)
let all_simple = arr.iter().all(|item| {
matches!(
item,
Value::String(_) | Value::Number(_) | Value::Bool(_) | Value::Null
)
});
if all_simple {
// Render inline for simple arrays, truncation will be handled by print_value if needed
let values: Vec<String> = arr
.iter()
.map(|item| match item {
Value::String(s) => s.clone(),
Value::Number(n) => n.to_string(),
Value::Bool(b) => b.to_string(),
Value::Null => "null".to_string(),
_ => unreachable!(),
})
.collect();
let joined_values = values.join(", ");
print_value_with_prefix(
&format!("{}{}: ", indent, style(key).dim()),
&Value::String(joined_values),
debug,
);
} else {
// Use the original multi-line format for complex arrays
match value {
Value::Object(map) => {
for (key, val) in map {
match val {
Value::Object(_) => {
println!("{}{}:", indent, style(key).dim());
for item in arr.iter() {
if let Value::Object(obj) = item {
print_params(val, depth + 1, debug);
}
Value::Array(arr) => {
// Check if all items are simple values (not objects or arrays)
let all_simple = arr.iter().all(|item| {
matches!(
item,
Value::String(_) | Value::Number(_) | Value::Bool(_) | Value::Null
)
});
if all_simple {
// Render inline for simple arrays, truncation will be handled by print_value if needed
let values: Vec<String> = arr
.iter()
.map(|item| match item {
Value::String(s) => s.clone(),
Value::Number(n) => n.to_string(),
Value::Bool(b) => b.to_string(),
Value::Null => "null".to_string(),
_ => unreachable!(),
})
.collect();
let joined_values = values.join(", ");
print_value_with_prefix(
&format!("{}{}: ", indent, style(key).dim()),
&Value::String(joined_values),
debug,
);
} else {
// Use the original multi-line format for complex arrays
println!("{}{}:", indent, style(key).dim());
for item in arr.iter() {
println!("{}{}- ", indent, INDENT);
print_params(&Some(obj.clone()), depth + 2, debug);
} else {
println!("{}{}- {}", indent, INDENT, item);
print_params(item, depth + 2, debug);
}
}
}
}
_ => {
print_value_with_prefix(
&format!("{}{}: ", indent, style(key).dim()),
val,
debug,
);
_ => {
print_value_with_prefix(
&format!("{}{}: ", indent, style(key).dim()),
val,
debug,
);
}
}
}
}
Value::Array(arr) => {
for (i, item) in arr.iter().enumerate() {
println!("{}{}.", indent, i + 1);
print_params(item, depth + 1, debug);
}
}
_ => print_value(value, debug, 0),
}
}