fix(mcp): preserve empty structured content in tool output#3575
fix(mcp): preserve empty structured content in tool output#3575mshsheikh wants to merge 1 commit into
Conversation
When `server.use_structured_content` is enabled, `invoke_mcp_tool()` should treat structured content as available whenever it is not `None`, even if it is an empty list or object. The current truthiness check drops valid empty JSON results and falls back to the content-based path instead. That conflicts with the code comment and documented behavior that structured content should be used exclusively when it is requested and available.
This change updates the condition from a truthiness check to an explicit `is not None` check so that empty structured results such as `[]` and `{}` are still serialized and returned correctly. This aligns the runtime behavior with the intended MCP output handling and addresses the empty-tool-output bug reported in openai#1035.
Testing:
- Add a regression test for `structuredContent=[]` with `use_structured_content=True`
- Add a regression test for `structuredContent={}` with `use_structured_content=True`
- Verify the method returns JSON text for both cases rather than falling back to `result.content`
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c382b8eb1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # If structured content is requested and available, use it exclusively | ||
| tool_output: ToolOutput | ||
| if server.use_structured_content and result.structuredContent: | ||
| if server.use_structured_content and result.structuredContent is not None: |
There was a problem hiding this comment.
When this branch now serializes empty structured content such as {} instead of falling back to result.content, the committed tests still assert the old fallback behavior (for example tests/mcp/test_mcp_util.py::test_structured_content_handling scenario 4 and test_empty_structured_content_fallback). That leaves the MCP test suite failing for the exact case this change intends to fix, so the expected outputs/regression tests need to be updated in the same change.
Useful? React with 👍 / 👎.
1 participant
When
server.use_structured_contentis enabled,invoke_mcp_tool()should treat structured content as available whenever it is notNone, even if it is an empty list or object. The current truthiness check drops valid empty JSON results and falls back to the content-based path instead. That conflicts with the code comment and documented behavior that structured content should be used exclusively when it is requested and available.This change updates the condition from a truthiness check to an explicit
is not Nonecheck so that empty structured results such as[]and{}are still serialized and returned correctly. This aligns the runtime behavior with the intended MCP output handling and addresses the empty-tool-output bug reported in #1035.Testing:
structuredContent=[]withuse_structured_content=TruestructuredContent={}withuse_structured_content=Trueresult.content