From e6632d51b674d7358d60c25d1251cabf4c2c42f0 Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Wed, 29 Jul 2026 14:11:27 +0200 Subject: [PATCH 1/2] Python: Defer provider-injected approvals to in-run execution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1 --- .../specs/004-python-function-calling-loop.md | 3 +- python/packages/ag-ui/AGENTS.md | 2 + .../ag-ui/agent_framework_ag_ui/_agent_run.py | 21 +- .../ag-ui/tests/ag_ui/test_endpoint.py | 196 ++++++++++++++++++ 4 files changed, 216 insertions(+), 6 deletions(-) diff --git a/docs/specs/004-python-function-calling-loop.md b/docs/specs/004-python-function-calling-loop.md index a0f04f63c6..188d7a2182 100644 --- a/docs/specs/004-python-function-calling-loop.md +++ b/docs/specs/004-python-function-calling-loop.md @@ -438,6 +438,7 @@ that manually replay messages own the equivalent rule: do not resend an approval | Standing tool rule | Tool-level approval applies only to later matching tools. | `test_tool_approval_middleware_always_approve_tool_rule` | | Hosted server boundary | Standing approval does not cross `server_label`. | `test_tool_approval_middleware_standing_rules_include_hosted_server_boundary` | | Argument-scoped rule | Exact arguments are required; empty arguments are not tool-wide. | `test_tool_approval_middleware_always_approve_tool_with_arguments_rule`, `test_tool_approval_middleware_empty_arguments_rule_is_not_tool_wide` | +| Provider-injected approval tool | A tool added during `before_run` defers to in-run resolution, executes once, and emits one result. | `packages/ag-ui/tests/ag_ui/test_endpoint.py::test_endpoint_agent_approval_deferred_provider_tool_executes` | ### Errors, control flow, and limits @@ -482,7 +483,6 @@ These scenarios are required but are not fully covered by merged tests on `main` | Gap | Tracking | |---|---| | Non-adjacent and reused-id call/result occurrences remain atomic during compaction. | #7212 | -| Provider-injected approval-required tools defer until `before_run` tools exist and still emit one result. | #7043 | | Service-side storage sends the current approval response while omitting the stored request. | #7125 | | Service-owned `previous_response_id` continuation cannot execute a terminal approval again on a later turn. | #6851 | | A provider that ignores `tool_choice="none"` after an invocation limit cannot expose an unanswered call. | #7045 | @@ -536,6 +536,7 @@ Before accepting an update, reviewers must confirm: - #7241 — approval-resolution result streaming - #7267 / #7271 and #7304 — replayed calls and reused ids +- #7043 — provider-injected approval execution - #6851 — duplicate side effects after approval continuation - #7383 — bind approval responses to framework-issued requests after this foundation merges - #6963 / #7095 — opaque reasoning-signature replay diff --git a/python/packages/ag-ui/AGENTS.md b/python/packages/ag-ui/AGENTS.md index 20d852b148..65bdc6aff1 100644 --- a/python/packages/ag-ui/AGENTS.md +++ b/python/packages/ag-ui/AGENTS.md @@ -31,6 +31,8 @@ AG-UI protocol integration for building agent UIs with the AG-UI standard. - `Interrupt` and `ResumeEntry` come from the `ag-ui-protocol` package (`ag_ui.core`), not from an Agent Framework-specific interrupt model. - Approval-time execution preserves each call's complete result group. Follow-up user-input requests remain in the resumed messages, while `TOOL_CALL_RESULT` events are emitted only for terminal `function_result` contents. +- Approval responses for tools injected during `before_run` are deferred to the in-run approval middleware rather + than executed or rejected by the transport before those tools exist. - SSE keepalive is endpoint-owned transport behavior configured through `add_agent_framework_fastapi_endpoint(keepalive_seconds=...)`. It emits SSE comments only; do not add `PING`, `HEARTBEAT`, or `KEEPALIVE` AG-UI events, and do not add runner-level keepalive settings. diff --git a/python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py b/python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py index cf99489f0d..3600396a17 100644 --- a/python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py +++ b/python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py @@ -40,6 +40,7 @@ from agent_framework._tools import ( _ALREADY_APPROVED_APPROVAL_REQUEST_GROUPS_KEY, # type: ignore _collect_approval_responses, # type: ignore + _get_tool_map, # type: ignore _replace_approval_contents_with_results, # type: ignore _TOOL_APPROVAL_STATE_KEY, # type: ignore _try_execute_function_call_groups, # type: ignore @@ -1308,8 +1309,17 @@ async def _resolve_approval_responses( approved_function_result_groups: list[list[Content]] = [] - # Execute approved tool calls - if approved_responses and tools: + # Partition approved responses into static (execute now) and deferred (execute during run) + tool_map = _get_tool_map(tools) if tools else {} + static_approved: list[Content] = [] + + for approval in approved_responses: + tool_name = approval.function_call.name if approval.function_call else None + if tool_name in tool_map: + static_approved.append(approval) + + # Execute only statically-available approved tool calls + if static_approved and tools: client = getattr(agent, "client", None) config = normalize_function_invocation_configuration(getattr(client, "function_invocation_configuration", None)) middleware_pipeline = FunctionMiddlewarePipeline( @@ -1321,7 +1331,7 @@ async def _resolve_approval_responses( try: approved_function_result_groups, _ = await _try_execute_function_call_groups( custom_args=tool_kwargs, - function_calls=approved_responses, + function_calls=static_approved, tools=tools, middleware_pipeline=middleware_pipeline, config=config, @@ -1330,10 +1340,11 @@ async def _resolve_approval_responses( logger.exception("Failed to execute approved tool calls; injecting error results: %s", e) approved_function_result_groups = [] - # Normalize one group per approval and collect only terminal results for TOOL_CALL_RESULT events. + # Normalize one group per static approval and collect only terminal results for TOOL_CALL_RESULT events. + # Deferred provider-injected approvals are left in messages for ToolApprovalMiddleware to process. replacement_groups: list[list[Content]] = [] approved_results: list[Content] = [] - for idx, approval in enumerate(approved_responses): + for idx, approval in enumerate(static_approved): result_group = approved_function_result_groups[idx] if idx < len(approved_function_result_groups) else [] if not result_group: func_call = approval.function_call diff --git a/python/packages/ag-ui/tests/ag_ui/test_endpoint.py b/python/packages/ag-ui/tests/ag_ui/test_endpoint.py index 303a307d91..8c9ae87b06 100644 --- a/python/packages/ag-ui/tests/ag_ui/test_endpoint.py +++ b/python/packages/ag-ui/tests/ag_ui/test_endpoint.py @@ -5233,3 +5233,199 @@ def resolve_scope(request: AGUIRequest) -> str: runner._resolve_workflow("thread-1", "tenant-b") # pyright: ignore[reportPrivateUsage] is created_workflows[1] ) + + +async def test_endpoint_agent_approval_defers_provider_injected_tools() -> None: + """Approving provider-injected tools should not produce rejection errors. + + When approval responses include tools not in the static tool map, + they should be deferred (not executed by transport) rather than + producing "Tool call invocation was rejected" errors. + + This prevents the scenario where approval mode loops with tool invocation failures. + Fixes issue #7043. + """ + executed_tools: list[str] = [] + + def static_tool() -> str: + executed_tools.append("static") + return "static result" + + # Create approval responses: one for static tool, one for provider tool (not in static list) + static_approval = Content.from_function_approval_response( + id="static_id", + approved=True, + function_call=Content.from_function_call( + call_id="call_static", + name="static_tool", + arguments="{}", + ), + ) + provider_approval = Content.from_function_approval_response( + id="provider_id", + approved=True, + function_call=Content.from_function_call( + call_id="call_provider", + name="provider_file_access", # Not in agent's tool list + arguments="{}", + ), + ) + + # Stub agent that returns the approval responses embedded in a message + agent = StubAgent( + updates=[ + AgentResponseUpdate(contents=[static_approval, provider_approval], role="user"), + ] + ) + + app = FastAPI() + wrapped_agent = AgentFrameworkAgent(agent=agent, require_confirmation=False) + add_agent_framework_fastapi_endpoint(app, wrapped_agent, path="/approval") + client = TestClient(app) + + # Submit approval responses (static and provider-injected) + response = client.post( + "/approval", + json={ + "runId": "run-approval", + "threadId": "thread-approval", + "messages": [ + { + "role": "user", + "content": [ + { + "type": "function_approval_response", + "id": "static_id", + "approved": True, + "function_call": { + "call_id": "call_static", + "name": "static_tool", + "arguments": "{}", + }, + }, + { + "type": "function_approval_response", + "id": "provider_id", + "approved": True, + "function_call": { + "call_id": "call_provider", + "name": "provider_file_access", + "arguments": "{}", + }, + }, + ], + } + ], + }, + ) + + assert response.status_code == 200 + events = _decode_sse_events(response) + response_text = json.dumps(events) + + # Verify no rejection error for provider tool + assert "Tool call invocation was rejected" not in response_text, ( + "Deferred provider tool should not produce rejection error" + ) + + +async def test_endpoint_agent_approval_deferred_provider_tool_executes(streaming_chat_client_stub) -> None: + """A provider-injected tool approved via AG-UI executes in-run instead of being rejected. + + Regression for #7043. A tool registered by a context provider during ``before_run`` is + absent from the transport's static tool map, so ``_resolve_approval_responses`` must defer + it (not execute or reject it) and leave it for the in-run ``ToolApprovalMiddleware`` to run. + This drives the full pause -> approve -> resume flow with a real provider-injected tool and + asserts the approved side effect actually happens without any rejection/failure result. + + The deferred tool result must still be returned to AG-UI exactly once. + """ + side_effects: list[str] = [] + state = {"phase": "pause"} + + def provider_write() -> str: + side_effects.append("wrote") + return "wrote to disk" + + provider_tool = FunctionTool( + name="provider_write", + description="Write to disk (provider-injected)", + func=provider_write, + approval_mode="always_require", + ) + + class ToolInjectingProvider(ContextProvider): + """Registers a tool during before_run, mimicking FileAccessProvider/CodeInterpreterProvider.""" + + async def before_run(self, *, agent, session, context, state) -> None: # type: ignore[override] # pyrefly: ignore # ty: ignore + del agent, session, state + context.extend_tools(self.source_id, [provider_tool]) + + async def stream_fn( + messages: list[Message], + options: dict[str, Any], + **kwargs: Any, + ) -> AsyncIterator[ChatResponseUpdate]: + del options, kwargs + if state["phase"] == "pause": + yield ChatResponseUpdate( + contents=[Content.from_function_call(call_id="call_provider", name="provider_write", arguments="{}")], + role="assistant", + ) + return + yield ChatResponseUpdate(contents=[Content.from_text(text="Done.")], role="assistant") + + # provider_write is intentionally NOT in the static tools list -- it is only injected via before_run. + agent = Agent( + name="test_agent", + instructions="Test", + client=streaming_chat_client_stub(stream_fn), + tools=[], + middleware=[ToolApprovalMiddleware()], + context_providers=[ToolInjectingProvider(source_id="tool_injector")], + ) + app = FastAPI() + add_agent_framework_fastapi_endpoint( + app, + AgentFrameworkAgent(agent=agent, require_confirmation=False), + path="/approval", + ) + client = TestClient(app) + + # Pause: the harness surfaces the provider-injected tool for approval, nothing executes yet. + pause_response = client.post( + "/approval", + json={ + "runId": "run-pause", + "threadId": "thread-provider", + "messages": [{"role": "user", "content": "Write something"}], + }, + ) + assert pause_response.status_code == 200 + pause_finished = [event for event in _decode_sse_events(pause_response) if event.get("type") == "RUN_FINISHED"] + assert [interrupt["id"] for interrupt in _run_finished_interrupts(pause_finished[-1])] == ["call_provider"] + assert side_effects == [] + + # Resume with approval: the deferred provider tool runs during agent.run. + state["phase"] = "resume" + resume_response = client.post( + "/approval", + json={ + "runId": "run-resume", + "threadId": "thread-provider", + "messages": [], + "resume": [{"interruptId": "call_provider", "status": "resolved", "payload": {"accepted": True}}], + }, + ) + assert resume_response.status_code == 200 + resume_events = _decode_sse_events(resume_response) + resume_text = json.dumps(resume_events) + + # The approved provider tool actually executed -- its side effect fired. + assert side_effects == ["wrote"] + tool_results = [event for event in resume_events if event.get("type") == "TOOL_CALL_RESULT"] + assert [(event["toolCallId"], event["content"]) for event in tool_results] == [("call_provider", "wrote to disk")] + # And it was neither rejected nor reported as a transport failure (the #7043 bug). + assert "Tool call invocation was rejected" not in resume_text + assert "Tool call invocation failed" not in resume_text + assert not [event for event in resume_events if event.get("type") == "RUN_ERROR"] From f986e972010f6346b5abf510502a63199683021a Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Thu, 30 Jul 2026 07:32:56 +0200 Subject: [PATCH 2/2] Python: Remove vacuous AG-UI approval test Drop the forged-approval test that was stripped by pending-approval validation; the real pause-approve-resume regression remains the authoritative provider-injected coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1 --- .../ag-ui/tests/ag_ui/test_endpoint.py | 94 ------------------- 1 file changed, 94 deletions(-) diff --git a/python/packages/ag-ui/tests/ag_ui/test_endpoint.py b/python/packages/ag-ui/tests/ag_ui/test_endpoint.py index 8c9ae87b06..0408de841d 100644 --- a/python/packages/ag-ui/tests/ag_ui/test_endpoint.py +++ b/python/packages/ag-ui/tests/ag_ui/test_endpoint.py @@ -5235,100 +5235,6 @@ def resolve_scope(request: AGUIRequest) -> str: ) -async def test_endpoint_agent_approval_defers_provider_injected_tools() -> None: - """Approving provider-injected tools should not produce rejection errors. - - When approval responses include tools not in the static tool map, - they should be deferred (not executed by transport) rather than - producing "Tool call invocation was rejected" errors. - - This prevents the scenario where approval mode loops with tool invocation failures. - Fixes issue #7043. - """ - executed_tools: list[str] = [] - - def static_tool() -> str: - executed_tools.append("static") - return "static result" - - # Create approval responses: one for static tool, one for provider tool (not in static list) - static_approval = Content.from_function_approval_response( - id="static_id", - approved=True, - function_call=Content.from_function_call( - call_id="call_static", - name="static_tool", - arguments="{}", - ), - ) - provider_approval = Content.from_function_approval_response( - id="provider_id", - approved=True, - function_call=Content.from_function_call( - call_id="call_provider", - name="provider_file_access", # Not in agent's tool list - arguments="{}", - ), - ) - - # Stub agent that returns the approval responses embedded in a message - agent = StubAgent( - updates=[ - AgentResponseUpdate(contents=[static_approval, provider_approval], role="user"), - ] - ) - - app = FastAPI() - wrapped_agent = AgentFrameworkAgent(agent=agent, require_confirmation=False) - add_agent_framework_fastapi_endpoint(app, wrapped_agent, path="/approval") - client = TestClient(app) - - # Submit approval responses (static and provider-injected) - response = client.post( - "/approval", - json={ - "runId": "run-approval", - "threadId": "thread-approval", - "messages": [ - { - "role": "user", - "content": [ - { - "type": "function_approval_response", - "id": "static_id", - "approved": True, - "function_call": { - "call_id": "call_static", - "name": "static_tool", - "arguments": "{}", - }, - }, - { - "type": "function_approval_response", - "id": "provider_id", - "approved": True, - "function_call": { - "call_id": "call_provider", - "name": "provider_file_access", - "arguments": "{}", - }, - }, - ], - } - ], - }, - ) - - assert response.status_code == 200 - events = _decode_sse_events(response) - response_text = json.dumps(events) - - # Verify no rejection error for provider tool - assert "Tool call invocation was rejected" not in response_text, ( - "Deferred provider tool should not produce rejection error" - ) - - async def test_endpoint_agent_approval_deferred_provider_tool_executes(streaming_chat_client_stub) -> None: """A provider-injected tool approved via AG-UI executes in-run instead of being rejected.