Fix multi tool calling #5855

DOsinga · 2025-11-24T12:56:01Z

Summary

Fix how we handle multiple tools, we need to interleave requests and responses, not bunch them up

Copilot

Pull request overview

This PR refactors the multi-tool calling logic to properly interleave tool requests and responses instead of bunching them together. The changes enable each tool request to have its own dedicated response message, creating a proper interleaved conversation pattern.

Key Changes:

Replaced a single shared message_tool_response with a HashMap mapping request IDs to individual response messages
Refactored tool execution to process tools sequentially, interleaving request/response pairs
Extracted handle_denied_tools as a separate helper function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
crates/goose/src/agents/tool_execution.rs	Updated `handle_approval_tool_requests` to accept a HashMap mapping request IDs to response messages, enabling individual response handling per tool request
crates/goose/src/agents/agent.rs	Refactored reply logic to create individual response messages for each tool, process frontend tools sequentially, and build properly interleaved request/response message pairs

Copilot · 2025-11-24T12:58:20Z

crates/goose/src/agents/agent.rs

+        Ok(tool_futures)
+    }
+
+    async fn handle_denied_tools(permission_check_result: &&PermissionCheckResult, request_to_response_map: &HashMap<String, Arc<Mutex<Message>>>) {


Unnecessary double reference in parameter. Change permission_check_result: &&PermissionCheckResult to permission_check_result: &PermissionCheckResult.

Suggested change

async fn handle_denied_tools(permission_check_result: &&PermissionCheckResult, request_to_response_map: &HashMap<String, Arc<Mutex<Message>>>) {

async fn handle_denied_tools(permission_check_result: &PermissionCheckResult, request_to_response_map: &HashMap<String, Arc<Mutex<Message>>>) {

Copilot · 2025-11-24T12:58:20Z

crates/goose/src/agents/agent.rs

+        for (idx, request) in frontend_requests.iter().enumerate() {
+            let requests = vec![request.clone()];
+            let mut frontend_tool_stream = self.handle_frontend_tool_requests(
+                &requests,
+                tool_response_messages[idx].clone(),
+            );

-                                let mut frontend_tool_stream = self.handle_frontend_tool_requests(
-                                    &frontend_requests,
-                                    message_tool_response.clone(),
+            while let Some(msg) = frontend_tool_stream.try_next().await? {
+                yield AgentEvent::Message(msg);
+            }
+        }


Incorrect indentation: these lines should be indented to match the surrounding code context (should be indented one level deeper).

Copilot · 2025-11-24T12:58:20Z

crates/goose/src/agents/agent.rs

-                                    while let Some((request_id, item)) = combined.next().await {
-                                        if is_token_cancelled(&cancel_token) {
+                // Build the final interleaved messages
+                // Build the final interleaved messages


Duplicate comment on consecutive lines. Remove the duplicate.

Suggested change

// Build the final interleaved messages

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Copilot · 2025-11-24T13:15:51Z

crates/goose/src/agents/agent.rs

+                .map(|_| Arc::new(Mutex::new(Message::user().with_id(
+                    format!("msg_{}", Uuid::new_v4())
+                ))))


[nitpick] Consider extracting message creation to a helper function. The closure creating Message::user().with_id(format!("msg_{}", Uuid::new_v4())) is duplicated at lines 1030-1032 when it could be a reusable function to improve maintainability.

Suggested change

.map(|_| Arc::new(Mutex::new(Message::user().with_id(

format!("msg_{}", Uuid::new_v4())

))))

.map(|_| Arc::new(Mutex::new(new_user_message_with_unique_id())))

Copilot · 2025-11-24T13:15:52Z

crates/goose/src/agents/agent.rs

+            for (idx, request) in frontend_requests.iter().enumerate() {
+                let requests = vec![request.clone()];
+                let mut frontend_tool_stream = self.handle_frontend_tool_requests(
+                    &requests,


Creating a single-element vector vec![request.clone()] for each iteration is inefficient. Consider refactoring handle_frontend_tool_requests to accept a single request instead of requiring a slice, or pass a slice of one element without cloning.

Suggested change

for (idx, request) in frontend_requests.iter().enumerate() {

let requests = vec![request.clone()];

let mut frontend_tool_stream = self.handle_frontend_tool_requests(

&requests,

for (idx, _request) in frontend_requests.iter().enumerate() {

let requests = &frontend_requests[idx..=idx];

let mut frontend_tool_stream = self.handle_frontend_tool_requests(

requests,

Copilot · 2025-11-24T13:15:52Z

crates/goose/src/agents/agent.rs

+            for (idx, request) in frontend_requests.iter()
+                                    .chain(remaining_requests.iter()).enumerate() {
+                if request.tool_call.is_ok() {
+                    let request_msg = Message::assistant()
+                        .with_id(format!("msg_{}", Uuid::new_v4()))
+                        .with_tool_request(request.id.clone(), request.tool_call.clone());
+                    messages_to_add.push(request_msg);
+                    let final_response = tool_response_messages[idx]
+                                            .lock().await.clone();
+                    yield AgentEvent::Message(final_response.clone());
+                    messages_to_add.push(final_response);


The indexing tool_response_messages[idx] assumes that the enumeration index matches the position in tool_response_messages, but this will fail if any requests have tool_call.is_err() since those requests are skipped but still increment idx. This could cause index out of bounds or incorrect message retrieval.

Suggested change

for (idx, request) in frontend_requests.iter()

.chain(remaining_requests.iter()).enumerate() {

if request.tool_call.is_ok() {

let request_msg = Message::assistant()

.with_id(format!("msg_{}", Uuid::new_v4()))

.with_tool_request(request.id.clone(), request.tool_call.clone());

messages_to_add.push(request_msg);

let final_response = tool_response_messages[idx]

.lock().await.clone();

yield AgentEvent::Message(final_response.clone());

messages_to_add.push(final_response);

let mut tool_response_idx = 0;

for request in frontend_requests.iter().chain(remaining_requests.iter()) {

if request.tool_call.is_ok() {

let request_msg = Message::assistant()

.with_id(format!("msg_{}", Uuid::new_v4()))

.with_tool_request(request.id.clone(), request.tool_call.clone());

messages_to_add.push(request_msg);

let final_response = tool_response_messages[tool_response_idx]

.lock().await.clone();

yield AgentEvent::Message(final_response.clone());

messages_to_add.push(final_response);

tool_response_idx += 1;

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Copilot · 2025-11-24T13:23:43Z

crates/goose/src/agents/agent.rs

+                                    .chain(remaining_requests.iter()).enumerate() {
+                if request.tool_call.is_ok() {
+                    let request_msg = Message::assistant()
+                        .with_id(format!("msg_{}", Uuid::new_v4()))
+                        .with_tool_request(request.id.clone(), request.tool_call.clone());
+                    messages_to_add.push(request_msg);
+                    let final_response = tool_response_messages[idx]
+                                            .lock().await.clone();
+                    yield AgentEvent::Message(final_response.clone());
+                    messages_to_add.push(final_response);
+                }


This loop processes all tool requests (frontend + remaining) but only emits tool_response_messages for requests where request.tool_call.is_ok(). If any request has tool_call.is_err(), the indexing into tool_response_messages[idx] will be off by one or more, causing a potential panic or mismatched response messages. The index should only increment for successful tool_calls, or filter should be applied before enumeration.

Suggested change

.chain(remaining_requests.iter()).enumerate() {

if request.tool_call.is_ok() {

let request_msg = Message::assistant()

.with_id(format!("msg_{}", Uuid::new_v4()))

.with_tool_request(request.id.clone(), request.tool_call.clone());

messages_to_add.push(request_msg);

let final_response = tool_response_messages[idx]

.lock().await.clone();

yield AgentEvent::Message(final_response.clone());

messages_to_add.push(final_response);

}

.chain(remaining_requests.iter())

.filter(|request| request.tool_call.is_ok())

.enumerate()

{

let request_msg = Message::assistant()

.with_id(format!("msg_{}", Uuid::new_v4()))

.with_tool_request(request.id.clone(), request.tool_call.clone());

messages_to_add.push(request_msg);

let final_response = tool_response_messages[idx]

.lock().await.clone();

yield AgentEvent::Message(final_response.clone());

messages_to_add.push(final_response);

Copilot · 2025-11-24T13:23:44Z

crates/goose/src/agents/agent.rs

+            let mut request_to_response_map = HashMap::new();
+            for (idx, request) in frontend_requests.iter().chain(remaining_requests.iter()).enumerate() {
+                request_to_response_map.insert(request.id.clone(), tool_response_messages[idx].clone());
+            }


The request_to_response_map is populated using all requests (frontend_requests.iter().chain(remaining_requests.iter())), but tool_response_messages is created based on num_tool_requests which is the total count. If any requests have tool_call.is_err(), there's a mismatch: the map will have entries for all requests, but later code at line 1165 only processes requests where tool_call.is_ok(), leading to unused message objects and potential index misalignment when accessing tool_response_messages[idx].

Copilot · 2025-11-24T13:23:44Z

crates/goose/src/agents/agent.rs

+            for (idx, request) in frontend_requests.iter().enumerate() {
+                let requests = vec![request.clone()];
+                let mut frontend_tool_stream = self.handle_frontend_tool_requests(
+                    &requests,
+                    tool_response_messages[idx].clone(),
+                );

-                                    // Process inspection results into permission decisions using the permission inspector
-                                    let permission_check_result = self.tool_inspection_manager
-                                        .process_inspection_results_with_permission_inspector(
-                                            &remaining_requests,
-                                            &inspection_results,
-                                        )
-                                        .unwrap_or_else(|| {
-                                            // Fallback if permission inspector not found - default to needs approval
-                                            let mut result = PermissionCheckResult {
-                                                approved: vec![],
-                                                needs_approval: vec![],
-                                                denied: vec![],
-                                            };
-                                            result.needs_approval.extend(remaining_requests.iter().cloned());
-                                            result
-                                        });
-
-                                    // Track extension requests for special handling
-                                    let mut enable_extension_request_ids = vec![];
-                                    for request in &remaining_requests {
-                                        if let Ok(tool_call) = &request.tool_call {
-                                            if tool_call.name == MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE {
-                                                enable_extension_request_ids.push(request.id.clone());
-                                            }
-                                        }
-                                    }
+                while let Some(msg) = frontend_tool_stream.try_next().await? {
+                    yield AgentEvent::Message(msg);
+                }
+            }


[nitpick] Frontend tool requests are processed sequentially using individual indices into tool_response_messages, but this assumes frontend_requests starts at index 0 in the shared array. Since tool_response_messages contains entries for both frontend and remaining requests, and remaining requests start at frontend_requests.len(), the indexing here is correct but fragile. Consider documenting this assumption or using a more explicit indexing scheme.

alexhancock

I don't understand the old code at all, but the new code makes sense to me

DOsinga · 2025-11-24T15:19:28Z

In the old code we collected all the tool requests in one message and all the tool responses in another one and then appended those to the conversation, which doesn't work on at least databricks

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Copilot · 2025-11-24T15:46:36Z

crates/goose/src/agents/agent.rs

+                }
+            }
+        no_tools_called = false;


Indentation error: no_tools_called = false; should be at the same indentation level as the code inside the if let Some(response) block (line 1009), not outdented. This line logically belongs to the tool execution path.

Suggested change

}

}

no_tools_called = false;

no_tools_called = false;

}

}

Copilot · 2025-11-24T15:46:36Z

crates/goose/src/agents/agent.rs

+                messages_to_add.push(response.clone());
+                continue;
+            }



The assistant response with tool requests is never added to messages_to_add. Line 1025 adds response.clone() when there are no tool requests, but when tools are called, the response containing the tool requests should be added before the individual tool request messages (line 1165-1167). Without this, the conversation history will be missing the assistant's response that triggered the tool calls.

Suggested change

// Always add the assistant's response that triggered the tool calls to the conversation history

messages_to_add.push(response.clone());

* main: docs: add DataHub MCP server extension documentation (#5769) docs: lowercase goose in remaining topics (#5861) docs: lowercase goose in getting-started and guides topics (#5857) Fix multi tool calling (#5855) fix(#5626 #5832): handle multiple content chunks & images better (#5839) chore: some old code hanging around, and mention configure cli (#5822) feat : add support for math / science symbology via katex (#5773) feat : add ability to see error message in toast (#5851)

Co-authored-by: Douwe Osinga <[email protected]>

Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: Sai Karthik <[email protected]>

Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: Blair Allan <[email protected]>

Douwe Osinga added 2 commits November 24, 2025 13:46

Fix multi toolcalling

306fdc0

Cleanup1

8244c7a

Copilot AI review requested due to automatic review settings November 24, 2025 12:56

Copilot started reviewing on behalf of DOsinga November 24, 2025 12:56 View session

remove

3383b64

Copilot finished reviewing on behalf of DOsinga November 24, 2025 12:57

Copilot AI reviewed Nov 24, 2025

View reviewed changes

Douwe Osinga added 2 commits November 24, 2025 14:00

Move around

6179946

Scenario testing

d1c2fec

Copilot AI review requested due to automatic review settings November 24, 2025 13:13

Copilot started reviewing on behalf of DOsinga November 24, 2025 13:13 View session

Copilot finished reviewing on behalf of DOsinga November 24, 2025 13:14

Copilot AI reviewed Nov 24, 2025

View reviewed changes

DOsinga requested a review from Copilot November 24, 2025 13:21

Copilot started reviewing on behalf of DOsinga November 24, 2025 13:21 View session

Copilot finished reviewing on behalf of DOsinga November 24, 2025 13:22

Copilot AI reviewed Nov 24, 2025

View reviewed changes

Small clean up

a02f0df

alexhancock approved these changes Nov 24, 2025

View reviewed changes

Clippy

17e718c

Copilot AI review requested due to automatic review settings November 24, 2025 15:44

Copilot started reviewing on behalf of DOsinga November 24, 2025 15:44 View session

Copilot finished reviewing on behalf of DOsinga November 24, 2025 15:46

Copilot AI reviewed Nov 24, 2025

View reviewed changes

DOsinga merged commit 3e9d9c7 into main Nov 24, 2025
23 checks passed

DOsinga deleted the fix-multi-tool-calling branch November 24, 2025 15:59

github-actions bot mentioned this pull request Nov 25, 2025

chore(release): release version 1.16.0 (minor) #5867

Closed

kskarthik pushed a commit to kskarthik/goose that referenced this pull request Nov 25, 2025

Fix multi tool calling (block#5855)

ec7a8cc

Co-authored-by: Douwe Osinga <[email protected]>

kskarthik pushed a commit to kskarthik/goose that referenced this pull request Nov 26, 2025

Fix multi tool calling (block#5855)

73a3cba

Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: Sai Karthik <[email protected]>

BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025

Fix multi tool calling (block#5855)

85cde1e

Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: Blair Allan <[email protected]>

zanesq mentioned this pull request Dec 3, 2025

Auto Tool chaining is not working as expected in goose 1.15.0 version compared to 1.14.0 version #5957

Closed

github-actions bot mentioned this pull request Dec 4, 2025

chore(release): release version 1.16.0 (minor) #5985

Merged

	async fn handle_denied_tools(permission_check_result: &&PermissionCheckResult, request_to_response_map: &HashMap<String, Arc<Mutex<Message>>>) {
	async fn handle_denied_tools(permission_check_result: &PermissionCheckResult, request_to_response_map: &HashMap<String, Arc<Mutex<Message>>>) {



	// Always add the assistant's response that triggered the tool calls to the conversation history
	messages_to_add.push(response.clone());

Fix multi tool calling #5855

Fix multi tool calling #5855

Uh oh!

Conversation

DOsinga commented Nov 24, 2025

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

alexhancock left a comment

Choose a reason for hiding this comment

Uh oh!

DOsinga commented Nov 24, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants