diff --git a/ads_mcp/tools/search.py b/ads_mcp/tools/search.py index 1aa48b6..a8e6ebf 100644 --- a/ads_mcp/tools/search.py +++ b/ads_mcp/tools/search.py @@ -27,9 +27,9 @@ def search( customer_id: str, fields: List[str], resource: str, - conditions: List[str] = None, - orderings: List[str] = None, - limit: int = None, + conditions: List[str] | None = None, + orderings: List[str] | None = None, + limit: int | None = None, ) -> List[Dict[str, Any]]: """Fetches data from the Google Ads API using the search method diff --git a/tests/smoke/golden_tools_list.json b/tests/smoke/golden_tools_list.json index eaae787..8f1ffaa 100644 --- a/tests/smoke/golden_tools_list.json +++ b/tests/smoke/golden_tools_list.json @@ -75,12 +75,19 @@ "additionalProperties": false, "properties": { "conditions": { + "anyOf": [ + { + "items": { + "type": "string" + }, + "type": "array" + }, + { + "type": "null" + } + ], "default": null, - "description": "List of conditions to filter the data, combined using AND clauses", - "items": { - "type": "string" - }, - "type": "array" + "description": "List of conditions to filter the data, combined using AND clauses" }, "customer_id": { "description": "The id of the customer", @@ -94,17 +101,31 @@ "type": "array" }, "limit": { + "anyOf": [ + { + "type": "integer" + }, + { + "type": "null" + } + ], "default": null, - "description": "The maximum number of rows to return", - "type": "integer" + "description": "The maximum number of rows to return" }, "orderings": { + "anyOf": [ + { + "items": { + "type": "string" + }, + "type": "array" + }, + { + "type": "null" + } + ], "default": null, - "description": "How the data is ordered", - "items": { - "type": "string" - }, - "type": "array" + "description": "How the data is ordered" }, "resource": { "description": "The resource to return fields from", diff --git a/tests/smoke/llm_cases.json b/tests/smoke/llm_cases.json index 3928276..e7ec800 100644 --- a/tests/smoke/llm_cases.json +++ b/tests/smoke/llm_cases.json @@ -2,100 +2,100 @@ { "prompt": "List the customers I can access.", "expected_tool": "list_accessible_customers", - "prompt_tokens": 524, + "prompt_tokens": 538, "output_tokens": 12, "model": "gemini-flash-latest", - "thought_tokens": 27, + "thought_tokens": 61, "tool_tokens": 170 }, { "prompt": "Find campaigns matching 'Test Campaign' for customer {customer_id}.", "expected_tool": "search", - "prompt_tokens": 538, - "output_tokens": 64, + "prompt_tokens": 552, + "output_tokens": 20, "model": "gemini-flash-latest", - "thought_tokens": 236, - "tool_tokens": 35 + "thought_tokens": 80, + "tool_tokens": 35429 }, { "prompt": "Show me all my accounts.", "expected_tool": "list_accessible_customers", - "prompt_tokens": 523, + "prompt_tokens": 537, "output_tokens": 12, "model": "gemini-flash-latest", - "thought_tokens": 25, + "thought_tokens": 46, "tool_tokens": 170 }, { "prompt": "Which customers do I have access to?", "expected_tool": "list_accessible_customers", - "prompt_tokens": 525, + "prompt_tokens": 539, "output_tokens": 12, "model": "gemini-flash-latest", - "thought_tokens": 29, + "thought_tokens": 66, "tool_tokens": 170 }, { "prompt": "Find enabled ad groups for customer {customer_id}.", "expected_tool": "search", - "prompt_tokens": 535, + "prompt_tokens": 549, "output_tokens": 22, "model": "gemini-flash-latest", - "thought_tokens": 152, - "tool_tokens": 24555 + "thought_tokens": 93, + "tool_tokens": 24699 }, { "prompt": "Get keywords for campaign 'Summer Sale' in account {customer_id}.", "expected_tool": "search", - "prompt_tokens": 539, - "output_tokens": 58, + "prompt_tokens": 553, + "output_tokens": 20, "model": "gemini-flash-latest", - "thought_tokens": 238, - "tool_tokens": 35 + "thought_tokens": 201, + "tool_tokens": 35429 }, { "prompt": "Show me the metrics for customer {customer_id}.", "expected_tool": "search", - "prompt_tokens": 535, + "prompt_tokens": 549, "output_tokens": 20, "model": "gemini-flash-latest", - "thought_tokens": 70, - "tool_tokens": 22505 + "thought_tokens": 150, + "tool_tokens": 22703 }, { "prompt": "Generate a daily report of clicks for all active campaigns in account {customer_id}.", "expected_tool": "search", - "prompt_tokens": 541, - "output_tokens": 51, - "thought_tokens": 198, + "prompt_tokens": 555, + "output_tokens": 20, + "thought_tokens": 88, "model": "gemini-flash-latest", - "tool_tokens": 70 + "tool_tokens": 35429 }, { "prompt": "What fields are available for the campaign resource?", "expected_tool": "get_resource_metadata", - "prompt_tokens": 526, + "prompt_tokens": 540, "output_tokens": 20, - "thought_tokens": 34, + "thought_tokens": 87, "model": "gemini-flash-latest", - "tool_tokens": 35231 + "tool_tokens": 35429 }, { "prompt": "Find metrics compatible with ad_group.", "expected_tool": "get_resource_metadata", - "prompt_tokens": 525, + "prompt_tokens": 539, "output_tokens": 22, - "thought_tokens": 32, + "thought_tokens": 81, "model": "gemini-flash-latest", - "tool_tokens": 24555 + "tool_tokens": 24699 }, { "prompt": "What segments can I select with the customer resource?", "expected_tool": "get_resource_metadata", - "prompt_tokens": 527, + "prompt_tokens": 541, "output_tokens": 20, - "thought_tokens": 38, + "thought_tokens": 90, "model": "gemini-flash-latest", - "tool_tokens": 22505 + "tool_tokens": 22703 } ] diff --git a/tests/tools/schema_test.py b/tests/tools/schema_test.py new file mode 100644 index 0000000..c275c2b --- /dev/null +++ b/tests/tools/schema_test.py @@ -0,0 +1,75 @@ +# Copyright 2026 Google LLC. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for tool schema correctness and type safety.""" + +import unittest +from ads_mcp.coordinator import mcp + +# Import server to ensure all tools are registered on the mcp object +from ads_mcp import server # noqa: F401 + + +class TestToolSchemas(unittest.IsolatedAsyncioTestCase): + """Verifies that tool schemas are properly defined and prevent Zod errors.""" + + async def test_optional_parameters_allow_null(self): + """Verifies that any tool parameter with a default of None allows 'null' in its schema. + + This prevents 'Expected array, received string' or similar client-side Zod validation + failures caused by schema type contradictions (e.g. type 'array' but default is 'null'). + """ + tools = await mcp.list_tools() + self.assertGreater( + len(tools), 0, "No tools are registered on the server" + ) + + for tool in tools: + input_schema = tool.parameters + properties = input_schema.get("properties", {}) + for param_name, param_schema in properties.items(): + # If a parameter has a default value of None (JSON null), the schema must permit null + if ( + "default" in param_schema + and param_schema["default"] is None + ): + has_null_type = False + + # Case 1: Schema uses anyOf (standard for Pydantic unions) + if "anyOf" in param_schema: + for option in param_schema["anyOf"]: + if option.get("type") == "null": + has_null_type = True + break + + # Case 2: Schema uses oneOf + elif "oneOf" in param_schema: + for option in param_schema["oneOf"]: + if option.get("type") == "null": + has_null_type = True + break + + # Case 3: Schema has list-based types or direct type 'null' + elif "type" in param_schema: + t = param_schema["type"] + if t == "null": + has_null_type = True + elif isinstance(t, list) and "null" in t: + has_null_type = True + + self.assertTrue( + has_null_type, + f"Tool '{tool.name}' parameter '{param_name}' has default=None, " + f"but its JSON schema does not permit 'null'. Schema: {param_schema}", + )