mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(kimi,mcp): Moonshot schema sanitizer + MCP schema robustness (#14805)
Fixes a broader class of 'tools.function.parameters is not a valid moonshot flavored json schema' errors on Nous / OpenRouter aggregators routing to moonshotai/kimi-k2.6 with MCP tools loaded. ## Moonshot sanitizer (agent/moonshot_schema.py, new) Model-name-routed (not base-URL-routed) so Nous / OpenRouter users are covered alongside api.moonshot.ai. Applied in ChatCompletionsTransport.build_kwargs when is_moonshot_model(model). Two repairs: 1. Fill missing 'type' on every property / items / anyOf-child schema node (structural walk — only schema-position dicts are touched, not container maps like properties/$defs). 2. Strip 'type' at anyOf parents; Moonshot rejects it. ## MCP normalizer hardened (tools/mcp_tool.py) Draft-07 $ref rewrite from PR #14802 now also does: - coerce missing / null 'type' on object-shaped nodes (salvages #4897) - prune 'required' arrays to names that exist in 'properties' (salvages #4651; Gemini 400s on dangling required) - apply recursively, not just top-level These repairs are provider-agnostic so the same MCP schema is valid on OpenAI, Anthropic, Gemini, and Moonshot in one pass. ## Crash fix: safe getattr for Tool.inputSchema _convert_mcp_schema now uses getattr(t, 'inputSchema', None) so MCP servers whose Tool objects omit the attribute entirely no longer abort registration (salvages #3882). ## Validation - tests/agent/test_moonshot_schema.py: 27 new tests (model detection, missing-type fill, anyOf-parent strip, non-mutation, real-world MCP shape) - tests/tools/test_mcp_tool.py: 7 new tests (missing / null type, required pruning, nested repair, safe getattr) - tests/agent/transports/test_chat_completions.py: 2 new integration tests (Moonshot route sanitizes, non-Moonshot route doesn't) - Targeted suite: 49 passed - E2E via execute_code with a realistic MCP tool carrying all three Moonshot rejection modes + dangling required + draft-07 refs: sanitizer produces a schema valid on Moonshot and Gemini
This commit is contained in:
parent
24f139e16a
commit
e26c4f0e34
6 changed files with 663 additions and 3 deletions
190
agent/moonshot_schema.py
Normal file
190
agent/moonshot_schema.py
Normal file
|
|
@ -0,0 +1,190 @@
|
||||||
|
"""Helpers for translating OpenAI-style tool schemas to Moonshot's schema subset.
|
||||||
|
|
||||||
|
Moonshot (Kimi) accepts a stricter subset of JSON Schema than standard OpenAI
|
||||||
|
tool calling. Requests that violate it fail with HTTP 400:
|
||||||
|
|
||||||
|
tools.function.parameters is not a valid moonshot flavored json schema,
|
||||||
|
details: <...>
|
||||||
|
|
||||||
|
Known rejection modes documented at
|
||||||
|
https://forum.moonshot.ai/t/tool-calling-specification-violation-on-moonshot-api/102
|
||||||
|
and MoonshotAI/kimi-cli#1595:
|
||||||
|
|
||||||
|
1. Every property schema must carry a ``type``. Standard JSON Schema allows
|
||||||
|
type to be omitted (the value is then unconstrained); Moonshot refuses.
|
||||||
|
2. When ``anyOf`` is used, ``type`` must be on the ``anyOf`` children, not
|
||||||
|
the parent. Presence of both causes "type should be defined in anyOf
|
||||||
|
items instead of the parent schema".
|
||||||
|
|
||||||
|
The ``#/definitions/...`` → ``#/$defs/...`` rewrite for draft-07 refs is
|
||||||
|
handled separately in ``tools/mcp_tool._normalize_mcp_input_schema`` so it
|
||||||
|
applies at MCP registration time for all providers.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import copy
|
||||||
|
from typing import Any, Dict, List
|
||||||
|
|
||||||
|
# Keys whose values are maps of name → schema (not schemas themselves).
|
||||||
|
# When we recurse, we walk the values of these maps as schemas, but we do
|
||||||
|
# NOT apply the missing-type repair to the map itself.
|
||||||
|
_SCHEMA_MAP_KEYS = frozenset({"properties", "patternProperties", "$defs", "definitions"})
|
||||||
|
|
||||||
|
# Keys whose values are lists of schemas.
|
||||||
|
_SCHEMA_LIST_KEYS = frozenset({"anyOf", "oneOf", "allOf", "prefixItems"})
|
||||||
|
|
||||||
|
# Keys whose values are a single nested schema.
|
||||||
|
_SCHEMA_NODE_KEYS = frozenset({"items", "contains", "not", "additionalProperties", "propertyNames"})
|
||||||
|
|
||||||
|
|
||||||
|
def _repair_schema(node: Any, is_schema: bool = True) -> Any:
|
||||||
|
"""Recursively apply Moonshot repairs to a schema node.
|
||||||
|
|
||||||
|
``is_schema=True`` means this dict is a JSON Schema node and gets the
|
||||||
|
missing-type + anyOf-parent repairs applied. ``is_schema=False`` means
|
||||||
|
it's a container map (e.g. the value of ``properties``) and we only
|
||||||
|
recurse into its values.
|
||||||
|
"""
|
||||||
|
if isinstance(node, list):
|
||||||
|
# Lists only show up under schema-list keys (anyOf/oneOf/allOf), so
|
||||||
|
# every element is itself a schema.
|
||||||
|
return [_repair_schema(item, is_schema=True) for item in node]
|
||||||
|
if not isinstance(node, dict):
|
||||||
|
return node
|
||||||
|
|
||||||
|
# Walk the dict, deciding per-key whether recursion is into a schema
|
||||||
|
# node, a container map, or a scalar.
|
||||||
|
repaired: Dict[str, Any] = {}
|
||||||
|
for key, value in node.items():
|
||||||
|
if key in _SCHEMA_MAP_KEYS and isinstance(value, dict):
|
||||||
|
# Map of name → schema. Don't treat the map itself as a schema
|
||||||
|
# (it has no type / properties of its own), but each value is.
|
||||||
|
repaired[key] = {
|
||||||
|
sub_key: _repair_schema(sub_val, is_schema=True)
|
||||||
|
for sub_key, sub_val in value.items()
|
||||||
|
}
|
||||||
|
elif key in _SCHEMA_LIST_KEYS and isinstance(value, list):
|
||||||
|
repaired[key] = [_repair_schema(v, is_schema=True) for v in value]
|
||||||
|
elif key in _SCHEMA_NODE_KEYS:
|
||||||
|
# items / not / additionalProperties: single nested schema.
|
||||||
|
# additionalProperties can also be a bool — leave those alone.
|
||||||
|
if isinstance(value, dict):
|
||||||
|
repaired[key] = _repair_schema(value, is_schema=True)
|
||||||
|
else:
|
||||||
|
repaired[key] = value
|
||||||
|
else:
|
||||||
|
# Scalars (description, title, format, enum values, etc.) pass through.
|
||||||
|
repaired[key] = value
|
||||||
|
|
||||||
|
if not is_schema:
|
||||||
|
return repaired
|
||||||
|
|
||||||
|
# Rule 2: when anyOf is present, type belongs only on the children.
|
||||||
|
if "anyOf" in repaired and isinstance(repaired["anyOf"], list):
|
||||||
|
repaired.pop("type", None)
|
||||||
|
return repaired
|
||||||
|
|
||||||
|
# Rule 1: property schemas without type need one. $ref nodes are exempt
|
||||||
|
# — their type comes from the referenced definition.
|
||||||
|
if "$ref" in repaired:
|
||||||
|
return repaired
|
||||||
|
return _fill_missing_type(repaired)
|
||||||
|
|
||||||
|
|
||||||
|
def _fill_missing_type(node: Dict[str, Any]) -> Dict[str, Any]:
|
||||||
|
"""Infer a reasonable ``type`` if this schema node has none."""
|
||||||
|
if "type" in node and node["type"] not in (None, ""):
|
||||||
|
return node
|
||||||
|
|
||||||
|
# Heuristic: presence of ``properties`` → object, ``items`` → array, ``enum``
|
||||||
|
# → type of first enum value, else fall back to ``string`` (safest scalar).
|
||||||
|
if "properties" in node or "required" in node or "additionalProperties" in node:
|
||||||
|
inferred = "object"
|
||||||
|
elif "items" in node or "prefixItems" in node:
|
||||||
|
inferred = "array"
|
||||||
|
elif "enum" in node and isinstance(node["enum"], list) and node["enum"]:
|
||||||
|
sample = node["enum"][0]
|
||||||
|
if isinstance(sample, bool):
|
||||||
|
inferred = "boolean"
|
||||||
|
elif isinstance(sample, int):
|
||||||
|
inferred = "integer"
|
||||||
|
elif isinstance(sample, float):
|
||||||
|
inferred = "number"
|
||||||
|
else:
|
||||||
|
inferred = "string"
|
||||||
|
else:
|
||||||
|
inferred = "string"
|
||||||
|
|
||||||
|
return {**node, "type": inferred}
|
||||||
|
|
||||||
|
|
||||||
|
def sanitize_moonshot_tool_parameters(parameters: Any) -> Dict[str, Any]:
|
||||||
|
"""Normalize tool parameters to a Moonshot-compatible object schema.
|
||||||
|
|
||||||
|
Returns a deep-copied schema with the two flavored-JSON-Schema repairs
|
||||||
|
applied. Input is not mutated.
|
||||||
|
"""
|
||||||
|
if not isinstance(parameters, dict):
|
||||||
|
return {"type": "object", "properties": {}}
|
||||||
|
|
||||||
|
repaired = _repair_schema(copy.deepcopy(parameters), is_schema=True)
|
||||||
|
if not isinstance(repaired, dict):
|
||||||
|
return {"type": "object", "properties": {}}
|
||||||
|
|
||||||
|
# Top-level must be an object schema
|
||||||
|
if repaired.get("type") != "object":
|
||||||
|
repaired["type"] = "object"
|
||||||
|
if "properties" not in repaired:
|
||||||
|
repaired["properties"] = {}
|
||||||
|
|
||||||
|
return repaired
|
||||||
|
|
||||||
|
|
||||||
|
def sanitize_moonshot_tools(tools: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
|
||||||
|
"""Apply ``sanitize_moonshot_tool_parameters`` to every tool's parameters."""
|
||||||
|
if not tools:
|
||||||
|
return tools
|
||||||
|
|
||||||
|
sanitized: List[Dict[str, Any]] = []
|
||||||
|
any_change = False
|
||||||
|
for tool in tools:
|
||||||
|
if not isinstance(tool, dict):
|
||||||
|
sanitized.append(tool)
|
||||||
|
continue
|
||||||
|
fn = tool.get("function")
|
||||||
|
if not isinstance(fn, dict):
|
||||||
|
sanitized.append(tool)
|
||||||
|
continue
|
||||||
|
params = fn.get("parameters")
|
||||||
|
repaired = sanitize_moonshot_tool_parameters(params)
|
||||||
|
if repaired is not params:
|
||||||
|
any_change = True
|
||||||
|
new_fn = {**fn, "parameters": repaired}
|
||||||
|
sanitized.append({**tool, "function": new_fn})
|
||||||
|
else:
|
||||||
|
sanitized.append(tool)
|
||||||
|
|
||||||
|
return sanitized if any_change else tools
|
||||||
|
|
||||||
|
|
||||||
|
def is_moonshot_model(model: str | None) -> bool:
|
||||||
|
"""True for any Kimi / Moonshot model slug, regardless of aggregator prefix.
|
||||||
|
|
||||||
|
Matches bare names (``kimi-k2.6``, ``moonshotai/Kimi-K2.6``) and aggregator-
|
||||||
|
prefixed slugs (``nous/moonshotai/kimi-k2.6``, ``openrouter/moonshotai/...``).
|
||||||
|
Detection by model name covers Nous / OpenRouter / other aggregators that
|
||||||
|
route to Moonshot's inference, where the base URL is the aggregator's, not
|
||||||
|
``api.moonshot.ai``.
|
||||||
|
"""
|
||||||
|
if not model:
|
||||||
|
return False
|
||||||
|
bare = model.strip().lower()
|
||||||
|
# Last path segment (covers aggregator-prefixed slugs)
|
||||||
|
tail = bare.rsplit("/", 1)[-1]
|
||||||
|
if tail.startswith("kimi-") or tail == "kimi":
|
||||||
|
return True
|
||||||
|
# Vendor-prefixed forms commonly used on aggregators
|
||||||
|
if "moonshot" in bare or "/kimi" in bare or bare.startswith("kimi"):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
@ -12,6 +12,7 @@ reasoning configuration, temperature handling, and extra_body assembly.
|
||||||
import copy
|
import copy
|
||||||
from typing import Any, Dict, List, Optional
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
|
from agent.moonshot_schema import is_moonshot_model, sanitize_moonshot_tools
|
||||||
from agent.prompt_builder import DEVELOPER_ROLE_MODELS
|
from agent.prompt_builder import DEVELOPER_ROLE_MODELS
|
||||||
from agent.transports.base import ProviderTransport
|
from agent.transports.base import ProviderTransport
|
||||||
from agent.transports.types import NormalizedResponse, ToolCall, Usage
|
from agent.transports.types import NormalizedResponse, ToolCall, Usage
|
||||||
|
|
@ -172,6 +173,11 @@ class ChatCompletionsTransport(ProviderTransport):
|
||||||
|
|
||||||
# Tools
|
# Tools
|
||||||
if tools:
|
if tools:
|
||||||
|
# Moonshot/Kimi uses a stricter flavored JSON Schema. Rewriting
|
||||||
|
# tool parameters here keeps aggregator routes (Nous, OpenRouter,
|
||||||
|
# etc.) compatible, in addition to direct moonshot.ai endpoints.
|
||||||
|
if is_moonshot_model(model):
|
||||||
|
tools = sanitize_moonshot_tools(tools)
|
||||||
api_kwargs["tools"] = tools
|
api_kwargs["tools"] = tools
|
||||||
|
|
||||||
# max_tokens resolution — priority: ephemeral > user > provider default
|
# max_tokens resolution — priority: ephemeral > user > provider default
|
||||||
|
|
|
||||||
254
tests/agent/test_moonshot_schema.py
Normal file
254
tests/agent/test_moonshot_schema.py
Normal file
|
|
@ -0,0 +1,254 @@
|
||||||
|
"""Tests for Moonshot/Kimi flavored-JSON-Schema sanitizer.
|
||||||
|
|
||||||
|
Moonshot's tool-parameter validator rejects several shapes that the rest of
|
||||||
|
the JSON Schema ecosystem accepts:
|
||||||
|
|
||||||
|
1. Properties without ``type`` — Moonshot requires ``type`` on every node.
|
||||||
|
2. ``type`` at the parent of ``anyOf`` — Moonshot requires it only inside
|
||||||
|
``anyOf`` children.
|
||||||
|
|
||||||
|
These tests cover the repairs applied by ``agent/moonshot_schema.py``.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from agent.moonshot_schema import (
|
||||||
|
is_moonshot_model,
|
||||||
|
sanitize_moonshot_tool_parameters,
|
||||||
|
sanitize_moonshot_tools,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestMoonshotModelDetection:
|
||||||
|
"""is_moonshot_model() must match across aggregator prefixes."""
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"model",
|
||||||
|
[
|
||||||
|
"kimi-k2.6",
|
||||||
|
"kimi-k2-thinking",
|
||||||
|
"moonshotai/Kimi-K2.6",
|
||||||
|
"moonshotai/kimi-k2.6",
|
||||||
|
"nous/moonshotai/kimi-k2.6",
|
||||||
|
"openrouter/moonshotai/kimi-k2-thinking",
|
||||||
|
"MOONSHOTAI/KIMI-K2.6",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_positive_matches(self, model):
|
||||||
|
assert is_moonshot_model(model) is True
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"model",
|
||||||
|
[
|
||||||
|
"",
|
||||||
|
None,
|
||||||
|
"anthropic/claude-sonnet-4.6",
|
||||||
|
"openai/gpt-5.4",
|
||||||
|
"google/gemini-3-flash-preview",
|
||||||
|
"deepseek-chat",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_negative_matches(self, model):
|
||||||
|
assert is_moonshot_model(model) is False
|
||||||
|
|
||||||
|
|
||||||
|
class TestMissingTypeFilled:
|
||||||
|
"""Rule 1: every property must carry a type."""
|
||||||
|
|
||||||
|
def test_property_without_type_gets_string(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"query": {"description": "a bare property"}},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert out["properties"]["query"]["type"] == "string"
|
||||||
|
|
||||||
|
def test_property_with_enum_infers_type_from_first_value(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"flag": {"enum": [True, False]}},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert out["properties"]["flag"]["type"] == "boolean"
|
||||||
|
|
||||||
|
def test_nested_properties_are_repaired(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"filter": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"field": {"description": "no type"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert out["properties"]["filter"]["properties"]["field"]["type"] == "string"
|
||||||
|
|
||||||
|
def test_array_items_without_type_get_repaired(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"tags": {
|
||||||
|
"type": "array",
|
||||||
|
"items": {"description": "tag entry"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert out["properties"]["tags"]["items"]["type"] == "string"
|
||||||
|
|
||||||
|
def test_ref_node_is_not_given_synthetic_type(self):
|
||||||
|
"""$ref nodes should NOT get a synthetic type — the referenced
|
||||||
|
definition supplies it, and Moonshot would reject the conflict."""
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"payload": {"$ref": "#/$defs/Payload"}},
|
||||||
|
"$defs": {"Payload": {"type": "object", "properties": {}}},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert "type" not in out["properties"]["payload"]
|
||||||
|
assert out["properties"]["payload"]["$ref"] == "#/$defs/Payload"
|
||||||
|
|
||||||
|
|
||||||
|
class TestAnyOfParentType:
|
||||||
|
"""Rule 2: type must not appear at the anyOf parent level."""
|
||||||
|
|
||||||
|
def test_parent_type_stripped_when_anyof_present(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"from_format": {
|
||||||
|
"type": "string",
|
||||||
|
"anyOf": [
|
||||||
|
{"type": "string"},
|
||||||
|
{"type": "null"},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
from_format = out["properties"]["from_format"]
|
||||||
|
assert "type" not in from_format
|
||||||
|
assert "anyOf" in from_format
|
||||||
|
|
||||||
|
def test_anyof_children_missing_type_get_filled(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"value": {
|
||||||
|
"anyOf": [
|
||||||
|
{"type": "string"},
|
||||||
|
{"description": "A typeless option"},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
children = out["properties"]["value"]["anyOf"]
|
||||||
|
assert children[0]["type"] == "string"
|
||||||
|
assert "type" in children[1]
|
||||||
|
|
||||||
|
|
||||||
|
class TestTopLevelGuarantees:
|
||||||
|
"""The returned top-level schema is always a well-formed object."""
|
||||||
|
|
||||||
|
def test_non_dict_input_returns_empty_object(self):
|
||||||
|
assert sanitize_moonshot_tool_parameters(None) == {"type": "object", "properties": {}}
|
||||||
|
assert sanitize_moonshot_tool_parameters("garbage") == {"type": "object", "properties": {}}
|
||||||
|
assert sanitize_moonshot_tool_parameters([]) == {"type": "object", "properties": {}}
|
||||||
|
|
||||||
|
def test_non_object_top_level_coerced(self):
|
||||||
|
params = {"type": "string"}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert out["type"] == "object"
|
||||||
|
assert "properties" in out
|
||||||
|
|
||||||
|
def test_does_not_mutate_input(self):
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"q": {"description": "no type"}},
|
||||||
|
}
|
||||||
|
snapshot = {
|
||||||
|
"type": params["type"],
|
||||||
|
"properties": {"q": dict(params["properties"]["q"])},
|
||||||
|
}
|
||||||
|
sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert params["type"] == snapshot["type"]
|
||||||
|
assert "type" not in params["properties"]["q"]
|
||||||
|
|
||||||
|
|
||||||
|
class TestToolListSanitizer:
|
||||||
|
"""sanitize_moonshot_tools() walks an OpenAI-format tool list."""
|
||||||
|
|
||||||
|
def test_applies_per_tool(self):
|
||||||
|
tools = [
|
||||||
|
{
|
||||||
|
"type": "function",
|
||||||
|
"function": {
|
||||||
|
"name": "search",
|
||||||
|
"description": "Search",
|
||||||
|
"parameters": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"q": {"description": "query"}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "function",
|
||||||
|
"function": {
|
||||||
|
"name": "noop",
|
||||||
|
"description": "Does nothing",
|
||||||
|
"parameters": {"type": "object", "properties": {}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]
|
||||||
|
out = sanitize_moonshot_tools(tools)
|
||||||
|
assert out[0]["function"]["parameters"]["properties"]["q"]["type"] == "string"
|
||||||
|
# Second tool already clean — should be structurally equivalent
|
||||||
|
assert out[1]["function"]["parameters"] == {"type": "object", "properties": {}}
|
||||||
|
|
||||||
|
def test_empty_list_is_passthrough(self):
|
||||||
|
assert sanitize_moonshot_tools([]) == []
|
||||||
|
assert sanitize_moonshot_tools(None) is None
|
||||||
|
|
||||||
|
def test_skips_malformed_entries(self):
|
||||||
|
"""Entries without a function dict are passed through untouched."""
|
||||||
|
tools = [{"type": "function"}, {"not": "a tool"}]
|
||||||
|
out = sanitize_moonshot_tools(tools)
|
||||||
|
assert out == tools
|
||||||
|
|
||||||
|
|
||||||
|
class TestRealWorldMCPShape:
|
||||||
|
"""End-to-end: a realistic MCP-style schema that used to 400 on Moonshot."""
|
||||||
|
|
||||||
|
def test_combined_rewrites(self):
|
||||||
|
# Shape: missing type on a property, anyOf with parent type, array
|
||||||
|
# items without type — all in one tool.
|
||||||
|
params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"query": {"description": "search text"},
|
||||||
|
"filter": {
|
||||||
|
"type": "string",
|
||||||
|
"anyOf": [
|
||||||
|
{"type": "string"},
|
||||||
|
{"type": "null"},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
"tags": {
|
||||||
|
"type": "array",
|
||||||
|
"items": {"description": "tag"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
"required": ["query"],
|
||||||
|
}
|
||||||
|
out = sanitize_moonshot_tool_parameters(params)
|
||||||
|
assert out["properties"]["query"]["type"] == "string"
|
||||||
|
assert "type" not in out["properties"]["filter"]
|
||||||
|
assert out["properties"]["filter"]["anyOf"][0]["type"] == "string"
|
||||||
|
assert out["properties"]["tags"]["items"]["type"] == "string"
|
||||||
|
assert out["required"] == ["query"]
|
||||||
|
|
@ -238,6 +238,56 @@ class TestChatCompletionsKimi:
|
||||||
)
|
)
|
||||||
assert kw["extra_body"]["thinking"] == {"type": "disabled"}
|
assert kw["extra_body"]["thinking"] == {"type": "disabled"}
|
||||||
|
|
||||||
|
def test_moonshot_tool_schemas_are_sanitized_by_model_name(self, transport):
|
||||||
|
"""Aggregator routes (Nous, OpenRouter) hit Moonshot by model name, not base URL."""
|
||||||
|
tools = [
|
||||||
|
{
|
||||||
|
"type": "function",
|
||||||
|
"function": {
|
||||||
|
"name": "search",
|
||||||
|
"description": "Search",
|
||||||
|
"parameters": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"q": {"description": "query"}, # missing type
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]
|
||||||
|
kw = transport.build_kwargs(
|
||||||
|
model="moonshotai/kimi-k2.6",
|
||||||
|
messages=[{"role": "user", "content": "Hi"}],
|
||||||
|
tools=tools,
|
||||||
|
max_tokens_param_fn=lambda n: {"max_tokens": n},
|
||||||
|
)
|
||||||
|
assert kw["tools"][0]["function"]["parameters"]["properties"]["q"]["type"] == "string"
|
||||||
|
|
||||||
|
def test_non_moonshot_tools_are_not_mutated(self, transport):
|
||||||
|
"""Other models don't go through the Moonshot sanitizer."""
|
||||||
|
original_params = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"q": {"description": "query"}}, # missing type
|
||||||
|
}
|
||||||
|
tools = [
|
||||||
|
{
|
||||||
|
"type": "function",
|
||||||
|
"function": {
|
||||||
|
"name": "search",
|
||||||
|
"description": "Search",
|
||||||
|
"parameters": original_params,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]
|
||||||
|
kw = transport.build_kwargs(
|
||||||
|
model="anthropic/claude-sonnet-4.6",
|
||||||
|
messages=[{"role": "user", "content": "Hi"}],
|
||||||
|
tools=tools,
|
||||||
|
max_tokens_param_fn=lambda n: {"max_tokens": n},
|
||||||
|
)
|
||||||
|
# The parameters dict is passed through untouched (no synthetic type)
|
||||||
|
assert "type" not in kw["tools"][0]["function"]["parameters"]["properties"]["q"]
|
||||||
|
|
||||||
|
|
||||||
class TestChatCompletionsValidate:
|
class TestChatCompletionsValidate:
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -186,6 +186,111 @@ class TestSchemaConversion:
|
||||||
assert schema["parameters"]["properties"]["items"]["items"]["$ref"] == "#/$defs/Entry"
|
assert schema["parameters"]["properties"]["items"]["items"]["$ref"] == "#/$defs/Entry"
|
||||||
assert schema["parameters"]["$defs"]["Entry"]["properties"]["child"]["$ref"] == "#/$defs/Child"
|
assert schema["parameters"]["$defs"]["Entry"]["properties"]["child"]["$ref"] == "#/$defs/Child"
|
||||||
|
|
||||||
|
def test_missing_type_on_object_is_coerced(self):
|
||||||
|
"""Schemas that describe an object but omit ``type`` get type='object'."""
|
||||||
|
from tools.mcp_tool import _normalize_mcp_input_schema
|
||||||
|
|
||||||
|
schema = _normalize_mcp_input_schema({
|
||||||
|
"properties": {"q": {"type": "string"}},
|
||||||
|
"required": ["q"],
|
||||||
|
})
|
||||||
|
|
||||||
|
assert schema["type"] == "object"
|
||||||
|
assert schema["properties"]["q"]["type"] == "string"
|
||||||
|
assert schema["required"] == ["q"]
|
||||||
|
|
||||||
|
def test_null_type_on_object_is_coerced(self):
|
||||||
|
"""type: None should be treated like missing type (common MCP server bug)."""
|
||||||
|
from tools.mcp_tool import _normalize_mcp_input_schema
|
||||||
|
|
||||||
|
schema = _normalize_mcp_input_schema({
|
||||||
|
"type": None,
|
||||||
|
"properties": {"x": {"type": "integer"}},
|
||||||
|
})
|
||||||
|
|
||||||
|
assert schema["type"] == "object"
|
||||||
|
|
||||||
|
def test_required_pruned_when_property_missing(self):
|
||||||
|
"""Gemini 400s on required names that don't exist in properties."""
|
||||||
|
from tools.mcp_tool import _normalize_mcp_input_schema
|
||||||
|
|
||||||
|
schema = _normalize_mcp_input_schema({
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"a": {"type": "string"}},
|
||||||
|
"required": ["a", "ghost", "phantom"],
|
||||||
|
})
|
||||||
|
|
||||||
|
assert schema["required"] == ["a"]
|
||||||
|
|
||||||
|
def test_required_removed_when_all_names_dangle(self):
|
||||||
|
from tools.mcp_tool import _normalize_mcp_input_schema
|
||||||
|
|
||||||
|
schema = _normalize_mcp_input_schema({
|
||||||
|
"type": "object",
|
||||||
|
"properties": {},
|
||||||
|
"required": ["ghost"],
|
||||||
|
})
|
||||||
|
|
||||||
|
assert "required" not in schema
|
||||||
|
|
||||||
|
def test_required_pruning_applies_recursively_inside_nested_objects(self):
|
||||||
|
"""Nested object schemas also get required pruning."""
|
||||||
|
from tools.mcp_tool import _normalize_mcp_input_schema
|
||||||
|
|
||||||
|
schema = _normalize_mcp_input_schema({
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"filter": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"field": {"type": "string"}},
|
||||||
|
"required": ["field", "missing"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
assert schema["properties"]["filter"]["required"] == ["field"]
|
||||||
|
|
||||||
|
def test_object_in_array_items_gets_properties_filled(self):
|
||||||
|
"""Array-item object schemas without properties get an empty dict."""
|
||||||
|
from tools.mcp_tool import _normalize_mcp_input_schema
|
||||||
|
|
||||||
|
schema = _normalize_mcp_input_schema({
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"items": {
|
||||||
|
"type": "array",
|
||||||
|
"items": {"type": "object"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
assert schema["properties"]["items"]["items"]["properties"] == {}
|
||||||
|
|
||||||
|
def test_convert_mcp_schema_survives_missing_inputschema_attribute(self):
|
||||||
|
"""A Tool object without .inputSchema must not crash registration."""
|
||||||
|
import types
|
||||||
|
|
||||||
|
from tools.mcp_tool import _convert_mcp_schema
|
||||||
|
|
||||||
|
bare_tool = types.SimpleNamespace(name="probe", description="Probe")
|
||||||
|
schema = _convert_mcp_schema("srv", bare_tool)
|
||||||
|
|
||||||
|
assert schema["name"] == "mcp_srv_probe"
|
||||||
|
assert schema["parameters"] == {"type": "object", "properties": {}}
|
||||||
|
|
||||||
|
def test_convert_mcp_schema_with_none_inputschema(self):
|
||||||
|
"""Tool with inputSchema=None produces a valid empty object schema."""
|
||||||
|
import types
|
||||||
|
|
||||||
|
from tools.mcp_tool import _convert_mcp_schema
|
||||||
|
|
||||||
|
# Note: _make_mcp_tool(input_schema=None) falls back to a default —
|
||||||
|
# build the namespace directly so .inputSchema really is None.
|
||||||
|
mcp_tool = types.SimpleNamespace(name="probe", description="Probe", inputSchema=None)
|
||||||
|
schema = _convert_mcp_schema("srv", mcp_tool)
|
||||||
|
|
||||||
|
assert schema["parameters"] == {"type": "object", "properties": {}}
|
||||||
|
|
||||||
def test_tool_name_prefix_format(self):
|
def test_tool_name_prefix_format(self):
|
||||||
from tools.mcp_tool import _convert_mcp_schema
|
from tools.mcp_tool import _convert_mcp_schema
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -2026,6 +2026,19 @@ def _normalize_mcp_input_schema(schema: dict | None) -> dict:
|
||||||
requires local refs to point into ``#/$defs/...`` instead. Normalize the
|
requires local refs to point into ``#/$defs/...`` instead. Normalize the
|
||||||
common draft-07 shape here so MCP tool schemas remain portable across
|
common draft-07 shape here so MCP tool schemas remain portable across
|
||||||
OpenAI-compatible providers.
|
OpenAI-compatible providers.
|
||||||
|
|
||||||
|
Additional MCP-server robustness repairs applied recursively:
|
||||||
|
|
||||||
|
* Missing or ``null`` ``type`` on an object-shaped node is coerced to
|
||||||
|
``"object"`` (some servers omit it). See PR #4897.
|
||||||
|
* When an ``object`` node lacks ``properties``, an empty ``properties``
|
||||||
|
dict is added so ``required`` entries don't dangle.
|
||||||
|
* ``required`` arrays are pruned to only names that exist in
|
||||||
|
``properties``; otherwise Google AI Studio / Gemini 400s with
|
||||||
|
``property is not defined``. See PR #4651.
|
||||||
|
|
||||||
|
All repairs are provider-agnostic and ideally produce a schema valid on
|
||||||
|
OpenAI, Anthropic, Gemini, and Moonshot in one pass.
|
||||||
"""
|
"""
|
||||||
if not schema:
|
if not schema:
|
||||||
return {"type": "object", "properties": {}}
|
return {"type": "object", "properties": {}}
|
||||||
|
|
@ -2044,10 +2057,52 @@ def _normalize_mcp_input_schema(schema: dict | None) -> dict:
|
||||||
return [_rewrite_local_refs(item) for item in node]
|
return [_rewrite_local_refs(item) for item in node]
|
||||||
return node
|
return node
|
||||||
|
|
||||||
normalized = _rewrite_local_refs(schema)
|
def _repair_object_shape(node):
|
||||||
|
"""Recursively repair object-shaped nodes: fill type, prune required."""
|
||||||
|
if isinstance(node, list):
|
||||||
|
return [_repair_object_shape(item) for item in node]
|
||||||
|
if not isinstance(node, dict):
|
||||||
|
return node
|
||||||
|
|
||||||
|
repaired = {k: _repair_object_shape(v) for k, v in node.items()}
|
||||||
|
|
||||||
|
# Coerce missing / null type when the shape is clearly an object
|
||||||
|
# (has properties or required but no type).
|
||||||
|
if not repaired.get("type") and (
|
||||||
|
"properties" in repaired or "required" in repaired
|
||||||
|
):
|
||||||
|
repaired["type"] = "object"
|
||||||
|
|
||||||
|
if repaired.get("type") == "object":
|
||||||
|
# Ensure properties exists so required can reference it safely
|
||||||
|
if "properties" not in repaired or not isinstance(
|
||||||
|
repaired.get("properties"), dict
|
||||||
|
):
|
||||||
|
repaired["properties"] = {} if "properties" not in repaired else repaired["properties"]
|
||||||
|
if not isinstance(repaired.get("properties"), dict):
|
||||||
|
repaired["properties"] = {}
|
||||||
|
|
||||||
|
# Prune required to only include names that exist in properties
|
||||||
|
required = repaired.get("required")
|
||||||
|
if isinstance(required, list):
|
||||||
|
props = repaired.get("properties") or {}
|
||||||
|
valid = [r for r in required if isinstance(r, str) and r in props]
|
||||||
|
if len(valid) != len(required):
|
||||||
|
if valid:
|
||||||
|
repaired["required"] = valid
|
||||||
|
else:
|
||||||
|
repaired.pop("required", None)
|
||||||
|
|
||||||
|
return repaired
|
||||||
|
|
||||||
|
normalized = _rewrite_local_refs(schema)
|
||||||
|
normalized = _repair_object_shape(normalized)
|
||||||
|
|
||||||
|
# Ensure top-level is a well-formed object schema
|
||||||
|
if not isinstance(normalized, dict):
|
||||||
|
return {"type": "object", "properties": {}}
|
||||||
if normalized.get("type") == "object" and "properties" not in normalized:
|
if normalized.get("type") == "object" and "properties" not in normalized:
|
||||||
return {**normalized, "properties": {}}
|
normalized = {**normalized, "properties": {}}
|
||||||
|
|
||||||
return normalized
|
return normalized
|
||||||
|
|
||||||
|
|
@ -2080,7 +2135,7 @@ def _convert_mcp_schema(server_name: str, mcp_tool) -> dict:
|
||||||
return {
|
return {
|
||||||
"name": prefixed_name,
|
"name": prefixed_name,
|
||||||
"description": mcp_tool.description or f"MCP tool {mcp_tool.name} from {server_name}",
|
"description": mcp_tool.description or f"MCP tool {mcp_tool.name} from {server_name}",
|
||||||
"parameters": _normalize_mcp_input_schema(mcp_tool.inputSchema),
|
"parameters": _normalize_mcp_input_schema(getattr(mcp_tool, "inputSchema", None)),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue