From cc3b7a26c66043eaf456fc5dae33cd0c9a2cb238 Mon Sep 17 00:00:00 2001 From: atty57 <99388680+atty57@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:18:19 -0400 Subject: [PATCH 1/3] Python: Fix duplicate function call on approval round-trip (#7267) `_replace_approval_contents_with_results` deduped restored function calls against only the message currently being scanned. On an approval round-trip the hosting layer replays the stored `function_call` item and its `mcp_approval_request` item as two separate assistant messages, so the per-message check never fired and the approval request restored a second copy of the call. Only one copy received the function result; the orphaned copy was left unanswered, which the Responses API rejects with "No tool output found for function call call_". Collect existing call ids across all messages instead, and add a restored call to that set so two approval requests for the same call cannot both expand. Co-Authored-By: Claude Opus 4.8 --- .../packages/core/agent_framework/_tools.py | 19 ++++++++--- .../core/test_function_invocation_logic.py | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 588ae93d5c0..922086a6295 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -2119,12 +2119,19 @@ def _replace_approval_contents_with_results( # Track which call_ids had their placeholders replaced placeholders_replaced: set[str] = set() - for msg in messages: - # First pass - collect existing function call IDs to avoid duplicates - existing_call_ids = { - content.call_id for content in msg.contents if content.type == "function_call" and content.call_id - } + # Collect existing function call IDs across *all* messages to avoid duplicates. The + # function call and its approval request are frequently carried in separate messages + # (e.g. when a hosting layer replays them as separate items on an approval round + # trip), so scoping this per-message would let the same call_id be restored twice and + # leave the copy without a result unanswered. + existing_call_ids = { + content.call_id + for msg in messages + for content in msg.contents + if content.type == "function_call" and content.call_id + } + for msg in messages: # Track approval requests that should be removed (duplicates) contents_to_remove: list[int] = [] @@ -2140,6 +2147,8 @@ def _replace_approval_contents_with_results( elif content.function_call is not None: # Put back the function call content only if it doesn't exist msg.contents[content_idx] = content.function_call + if content.function_call.call_id: + existing_call_ids.add(content.function_call.call_id) elif content.type == "function_approval_response": # Skip hosted tool approvals — they must pass through to the API unchanged if _is_hosted_tool_approval(content): 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 6426fade633..3dca0f5f455 100644 --- a/python/packages/core/tests/core/test_function_invocation_logic.py +++ b/python/packages/core/tests/core/test_function_invocation_logic.py @@ -2338,6 +2338,39 @@ def test_replace_approval_contents_with_results_uses_result_call_ids_without_pla ] +def test_replace_approval_contents_with_results_dedupes_call_across_messages() -> None: + """A function call and its approval request may arrive in separate messages. + + Hosting layers replay a stored ``function_call`` item and its + ``mcp_approval_request`` item as two assistant messages. The approval request + must not restore a second copy of the call, or that copy is left without a + result and the service rejects the turn with "No tool output found for + function call". + """ + from agent_framework._tools import _collect_approval_responses, _replace_approval_contents_with_results + + call, request, response = _build_approved_tool_roundtrip( + call_id="call_1", approval_id="approval_1", tool_name="run_skill_script" + ) + + messages = [ + Message(role="assistant", contents=[call]), + Message(role="assistant", contents=[request]), + Message(role="user", contents=[response]), + ] + + _replace_approval_contents_with_results( + messages, + _collect_approval_responses(messages), + [Content.from_function_result(call_id="call_1", result="script output")], + ) + + function_calls = [c for m in messages for c in m.contents if c.type == "function_call"] + assert [c.call_id for c in function_calls] == ["call_1"] + results = [c for m in messages for c in m.contents if c.type == "function_result"] + assert [(c.call_id, c.result) for c in results] == [("call_1", "script output")] + + def test_replace_approval_contents_with_results_uses_result_call_ids_for_placeholders() -> None: from agent_framework._tools import _collect_approval_responses, _replace_approval_contents_with_results From 99d225e7943b481a08b41def2b7ce68a86b31b16 Mon Sep 17 00:00:00 2001 From: Atharva Vichare <99388680+atty57@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:23:39 -0400 Subject: [PATCH 2/3] Refactor approval placeholder result handling Refactor approval handling logic to improve clarity and maintainability. --- .../packages/core/agent_framework/_tools.py | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 922086a6295..5d783cb0368 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -2096,6 +2096,12 @@ def _collect_approval_responses( return fcc_todo +def _is_approval_placeholder_result(content: Content) -> bool: + """Whether a function_result is the stand-in emitted while approval is pending.""" + result = getattr(content, "result", None) + return isinstance(result, str) and "[APPROVAL_PENDING]" in result + + def _replace_approval_contents_with_results( messages: list[Message], fcc_todo: dict[str, Content], @@ -2119,16 +2125,27 @@ def _replace_approval_contents_with_results( # Track which call_ids had their placeholders replaced placeholders_replaced: set[str] = set() - # Collect existing function call IDs across *all* messages to avoid duplicates. The + # Collect *pending* function call IDs across all messages to avoid duplicates. The # function call and its approval request are frequently carried in separate messages - # (e.g. when a hosting layer replays them as separate items on an approval round - # trip), so scoping this per-message would let the same call_id be restored twice and - # leave the copy without a result unanswered. + # (e.g. when a hosting layer replays them as separate items on an approval round trip), + # so scoping this per-message would let the same call_id be restored twice and leave + # the copy without a result unanswered. + # + # Calls that already carry a real result are excluded: reusing a call_id for a later + # invocation is supported, and a completed pair must not suppress the fresh request — + # that would drop the new call and attach its result to the old one. Placeholder + # results still count as pending, since the call they answer is the one being restored. + answered_call_ids = { + content.call_id + for msg in messages + for content in msg.contents + if content.type == "function_result" and content.call_id and not _is_approval_placeholder_result(content) + } existing_call_ids = { content.call_id for msg in messages for content in msg.contents - if content.type == "function_call" and content.call_id + if content.type == "function_call" and content.call_id and content.call_id not in answered_call_ids } for msg in messages: @@ -2178,12 +2195,7 @@ def _replace_approval_contents_with_results( msg.role = "tool" elif content.type == "function_result": # Check if this is a placeholder result that should be replaced - if ( - hasattr(content, "result") - and isinstance(content.result, str) - and "[APPROVAL_PENDING]" in content.result - and content.call_id in result_by_call_id - ): + if _is_approval_placeholder_result(content) and content.call_id in result_by_call_id: # Replace placeholder with actual result msg.contents[content_idx] = result_by_call_id[content.call_id] placeholders_replaced.add(content.call_id) From 212d2dee6a33b11b70caf5415b6a75d51b6b74dd Mon Sep 17 00:00:00 2001 From: Atharva Vichare <99388680+atty57@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:22:26 -0400 Subject: [PATCH 3/3] Refactor test to support reused call IDs after completion Updated the test to allow reused call IDs after completion, ensuring that a completed call does not suppress later approval requests with the same ID. Adjusted assertions to reflect the new behavior. --- .../core/test_function_invocation_logic.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) 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 3dca0f5f455..6deb7090c7d 100644 --- a/python/packages/core/tests/core/test_function_invocation_logic.py +++ b/python/packages/core/tests/core/test_function_invocation_logic.py @@ -2338,23 +2338,24 @@ def test_replace_approval_contents_with_results_uses_result_call_ids_without_pla ] -def test_replace_approval_contents_with_results_dedupes_call_across_messages() -> None: - """A function call and its approval request may arrive in separate messages. - - Hosting layers replay a stored ``function_call`` item and its - ``mcp_approval_request`` item as two assistant messages. The approval request - must not restore a second copy of the call, or that copy is left without a - result and the service rejects the turn with "No tool output found for - function call". +def test_replace_approval_contents_with_results_allows_reused_call_id_after_completion() -> None: + """A completed call must not suppress a later approval request that reuses its id. + + Re-approving the same ``(call_id, function)`` is supported behaviour. If the dedupe + matched every occurrence of the id, the fresh request would be dropped and its result + attached to the already-answered call, leaving one call with two results. """ from agent_framework._tools import _collect_approval_responses, _replace_approval_contents_with_results - call, request, response = _build_approved_tool_roundtrip( - call_id="call_1", approval_id="approval_1", tool_name="run_skill_script" + completed_call = Content.from_function_call(call_id="call_reused", name="run_skill_script", arguments="{}") + completed_result = Content.from_function_result(call_id="call_reused", result="first output") + _, request, response = _build_approved_tool_roundtrip( + call_id="call_reused", approval_id="approval_2", tool_name="run_skill_script" ) messages = [ - Message(role="assistant", contents=[call]), + Message(role="assistant", contents=[completed_call]), + Message(role="tool", contents=[completed_result]), Message(role="assistant", contents=[request]), Message(role="user", contents=[response]), ] @@ -2362,14 +2363,16 @@ def test_replace_approval_contents_with_results_dedupes_call_across_messages() - _replace_approval_contents_with_results( messages, _collect_approval_responses(messages), - [Content.from_function_result(call_id="call_1", result="script output")], + [Content.from_function_result(call_id="call_reused", result="second output")], ) function_calls = [c for m in messages for c in m.contents if c.type == "function_call"] - assert [c.call_id for c in function_calls] == ["call_1"] + assert [c.call_id for c in function_calls] == ["call_reused", "call_reused"] results = [c for m in messages for c in m.contents if c.type == "function_result"] - assert [(c.call_id, c.result) for c in results] == [("call_1", "script output")] - + assert [(c.call_id, c.result) for c in results] == [ + ("call_reused", "first output"), + ("call_reused", "second output"), + ] def test_replace_approval_contents_with_results_uses_result_call_ids_for_placeholders() -> None: from agent_framework._tools import _collect_approval_responses, _replace_approval_contents_with_results