Python: drop hosted MCP calls when reasoning is stripped#6210
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds handling for a replay scenario where reasoning content is stripped (when not using service-side storage) by also dropping the paired hosted-MCP tool call/result to avoid emitting invalid item sequences to the OpenAI API.
Changes:
- Add a unit test that asserts hosted-MCP call/result are dropped when paired reasoning is stripped (no service-side storage).
- Update
_prepare_message_for_openaito detect “reasoning will be dropped” and skip serializingmcp_callaccordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/openai/tests/openai/test_openai_chat_client.py | Adds regression test for dropping hosted-MCP call/result when reasoning is stripped. |
| python/packages/openai/agent_framework_openai/_chat_client.py | Drops mcp_call when reasoning is stripped without storage to prevent invalid payloads. |
| drops_reasoning_without_storage = not request_uses_service_side_storage and any( | ||
| content.type == "text_reasoning" for content in message.contents | ||
| ) |
| # | ||
| # Without storage, a reasoning + hosted-MCP pair cannot be replayed | ||
| # partially: reasoning is stripped above, and a bare mcp_call is rejected. | ||
| if request_uses_service_side_storage or drops_reasoning_without_storage: |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
moonbox3
left a comment
There was a problem hiding this comment.
Please check the failing tests as well.
| # | ||
| # Without storage, a reasoning + hosted-MCP pair cannot be replayed | ||
| # partially: reasoning is stripped above, and a bare mcp_call is rejected. | ||
| if request_uses_service_side_storage or drops_reasoning_without_storage: |
There was a problem hiding this comment.
Should the same drop apply to function_call? Under no-storage this branch keeps the call but reasoning is stripped above, the exact orphan we're fixing for mcp_call. The docstring on test_prepare_message_for_openai_includes_reasoning_with_function_call says that combo 400s: "function_call was provided without its required reasoning item". So either function_call needs the same drops_reasoning_without_storage guard and this PR fixes only half the problem, or function_call and mcp_call genuinely differ at the API and it's worth a note on why only mcp_call is all-or-nothing. Reasoning-model + intermittent, so the new test wouldn't surface it either way.
| # marker, since the server-stored items would otherwise duplicate the inline ones. Without | ||
| # storage, standalone reasoning items are invalid per the API ("reasoning was provided | ||
| # without its required following item"), so the reasoning branch always drops. | ||
| drops_reasoning_without_storage = not request_uses_service_side_storage and any( |
There was a problem hiding this comment.
Flag is message-scoped, so it only fires when reasoning and the mcp_call share one Message. For responses parsed by this client that always holds (streaming updates coalesce into one assistant Message, non-streaming builds one), so live traffic is fine. But if a replayed or checkpointed history ever splits a turn so reasoning sits in a separate message, this is False for the mcp_call's message and the bare call survives, the exact state the API rejects. Worth a no-storage test with reasoning and the call in separate messages to pin the contract, or a note that the split can't happen?
b20225e to
67f2890
Compare
|
Rebased this branch onto latest Local validation:
I also checked the failed GitHub Actions job. The failing cases are the Docker shell-tool tests, and the failure is from pulling Those tests do not touch the OpenAI message serialization path changed in this PR. I did not change Docker-related code. One local note: running the full |
Motivation and Context
Fixes #6074.
When OpenAI Responses history is replayed without service-side storage, the client strips text_reasoning because replaying standalone reasoning items is rejected by the API. Hosted MCP calls from the same model output were still kept, which can leave the request with a bare mcp_call after its paired reasoning item has been removed.
Description
This change treats that stateless replay path as an all-or-nothing pair for hosted MCP items. If a message contains reasoning that will be stripped, hosted MCP call/result contents in the same message are skipped too. Any later unmatched MCP result marker is then dropped by the existing coalescing logic.
The existing behavior for hosted MCP calls without reasoning is unchanged, and storage-backed continuation still strips server-issued MCP items as before.
A regression test covers a reasoning plus hosted MCP call/result history and verifies that the prepared input does not contain a bare reasoning, mcp_call, or function_call_output item.
Contribution Checklist