fix(mcp): Transform tool arguments to match schema #3375
Conversation
Adds argument transformation layer to handle cases where model output simplifies array-of-objects to array-of-primitives for single-property object schemas.
Example: Model generates ["web", "images"] for schema expecting
[{"type": "web"}, {"type": "images"}], causing MCP validation errors.
The transformation:
- Only applies to arrays with object items containing one property
- Preserves already-correct arguments
- Returns original values when transformation doesn't apply
Fixes validation errors with MCP tools that define array parameters
with single-property object items (e.g., Firecrawl search sources).
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @leonardogrig, 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 the output from language models, when simplified, caused MCP tools to fail due to a mismatch with their expected input schemas. Specifically, it resolves cases where an array of single-property objects was incorrectly simplified to an array of primitives. The solution introduces an intelligent argument transformation mechanism that automatically re-formats these simplified inputs to conform to the tool's schema, ensuring successful tool execution and improving the robustness of agent-tool interactions. 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
|
|
Response from ADK Triaging Agent Hello @leonardogrig, thank you for creating this PR! It looks like you have not signed the Contributor License Agreement (CLA) yet. Please make sure to sign the CLA so we can proceed with reviewing your contribution. Thank you for providing a detailed testing plan, it is very helpful for the reviewers! |
![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 introduces a transformation layer to handle cases where the language model simplifies arguments for MCP tools, for example, by turning an array of single-property objects into an array of primitives. The implementation is solid and well-tested for the intended scenarios. My main feedback is to improve the robustness of the transformation logic in _transform_value_to_schema to correctly handle mixed-type arrays, which could otherwise lead to runtime errors. I've provided a specific code suggestion to address this. Overall, this is a valuable fix.
| if schema_type == "array" and isinstance(value, list) and value: | ||
| items_schema = schema.get("items") | ||
| if not items_schema or items_schema.get("type") != "object": | ||
| return value | ||
|
|
||
| if not isinstance(value[0], dict): | ||
| if not all(not isinstance(item, dict) for item in value): | ||
| logger.warning( | ||
| "Mixed types in array for MCP tool %s", self.name | ||
| ) | ||
| return value | ||
|
|
||
| item_properties = items_schema.get("properties", {}) | ||
| if len(item_properties) == 1: | ||
| property_name = next(iter(item_properties)) | ||
| logger.debug( | ||
| "Transforming array for MCP tool %s with property '%s'", | ||
| self.name, | ||
| property_name, | ||
| ) | ||
| return [{property_name: item} for item in value] |
There was a problem hiding this comment.
The current logic for transforming values has a potential issue with mixed-type arrays. It assumes that if the first element in a list is a dictionary, the entire list is correctly formatted. This could lead to errors if the list contains a mix of dictionaries and primitives (e.g., [{'type': 'web'}, 'images']).
The suggested change refactors the logic to be more robust by explicitly checking if the list contains all dictionaries, all primitives, or a mix of both, ensuring that mixed lists are handled gracefully by logging a warning and returning the original value. This prevents potential downstream errors in the MCP tool.
It would also be beneficial to add a new unit test to cover this mixed-list scenario to prevent future regressions.
if schema_type == "array" and isinstance(value, list) and value:
items_schema = schema.get("items")
if not items_schema or items_schema.get("type") != "object":
return value
is_list_of_dicts = all(isinstance(item, dict) for item in value)
if is_list_of_dicts:
return value
is_list_of_primitives = all(not isinstance(item, dict) for item in value)
if not is_list_of_primitives:
logger.warning(
"Mixed types in array for MCP tool %s", self.name
)
return value
item_properties = items_schema.get("properties", {})
if len(item_properties) == 1:
property_name = next(iter(item_properties))
logger.debug(
"Transforming array for MCP tool %s with property '%s'",
self.name,
property_name,
)
return [{property_name: item} for item in value]
There was a problem hiding this comment.
The current logic for validating the array hinges on the type of the first element (value[0]). This creates a potential failure point if the array contains mixed types.
3 participants
Link to Issue or Description of Change
Problem:
MCP tools fail when model simplifies
[{"type": "web"}]to["web"].Error:
Solution:
Added input transformation in
McpTool._run_async_impl()to convert["web"]→[{"type": "web"}]when schema expects array of single-property objects.Testing Plan
Unit Tests:
test_mcp_tool.pyTests:
Manual E2E Test:
Tested with Firecrawl MCP
firecrawl_searchtool.Before:
After:
✅ Tool executes successfully
Checklist
Additional context
Implementation:
_transform_args_to_mcp_format()and_transform_value_to_schema()Test Code Used
The following agent code was used to test the fix with the query: "can you fetch recent ai news?"
Current MCP tool usage error:
Fix after this PR:
