From a300fe86b6118ad77facd0f6f740747867458e7c Mon Sep 17 00:00:00 2001 From: tonydzi Date: Sun, 9 Aug 2026 13:09:32 +0100 Subject: [PATCH] Python: bind tool-approval responses to surfaced approval requests Mirrors the .NET behavior from #7111 for the Python core, closing the gap tracked in #7383: an inbound function_approval_response previously executed whatever function_call it carried, so an edited or replayed response could execute a call that was never surfaced for approval. - surfaced approval requests are recorded in the session tool-approval state bag when the batch pauses (same gating as the existing already-approved-siblings mechanism) - on resume, each decision is bound back to its recorded request: the recorded function call is executed, a differing embedded call is logged and ignored - records are one-time-use: consumed by the first decision, approved or rejected - sessionless flows are unchanged (nothing recorded, responses pass through as before) Tests: two new cases in test_harness_tool_approval.py (edited response executes the surfaced call; record consumed on decision). Full runs of test_harness_tool_approval.py, test_function_invocation_logic.py, test_sessions.py and test_security.py: 475 passed, 4 pre-existing environment failures (aiohttp/mcp deps) that also fail on clean main. Assisted-by: Claude (Anthropic) --- .../packages/core/agent_framework/_tools.py | 83 ++++++++++++++++++- .../tests/core/test_harness_tool_approval.py | 80 ++++++++++++++++++ 2 files changed, 161 insertions(+), 2 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index e7760c9cbd..f0be51e20f 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -97,6 +97,7 @@ SHELL_TOOL_KIND_VALUE: Final[str] = "shell" _TOOL_APPROVAL_STATE_KEY: Final[str] = "tool_approval" _ALREADY_APPROVED_APPROVAL_REQUEST_GROUPS_KEY: Final[str] = "already_approved_approval_request_groups" +_SURFACED_APPROVAL_REQUESTS_KEY: Final[str] = "surfaced_approval_requests" _FUNCTION_INVOCATION_BUDGET_STATE_KEY: Final[str] = "_function_invocation_budget_state" _FUNCTION_INVOCATION_LIMIT_FALLBACK_TEXT: Final[str] = ( "Function invocation limit reached before a final answer could be produced." @@ -1813,6 +1814,7 @@ async def _try_execute_function_call_groups( visible_requests, already_approved_requests, ) + _store_surfaced_approval_requests(invocation_session, visible_requests) return [[request] for request in visible_requests], False if has_declaration_only_call: # Declaration-only calls are returned as user input rather than executed locally. @@ -2117,6 +2119,78 @@ def _store_already_approved_approval_requests( state[_ALREADY_APPROVED_APPROVAL_REQUEST_GROUPS_KEY] = pending_groups +def _store_surfaced_approval_requests( + invocation_session: AgentSession | None, + visible_approval_requests: Sequence[Content], +) -> None: + """Record surfaced approval requests so inbound decisions can be bound back to them.""" + state = _get_tool_approval_state(invocation_session) + if state is None: + return + raw_surfaced = state.get(_SURFACED_APPROVAL_REQUESTS_KEY) + surfaced_requests = dict(cast(dict[str, Any], raw_surfaced)) if isinstance(raw_surfaced, dict) else {} + for request in visible_approval_requests: + if request.id and request.function_call is not None: + surfaced_requests[request.id] = request.function_call.to_dict() + if surfaced_requests: + state[_SURFACED_APPROVAL_REQUESTS_KEY] = surfaced_requests + + +def _bind_approval_responses_to_surfaced_requests( + invocation_session: AgentSession | None, + approval_responses: Sequence[Content], +) -> list[Content]: + """Bind inbound approval decisions to the surfaced requests they answer, consuming each record. + + An approval response is a decision token, not a work order: what executes after an + approval must be the function call the host actually saw when it decided. For + session-backed runs the surfaced requests are recorded, so each inbound decision is + rebound to its recorded call and the record is consumed (one decision per surfaced + request, approved or rejected). A response whose embedded call differs from the + recorded one is logged and the recorded call wins, so an edited or replayed response + cannot execute a call that was never surfaced. Without a session there is no record + and responses pass through unchanged. + + Returns the approved responses to execute, in their original order. + """ + state = _get_tool_approval_state(invocation_session) + raw_surfaced = state.get(_SURFACED_APPROVAL_REQUESTS_KEY) if state is not None else None + if state is None or not isinstance(raw_surfaced, dict): + return [response for response in approval_responses if response.approved] + surfaced_requests = cast(dict[str, Any], raw_surfaced) + + from ._types import Content + + responses_to_execute: list[Content] = [] + for response in approval_responses: + recorded = _content_from_state(surfaced_requests.pop(response.id, None)) if response.id else None + if not response.approved: + # The record is consumed above: a rejected request must not be approvable later. + continue + if recorded is None or recorded.type != "function_call": + responses_to_execute.append(response) + continue + inbound_call = response.function_call + if inbound_call is None or inbound_call.to_dict() != recorded.to_dict(): + logger.warning( + "Approval response %s does not match the surfaced approval request; " + "executing the surfaced function call instead.", + response.id, + ) + responses_to_execute.append( + Content.from_function_approval_response( + id=response.id, # type: ignore[arg-type] + function_call=recorded, + approved=True, + ) + ) + if surfaced_requests: + state[_SURFACED_APPROVAL_REQUESTS_KEY] = surfaced_requests + else: + state.pop(_SURFACED_APPROVAL_REQUESTS_KEY, None) + return responses_to_execute + + def _pop_already_approved_approval_responses( invocation_session: AgentSession | None, approval_response_ids: set[str], @@ -2748,8 +2822,13 @@ async def _resolve_approval_responses( _remove_unanswered_approval_batches_from_model_input(prepared_messages) return _FunctionProcessingResult(errors_in_a_row=errors_in_a_row) - # 3. Execute approved decisions once. Rejected decisions are converted to results during normalization below. - responses_to_execute = [response for response in pending_approval_responses.values() if response.approved] + # 3. Execute approved decisions once, each bound back to the approval request that was + # actually surfaced (session-backed runs record them; see #7383). Rejected decisions are + # converted to results during normalization below. + responses_to_execute = _bind_approval_responses_to_surfaced_requests( + invocation_session, + list(pending_approval_responses.values()), + ) execution_result_groups: list[list[Content]] = [] should_terminate = False reached_error_limit = False diff --git a/python/packages/core/tests/core/test_harness_tool_approval.py b/python/packages/core/tests/core/test_harness_tool_approval.py index 257b52c966..0f66bae48e 100644 --- a/python/packages/core/tests/core/test_harness_tool_approval.py +++ b/python/packages/core/tests/core/test_harness_tool_approval.py @@ -1253,3 +1253,83 @@ def optional_args_tool(value: str = "default") -> str: requests = _approval_requests(second_response.messages) assert [_function_call(request).arguments for request in requests] == ['{"value": "custom"}'] assert calls == 1 + + +async def test_approval_resume_binds_decision_to_surfaced_request( + chat_client_base: MockBaseChatClient, +) -> None: + """An approval response carrying an edited call must execute the surfaced call, not the edited one (#7383).""" + received_amounts: list[str] = [] + + @tool(name="guarded_transfer", approval_mode="always_require") + def guarded_transfer(amount: str) -> str: + received_amounts.append(amount) + return f"transferred {amount}" + + agent = Agent(client=chat_client_base, tools=[guarded_transfer]) + session = AgentSession(session_id="bind-approval-to-surfaced-request") + function_call = Content.from_function_call( + call_id="call_transfer", + name="guarded_transfer", + arguments='{"amount": "10"}', + ) + chat_client_base.run_responses = [ChatResponse(messages=Message(role="assistant", contents=[function_call]))] + + first_response = await agent.run("run guarded", session=session) + approval_request = first_response.user_input_requests[0] + + edited_call = Content.from_function_call( + call_id="call_transfer", + name="guarded_transfer", + arguments='{"amount": "10000"}', + ) + edited_response = Content.from_function_approval_response( + id=approval_request.id, + function_call=edited_call, + approved=True, + ) + + chat_client_base.run_responses = [ChatResponse(messages=Message(role="assistant", contents=["done"]))] + second_response = await agent.run(Message(role="user", contents=[edited_response]), session=session) + + assert received_amounts == ["10"] + result = next( + content + for message in second_response.messages + for content in message.contents + if content.type == "function_result" + ) + assert result.result == "transferred 10" + + +async def test_surfaced_approval_request_record_is_consumed_on_decision( + chat_client_base: MockBaseChatClient, +) -> None: + """The surfaced-request record is one-time-use: consumed by the first decision, approved or rejected.""" + calls = 0 + + @tool(name="guarded_tool", approval_mode="always_require") + def guarded_tool() -> str: + nonlocal calls + calls += 1 + return "approved result" + + agent = Agent(client=chat_client_base, tools=[guarded_tool]) + session = AgentSession(session_id="surfaced-record-consumed") + function_call = Content.from_function_call(call_id="call_guarded", name="guarded_tool", arguments="{}") + chat_client_base.run_responses = [ChatResponse(messages=Message(role="assistant", contents=[function_call]))] + + first_response = await agent.run("run guarded", session=session) + approval_request = first_response.user_input_requests[0] + + tool_state = session.state.get("tool_approval") + assert isinstance(tool_state, dict) + assert approval_request.id in tool_state.get("surfaced_approval_requests", {}) + + rejection = approval_request.to_function_approval_response(approved=False) + chat_client_base.run_responses = [ChatResponse(messages=Message(role="assistant", contents=["done"]))] + await agent.run(Message(role="user", contents=[rejection]), session=session) + + tool_state = session.state.get("tool_approval") or {} + assert "surfaced_approval_requests" not in tool_state + assert calls == 0