chore: code mode tool reporting #6133
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds tool call tracking and visibility for "code mode" execution. When JavaScript code executes and makes tool calls via execute_code, those underlying tool calls are now tracked and displayed in the UI as individual tool requests/responses. This provides better transparency into what tools are being invoked during batched code execution.
- Introduces data structures (
TrackedToolCall,TrackedToolResult,TrackedContent) to capture tool call information during JS execution - Modifies
handle_execute_codeto collect and return tracked tool calls alongside execution results, attaching them to the response metadata - Adds
expand_nested_tool_callsfunction in the agent to convert tracked calls from metadata into visible message pairs for the UI
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/goose/src/agents/code_execution_extension.rs | Adds tracking structures and modifies tool handling to capture tool calls made during code execution, returning them in metadata |
| crates/goose/src/agents/agent.rs | Implements expansion of nested tool calls from metadata into separate message pairs for UI visibility |
| /// Expands nested tool calls from code_execution into real ToolRequest/ToolResponse pairs. | ||
| /// This makes batched tool calls visible in the UI as individual tool calls when in code mode | ||
| fn expand_nested_tool_calls( | ||
| request_msg: Message, | ||
| response_msg: Message, | ||
| ) -> (Message, Message, Option<(Message, Message)>) { | ||
| // Find tool responses with nested calls in meta | ||
| let nested_calls: Vec<(String, serde_json::Value)> = response_msg | ||
| .content | ||
| .iter() | ||
| .filter_map(|c| { | ||
| if let MessageContent::ToolResponse(ref tr) = c { | ||
| if let Ok(ref result) = tr.tool_result { | ||
| if let Some(ref meta) = result.meta { | ||
| if let Some(nested) = meta.0.get("nestedToolCalls") { | ||
| return Some((tr.id.clone(), nested.clone())); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| }) | ||
| .collect(); | ||
|
|
||
| if nested_calls.is_empty() { | ||
| return (request_msg, response_msg, None); | ||
| } | ||
|
|
||
| let mut nested_request_msg = | ||
| Message::assistant().with_id(format!("msg_{}", Uuid::new_v4())); | ||
| let mut nested_response_msg = Message::user().with_id(format!("msg_{}", Uuid::new_v4())); | ||
|
|
||
| for (_parent_id, nested_json) in nested_calls { | ||
| if let Some(calls) = nested_json.as_array() { | ||
| for call in calls { | ||
| let Some(name) = call.get("name").and_then(|n| n.as_str()) else { | ||
| continue; | ||
| }; | ||
| let arguments = call.get("arguments").and_then(|a| a.as_object()).cloned(); | ||
|
|
||
| // Generate a unique ID for this nested call | ||
| let nested_id = format!("nested_{}", Uuid::new_v4()); | ||
|
|
||
| let tool_call_param = CallToolRequestParam { | ||
| name: name.to_string().into(), | ||
| arguments, | ||
| }; | ||
| nested_request_msg = nested_request_msg | ||
| .with_tool_request(nested_id.clone(), Ok(tool_call_param)); | ||
|
|
||
| let tool_result = if let Some(result_obj) = call.get("result") { | ||
| if result_obj.get("status").and_then(|s| s.as_str()) == Some("success") { | ||
| let content: Vec<Content> = result_obj | ||
| .get("content") | ||
| .and_then(|c| c.as_array()) | ||
| .map(|arr| { | ||
| arr.iter() | ||
| .filter_map(|item| { | ||
| item.get("text") | ||
| .and_then(|t| t.as_str()) | ||
| .map(|text| Content::text(text.to_string())) | ||
| }) | ||
| .collect() | ||
| }) | ||
| .unwrap_or_default(); | ||
| Ok(CallToolResult { | ||
| content, | ||
| is_error: Some(false), | ||
| structured_content: None, | ||
| meta: None, | ||
| }) | ||
| } else { | ||
| let message = result_obj | ||
| .get("message") | ||
| .and_then(|m| m.as_str()) | ||
| .unwrap_or("Unknown error") | ||
| .to_string(); | ||
| Err(ErrorData::new(ErrorCode::INTERNAL_ERROR, message, None)) | ||
| } | ||
| } else { | ||
| Err(ErrorData::new( | ||
| ErrorCode::INTERNAL_ERROR, | ||
| "Missing result".to_string(), | ||
| None, | ||
| )) | ||
| }; | ||
|
|
||
| nested_response_msg = | ||
| nested_response_msg.with_tool_response(nested_id, tool_result); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ( | ||
| request_msg, | ||
| response_msg, | ||
| Some((nested_request_msg, nested_response_msg)), | ||
| ) | ||
| } |
There was a problem hiding this comment.
The expand_nested_tool_calls function lacks test coverage. Consider adding tests that verify: 1) nested tool calls are correctly extracted from metadata, 2) the function handles missing or malformed metadata gracefully, 3) successful and error tool results are correctly converted to message pairs.
| ( | ||
| request_msg, | ||
| response_msg, | ||
| Some((nested_request_msg, nested_response_msg)), | ||
| ) |
There was a problem hiding this comment.
This function may return empty messages when all nested tool calls fail to parse. If all calls in the array lack a "name" field, the nested request and response messages will have empty content vectors, but will still be returned as Some and added to messages_to_add. Consider checking if the messages have any content before returning them, or only create the messages after confirming at least one valid call exists.
| ( | |
| request_msg, | |
| response_msg, | |
| Some((nested_request_msg, nested_response_msg)), | |
| ) | |
| if !nested_request_msg.content.is_empty() || !nested_response_msg.content.is_empty() { | |
| ( | |
| request_msg, | |
| response_msg, | |
| Some((nested_request_msg, nested_response_msg)), | |
| ) | |
| } else { | |
| ( | |
| request_msg, | |
| response_msg, | |
| None, | |
| ) | |
| } |
|
yeah I don't like that it implies that it needs a bespoke UI implementation (it should not - if that review is to believed @tlongwell-block !) |
No, that review was terrible. Please ignore. Sorry for the spam. Working on the goose reviewer and this was too much for it. |
This is an attempt to make it show the underlying tool calls in the UI when "code mode" is used:
cc @alexhancock
don't like how clumsy this feels and special case-ey, but I think we need something functionally like this?