fix(mcp): use JSON Schema types in tool input models; exclude None from MCP calls
When non-Anthropic models (MiniMax, Moonshot, etc.) call MCP tools, they
sometimes send `null` for parameters they consider optional. The previous
implementation typed all fields as `object | None`, so:
1. Pydantic accepted `null` even for required fields (e.g. `query`)
2. `model_dump()` serialized these as `{"query": null, ...}`
3. The MCP server received null for required parameters → validation error
Fix:
- Map JSON Schema types to proper Python types (string→str, integer→int,
etc.) so Pydantic rejects null for required fields
- Use `exclude_none=True` in model_dump to strip optional null parameters
before passing to the MCP server
This affects ALL MCP tool servers, not just specific ones.
Tested with: MiniMax-M2.7-highspeed + dongtian MCP server (15 tools).
This commit is contained in:
@@ -29,13 +29,23 @@ class McpToolAdapter(BaseTool):
|
||||
output = await self._manager.call_tool(
|
||||
self._tool_info.server_name,
|
||||
self._tool_info.name,
|
||||
arguments.model_dump(mode="json"),
|
||||
arguments.model_dump(mode="json", exclude_none=True),
|
||||
)
|
||||
except McpServerNotConnectedError as exc:
|
||||
return ToolResult(output=str(exc), is_error=True)
|
||||
return ToolResult(output=output)
|
||||
|
||||
|
||||
_JSON_TYPE_MAP: dict[str, type] = {
|
||||
"string": str,
|
||||
"integer": int,
|
||||
"number": float,
|
||||
"boolean": bool,
|
||||
"array": list,
|
||||
"object": dict,
|
||||
}
|
||||
|
||||
|
||||
def _input_model_from_schema(tool_name: str, schema: dict[str, object]) -> type[BaseModel]:
|
||||
properties = schema.get("properties", {})
|
||||
if not isinstance(properties, dict):
|
||||
@@ -44,8 +54,12 @@ def _input_model_from_schema(tool_name: str, schema: dict[str, object]) -> type[
|
||||
fields = {}
|
||||
required = set(schema.get("required", [])) if isinstance(schema.get("required", []), list) else set()
|
||||
for key in properties:
|
||||
default = ... if key in required else None
|
||||
fields[key] = (object | None, Field(default=default))
|
||||
prop = properties[key] if isinstance(properties[key], dict) else {}
|
||||
py_type = _JSON_TYPE_MAP.get(str(prop.get("type", "")), object)
|
||||
if key in required:
|
||||
fields[key] = (py_type, Field(default=...))
|
||||
else:
|
||||
fields[key] = (py_type | None, Field(default=None))
|
||||
return create_model(f"{tool_name.title().replace('-', '_')}Input", **fields)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
"""Tests for MCP tool adapters — input model generation and argument serialization."""
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from openharness.tools.mcp_tool import _input_model_from_schema
|
||||
|
||||
|
||||
class TestInputModelFromSchema:
|
||||
"""Verify _input_model_from_schema maps JSON Schema types correctly."""
|
||||
|
||||
def test_required_string_rejects_none(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
}
|
||||
Model = _input_model_from_schema("search", schema)
|
||||
with pytest.raises(ValidationError):
|
||||
Model(query=None)
|
||||
|
||||
def test_required_string_accepts_value(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
}
|
||||
Model = _input_model_from_schema("search", schema)
|
||||
m = Model(query="zigzag")
|
||||
assert m.query == "zigzag"
|
||||
|
||||
def test_optional_string_defaults_to_none(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string"},
|
||||
"wing": {"type": "string"},
|
||||
},
|
||||
"required": ["query"],
|
||||
}
|
||||
Model = _input_model_from_schema("search", schema)
|
||||
m = Model(query="test")
|
||||
assert m.wing is None
|
||||
|
||||
def test_exclude_none_omits_optional_keeps_required(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string"},
|
||||
"wing": {"type": "string"},
|
||||
"limit": {"type": "integer"},
|
||||
},
|
||||
"required": ["query"],
|
||||
}
|
||||
Model = _input_model_from_schema("search", schema)
|
||||
m = Model(query="test")
|
||||
dumped = m.model_dump(mode="json", exclude_none=True)
|
||||
assert dumped == {"query": "test"}
|
||||
|
||||
def test_all_json_types_mapped(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"count": {"type": "integer"},
|
||||
"score": {"type": "number"},
|
||||
"active": {"type": "boolean"},
|
||||
"tags": {"type": "array"},
|
||||
"meta": {"type": "object"},
|
||||
},
|
||||
"required": ["name", "count", "score", "active", "tags", "meta"],
|
||||
}
|
||||
Model = _input_model_from_schema("full", schema)
|
||||
m = Model(name="x", count=1, score=0.5, active=True, tags=["a"], meta={"k": "v"})
|
||||
dumped = m.model_dump(mode="json")
|
||||
assert dumped == {
|
||||
"name": "x", "count": 1, "score": 0.5,
|
||||
"active": True, "tags": ["a"], "meta": {"k": "v"},
|
||||
}
|
||||
|
||||
def test_empty_schema_creates_valid_model(self):
|
||||
Model = _input_model_from_schema("empty", {"type": "object"})
|
||||
m = Model()
|
||||
assert m.model_dump(mode="json") == {}
|
||||
|
||||
def test_model_rejects_null_for_required_integer(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {"limit": {"type": "integer"}},
|
||||
"required": ["limit"],
|
||||
}
|
||||
Model = _input_model_from_schema("limited", schema)
|
||||
with pytest.raises(ValidationError):
|
||||
Model(limit=None)
|
||||
Reference in New Issue
Block a user