From e04817b822180a85f4ae74b1d57f2056e4d0f285 Mon Sep 17 00:00:00 2001 From: King Star Date: Mon, 8 Jun 2026 23:56:28 +0800 Subject: [PATCH] fix: execute non-approval tools in mixed approval batches --- .../packages/core/agent_framework/_tools.py | 82 ++++++++++--------- .../core/test_function_invocation_logic.py | 64 ++++++++++++--- 2 files changed, 94 insertions(+), 52 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 7bb54ee2c9..bd5059b9b3 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -1238,8 +1238,8 @@ def tool( Note: When approval_mode is set to "always_require", the function will not be executed until explicit approval is given, this only applies to the auto-invocation flow. - It is also important to note that if the model returns multiple function calls, some that require approval - and others that do not, it will ask approval for all of them. + If the model returns multiple function calls, only the calls for tools that require approval + will ask for approval. Example: @@ -1675,8 +1675,8 @@ async def _try_execute_function_calls( Returns: A tuple of: - A list of Content containing the results of each function call, - or the approval requests if any function requires approval, - or the original function calls if any are declaration only. + approval requests for function calls that require approval, + or original function calls for declaration-only requests. - Always False; termination via middleware is no longer supported. """ from ._types import Content @@ -1685,20 +1685,18 @@ async def _try_execute_function_calls( # The live tools list (when tools is the run-local list) is exposed on the # FunctionInvocationContext so tools can add/remove tools during the run. live_tools: list[ToolTypes] | None = cast("list[ToolTypes]", tools) if isinstance(tools, list) else None - approval_tools = [tool_name for tool_name, tool in tool_map.items() if tool.approval_mode == "always_require"] + approval_tools = {tool_name for tool_name, tool in tool_map.items() if tool.approval_mode == "always_require"} logger.debug( "_try_execute_function_calls: tool_map keys=%s, approval_tools=%s", list(tool_map.keys()), - approval_tools, + list(approval_tools), ) declaration_only = [tool_name for tool_name, tool in tool_map.items() if tool.declaration_only] configured_additional_tools = config.get("additional_tools") or [] additional_tool_names = [tool.name for tool in configured_additional_tools] - # check if any are calling functions that need approval - # if so, we return approval request for all - approval_needed = False - declaration_only_flag = False - for fcc in function_calls: + deferred_results_by_index: dict[int, Content] = {} + function_calls_to_execute: list[tuple[int, Content]] = [] + for idx, fcc in enumerate(function_calls): fcc_name = getattr(fcc, "name", None) logger.debug( "Checking function call: type=%s, name=%s, in approval_tools=%s", @@ -1708,36 +1706,25 @@ async def _try_execute_function_calls( ) if fcc.type == "function_call" and fcc.name in approval_tools: # type: ignore[attr-defined] logger.debug("Approval needed for function: %s", fcc.name) - approval_needed = True - break + deferred_results_by_index[idx] = Content.from_function_approval_request( + id=fcc.call_id, # type: ignore[arg-type, attr-defined] + function_call=fcc, # type: ignore[arg-type] + ) + continue if fcc.type == "function_call" and (fcc.name in declaration_only or fcc.name in additional_tool_names): # type: ignore[attr-defined] - declaration_only_flag = True - break + fcc.user_input_request = True + fcc.id = fcc.call_id + deferred_results_by_index[idx] = fcc + continue if ( config.get("terminate_on_unknown_calls", False) and fcc.type == "function_call" and fcc.name not in tool_map # type: ignore[attr-defined] ): raise KeyError(f'Error: Requested function "{fcc.name}" not found.') # type: ignore[attr-defined] - if approval_needed: - # approval can only be needed for Function Call Content, not Approval Responses. - logger.debug("Returning function_approval_request contents") - return ( - [ - Content.from_function_approval_request(id=fcc.call_id, function_call=fcc) # type: ignore[attr-defined, arg-type] - for fcc in function_calls - if fcc.type == "function_call" - ], - False, - ) - if declaration_only_flag: - # return the declaration only tools to the user, since we cannot execute them. - # Mark as user_input_request so AgentExecutor emits request_info events and pauses the workflow. - declaration_only_calls: list[Content] = [] - for fcc in function_calls: - if fcc.type == "function_call": - fcc.user_input_request = True - fcc.id = fcc.call_id - declaration_only_calls.append(fcc) - return (declaration_only_calls, False) + function_calls_to_execute.append((idx, fcc)) + + if deferred_results_by_index and not function_calls_to_execute: + logger.debug("Returning deferred function call contents") + return ([deferred_results_by_index[idx] for idx in sorted(deferred_results_by_index)], False) # Run all function calls concurrently, handling MiddlewareTermination from ._middleware import MiddlewareTermination @@ -1794,11 +1781,23 @@ async def invoke_with_termination_handling( ) execution_results = await asyncio.gather(*[ - invoke_with_termination_handling(function_call, seq_idx) for seq_idx, function_call in enumerate(function_calls) + invoke_with_termination_handling(function_call, seq_idx) + for seq_idx, function_call in function_calls_to_execute ]) # Unpack results - each is (Content, terminate_flag) - contents: list[Content] = [result[0] for result in execution_results] + if deferred_results_by_index: + execution_results_by_index = { + seq_idx: result for (seq_idx, _), result in zip(function_calls_to_execute, execution_results) + } + contents = [ + deferred_results_by_index[idx] + if idx in deferred_results_by_index + else execution_results_by_index[idx][0] + for idx in range(len(function_calls)) + ] + else: + contents = [result[0] for result in execution_results] contents.extend(extra_user_input_contents) # If any function requested termination, terminate the loop should_terminate = any(result[1] for result in execution_results) @@ -2108,12 +2107,17 @@ def _handle_function_call_results( ): # Only add items that aren't already in the message (e.g. function_approval_request wrappers). # Declaration-only function_call items are already present from the LLM response. - new_items = [fccr for fccr in function_call_results if fccr.type != "function_call"] + new_items = [ + fccr for fccr in function_call_results if fccr.type not in {"function_call", "function_result"} + ] if new_items: if response.messages and response.messages[0].role == "assistant": response.messages[0].contents.extend(new_items) else: response.messages.append(Message(role="assistant", contents=new_items)) + function_result_items = [fccr for fccr in function_call_results if fccr.type == "function_result"] + if function_result_items: + response.messages.append(Message(role="tool", contents=function_result_items)) return { "action": "return", "errors_in_a_row": errors_in_a_row, diff --git a/python/packages/core/tests/core/test_function_invocation_logic.py b/python/packages/core/tests/core/test_function_invocation_logic.py index f96ca99ad5..bb0eeb4a6a 100644 --- a/python/packages/core/tests/core/test_function_invocation_logic.py +++ b/python/packages/core/tests/core/test_function_invocation_logic.py @@ -117,6 +117,42 @@ def ai_func(arg1: str) -> str: assert response.messages[1].contents[0].result == "Processed value1" +async def test_mixed_batch_only_approval_tool_gets_wrapped() -> None: + from agent_framework._tools import _try_execute_function_calls, normalize_function_invocation_configuration + + exec_counter = 0 + + @tool(name="no_approval_func", approval_mode="never_require") + def func_no_approval(arg1: str) -> str: + nonlocal exec_counter + exec_counter += 1 + return f"Processed {arg1}" + + @tool(name="approval_func", approval_mode="always_require") + def func_with_approval(arg1: str) -> str: + return f"Approved {arg1}" + + results, should_terminate = await _try_execute_function_calls( + custom_args={}, + attempt_idx=0, + function_calls=[ + Content.from_function_call(call_id="1", name="no_approval_func", arguments='{"arg1": "value1"}'), + Content.from_function_call(call_id="2", name="approval_func", arguments='{"arg1": "value2"}'), + ], + tools=[func_no_approval, func_with_approval], + config=normalize_function_invocation_configuration(None), + ) + + assert should_terminate is False + assert exec_counter == 1 + assert [(content.type, content.call_id) for content in results] == [ + ("function_result", "1"), + ("function_approval_request", None), + ] + assert results[0].result == "Processed value1" + assert results[1].function_call.name == "approval_func" + + @pytest.mark.parametrize("max_iterations", [3]) async def test_base_client_with_function_calling_resets(chat_client_base: SupportsChatGetResponse): exec_counter = 0 @@ -550,7 +586,7 @@ async def test_function_invocation_scenarios( This test covers: - Single function without approval: 3 messages (call, result, final) - Single function with approval: 2 messages (call, approval request) - - Two functions with mixed approval: varies based on approval flow + - Two functions with mixed approval: non-approval calls execute, approval calls request approval - All scenarios tested with both streaming and non-streaming - Thread scenarios: no thread, local thread (in-memory), and service thread (conversation_id) """ @@ -690,26 +726,28 @@ def func_with_approval(arg1: str) -> str: else: # num_functions == 2 # Two functions with mixed approval if not streaming: - # Mixed: assistant message has both calls + approval requests (4 items total) - # (because when one requires approval, all are batched for approval) - assert len(messages) == 1 - # Should have: 2 FunctionCallContent + 2 FunctionApprovalRequestContent - assert len(messages[0].contents) == 4 + assert len(messages) == 2 assert messages[0].contents[0].type == "function_call" assert messages[0].contents[1].type == "function_call" - # Both should result in approval requests approval_requests = [c for c in messages[0].contents if c.type == "function_approval_request"] - assert len(approval_requests) == 2 - assert exec_counter == 0 # Neither function executed yet + assert len(approval_requests) == 1 + assert approval_requests[0].function_call.name == "approval_func" + assert messages[1].role == "tool" + assert messages[1].contents[0].type == "function_result" + assert messages[1].contents[0].call_id == "1" + assert messages[1].contents[0].result == "Processed value1" + assert exec_counter == 1 else: - # Streaming: 2 function call updates + 1 approval request with 2 contents assert len(messages) == 3 assert messages[0].contents[0].type == "function_call" assert messages[1].contents[0].type == "function_call" - # The approval request message contains both approval requests assert len(messages[2].contents) == 2 - assert all(c.type == "function_approval_request" for c in messages[2].contents) - assert exec_counter == 0 # Neither function executed yet + assert messages[2].contents[0].type == "function_result" + assert messages[2].contents[0].call_id == "1" + assert messages[2].contents[0].result == "Processed value1" + assert messages[2].contents[1].type == "function_approval_request" + assert messages[2].contents[1].function_call.name == "approval_func" + assert exec_counter == 1 async def test_rejected_approval(chat_client_base: SupportsChatGetResponse):