fix(mcp): preserve nullable schema coercion
This commit is contained in:
parent
1350d12b0b
commit
aa94883288
@ -415,24 +415,27 @@ def coerce_tool_args(tool_name: str, args: Dict[str, Any]) -> Dict[str, Any]:
|
||||
if not prop_schema:
|
||||
continue
|
||||
expected = prop_schema.get("type")
|
||||
if not expected:
|
||||
if not expected and not _schema_allows_null(prop_schema):
|
||||
continue
|
||||
coerced = _coerce_value(value, expected)
|
||||
coerced = _coerce_value(value, expected, schema=prop_schema)
|
||||
if coerced is not value:
|
||||
args[key] = coerced
|
||||
|
||||
return args
|
||||
|
||||
|
||||
def _coerce_value(value: str, expected_type):
|
||||
def _coerce_value(value: str, expected_type, schema: dict | None = None):
|
||||
"""Attempt to coerce a string *value* to *expected_type*.
|
||||
|
||||
Returns the original string when coercion is not applicable or fails.
|
||||
"""
|
||||
if _schema_allows_null(schema) and value.strip().lower() == "null":
|
||||
return None
|
||||
|
||||
if isinstance(expected_type, list):
|
||||
# Union type — try each in order, return first successful coercion
|
||||
for t in expected_type:
|
||||
result = _coerce_value(value, t)
|
||||
result = _coerce_value(value, t, schema=schema)
|
||||
if result is not value:
|
||||
return result
|
||||
return value
|
||||
@ -445,9 +448,35 @@ def _coerce_value(value: str, expected_type):
|
||||
return _coerce_json(value, list)
|
||||
if expected_type == "object":
|
||||
return _coerce_json(value, dict)
|
||||
if expected_type == "null" and value.strip().lower() == "null":
|
||||
return None
|
||||
return value
|
||||
|
||||
|
||||
def _schema_allows_null(schema: dict | None) -> bool:
|
||||
"""Return True when a JSON Schema fragment explicitly permits null."""
|
||||
if not isinstance(schema, dict):
|
||||
return False
|
||||
|
||||
schema_type = schema.get("type")
|
||||
if schema_type == "null":
|
||||
return True
|
||||
if isinstance(schema_type, list) and "null" in schema_type:
|
||||
return True
|
||||
if schema.get("nullable") is True:
|
||||
return True
|
||||
|
||||
for union_key in ("anyOf", "oneOf"):
|
||||
variants = schema.get(union_key)
|
||||
if not isinstance(variants, list):
|
||||
continue
|
||||
for variant in variants:
|
||||
if isinstance(variant, dict) and variant.get("type") == "null":
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
def _coerce_json(value: str, expected_python_type: type):
|
||||
"""Parse *value* as JSON when the schema expects an array or object.
|
||||
|
||||
|
||||
@ -67,7 +67,7 @@ class TestCoerceNumber:
|
||||
def test_inf_stays_string_for_integer_only(self):
|
||||
"""Infinity should not be converted to int."""
|
||||
result = _coerce_number("inf")
|
||||
assert result == float("inf")
|
||||
assert result == "inf"
|
||||
|
||||
def test_negative_float(self):
|
||||
assert _coerce_number("-2.5") == -2.5
|
||||
@ -255,6 +255,35 @@ class TestCoerceToolArgs:
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["config"] == {"max": 50}
|
||||
|
||||
def test_coerces_string_null_for_nullable_object_arg(self):
|
||||
"""Models often emit literal "null" for optional MCP object args."""
|
||||
schema = self._mock_schema({
|
||||
"setting": {
|
||||
"type": "object",
|
||||
"additionalProperties": True,
|
||||
"nullable": True,
|
||||
"default": None,
|
||||
},
|
||||
})
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"setting": "null"}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["setting"] is None
|
||||
|
||||
def test_coerces_string_null_for_nullable_array_arg(self):
|
||||
schema = self._mock_schema({
|
||||
"stages": {
|
||||
"type": "array",
|
||||
"items": {"type": "object"},
|
||||
"nullable": True,
|
||||
"default": None,
|
||||
},
|
||||
})
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"stages": "null"}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["stages"] is None
|
||||
|
||||
def test_invalid_json_array_preserved_as_string(self):
|
||||
"""If the string isn't valid JSON, pass it through — let the tool decide."""
|
||||
schema = self._mock_schema({"items": {"type": "array"}})
|
||||
|
||||
@ -285,6 +285,7 @@ class TestSchemaConversion:
|
||||
|
||||
assert schema["properties"]["workdir"] == {
|
||||
"type": "string",
|
||||
"nullable": True,
|
||||
"default": None,
|
||||
"description": "Optional working directory",
|
||||
}
|
||||
@ -314,6 +315,7 @@ class TestSchemaConversion:
|
||||
assert schema["properties"]["filters"]["items"] == {
|
||||
"type": "object",
|
||||
"properties": {"field": {"type": "string"}},
|
||||
"nullable": True,
|
||||
}
|
||||
|
||||
def test_convert_mcp_schema_survives_missing_inputschema_attribute(self):
|
||||
|
||||
@ -2441,6 +2441,7 @@ def _normalize_mcp_input_schema(schema: dict | None) -> dict:
|
||||
]
|
||||
if len(non_null) == 1 and len(non_null) != len(variants):
|
||||
replacement = dict(non_null[0]) if isinstance(non_null[0], dict) else {}
|
||||
replacement.setdefault("nullable", True)
|
||||
for meta_key in ("title", "description", "default", "examples"):
|
||||
if meta_key in stripped and meta_key not in replacement:
|
||||
replacement[meta_key] = stripped[meta_key]
|
||||
|
||||
Loading…
Reference in New Issue
Block a user