From 3557a4139707950a647849aa3c9bce44bcae5533 Mon Sep 17 00:00:00 2001 From: AnvitDevadiga Date: Thu, 17 Sep 2026 08:49:51 +0530 Subject: [PATCH] fix: enforce denied tool confirmations centrally --- .../flows/llm_flows/tools/_confirmation.py | 52 +++++++++++++++---- .../llm_flows/tools/test_confirmation.py | 32 +++--------- .../runners/test_run_tool_confirmation.py | 14 +++-- 3 files changed, 60 insertions(+), 38 deletions(-) diff --git a/src/google/adk/flows/llm_flows/tools/_confirmation.py b/src/google/adk/flows/llm_flows/tools/_confirmation.py index 80a98b87258..133d3b269b3 100644 --- a/src/google/adk/flows/llm_flows/tools/_confirmation.py +++ b/src/google/adk/flows/llm_flows/tools/_confirmation.py @@ -354,17 +354,51 @@ async def run_async( if not tools_to_resume_with_confirmation: return - # Step 4: Re-execute the confirmed tools. + # Step 4: Re-execute only confirmed tools. Denials are handled here at the + # framework boundary so custom BaseTool implementations cannot accidentally + # perform a side effect after the user declines confirmation. from .. import functions - if function_response_event := await functions.handle_function_call_list_async( - invocation_context, - list(tools_to_resume_with_args.values()), - tools_dict, - set(tools_to_resume_with_confirmation.keys()), - tools_to_resume_with_confirmation, - ): - yield function_response_event + denied_parts: list[types.Part] = [] + confirmed_args: list[types.FunctionCall] = [] + confirmed_ids: set[str] = set() + confirmed_tools: dict[str, ToolConfirmation] = {} + for function_call_id, function_call in tools_to_resume_with_args.items(): + confirmation = tools_to_resume_with_confirmation[function_call_id] + if confirmation.confirmed: + confirmed_args.append(function_call) + confirmed_ids.add(function_call_id) + confirmed_tools[function_call_id] = confirmation + else: + denied_parts.append( + types.Part( + function_response=types.FunctionResponse( + name=function_call.name, + id=function_call_id, + response={"error": "Tool execution not confirmed"}, + ) + ) + ) + + if confirmed_args: + if function_response_event := await functions.handle_function_call_list_async( + invocation_context, + confirmed_args, + tools_dict, + confirmed_ids, + confirmed_tools, + ): + denied_parts.extend(function_response_event.content.parts) + yield function_response_event.model_copy(update={ + "content": types.Content(parts=denied_parts) + }) + return + + if denied_parts: + yield Event( + author=invocation_context.agent.name if invocation_context.agent else "agent", + content=types.Content(parts=denied_parts), + ) return diff --git a/tests/unittests/flows/llm_flows/tools/test_confirmation.py b/tests/unittests/flows/llm_flows/tools/test_confirmation.py index 0acd73915bf..65ae514ca78 100644 --- a/tests/unittests/flows/llm_flows/tools/test_confirmation.py +++ b/tests/unittests/flows/llm_flows/tools/test_confirmation.py @@ -301,21 +301,6 @@ async def test_request_confirmation_processor_tool_not_confirmed(): with patch( "google.adk.flows.llm_flows.functions.handle_function_call_list_async" ) as mock_handle_function_call_list_async: - mock_handle_function_call_list_async.return_value = Event( - author="agent", - content=types.Content( - parts=[ - types.Part( - function_response=types.FunctionResponse( - name=MOCK_TOOL_NAME, - id=MOCK_FUNCTION_CALL_ID, - response={"error": "Tool execution not confirmed"}, - ) - ) - ] - ), - ) - events = [] async for event in request_processor.run_async( invocation_context, llm_request @@ -323,11 +308,10 @@ async def test_request_confirmation_processor_tool_not_confirmed(): events.append(event) assert len(events) == 1 - mock_handle_function_call_list_async.assert_called_once() - args, _ = mock_handle_function_call_list_async.call_args - assert ( - args[4][MOCK_FUNCTION_CALL_ID] == user_confirmation - ) # tool_confirmation_dict + mock_handle_function_call_list_async.assert_not_called() + assert events[0].content.parts[0].function_response.response == { + "error": "Tool execution not confirmed" + } TRANSFER_TOOL_NAME = "transfer_to_agent" @@ -509,10 +493,10 @@ async def test_request_confirmation_transfer_to_agent_rejected(): events.append(event) assert len(events) == 1 - mock_handle.assert_called_once() - args, _ = mock_handle.call_args - tools_dict = args[2] - assert TRANSFER_TOOL_NAME in tools_dict + mock_handle.assert_not_called() + assert events[0].content.parts[0].function_response.response == { + "error": "Tool execution not confirmed" + } @pytest.mark.asyncio diff --git a/tests/unittests/runners/test_run_tool_confirmation.py b/tests/unittests/runners/test_run_tool_confirmation.py index 005fb98b5af..7dc383bce33 100644 --- a/tests/unittests/runners/test_run_tool_confirmation.py +++ b/tests/unittests/runners/test_run_tool_confirmation.py @@ -211,7 +211,7 @@ async def test_confirmation_flow( name=tools[0].name, response={"result": f"confirmed={tool_call_confirmed}"} if tool_call_confirmed - else {"error": "This tool call is rejected."}, + else {"error": "Tool execution not confirmed"}, ) ), ), @@ -352,10 +352,14 @@ async def test_confirmation_flow( ) events = await runner.run_async(user_confirmation) - expected_response = { - "result": f"confirmed={tool_call_confirmed}", - "custom_payload": custom_payload, - } + expected_response = ( + { + "result": "confirmed=True", + "custom_payload": custom_payload, + } + if tool_call_confirmed + else {"error": "Tool execution not confirmed"} + ) expected_parts_final = [ ( agent.name,