Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions crates/goose-cli/src/session/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ fn render_tool_request(req: &ToolRequest, theme: Theme, debug: bool) {
Ok(call) => match call.name.to_string().as_str() {
"developer__text_editor" => render_text_editor_request(call, debug),
"developer__shell" => render_shell_request(call, debug),
"code_execution__execute_code" => render_execute_code_request(call, debug),
"subagent" => render_subagent_request(call, debug),
"todo__write" => render_todo_request(call, debug),
_ => render_default_request(call, debug),
Expand Down Expand Up @@ -445,6 +446,61 @@ fn render_shell_request(call: &CallToolRequestParam, debug: bool) {
println!();
}

fn render_execute_code_request(call: &CallToolRequestParam, debug: bool) {
let tool_graph = call
.arguments
.as_ref()
.and_then(|args| args.get("tool_graph"))
.and_then(Value::as_array)
.filter(|arr| !arr.is_empty());

let Some(tool_graph) = tool_graph else {
return render_default_request(call, debug);
};

let count = tool_graph.len();
let plural = if count == 1 { "" } else { "s" };
println!();
println!(
"─── {} tool call{} | {} ──────────────────────────",
style(count).cyan(),
plural,
style("execute_code").magenta().dim()
);

for (i, node) in tool_graph.iter().filter_map(Value::as_object).enumerate() {
let tool = node
.get("tool")
.and_then(Value::as_str)
.unwrap_or("unknown");
let desc = node
.get("description")
.and_then(Value::as_str)
.unwrap_or("");
let deps: Vec<_> = node
.get("depends_on")
.and_then(Value::as_array)
.into_iter()
.flatten()
.filter_map(Value::as_u64)
.map(|d| (d + 1).to_string())
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency indices are displayed without validation. If a dependency index is out of bounds or references a later node, the displayed number will be misleading. Consider adding validation or at least bounds checking before displaying.

Suggested change
.map(|d| (d + 1).to_string())
.filter_map(|d| {
let max_index = count as u64;
let current_index = i as u64;
if d < max_index && d <= current_index {
Some((d + 1).to_string())
} else {
None
}
})

Copilot uses AI. Check for mistakes.
.collect();
let deps_str = if deps.is_empty() {
String::new()
} else {
format!(" (uses {})", deps.join(", "))
};
println!(
" {}. {}: {}{}",
style(i + 1).dim(),
style(tool).cyan(),
style(desc).green(),
style(deps_str).dim()
);
}
println!();
}

fn render_subagent_request(call: &CallToolRequestParam, debug: bool) {
print_tool_header(call);

Expand Down
25 changes: 25 additions & 0 deletions crates/goose/src/agents/code_execution_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,25 @@ type ToolCallRequest = (
tokio::sync::oneshot::Sender<Result<String, String>>,
);

#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
struct ToolGraphNode {
/// Tool name in format "server/tool" (e.g., "developer/shell")
tool: String,
/// Brief description of what this call does (e.g., "list files in /src")
description: String,
/// Indices of nodes this depends on (empty if no dependencies)
#[serde(default)]
depends_on: Vec<usize>,
}

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
struct ExecuteCodeParams {
/// JavaScript code with ES6 imports for MCP tools.
code: String,
/// DAG of tool calls showing execution flow. Each node represents a tool call.
/// Use depends_on to show data flow (e.g., node 1 uses output from node 0).
#[serde(default)]
tool_graph: Vec<ToolGraphNode>,
}

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
Expand Down Expand Up @@ -701,6 +716,7 @@ impl McpClientTrait for CodeExecutionClient {
Err(Error::TransportClosed)
}

#[allow(clippy::too_many_lines)]
async fn list_tools(
&self,
_next_cursor: Option<String>,
Expand Down Expand Up @@ -746,6 +762,15 @@ impl McpClientTrait for CodeExecutionClient {
- Last expression is the result
- No comments in code

TOOL_GRAPH: Always provide tool_graph to describe the execution flow for the UI.
Each node has: tool (server/name), description (what it does), depends_on (indices of dependencies).
Example for chained operations:
[
{"tool": "developer/shell", "description": "list files", "depends_on": []},
{"tool": "developer/text_editor", "description": "read README.md", "depends_on": []},
{"tool": "developer/text_editor", "description": "write output.txt", "depends_on": [0, 1]}
]
Comment on lines +765 to +772
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation states "Always provide tool_graph" but the field is marked as optional with #[serde(default)]. This creates inconsistency between the instruction and the actual schema. Either make the field required or update the documentation to say "Provide tool_graph when possible" or similar.

Copilot uses AI. Check for mistakes.

BEFORE CALLING: Use the read_module tool to check required parameters.
"#}
.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ impl MappingReport {
}
}

#[allow(clippy::too_many_lines)]
async fn build_canonical_models() -> Result<()> {
println!("Fetching models from OpenRouter API...");

Expand Down
6 changes: 4 additions & 2 deletions scripts/test_providers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ fi
if [ "$CODE_EXEC_MODE" = true ]; then
echo "Mode: code_execution (JS batching)"
BUILTINS="developer,code_execution"
# Match "execute_code | code_execution" or "read_module | code_execution" in output
SUCCESS_PATTERN="(execute_code \| code_execution)|(read_module \| code_execution)"
# Match code_execution tool usage:
# - "execute_code | code_execution" or "read_module | code_execution" (fallback format)
# - "tool call | execute_code" or "tool calls | execute_code" (new format with tool_graph)
SUCCESS_PATTERN="(execute_code \| code_execution)|(read_module \| code_execution)|(tool calls? \| execute_code)"
SUCCESS_MSG="code_execution tool called"
FAILURE_MSG="no code_execution tools called"
else
Expand Down
93 changes: 87 additions & 6 deletions ui/desktop/src/components/ToolCallWithResponse.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import MCPUIResourceRenderer from './MCPUIResourceRenderer';
import { isUIResource } from '@mcp-ui/client';
import { CallToolResponse, Content, EmbeddedResource } from '../api';

interface ToolGraphNode {
tool: string;
description: string;
depends_on: number[];
}

interface ToolCallWithResponseProps {
isCancelledMessage: boolean;
toolRequest: ToolRequestMessageContent;
Expand Down Expand Up @@ -410,6 +416,20 @@ function ToolCallView({
case 'computer_control':
return `poking around...`;

case 'execute_code': {
const toolGraph = args.tool_graph as unknown as ToolGraphNode[] | undefined;
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double cast through unknown bypasses TypeScript's type safety. Consider defining a proper type guard function or adding a runtime validation function that checks the structure matches ToolGraphNode[] before casting.

Copilot uses AI. Check for mistakes.
if (toolGraph && Array.isArray(toolGraph) && toolGraph.length > 0) {
if (toolGraph.length === 1) {
return `${toolGraph[0].description}`;
}
if (toolGraph.length === 2) {
return `${toolGraph[0].tool}, ${toolGraph[1].tool}`;
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When displaying two tools, this shows the tool names but not their descriptions. This is inconsistent with the single-tool case (line 423) which shows the description. For two tools, consider showing descriptions instead of tool names for consistency, or show both items in a format like "description1, description2".

Suggested change
return `${toolGraph[0].tool}, ${toolGraph[1].tool}`;
const firstLabel = toolGraph[0].description || toolGraph[0].tool;
const secondLabel = toolGraph[1].description || toolGraph[1].tool;
return `${firstLabel}, ${secondLabel}`;

Copilot uses AI. Check for mistakes.
}
return `${toolGraph.length} tools used`;
}
return 'executing code';
}

default: {
// Generic fallback for unknown tools: ToolName + CompactArguments
// This ensures any MCP tool works without explicit handling
Expand Down Expand Up @@ -489,12 +509,34 @@ function ToolCallView({
)
}
>
{/* Tool Details */}
{isToolDetails && (
<div className="border-t border-borderSubtle">
<ToolDetailsView toolCall={toolCall} isStartExpanded={isExpandToolDetails} />
</div>
)}
{(() => {
const toolName = toolCall.name.substring(toolCall.name.lastIndexOf('__') + 2);
const toolGraph = toolCall.arguments?.tool_graph as unknown as ToolGraphNode[] | undefined;
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double cast through unknown bypasses TypeScript's type safety. Consider defining a proper type guard function or adding a runtime validation function that checks the structure matches ToolGraphNode[] before casting.

Copilot uses AI. Check for mistakes.
const code = toolCall.arguments?.code as unknown as string | undefined;
const hasToolGraph =
toolName === 'execute_code' &&
toolGraph &&
Array.isArray(toolGraph) &&
toolGraph.length > 0;

if (hasToolGraph) {
return (
<div className="border-t border-borderSubtle">
<ToolGraphView toolGraph={toolGraph} code={code} />
</div>
);
}

if (isToolDetails) {
return (
<div className="border-t border-borderSubtle">
<ToolDetailsView toolCall={toolCall} isStartExpanded={isExpandToolDetails} />
</div>
);
}

return null;
})()}

{logs && logs.length > 0 && (
<div className="border-t border-borderSubtle">
Expand Down Expand Up @@ -553,6 +595,45 @@ function ToolDetailsView({ toolCall, isStartExpanded }: ToolDetailsViewProps) {
);
}

interface ToolGraphViewProps {
toolGraph: ToolGraphNode[];
code?: string;
}

function ToolGraphView({ toolGraph, code }: ToolGraphViewProps) {
const renderGraph = () => {
if (toolGraph.length === 0) return null;

const lines: string[] = [];

toolGraph.forEach((node, index) => {
const deps =
node.depends_on.length > 0 ? ` (uses ${node.depends_on.map((d) => d + 1).join(', ')})` : '';
Comment on lines +610 to +611
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency indices are displayed without validation. If a dependency index is out of bounds (>= toolGraph.length) or references a later node (>= current index), the displayed number will be misleading. Consider adding validation or at least bounds checking before displaying.

Suggested change
const deps =
node.depends_on.length > 0 ? ` (uses ${node.depends_on.map((d) => d + 1).join(', ')})` : '';
const validDeps = node.depends_on.filter(
(d) =>
Number.isInteger(d) &&
d >= 0 &&
d < toolGraph.length &&
d < index,
);
const deps =
validDeps.length > 0 ? ` (uses ${validDeps.map((d) => d + 1).join(', ')})` : '';

Copilot uses AI. Check for mistakes.
lines.push(`${index + 1}. ${node.tool}: ${node.description}${deps}`);
});

return lines.join('\n');
};

return (
<div className="px-4 py-2">
<pre className="font-mono text-xs text-textSubtle whitespace-pre-wrap">{renderGraph()}</pre>
{code && (
<div className="border-t border-borderSubtle -mx-4 mt-2">
<ToolCallExpandable
label={<span className="pl-4 font-sans text-sm">Code</span>}
isStartExpanded={false}
>
<pre className="font-mono text-xs text-textSubtle whitespace-pre-wrap overflow-x-auto px-4 py-2">
{code}
</pre>
</ToolCallExpandable>
</div>
)}
</div>
);
}

interface ToolResultViewProps {
result: Content;
isStartExpanded: boolean;
Expand Down
Loading