Fix: Prevent double JSON serialization of MCP tool responses #3678
Conversation
Fixes google#3676 MCP tool responses arrive as JSON strings but were being double-serialized by _safe_json_serialize(), creating triple-nested JSON that prevented Claude and GPT from parsing tool results. Example of the bug: '{"content": [{"type": "text", "text": "{\n \"type\"..."}]}' This fix adds an isinstance(str) check before serialization. If the response is already a string (from MCP or other sources), it's used directly. Otherwise, it's serialized normally. Impact: Without this fix, agents using LiteLLM with MCP tools would successfully call tools but fail to present results to users, appearing to hang or produce incomplete responses. Tested with Claude Sonnet 4.5 and GPT-5 via Azure OpenAI with MCP tools (Google Drive, HubSpot CRM) in production multi-agent system.
Summary of ChangesHello @thesynapses, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug where tool responses, especially from MCP tools, were being double-serialized into triple-nested JSON structures. This prevented large language models from correctly parsing the tool results, leading to agents appearing unresponsive or failing to display output. The fix introduces a conditional check to ensure that only non-string responses are serialized, thereby guaranteeing proper parsing and display of tool outputs for a smoother agent experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a critical issue where MCP tool responses were being double-serialized, leading to parsing failures in downstream models. The fix is direct and correct, adding a check to see if the response is already a string before attempting serialization. The pull request description is excellent, clearly detailing the problem, the fix, and the testing performed. I have one suggestion to make the code slightly more concise, but the logic is sound.
| # FIX: Check if response is already a string before serializing. | ||
| # MCP tool responses come as JSON strings, but _safe_json_serialize was | ||
| # double-serializing them (json.dumps on already-JSON strings), causing | ||
| # triple-nested JSON like: '{"content": [{"type": "text", "text": "{\n \"type\"..."}]}' | ||
| # This prevented Claude/GPT from parsing tool results correctly. | ||
| response_content = ( | ||
| part.function_response.response | ||
| if isinstance(part.function_response.response, str) | ||
| else _safe_json_serialize(part.function_response.response) | ||
| ) | ||
| tool_messages.append( | ||
| ChatCompletionToolMessage( | ||
| role="tool", | ||
| tool_call_id=part.function_response.id, | ||
| content=_safe_json_serialize(part.function_response.response), | ||
| content=response_content, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
While the logic is correct, this implementation can be made more concise. The temporary response_content variable can be inlined directly into the ChatCompletionToolMessage constructor. Additionally, the detailed comment is great for a pull request description but could be summarized for better long-term readability within the code.
# MCP tool responses can be pre-serialized JSON strings. Avoid
# double-serializing them to prevent parsing issues by downstream models.
tool_messages.append(
ChatCompletionToolMessage(
role="tool",
tool_call_id=part.function_response.id,
content=(
part.function_response.response
if isinstance(part.function_response.response, str)
else _safe_json_serialize(part.function_response.response)
),
)
)
|
Response from ADK Triaging Agent Hello @thesynapses, thank you for creating this PR! This is a great contribution. Could you please add unit tests for this change? This will help to ensure the quality of the code and prevent regressions. This information will help reviewers to review your PR more efficiently. Thanks! |
3 participants
Fixes #3676
MCP tool responses arrive as JSON strings but were being double-serialized by _safe_json_serialize(), creating triple-nested JSON that prevented Claude and GPT from parsing tool results.
Example of the bug:
'{"content": [{"type": "text", "text": "{\n "type"..."}]}'
This fix adds an isinstance(str) check before serialization. If the response is already a string (from MCP or other sources), it's used directly. Otherwise, it's serialized normally.
Impact: Without this fix, agents using LiteLLM with MCP tools would successfully call tools but fail to present results to users, appearing to hang or produce incomplete responses.
Tested with Claude Sonnet 4.5 and GPT-5 via Azure OpenAI with MCP tools (Google Drive, HubSpot CRM) in a production multi-agent system.
Link to Issue or Description of Change
1. Link to an existing issue:
Testing Plan
Problem:
MCP tool responses come as JSON strings. The code called
_safe_json_serialize()on these already-serialized strings, causing triple-nested JSON like:'{"content": [{"type": "text", "text": "{\\n \\"type\\"..."}]}'. This prevented Claude/GPT from parsing the tool results.Solution:
Added
isinstance(str)check before serialization in_content_to_message_param()(line 369).If the response is already a string, use it directly. Otherwise, serialize normally.
Unit Tests:
Manual End-to-End (E2E) Tests:
Setup:
vertex_ai/claude-sonnet-4-5@20250929)azure/gpt-5-openai-latest)Test Cases:
Before Fix:
'{"content": [{"type": "text", "text": "{\\n..."}]}'After Fix:
Checklist
Additional context
This fix is critical for production systems using MCP tools with LiteLLM models. The bug affects any pre-serialized JSON responses, not just MCP. The fix maintains backward compatibility with non-string responses while properly handling already-serialized strings.