Python: fix(ag-ui): Execute tools with approval_mode, fix shared state, code cleanup

python/packages/ag-ui/agent_framework_ag_ui/_message_adapters.py

@@ -2,6 +2,7 @@
 
 """Message format conversion between AG-UI and Agent Framework."""
 
+import json
 from typing import Any, cast
 
 from agent_framework import (
@@ -59,21 +60,65 @@ def agui_messages_to_agent_framework(messages: list[dict[str, Any]]) -> list[Cha
             # Distinguish approval payloads from actual tool results
             is_approval = False
             if isinstance(result_content, str) and result_content:
-                import json as _json
-
                 try:
-                    parsed = _json.loads(result_content)
+                    parsed = json.loads(result_content)
                     is_approval = isinstance(parsed, dict) and "accepted" in parsed
                 except Exception:
                     is_approval = False
 
             if is_approval:
-                # Approval responses should be treated as user messages to trigger human-in-the-loop flow
-                chat_msg = ChatMessage(
-                    role=Role.USER,
-                    contents=[TextContent(text=str(result_content))],
-                    additional_properties={"is_tool_result": True, "tool_call_id": str(tool_call_id or "")},
-                )
+                # Look for the matching function call in previous messages to create
+                # a proper FunctionApprovalResponseContent. This enables the agent framework
+                # to execute the approved tool (fix for GitHub issue #3034).
+                parsed_approval = json.loads(result_content)
+                accepted = parsed_approval.get("accepted", False)
+
+                # Find the function call that matches this tool_call_id
+                matching_func_call = None
+                for prev_msg in result:
+                    role_val = prev_msg.role.value if hasattr(prev_msg.role, "value") else str(prev_msg.role)
+                    if role_val != "assistant":
+                        continue
+                    for content in prev_msg.contents or []:
+                        if isinstance(content, FunctionCallContent):
+                            if content.call_id == tool_call_id and content.name != "confirm_changes":
+                                matching_func_call = content
+                                break
+
+                if matching_func_call:
+                    # Remove any existing tool result for this call_id since the framework
+                    # will re-execute the tool after approval. Keeping old results causes
+                    # OpenAI API errors ("tool message must follow assistant with tool_calls").
+                    result = [
+                        m
+                        for m in result
+                        if not (
+                            (m.role.value if hasattr(m.role, "value") else str(m.role)) == "tool"
+                            and any(
+                                isinstance(c, FunctionResultContent) and c.call_id == tool_call_id
+                                for c in (m.contents or [])
+                            )
+                        )
+                    ]
+
+                    # Create FunctionApprovalResponseContent for the agent framework
+                    approval_response = FunctionApprovalResponseContent(
+                        approved=accepted,
+                        id=str(tool_call_id),
+                        function_call=matching_func_call,
+                    )
+                    chat_msg = ChatMessage(
+                        role=Role.USER,
+                        contents=[approval_response],
+                    )
+                else:
+                    # No matching function call found - this is likely a confirm_changes approval
+                    # Keep the old behavior for backwards compatibility
+                    chat_msg = ChatMessage(
+                        role=Role.USER,
+                        contents=[TextContent(text=str(result_content))],
+                        additional_properties={"is_tool_result": True, "tool_call_id": str(tool_call_id or "")},
+                    )
                 if "id" in msg:
                     chat_msg.message_id = msg["id"]
                 result.append(chat_msg)

python/packages/ag-ui/agent_framework_ag_ui/_orchestration/_message_hygiene.py

@@ -6,7 +6,13 @@
 import logging
 from typing import Any
 
-from agent_framework import ChatMessage, FunctionCallContent, FunctionResultContent, TextContent
+from agent_framework import (
+    ChatMessage,
+    FunctionApprovalResponseContent,
+    FunctionCallContent,
+    FunctionResultContent,
+    TextContent,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -40,6 +46,22 @@ def sanitize_tool_history(messages: list[ChatMessage]) -> list[ChatMessage]:
             continue
 
         if role_value == "user":
+            # Check if this message contains FunctionApprovalResponseContent
+            # If so, the framework will handle tool execution - don't inject synthetic results
+            approval_call_ids: set[str] = set()
+            for content in msg.contents or []:
+                if isinstance(content, FunctionApprovalResponseContent):
+                    if content.function_call and content.function_call.call_id:
+                        approval_call_ids.add(str(content.function_call.call_id))
+
+            if approval_call_ids and pending_tool_call_ids:
+                # Remove approved call_ids from pending - the framework will execute them
+                pending_tool_call_ids -= approval_call_ids
+                logger.info(
+                    f"FunctionApprovalResponseContent found for call_ids={approval_call_ids} - "
+                    "framework will handle execution"
+                )
+
             if pending_confirm_changes_id:
                 user_text = ""
                 for content in msg.contents or []:

python/packages/ag-ui/tests/test_agent_wrapper_comprehensive.py

@@ -630,3 +630,181 @@ async def stream_fn(
     # Should contain some reference to the document
     full_text = "".join(e.delta for e in text_events)
     assert "written" in full_text.lower() or "document" in full_text.lower()
+
+
+async def test_function_approval_mode_executes_tool():
+    """Test that function approval with approval_mode='always_require' sends the correct messages."""
+    from agent_framework import FunctionApprovalResponseContent, ai_function
+    from agent_framework.ag_ui import AgentFrameworkAgent
+
+    messages_received: list[Any] = []
+
+    @ai_function(
+        name="get_datetime",
+        description="Get the current date and time",
+        approval_mode="always_require",
+    )
+    def get_datetime() -> str:
+        return "2025/12/01 12:00:00"
+
+    async def stream_fn(
+        messages: MutableSequence[ChatMessage], chat_options: ChatOptions, **kwargs: Any
+    ) -> AsyncIterator[ChatResponseUpdate]:
+        # Capture the messages received by the chat client
+        messages_received.clear()
+        messages_received.extend(messages)
+        yield ChatResponseUpdate(contents=[TextContent(text="Processing completed")])
+
+    agent = ChatAgent(
+        name="test_agent",
+        instructions="Test",
+        chat_client=StreamingChatClientStub(stream_fn),
+        tools=[get_datetime],
+    )
+    wrapper = AgentFrameworkAgent(agent=agent)
+
+    # Simulate the conversation history with:
+    # 1. User message asking for time
+    # 2. Assistant message with the function call that needs approval
+    # 3. Tool approval message from user
+    tool_result: dict[str, Any] = {"accepted": True}
+    input_data: dict[str, Any] = {
+        "messages": [
+            {
+                "role": "user",
+                "content": "What time is it?",
+            },
+            {
+                "role": "assistant",
+                "content": "",
+                "tool_calls": [
+                    {
+                        "id": "call_get_datetime_123",
+                        "type": "function",
+                        "function": {
+                            "name": "get_datetime",
+                            "arguments": "{}",
+                        },
+                    }
+                ],
+            },
+            {
+                "role": "tool",
+                "content": json.dumps(tool_result),
+                "toolCallId": "call_get_datetime_123",
+            },
+        ],
+    }
+
+    events: list[Any] = []
+    async for event in wrapper.run_agent(input_data):
+        events.append(event)
+
+    # Verify the run completed successfully
+    run_started = [e for e in events if e.type == "RUN_STARTED"]
+    run_finished = [e for e in events if e.type == "RUN_FINISHED"]
+    assert len(run_started) == 1
+    assert len(run_finished) == 1
+
+    # Verify that a FunctionApprovalResponseContent was created and sent to the agent
+    # This is the key fix - the orchestrator should create an approval response
+    approval_responses_found = False
+    for msg in messages_received:
+        for content in msg.contents:
+            if isinstance(content, FunctionApprovalResponseContent):
+                approval_responses_found = True
+                assert content.approved is True
+                assert content.function_call.name == "get_datetime"
+                assert content.function_call.call_id == "call_get_datetime_123"
+                break
+
+    assert approval_responses_found, (
+        "FunctionApprovalResponseContent should be included in messages sent to agent. "
+        "This is required for the agent framework to execute the approved function."
+    )
+
+
+async def test_function_approval_mode_rejection():
+    """Test that function approval rejection creates a rejection response."""
+    from agent_framework import FunctionApprovalResponseContent, ai_function
+    from agent_framework.ag_ui import AgentFrameworkAgent
+
+    messages_received: list[Any] = []
+
+    @ai_function(
+        name="delete_all_data",
+        description="Delete all user data",
+        approval_mode="always_require",
+    )
+    def delete_all_data() -> str:
+        return "All data deleted"
+
+    async def stream_fn(
+        messages: MutableSequence[ChatMessage], chat_options: ChatOptions, **kwargs: Any
+    ) -> AsyncIterator[ChatResponseUpdate]:
+        # Capture the messages received by the chat client
+        messages_received.clear()
+        messages_received.extend(messages)
+        yield ChatResponseUpdate(contents=[TextContent(text="Operation cancelled")])
+
+    agent = ChatAgent(
+        name="test_agent",
+        instructions="Test",
+        chat_client=StreamingChatClientStub(stream_fn),
+        tools=[delete_all_data],
+    )
+    wrapper = AgentFrameworkAgent(agent=agent)
+
+    # Simulate rejection
+    tool_result: dict[str, Any] = {"accepted": False}
+    input_data: dict[str, Any] = {
+        "messages": [
+            {
+                "role": "user",
+                "content": "Delete all my data",
+            },
+            {
+                "role": "assistant",
+                "content": "",
+                "tool_calls": [
+                    {
+                        "id": "call_delete_123",
+                        "type": "function",
+                        "function": {
+                            "name": "delete_all_data",
+                            "arguments": "{}",
+                        },
+                    }
+                ],
+            },
+            {
+                "role": "tool",
+                "content": json.dumps(tool_result),
+                "toolCallId": "call_delete_123",
+            },
+        ],
+    }
+
+    events: list[Any] = []
+    async for event in wrapper.run_agent(input_data):
+        events.append(event)
+
+    # Verify the run completed
+    run_finished = [e for e in events if e.type == "RUN_FINISHED"]
+    assert len(run_finished) == 1
+
+    # Verify that a FunctionApprovalResponseContent with approved=False was created
+    rejection_found = False
+    for msg in messages_received:
+        for content in msg.contents:
+            if isinstance(content, FunctionApprovalResponseContent):
+                rejection_found = True
+                assert content.approved is False
+                assert content.function_call.name == "delete_all_data"
+                assert content.function_call.call_id == "call_delete_123"
+                break
+
+    assert rejection_found, (
+        "FunctionApprovalResponseContent with approved=False should be included in messages sent to agent. "
+        "This tells the agent framework that the tool was rejected."
+    )