Previously, the model couldn't see why MCP tool calls failed, many of which were the model using the parameters incorrectly. A common failure is the model stringifying the json for the notion-update-page tool which it then couldn't correct. I want to do some system prompt massaging around this as well. However, it is crucial that the model sees the error so it can fix it. Before: <img width="2984" height="832" alt="CleanShot 2025-10-17 at 13 02 36" src="https://github.com/user-attachments/assets/709a3d27-b71b-4d8d-87b6-9b2d7fe4e6f2" /> After: <img width="2488" height="1550" alt="CleanShot 2025-10-17 at 13 01 18" src="https://github.com/user-attachments/assets/13a0b7dc-fdad-4996-bf2d-0772872c34fc" /> 🎉 <img width="1078" height="568" alt="CleanShot 2025-10-17 at 13 09 30" src="https://github.com/user-attachments/assets/64cde8be-9e6c-4e61-b971-c2ba22504292" /> Fixes #4707
84 lines
2.6 KiB
Rust
84 lines
2.6 KiB
Rust
use std::time::Instant;
|
|
|
|
use tracing::error;
|
|
|
|
use crate::codex::Session;
|
|
use crate::protocol::Event;
|
|
use crate::protocol::EventMsg;
|
|
use crate::protocol::McpInvocation;
|
|
use crate::protocol::McpToolCallBeginEvent;
|
|
use crate::protocol::McpToolCallEndEvent;
|
|
use codex_protocol::models::FunctionCallOutputPayload;
|
|
use codex_protocol::models::ResponseInputItem;
|
|
|
|
/// Handles the specified tool call dispatches the appropriate
|
|
/// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`.
|
|
pub(crate) async fn handle_mcp_tool_call(
|
|
sess: &Session,
|
|
sub_id: &str,
|
|
call_id: String,
|
|
server: String,
|
|
tool_name: String,
|
|
arguments: String,
|
|
) -> ResponseInputItem {
|
|
// Parse the `arguments` as JSON. An empty string is OK, but invalid JSON
|
|
// is not.
|
|
let arguments_value = if arguments.trim().is_empty() {
|
|
None
|
|
} else {
|
|
match serde_json::from_str::<serde_json::Value>(&arguments) {
|
|
Ok(value) => Some(value),
|
|
Err(e) => {
|
|
error!("failed to parse tool call arguments: {e}");
|
|
return ResponseInputItem::FunctionCallOutput {
|
|
call_id: call_id.clone(),
|
|
output: FunctionCallOutputPayload {
|
|
content: format!("err: {e}"),
|
|
success: Some(false),
|
|
},
|
|
};
|
|
}
|
|
}
|
|
};
|
|
|
|
let invocation = McpInvocation {
|
|
server: server.clone(),
|
|
tool: tool_name.clone(),
|
|
arguments: arguments_value.clone(),
|
|
};
|
|
|
|
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
|
|
call_id: call_id.clone(),
|
|
invocation: invocation.clone(),
|
|
});
|
|
notify_mcp_tool_call_event(sess, sub_id, tool_call_begin_event).await;
|
|
|
|
let start = Instant::now();
|
|
// Perform the tool call.
|
|
let result = sess
|
|
.call_tool(&server, &tool_name, arguments_value.clone())
|
|
.await
|
|
.map_err(|e| format!("tool call error: {e:?}"));
|
|
if let Err(e) = &result {
|
|
tracing::warn!("MCP tool call error: {e:?}");
|
|
}
|
|
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
|
|
call_id: call_id.clone(),
|
|
invocation,
|
|
duration: start.elapsed(),
|
|
result: result.clone(),
|
|
});
|
|
|
|
notify_mcp_tool_call_event(sess, sub_id, tool_call_end_event.clone()).await;
|
|
|
|
ResponseInputItem::McpToolCallOutput { call_id, result }
|
|
}
|
|
|
|
async fn notify_mcp_tool_call_event(sess: &Session, sub_id: &str, event: EventMsg) {
|
|
sess.send_event(Event {
|
|
id: sub_id.to_string(),
|
|
msg: event,
|
|
})
|
|
.await;
|
|
}
|