mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix MCP tool schema normalization for zero-argument tools missing 'properties' key (#4771)
* Fix zero-argument MCP tool schema missing 'properties' key (#4540) MCP servers for zero-argument tools (e.g. matlab-mcp-core-server's detect_matlab_toolboxes) declare inputSchema as {"type": "object"} without a "properties" key. OpenAI's API requires "properties" to be present on object schemas, causing a 400 invalid_request_error. Normalize inputSchema at MCP ingestion in load_tools() to inject an empty "properties": {} when it is missing from object-type schemas. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback for #4540: improve test robustness and add defensive guard - Look up loaded functions by name instead of index to avoid brittle ordering assumptions - Add negative-path test cases: non-object schema (type: string) and empty schema ({}) to verify guard clause skips them correctly - Assert original inputSchema dicts are not mutated by load_tools() - Add defensive guard for tool.inputSchema being None Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback for #4540: Python: [Bug]: Local stdio MCP works for calculator but fails for official matlab-mcp-core-server on LM Studio /v1/responses --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
100086a276
commit
4c287c2424
@@ -902,13 +902,21 @@ class MCPTool:
|
||||
continue
|
||||
|
||||
approval_mode = self._determine_approval_mode(local_name, normalized_name, tool.name)
|
||||
# Normalize inputSchema: ensure "properties" exists for object schemas.
|
||||
# Some MCP servers (e.g. zero-argument tools) omit "properties",
|
||||
# which causes OpenAI API to reject the schema with a 400 error.
|
||||
# Guard against non-conforming MCP servers that send inputSchema=None
|
||||
# despite the MCP spec typing it as dict[str, Any].
|
||||
input_schema = dict(tool.inputSchema or {})
|
||||
if input_schema.get("type") == "object" and "properties" not in input_schema:
|
||||
input_schema["properties"] = {}
|
||||
# Create FunctionTools out of each tool
|
||||
func: FunctionTool = FunctionTool(
|
||||
func=partial(self.call_tool, tool.name),
|
||||
name=local_name,
|
||||
description=tool.description or "",
|
||||
approval_mode=approval_mode,
|
||||
input_model=tool.inputSchema,
|
||||
input_model=input_schema,
|
||||
additional_properties={
|
||||
_MCP_REMOTE_NAME_KEY: tool.name,
|
||||
_MCP_NORMALIZED_NAME_KEY: normalized_name,
|
||||
|
||||
@@ -2042,6 +2042,100 @@ async def test_load_tools_with_pagination():
|
||||
assert [f.name for f in tool._functions] == ["tool_1", "tool_2", "tool_3", "tool_4"]
|
||||
|
||||
|
||||
async def test_load_tools_adds_properties_to_zero_arg_tool_schema():
|
||||
"""Test that load_tools normalizes inputSchema for zero-argument MCP tools.
|
||||
|
||||
Some MCP servers (e.g. matlab-mcp-core-server) declare zero-argument tools
|
||||
with inputSchema={"type": "object"} and no "properties" key. OpenAI's API
|
||||
requires "properties" to be present on object schemas, so load_tools must
|
||||
inject an empty "properties" dict when it is missing.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
from agent_framework._mcp import MCPTool
|
||||
|
||||
tool = MCPTool(name="test_tool")
|
||||
|
||||
mock_session = AsyncMock()
|
||||
tool.session = mock_session
|
||||
tool.load_tools_flag = True
|
||||
|
||||
original_zero_arg_schema = {"type": "object"}
|
||||
original_string_schema = {"type": "string"}
|
||||
original_empty_schema: dict[str, object] = {}
|
||||
|
||||
page = MagicMock()
|
||||
page.tools = [
|
||||
types.Tool(
|
||||
name="zero_arg_tool",
|
||||
description="A tool with no parameters",
|
||||
inputSchema=original_zero_arg_schema,
|
||||
),
|
||||
types.Tool(
|
||||
name="normal_tool",
|
||||
description="A tool with parameters",
|
||||
inputSchema={"type": "object", "properties": {"x": {"type": "string"}}, "required": ["x"]},
|
||||
),
|
||||
types.Tool(
|
||||
name="string_schema_tool",
|
||||
description="A tool with a non-object schema",
|
||||
inputSchema=original_string_schema,
|
||||
),
|
||||
types.Tool(
|
||||
name="empty_schema_tool",
|
||||
description="A tool with an empty schema",
|
||||
inputSchema=original_empty_schema,
|
||||
),
|
||||
]
|
||||
|
||||
# Simulate a non-conforming MCP server that sends inputSchema=None.
|
||||
# types.Tool requires inputSchema to be a dict, so we use a MagicMock.
|
||||
none_schema_tool = MagicMock()
|
||||
none_schema_tool.name = "none_schema_tool"
|
||||
none_schema_tool.description = "A tool with None inputSchema"
|
||||
none_schema_tool.inputSchema = None
|
||||
page.tools.append(none_schema_tool)
|
||||
page.nextCursor = None
|
||||
|
||||
mock_session.list_tools = AsyncMock(return_value=page)
|
||||
|
||||
await tool.load_tools()
|
||||
|
||||
assert len(tool._functions) == 5
|
||||
|
||||
funcs_by_name = {f.name: f for f in tool._functions}
|
||||
|
||||
# Zero-arg tool must have "properties" injected
|
||||
zero_params = funcs_by_name["zero_arg_tool"].parameters()
|
||||
assert "properties" in zero_params
|
||||
assert zero_params["properties"] == {}
|
||||
assert zero_params["type"] == "object"
|
||||
|
||||
# Normal tool must retain its existing properties
|
||||
normal_params = funcs_by_name["normal_tool"].parameters()
|
||||
assert "properties" in normal_params
|
||||
assert "x" in normal_params["properties"]
|
||||
assert normal_params["required"] == ["x"]
|
||||
|
||||
# Non-object schema must NOT have "properties" injected
|
||||
string_params = funcs_by_name["string_schema_tool"].parameters()
|
||||
assert "properties" not in string_params
|
||||
assert string_params["type"] == "string"
|
||||
|
||||
# Empty schema (no "type" key) must NOT have "properties" injected
|
||||
empty_params = funcs_by_name["empty_schema_tool"].parameters()
|
||||
assert "properties" not in empty_params
|
||||
|
||||
# None inputSchema must produce an empty dict (guard against non-conforming servers)
|
||||
none_params = funcs_by_name["none_schema_tool"].parameters()
|
||||
assert none_params == {}
|
||||
|
||||
# Original inputSchema dicts must not be mutated
|
||||
assert "properties" not in original_zero_arg_schema
|
||||
assert "properties" not in original_string_schema
|
||||
assert "properties" not in original_empty_schema
|
||||
|
||||
|
||||
async def test_load_prompts_with_pagination():
|
||||
"""Test that load_prompts handles pagination correctly."""
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
Reference in New Issue
Block a user