From 47f1912f803cbaaab0fa436ee15b2b5db4d869eb Mon Sep 17 00:00:00 2001 From: CorgiBoyG Date: Sat, 5 Sep 2026 02:31:41 +0800 Subject: [PATCH 1/4] fix: enforce tool approval regardless of call order in mixed batches Scan the whole batch before deciding, applying priority approval > declaration-only > unknown-call termination, so an always_require tool is no longer bypassed when a declaration-only or unknown call precedes it. Fixes #8079 --- .../packages/core/agent_framework/_tools.py | 21 +++-- .../core/test_function_invocation_logic.py | 84 +++++++++++++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 9b40754760..01bbd44977 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -1782,9 +1782,12 @@ async def _try_execute_function_call_groups( ] # Classify the entire batch first: any required user interaction pauses the batch before execution. + # Scan every call before deciding so classification travels with each call, not with its position in + # the batch. Priority (highest first): approval pause > declaration-only user-input > unknown-call + # termination. A user-input pause therefore takes precedence over unknown-call termination in mixed batches. requires_approval = False has_declaration_only_call = False - # A user-input pause takes precedence over unknown-call termination in mixed batches. + unknown_call_name: str | None = None for function_call in actionable_calls: function_name = function_call.name logger.debug( @@ -1796,12 +1799,20 @@ async def _try_execute_function_call_groups( if function_name in approval_tool_names: logger.debug("Approval needed for function: %s", function_name) requires_approval = True - break + continue if function_name in declaration_only_tool_names or function_name in additional_tool_names: has_declaration_only_call = True - break - if config.get("terminate_on_unknown_calls", False) and function_name not in tool_map: - raise KeyError(f'Error: Requested function "{function_name}" not found.') + continue + if ( + unknown_call_name is None + and config.get("terminate_on_unknown_calls", False) + and function_name not in tool_map + ): + unknown_call_name = function_name + # Defer unknown-call termination until the whole batch is classified so a higher-priority approval or + # declaration-only pause anywhere in the batch is not skipped by an earlier unknown call. + if not requires_approval and not has_declaration_only_call and unknown_call_name is not None: + raise KeyError(f'Error: Requested function "{unknown_call_name}" not found.') if requires_approval: # Surface only the approvals the host must decide; session-backed safe siblings wait for that resume. # approval can only be needed for Function Call Content, not Approval Responses. 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 5a2ba3e719..a16e29834c 100644 --- a/python/packages/core/tests/core/test_function_invocation_logic.py +++ b/python/packages/core/tests/core/test_function_invocation_logic.py @@ -2812,6 +2812,90 @@ def known_func(arg1: str) -> str: assert exec_counter == 0 +@pytest.mark.parametrize("approval_first", [True, False], ids=["approval-first", "declaration-only-first"]) +async def test_mixed_batch_approval_enforced_regardless_of_call_order( + chat_client_base: SupportsChatGetResponse, approval_first: bool +): + """A tool with approval_mode='always_require' must pause for approval even when a declaration-only + call precedes it in the same batch. + + Regression: batch classification used to stop at the first matching call, so a declaration-only call + appearing before an approval-required call caused the whole batch to be returned as user input and the + approval gate to be silently bypassed. Classification must travel with each call, not with its position. + """ + from agent_framework import FunctionTool + + @tool(name="approval_func", approval_mode="always_require") + def approval_func(arg1: str) -> str: + return f"Approved {arg1}" + + declaration_func = FunctionTool( + name="declaration_func", + func=None, + description="A declaration-only function for testing", + input_model={"type": "object", "properties": {"arg1": {"type": "string"}}, "required": ["arg1"]}, + ) + + approval_call = Content.from_function_call(call_id="a1", name="approval_func", arguments='{"arg1": "x"}') + declaration_call = Content.from_function_call(call_id="d1", name="declaration_func", arguments='{"arg1": "y"}') + contents = [approval_call, declaration_call] if approval_first else [declaration_call, approval_call] + + chat_client_base.run_responses = [ # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] + ChatResponse(messages=Message(role="assistant", contents=contents)), + ChatResponse(messages=Message(role="assistant", contents=["done"])), + ] + + response = await chat_client_base.get_response( + [Message(role="user", contents=["hello"])], + options={"tool_choice": "auto", "tools": [approval_func, declaration_func]}, + ) + + approval_requests = [ + content + for msg in response.messages + for content in msg.contents + if content.type == "function_approval_request" and content.function_call.name == "approval_func" + ] + assert len(approval_requests) == 1, "approval gate must be surfaced regardless of call order" + + +async def test_mixed_batch_approval_takes_precedence_over_unknown_call_termination( + chat_client_base: SupportsChatGetResponse, +): + """An approval-required call anywhere in the batch must pause before an unknown call terminates it. + + Regression: with terminate_on_unknown_calls=True, an unknown call appearing before an approval-required + call raised KeyError first, so the approval pause (higher priority) was never reached. + """ + + @tool(name="approval_func", approval_mode="always_require") + def approval_func(arg1: str) -> str: + return f"Approved {arg1}" + + chat_client_base.function_invocation_configuration["terminate_on_unknown_calls"] = True # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] + chat_client_base.run_responses = [ # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] + ChatResponse( + messages=Message( + role="assistant", + contents=[ + Content.from_function_call(call_id="u1", name="unknown_function", arguments='{"arg1": "x"}'), + Content.from_function_call(call_id="a1", name="approval_func", arguments='{"arg1": "y"}'), + ], + ) + ), + ChatResponse(messages=Message(role="assistant", contents=["done"])), + ] + + response = await chat_client_base.get_response( + [Message(role="user", contents=["hello"])], options={"tool_choice": "auto", "tools": [approval_func]} + ) + + approval_requests = [ + content for msg in response.messages for content in msg.contents if content.type == "function_approval_request" + ] + assert len(approval_requests) >= 1, "approval pause must take precedence over unknown-call termination" + + async def test_function_invocation_config_additional_tools(chat_client_base: SupportsChatGetResponse): """Test that additional_tools are available but treated as declaration_only.""" exec_counter_visible = 0 From d3c94e7e66fe38d4c5ceaea02c418d44118788a0 Mon Sep 17 00:00:00 2001 From: CorgiBoyG Date: Sat, 5 Sep 2026 03:08:36 +0800 Subject: [PATCH 2/4] fix: preserve unknown-call termination after approval resume --- .../packages/core/agent_framework/_tools.py | 25 ++++++++----------- .../core/test_function_invocation_logic.py | 20 +++++++++++++++ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 01bbd44977..116446f337 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -1777,41 +1777,38 @@ async def _try_execute_function_call_groups( ) declaration_only_tool_names = {tool_name for tool_name, tool in tool_map.items() if tool.declaration_only} additional_tool_names = {tool.name for tool in config.get("additional_tools") or []} - actionable_calls = [ - function_call for function_call in function_calls if _is_actionable_function_call(function_call) - ] - # Classify the entire batch first: any required user interaction pauses the batch before execution. # Scan every call before deciding so classification travels with each call, not with its position in # the batch. Priority (highest first): approval pause > declaration-only user-input > unknown-call # termination. A user-input pause therefore takes precedence over unknown-call termination in mixed batches. requires_approval = False has_declaration_only_call = False + unknown_call_found = False unknown_call_name: str | None = None - for function_call in actionable_calls: - function_name = function_call.name + for function_call in function_calls: + source_function_call = _underlying_function_call(function_call) + function_name = source_function_call.name logger.debug( "Checking function call: type=%s, name=%s, in approval_tools=%s", function_call.type, function_name, function_name in approval_tool_names, ) - if function_name in approval_tool_names: + if _is_actionable_function_call(function_call) and function_name in approval_tool_names: logger.debug("Approval needed for function: %s", function_name) requires_approval = True continue - if function_name in declaration_only_tool_names or function_name in additional_tool_names: + if _is_actionable_function_call(function_call) and ( + function_name in declaration_only_tool_names or function_name in additional_tool_names + ): has_declaration_only_call = True continue - if ( - unknown_call_name is None - and config.get("terminate_on_unknown_calls", False) - and function_name not in tool_map - ): + if not unknown_call_found and config.get("terminate_on_unknown_calls", False) and function_name not in tool_map: + unknown_call_found = True unknown_call_name = function_name # Defer unknown-call termination until the whole batch is classified so a higher-priority approval or # declaration-only pause anywhere in the batch is not skipped by an earlier unknown call. - if not requires_approval and not has_declaration_only_call and unknown_call_name is not None: + if not requires_approval and not has_declaration_only_call and unknown_call_found: raise KeyError(f'Error: Requested function "{unknown_call_name}" not found.') if requires_approval: # Surface only the approvals the host must decide; session-backed safe siblings wait for that resume. 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 a16e29834c..2c08a5e9b3 100644 --- a/python/packages/core/tests/core/test_function_invocation_logic.py +++ b/python/packages/core/tests/core/test_function_invocation_logic.py @@ -2895,6 +2895,26 @@ def approval_func(arg1: str) -> str: ] assert len(approval_requests) >= 1, "approval pause must take precedence over unknown-call termination" + approval_responses = [request.to_function_approval_response(approved=True) for request in approval_requests] + with pytest.raises(KeyError, match='Requested function "unknown_function" not found'): + await chat_client_base.get_response( + [Message(role="user", contents=approval_responses)], + options={"tool_choice": "auto", "tools": [approval_func]}, + ) + + +async def test_nameless_call_honors_unknown_call_termination(): + """A nameless call is still unknown and must terminate when configured to do so.""" + from agent_framework._tools import _try_execute_function_call_groups + + with pytest.raises(KeyError, match='Requested function "None" not found'): + await _try_execute_function_call_groups( + custom_args={}, + function_calls=[Content("function_call", call_id="nameless-call", arguments="{}")], + tools=[], + config={"terminate_on_unknown_calls": True}, + ) + async def test_function_invocation_config_additional_tools(chat_client_base: SupportsChatGetResponse): """Test that additional_tools are available but treated as declaration_only.""" From 97c3bf4af90a76e5c64e2213e3e660429a13a240 Mon Sep 17 00:00:00 2001 From: CorgiBoyG Date: Sat, 5 Sep 2026 11:11:17 +0800 Subject: [PATCH 3/4] fix: keep per-call classification inside approval-pausing batches An approval-required call forced the whole batch through the approval branch, which wrapped every sibling as an approval request. Two fail-open consequences followed: - A declaration-only sibling became an approval request, so approving the batch drove it into local execution on resume, where it raised because it has no implementation. Declaration-only and additional tools must surface as user input and never execute locally (spec 004), regardless of an approval-required sibling. - An unknown call in a batch configured to terminate was downgraded into a rejectable approval request, so a rejection or a dropped response silently skipped the fail-closed abort. terminate_on_unknown_calls is a security gate and must abort the batch before any approval is solicited. Classify each call individually inside the approval branch: surface declaration-only calls as user-input pauses alongside approval pauses, and raise unknown-call termination up front as an unconditional fail-closed gate. Adds regression coverage: mixed-batch approval-vs-user-input surfacing, an end-to-end approve-resume proving the declaration-only sibling is never executed, and unknown-call fail-closed precedence. --- .../packages/core/agent_framework/_tools.py | 42 ++++--- .../core/test_function_invocation_logic.py | 106 +++++++++++++++--- 2 files changed, 118 insertions(+), 30 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 116446f337..e852d66e1b 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -1806,9 +1806,12 @@ async def _try_execute_function_call_groups( if not unknown_call_found and config.get("terminate_on_unknown_calls", False) and function_name not in tool_map: unknown_call_found = True unknown_call_name = function_name - # Defer unknown-call termination until the whole batch is classified so a higher-priority approval or - # declaration-only pause anywhere in the batch is not skipped by an earlier unknown call. - if not requires_approval and not has_declaration_only_call and unknown_call_found: + # Fail-closed precedence: an unknown call in a batch configured to terminate aborts the whole batch + # before any approval is solicited or any sibling executes. unknown_call_found is only set when + # terminate_on_unknown_calls is enabled (see the scan above), so this never fires for tolerated unknown + # calls. Wrapping the unknown call as a rejectable approval request instead would let a rejection or a + # dropped response silently skip the abort, turning a fail-closed gate back into a running loop. + if unknown_call_found: raise KeyError(f'Error: Requested function "{unknown_call_name}" not found.') if requires_approval: # Surface only the approvals the host must decide; session-backed safe siblings wait for that resume. @@ -1816,24 +1819,29 @@ async def _try_execute_function_call_groups( logger.debug("Returning visible function_approval_request contents and storing already-approved requests") visible_requests: list[Content] = [] already_approved_requests: list[Content] = [] + declaration_only_calls: list[Content] = [] for function_call in function_calls: if function_call.type != "function_call": continue + tool_name = function_call.name + # Declaration-only and additional tools are surfaced as user input and never executed locally + # (spec 004). Wrapping them as approval requests here made an "approve" decision drive them into + # local execution on resume, where they raise because they have no implementation. Classification + # must travel with each call even inside an approval-pausing batch. + if tool_name is not None and ( + tool_name in declaration_only_tool_names or tool_name in additional_tool_names + ): + function_call.user_input_request = True + if function_call.id is None: + function_call.id = function_call.call_id + declaration_only_calls.append(function_call) + continue approval_request = Content.from_function_approval_request( id=function_call.id or function_call.call_id, # type: ignore[arg-type] function_call=function_call, ) - tool_name = function_call.name - if tool_name is None: - visible_requests.append(approval_request) - continue - tool = tool_map.get(tool_name) - if ( - tool_name in approval_tool_names - or tool is None - or tool_name in declaration_only_tool_names - or tool_name in additional_tool_names - ): + tool = tool_map.get(tool_name) if tool_name is not None else None + if tool_name is None or tool_name in approval_tool_names or tool is None: visible_requests.append(approval_request) continue if invocation_session is None: @@ -1846,7 +1854,11 @@ async def _try_execute_function_call_groups( already_approved_requests, ) _store_pending_approval_requests(invocation_session, visible_requests) - return [[request] for request in visible_requests], False + # Surface approval pauses and declaration-only user-input pauses together so a mixed batch neither + # bypasses approval nor executes a declaration-only call. + pause_groups: list[list[Content]] = [[request] for request in visible_requests] + pause_groups.extend([call] for call in declaration_only_calls) + return pause_groups, False if has_declaration_only_call: # Declaration-only calls are returned as user input rather than executed locally. # return the declaration only tools to the user, since we cannot execute them. 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 2c08a5e9b3..98b413a138 100644 --- a/python/packages/core/tests/core/test_function_invocation_logic.py +++ b/python/packages/core/tests/core/test_function_invocation_logic.py @@ -2857,15 +2857,34 @@ def approval_func(arg1: str) -> str: if content.type == "function_approval_request" and content.function_call.name == "approval_func" ] assert len(approval_requests) == 1, "approval gate must be surfaced regardless of call order" + # The declaration-only sibling must be surfaced as user input, never wrapped as an approval request, + # so that an "approve" decision cannot drive it into local execution on resume (spec 004). + declaration_as_approval = [ + content + for msg in response.messages + for content in msg.contents + if content.type == "function_approval_request" and content.function_call.name == "declaration_func" + ] + assert not declaration_as_approval, "declaration-only call must not be wrapped as an approval request" + declaration_user_input = [ + content + for msg in response.messages + for content in msg.contents + if content.type == "function_call" and content.user_input_request and content.name == "declaration_func" + ] + assert len(declaration_user_input) == 1, "declaration-only call must be surfaced as user input" -async def test_mixed_batch_approval_takes_precedence_over_unknown_call_termination( +async def test_mixed_batch_unknown_call_fails_closed_before_approval( chat_client_base: SupportsChatGetResponse, ): - """An approval-required call anywhere in the batch must pause before an unknown call terminates it. + """With terminate_on_unknown_calls=True, an unknown call aborts the batch fail-closed before any + approval is solicited. - Regression: with terminate_on_unknown_calls=True, an unknown call appearing before an approval-required - call raised KeyError first, so the approval pause (higher priority) was never reached. + Regression: an earlier fix inverted this, letting an approval pause take precedence over unknown-call + termination. That downgraded the unknown call into a rejectable approval request, so rejecting it (or + dropping its response) silently skipped the fail-closed abort. terminate_on_unknown_calls is a security + gate and must win outright. """ @tool(name="approval_func", approval_mode="always_require") @@ -2886,21 +2905,78 @@ def approval_func(arg1: str) -> str: ChatResponse(messages=Message(role="assistant", contents=["done"])), ] - response = await chat_client_base.get_response( - [Message(role="user", contents=["hello"])], options={"tool_choice": "auto", "tools": [approval_func]} + with pytest.raises(KeyError, match='Requested function "unknown_function" not found'): + await chat_client_base.get_response( + [Message(role="user", contents=["hello"])], options={"tool_choice": "auto", "tools": [approval_func]} + ) + + +async def test_mixed_batch_declaration_only_not_executed_after_approval_resume( + chat_client_base: SupportsChatGetResponse, +): + """After approving a mixed batch, the declaration-only sibling must not be executed locally. + + Regression: the approval branch wrapped every call in the batch as an approval request, so approving + the batch drove the declaration-only call into local execution on resume, where it raised because it + has no implementation. A declaration-only call must surface as user input and never produce a local + result or error, regardless of an approval-required sibling in the same batch. + """ + from agent_framework import FunctionTool + + approval_calls = 0 + + @tool(name="approval_func", approval_mode="always_require") + def approval_func(arg1: str) -> str: + nonlocal approval_calls + approval_calls += 1 + return f"Approved {arg1}" + + declaration_func = FunctionTool( + name="declaration_func", + func=None, + description="A declaration-only function for testing", + input_model={"type": "object", "properties": {"arg1": {"type": "string"}}, "required": ["arg1"]}, ) - approval_requests = [ - content for msg in response.messages for content in msg.contents if content.type == "function_approval_request" + chat_client_base.run_responses = [ # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] + ChatResponse( + messages=Message( + role="assistant", + contents=[ + Content.from_function_call(call_id="d1", name="declaration_func", arguments='{"arg1": "y"}'), + Content.from_function_call(call_id="a1", name="approval_func", arguments='{"arg1": "x"}'), + ], + ) + ), + ChatResponse(messages=Message(role="assistant", contents=["done"])), ] - assert len(approval_requests) >= 1, "approval pause must take precedence over unknown-call termination" - approval_responses = [request.to_function_approval_response(approved=True) for request in approval_requests] - with pytest.raises(KeyError, match='Requested function "unknown_function" not found'): - await chat_client_base.get_response( - [Message(role="user", contents=approval_responses)], - options={"tool_choice": "auto", "tools": [approval_func]}, - ) + first_response = await chat_client_base.get_response( + [Message(role="user", contents=["hello"])], + options={"tool_choice": "auto", "tools": [approval_func, declaration_func]}, + ) + + approval_responses = [ + content.to_function_approval_response(approved=True) + for msg in first_response.messages + for content in msg.contents + if content.type == "function_approval_request" + ] + assert len(approval_responses) == 1, "only the approval-required call should surface an approval request" + + resumed_response = await chat_client_base.get_response( + [Message(role="user", contents=approval_responses)], + options={"tool_choice": "auto", "tools": [approval_func, declaration_func]}, + ) + + declaration_results = [ + content + for msg in resumed_response.messages + for content in msg.contents + if content.type == "function_result" and content.call_id == "d1" + ] + assert not declaration_results, "declaration-only call must not be executed locally on approval resume" + assert approval_calls == 1, "approved tool executes exactly once" async def test_nameless_call_honors_unknown_call_termination(): From f3f7a3c59ae1cb73f2c54f1578d4a0eef620a413 Mon Sep 17 00:00:00 2001 From: CorgiBoyG Date: Sat, 5 Sep 2026 11:34:46 +0800 Subject: [PATCH 4/4] docs: correct batch-classification precedence comment to match fail-closed behavior The scan comment still described the earlier ordering (approval > declaration-only > unknown-call termination, user-input winning over termination), which the implementation had already superseded: unknown-call termination is now an unconditional fail-closed gate that aborts the batch before any approval is solicited. Align the comment with the code so the security-relevant precedence is not misread. No behavior change. --- python/packages/core/agent_framework/_tools.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index e852d66e1b..8b31e96196 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -1777,10 +1777,13 @@ async def _try_execute_function_call_groups( ) declaration_only_tool_names = {tool_name for tool_name, tool in tool_map.items() if tool.declaration_only} additional_tool_names = {tool.name for tool in config.get("additional_tools") or []} - # Classify the entire batch first: any required user interaction pauses the batch before execution. - # Scan every call before deciding so classification travels with each call, not with its position in - # the batch. Priority (highest first): approval pause > declaration-only user-input > unknown-call - # termination. A user-input pause therefore takes precedence over unknown-call termination in mixed batches. + # Classify the entire batch first so classification travels with each call, not with its position in + # the batch. Scan every call before acting on any of it. Precedence (highest first): unknown-call + # termination > approval pause > declaration-only user-input. unknown-call termination is a fail-closed + # security gate: when terminate_on_unknown_calls is enabled, an unknown call aborts the whole batch up + # front, before any approval is solicited or any sibling executes, so it can never be downgraded into a + # rejectable approval request and slip past the abort on a rejection or a dropped response. Approval and + # declaration-only calls otherwise pause together (see the approval branch) so neither is bypassed. requires_approval = False has_declaration_only_call = False unknown_call_found = False