From 317ef4491eb5092c21c9234b5ee10138f23a1fac Mon Sep 17 00:00:00 2001 From: Evan Mattson <35585003+moonbox3@users.noreply.github.com> Date: Fri, 1 May 2026 06:01:52 +0900 Subject: [PATCH] Python: Fix hosted MCP replay producing orphan function_call_output (#5581) * Python: Fix hosted MCP replay producing orphan function_call_output Resolves part of #5546. After a turn ran a hosted MCP / Foundry-toolbox-MCP tool, the next turn's replayed input array carried a function_call_output with an mcp_* call_id and no matching function_call, and the Responses API returned a 400. Two layers covered here: * Chat-client serialize layer (packages/openai): adds mcp_server_tool_call and mcp_server_tool_result cases to _prepare_message_for_openai and _prepare_content_for_openai. Pairs are coalesced via a post-pass into a single mcp_call input item carrying both arguments and output. Orphan results are dropped (debug-logged) rather than serialized as orphan function_call_output, which is what the Responses API rejected. * Host read layer (packages/foundry_hosting): _item_to_message and _output_item_to_message now route custom_tool_call_output whose call_id.startswith("mcp_") to Content.from_mcp_server_tool_result. Non-mcp_ call_ids continue to produce Content.from_function_result. Symmetric with the host write-side choice for hosted-MCP results. Two further fixes (agentserver SDK additions, host write-side single-item emission) remain tracked on the issue and depend on an SDK release. * Python: Fix pyright unknown-type in _stringify_mcp_output cast(Sequence[Any], output) after the isinstance check so pyright stops flagging the loop variable as unknown. Also normalizes a couple of em-dashes in docstrings I introduced in the prior commit. * Python: Harden _stringify_mcp_output for dict-shaped MCP outputs Address Copilot review on PR #5581. Today the helper falls back to str() for any non-string, non-text-attribute entry, which produces Python repr (single-quoted dicts) for the canonical MCP raw-JSON text-content shape `{"type": "text", "text": "..."}` and any other dict-shaped output. Three small changes: * List-entry path: prefer plain string entries, then `.text` attribute (Content objects), then `entry["text"]` for Mapping entries in the canonical MCP shape, then JSON-encode anything else. * Final fallback: `json.dumps(output, default=str)` so Mappings and scalars produce valid JSON rather than Python repr. * Two new unit tests covering the dict-with-text shape and the non-text-dict JSON fallback. * Python: Suppress mypy redundant-cast on _stringify_mcp_output narrowing The cast is needed by pyright (reportUnknownVariableType) but mypy considers it redundant after the preceding isinstance narrowing. Pyright's behavior is correct for the strict-mode reporting we run, so keep the cast and silence mypy on the line. --- .../_responses.py | 22 +++ .../foundry_hosting/tests/test_responses.py | 50 +++++++ .../agent_framework_openai/_chat_client.py | 128 ++++++++++++++++- .../tests/openai/test_openai_chat_client.py | 133 ++++++++++++++++++ 4 files changed, 332 insertions(+), 1 deletion(-) diff --git a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py index 7da9e8413a..64b50f236a 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py @@ -806,6 +806,18 @@ def _item_to_message(item: Item) -> Message: if item.type == "custom_tool_call_output": cto = cast(ItemCustomToolCallOutput, item) output = cto.output if isinstance(cto.output, str) else str(cto.output) + # Hosted-MCP results land here because the host writes them via + # `aoutput_item_custom_tool_call_output` (see `_to_outputs` for + # `mcp_server_tool_result`). The persisted `call_id` keeps its + # `mcp_*` prefix; on read, route those back to a hosted-MCP result + # Content so the chat-client serialize layer can coalesce them + # onto a single `mcp_call` input item with `output` populated. + # Issue #5546. + if cto.call_id and cto.call_id.startswith("mcp_"): + return Message( + role="tool", + contents=[Content.from_mcp_server_tool_result(call_id=cto.call_id, output=output)], + ) return Message( role="tool", contents=[Content.from_function_result(cto.call_id, result=output)], @@ -1054,6 +1066,16 @@ def _output_item_to_message(item: OutputItem) -> Message: if item.type == "custom_tool_call_output": cto = cast(OutputItemCustomToolCallOutput, item) output = cto.output if isinstance(cto.output, str) else str(cto.output) + # Hosted-MCP results land here because the host writes them via + # `aoutput_item_custom_tool_call_output`. Route `mcp_*` call_ids + # back to a hosted-MCP result Content so the chat-client serialize + # layer can coalesce onto the matching `mcp_call` input item. + # Issue #5546. + if cto.call_id and cto.call_id.startswith("mcp_"): + return Message( + role="tool", + contents=[Content.from_mcp_server_tool_result(call_id=cto.call_id, output=output)], + ) return Message( role="tool", contents=[Content.from_function_result(cto.call_id, result=output)], diff --git a/python/packages/foundry_hosting/tests/test_responses.py b/python/packages/foundry_hosting/tests/test_responses.py index e7c0599ad3..83ac6b3956 100644 --- a/python/packages/foundry_hosting/tests/test_responses.py +++ b/python/packages/foundry_hosting/tests/test_responses.py @@ -879,6 +879,30 @@ class TestOutputItemToMessage: assert msg.contents[0].type == "function_result" assert msg.contents[0].result == "result text" + def test_custom_tool_call_output_with_mcp_call_id_routes_to_mcp_server_tool_result(self) -> None: + """When the host wrote a hosted-MCP result via + `aoutput_item_custom_tool_call_output`, the persisted call_id keeps + its `mcp_*` prefix. On read, that result must reconstruct as a + `mcp_server_tool_result` Content (not `function_result`), so the + chat-client serialize layer treats it as a hosted-MCP result and + does not produce an orphan `function_call_output`. + """ + from azure.ai.agentserver.responses.models import OutputItemCustomToolCallOutput + + item = OutputItemCustomToolCallOutput({ + "type": "custom_tool_call_output", + "call_id": "mcp_06b686e11f118cf40169f0e5badb3081979842929d5cf04920", + "output": "found 10 cats", + }) + msg = _output_item_to_message(item) + assert msg.role == "tool" + assert len(msg.contents) == 1 + c = msg.contents[0] + assert c.type == "mcp_server_tool_result", ( + f"expected mcp_server_tool_result for mcp_-prefixed call_id; got {c.type}" + ) + assert c.call_id == "mcp_06b686e11f118cf40169f0e5badb3081979842929d5cf04920" + def test_apply_patch_call(self) -> None: from azure.ai.agentserver.responses.models import ApplyPatchUpdateFileOperation, OutputItemApplyPatchToolCall @@ -1329,6 +1353,32 @@ class TestItemToMessage: assert msg is not None assert msg.contents[0].result == "123" + def test_custom_tool_call_output_with_mcp_call_id_routes_to_mcp_server_tool_result(self) -> None: + """Issue #5546: input items carrying a hosted-MCP result (from a + prior turn that the framework wrote via + `aoutput_item_custom_tool_call_output`) must reconstruct as a + `mcp_server_tool_result` Content, not `function_result`. Otherwise + the chat-client serialize layer turns it into an orphan + `function_call_output` with `mcp_*` call_id and the Responses API + rejects the next turn. + """ + from azure.ai.agentserver.responses.models import ItemCustomToolCallOutput + + item = ItemCustomToolCallOutput({ + "type": "custom_tool_call_output", + "call_id": "mcp_06b686e11f118cf40169f0e5badb3081979842929d5cf04920", + "output": "found 10 cats", + }) + msg = _item_to_message(item) + assert msg is not None + assert msg.role == "tool" + assert len(msg.contents) == 1 + c = msg.contents[0] + assert c.type == "mcp_server_tool_result", ( + f"expected mcp_server_tool_result for mcp_-prefixed call_id; got {c.type}" + ) + assert c.call_id == "mcp_06b686e11f118cf40169f0e5badb3081979842929d5cf04920" + def test_apply_patch_call(self) -> None: from azure.ai.agentserver.responses.models import ApplyPatchToolCallItemParam, ApplyPatchUpdateFileOperation diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index 5e03494ea9..46b8375224 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -121,6 +121,14 @@ OPENAI_LOCAL_SHELL_COMMAND_PARTS_KEY = "openai.local_shell_command_parts" OPENAI_SHELL_OUTPUT_TYPE_SHELL_CALL = "shell_call_output" OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL = "local_shell_call_output" +# Internal marker emitted by `_prepare_content_for_openai` for an +# `mcp_server_tool_result` Content. The Responses API expects an `mcp_call` +# input item to carry both arguments and output as one item, so result +# Contents cannot be serialized standalone. `_prepare_messages_for_openai` +# coalesces these markers into the most recent matching `mcp_call` input +# item before returning, dropping any that are unmatched. +_AF_MCP_PENDING_OUTPUT_KEY = "__af_pending_mcp_result__" + class OpenAIContinuationToken(ContinuationToken): """Continuation token for OpenAI Responses API background operations.""" @@ -1363,7 +1371,10 @@ class RawOpenAIChatClient( # type: ignore[misc] for message in chat_messages ] # Flatten the list of lists into a single list - return list(chain.from_iterable(list_of_list)) + flat = list(chain.from_iterable(list_of_list)) + # Coalesce hosted-MCP result markers onto matching mcp_call input + # items (drop unmatched). See `_AF_MCP_PENDING_OUTPUT_KEY`. + return self._coalesce_pending_mcp_results(flat) def _prepare_message_for_openai( self, @@ -1428,6 +1439,18 @@ class RawOpenAIChatClient( # type: ignore[misc] ) if prepared: all_messages.append(prepared) + case "mcp_server_tool_call" | "mcp_server_tool_result": + # Hosted MCP call/result contents serialize as a single + # top-level mcp_call input item; the result side emits an + # internal marker that `_prepare_messages_for_openai` + # coalesces onto the matching call (or drops if unmatched). + prepared_mcp = self._prepare_content_for_openai( + message.role, + content, + replays_local_storage=replays_local_storage, + ) + if prepared_mcp: + all_messages.append(prepared_mcp) case _: prepared_content = self._prepare_content_for_openai( message.role, @@ -1606,6 +1629,24 @@ class RawOpenAIChatClient( # type: ignore[misc] "approval_request_id": content.id, "approve": content.approved, } + case "mcp_server_tool_call": + if not content.call_id: + return {} + return { + "type": "mcp_call", + "id": content.call_id, + "server_label": content.server_name or "", + "name": content.tool_name or "", + "arguments": self._stringify_mcp_arguments(content.arguments), + } + case "mcp_server_tool_result": + if not content.call_id: + return {} + return { + _AF_MCP_PENDING_OUTPUT_KEY: True, + "call_id": content.call_id, + "output": self._stringify_mcp_output(content.output), + } case "hosted_file": # `input_file` is an input-only content type in the Responses API and is rejected # inside an assistant message. Hosted-file content on an assistant message @@ -1681,6 +1722,91 @@ class RawOpenAIChatClient( # type: ignore[misc] """Join shell commands into a single executable command string.""" return "\n".join(command for command in commands if command).strip() + @staticmethod + def _stringify_mcp_arguments(arguments: Any) -> str: + """Render hosted-MCP tool-call arguments as a JSON string for the Responses API.""" + if arguments is None: + return "" + if isinstance(arguments, str): + return arguments + try: + return json.dumps(arguments) + except (TypeError, ValueError): + return str(arguments) + + @staticmethod + def _stringify_mcp_output(output: Any) -> str: + """Render a hosted-MCP tool-call result into the string `mcp_call.output` field. + + Accepts a string, a list of text-bearing Content objects (the form + the chat client produces when parsing an `mcp_call` Responses item), + or any other value. List entries that are dicts with the canonical + MCP text-content shape (`{"text": "..."}`) are unwrapped to their + text. Anything else falls back to JSON encoding rather than Python + `repr`, so the wire payload stays parseable for downstream callers. + """ + if output is None: + return "" + if isinstance(output, str): + return output + if isinstance(output, Sequence) and not isinstance(output, (str, bytes, bytearray)): + # cast is for pyright (reportUnknownVariableType); mypy considers + # it redundant after the isinstance narrowing. + entries = cast(Sequence[Any], output) # type: ignore[redundant-cast] + parts: list[str] = [] + for entry in entries: + if isinstance(entry, str): + parts.append(entry) + continue + text = getattr(entry, "text", None) + if isinstance(text, str): + parts.append(text) + continue + if isinstance(entry, Mapping): + mapping_text = cast(Any, entry).get("text") + if isinstance(mapping_text, str): + parts.append(mapping_text) + continue + parts.append(json.dumps(entry, default=str)) + return "".join(parts) + return json.dumps(output, default=str) + + @staticmethod + def _coalesce_pending_mcp_results(items: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Merge pending hosted-MCP result markers onto matching mcp_call input items. + + See `_AF_MCP_PENDING_OUTPUT_KEY`. The Responses API expects a single + `mcp_call` input item carrying both `arguments` and `output`, so a + result Content cannot be its own input item. Any unmatched markers + are dropped (debug-logged); surfacing them as standalone items + would produce the orphan `function_call_output` / `mcp_call_output` + the API rejects. + """ + out: list[dict[str, Any]] = [] + for item in items: + if item.get(_AF_MCP_PENDING_OUTPUT_KEY): + target_call_id = item.get("call_id") + target = next( + ( + existing + for existing in reversed(out) + if existing.get("type") == "mcp_call" and existing.get("id") == target_call_id + ), + None, + ) + if target is not None: + if target.get("output") is None: + target["output"] = item.get("output") + else: + logger.debug( + "Dropping orphan mcp_server_tool_result for call_id=%s; " + "no matching mcp_call appeared in input.", + target_call_id, + ) + continue + out.append(item) + return out + @staticmethod def _serialize_provider_payload(value: Any) -> Any: """Convert OpenAI SDK objects into JSON-serializable Python values.""" diff --git a/python/packages/openai/tests/openai/test_openai_chat_client.py b/python/packages/openai/tests/openai/test_openai_chat_client.py index 2bce3562b2..1e99224044 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_client.py @@ -5133,4 +5133,137 @@ def test_prepare_messages_for_openai_filters_none_fc_id() -> None: assert fc_item["id"].startswith("fc_") +# region: hosted MCP round-trip (issue #5546) + + +def test_prepare_messages_for_openai_serializes_mcp_server_tool_call_as_mcp_call_input_item() -> None: + """A Message containing only an mcp_server_tool_call Content should produce + a top-level mcp_call input item, not be silently dropped (which today's + _prepare_content_for_openai default branch does). + """ + client = OpenAIChatClient(model="test-model", api_key="test-key") + + messages = [ + Message( + role="assistant", + contents=[ + Content.from_mcp_server_tool_call( + call_id="mcp_abc123", + tool_name="search", + server_name="api_specs", + arguments='{"q": "cats"}', + ) + ], + ), + ] + + result = client._prepare_messages_for_openai(messages) + + mcp_items = [item for item in result if isinstance(item, dict) and item.get("type") == "mcp_call"] + assert len(mcp_items) == 1, f"expected exactly one mcp_call item; got result={result}" + item = mcp_items[0] + assert item["id"] == "mcp_abc123" + assert item["server_label"] == "api_specs" + assert item["name"] == "search" + assert item["arguments"] == '{"q": "cats"}' + assert "output" not in item or item["output"] is None + + +def test_prepare_messages_for_openai_coalesces_mcp_call_and_result_into_single_item() -> None: + """An mcp_server_tool_call followed by an mcp_server_tool_result with the + same call_id (in same or separate Messages) must produce ONE mcp_call + input item carrying both arguments and output. Two items would let the + Responses API see an orphaned output and reject the request. + """ + client = OpenAIChatClient(model="test-model", api_key="test-key") + + messages = [ + Message( + role="assistant", + contents=[ + Content.from_mcp_server_tool_call( + call_id="mcp_abc123", + tool_name="search", + server_name="api_specs", + arguments='{"q": "cats"}', + ) + ], + ), + Message( + role="tool", + contents=[ + Content.from_mcp_server_tool_result( + call_id="mcp_abc123", + output=[Content.from_text(text="found 10 cats")], + ) + ], + ), + ] + + result = client._prepare_messages_for_openai(messages) + + mcp_items = [item for item in result if isinstance(item, dict) and item.get("type") == "mcp_call"] + assert len(mcp_items) == 1, f"expected one coalesced mcp_call item carrying both arguments and output; got {result}" + item = mcp_items[0] + assert item["id"] == "mcp_abc123" + assert item["arguments"] == '{"q": "cats"}' + assert item.get("output") == "found 10 cats" + + # And no orphaned function_call_output should appear anywhere in the input. + fco_items = [item for item in result if isinstance(item, dict) and item.get("type") == "function_call_output"] + assert fco_items == [], f"unexpected orphan function_call_output items: {fco_items}" + + +def test_prepare_messages_for_openai_drops_orphan_mcp_server_tool_result() -> None: + """When an mcp_server_tool_result has no matching mcp_server_tool_call in + the message list, it must be dropped, NOT serialized as a + function_call_output. An orphan function_call_output is what triggers the + Responses API 400 reported in #5546. + """ + client = OpenAIChatClient(model="test-model", api_key="test-key") + + messages = [ + Message( + role="tool", + contents=[ + Content.from_mcp_server_tool_result( + call_id="mcp_orphan_id", + output=[Content.from_text(text="dangling output")], + ) + ], + ), + ] + + result = client._prepare_messages_for_openai(messages) + + fco_items = [item for item in result if isinstance(item, dict) and item.get("type") == "function_call_output"] + assert fco_items == [], f"orphan mcp_server_tool_result must not serialize as function_call_output; got {fco_items}" + mcp_items = [item for item in result if isinstance(item, dict) and item.get("type") == "mcp_call"] + assert mcp_items == [], f"orphan mcp_server_tool_result must not synthesize a stand-alone mcp_call; got {mcp_items}" + + +def test_stringify_mcp_output_extracts_text_from_dict_entries() -> None: + """A list of dicts in the canonical MCP text-content shape + (`{"type": "text", "text": "..."}`, e.g. from raw-JSON-decoded MCP + responses) must unwrap to plain text rather than Python `repr`. + """ + result = OpenAIChatClient._stringify_mcp_output([{"type": "text", "text": "found 10 cats"}]) + assert result == "found 10 cats" + + +def test_stringify_mcp_output_falls_back_to_json_for_non_text_dict_entries() -> None: + """Dict entries that are not in the canonical text-content shape must + serialize as JSON, not Python `repr`. Python `repr` for a dict uses + single quotes and would not round-trip through any JSON-aware consumer. + """ + result = OpenAIChatClient._stringify_mcp_output([{"type": "image", "url": "https://example.com/x"}]) + # Valid JSON: starts with `{`, contains the keys, no Python-repr single quotes. + assert result.startswith("{") + assert '"url"' in result + assert "'" not in result + + +# endregion + + # endregion