fix(tools): reject malformed tool parameter schemas at registration
Port from earendil-works/pi#9300 fix (acaa253cc): a plugin registering a tool whose schema["parameters"] is not a dict (a list, string, etc.) previously registered fine and the malformed schema was serialized into every provider request, 400-ing turns far from the offending plugin. Live probe on main confirmed the bad schema flows into _fn_def() and the OpenAI wire unchanged. Fail at registry.register() with the tool name in the error instead. The plugin loader already catches registration exceptions and marks the plugin errored, so a broken plugin degrades gracefully rather than breaking every session. Schemas that omit "parameters" stay valid (no-argument tools); MCP tools are unaffected (their schemas pass through _normalize_mcp_input_schema first, which always returns a dict).
This commit is contained in:
@@ -6,6 +6,8 @@ import threading
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.registry import (
|
||||
ToolRegistry,
|
||||
_MAX_LOGGED_ERROR_CHARS,
|
||||
@@ -40,6 +42,23 @@ class TestRegisterAndDispatch:
|
||||
result = json.loads(reg.dispatch("alpha", {}))
|
||||
assert result == {"ok": True}
|
||||
|
||||
def test_register_rejects_non_dict_parameters(self):
|
||||
"""A list/str ``parameters`` fails at registration, not in a provider request (pi acaa253cc)."""
|
||||
reg = ToolRegistry()
|
||||
bad = {"name": "bad", "description": "x", "parameters": ["not", "an", "object"]}
|
||||
with pytest.raises(ValueError, match="parameters"):
|
||||
reg.register(name="bad", toolset="core", schema=bad, handler=_dummy_handler)
|
||||
assert reg.get_entry("bad") is None
|
||||
|
||||
def test_register_rejects_non_dict_schema(self):
|
||||
reg = ToolRegistry()
|
||||
with pytest.raises(ValueError, match="schema must be a dict"):
|
||||
reg.register(name="bad2", toolset="core", schema=None, handler=_dummy_handler)
|
||||
# Omitted parameters stays allowed (some tools take no arguments).
|
||||
reg.register(name="noargs", toolset="core",
|
||||
schema={"name": "noargs", "description": "x"}, handler=_dummy_handler)
|
||||
assert reg.get_entry("noargs") is not None
|
||||
|
||||
|
||||
def test_cross_mcp_toolsets_do_not_overwrite_atomically(self, caplog):
|
||||
"""Parallel MCP registrations with one name leave exactly one owner."""
|
||||
|
||||
@@ -602,6 +602,17 @@ class ToolRegistry:
|
||||
"""Register a tool (called at import time by each tool file). ``override=True`` is an
|
||||
explicit opt-in for plugins replacing a built-in implementation (e.g. a headed-Chrome
|
||||
browser backend); without it, cross-toolset shadowing is rejected."""
|
||||
# Reject malformed schemas at registration, not at request time: a non-dict
|
||||
# ``parameters`` (e.g. a list) serializes into every provider request and 400s the
|
||||
# whole turn far from the offending plugin. Failing here names the culprit instead.
|
||||
if not isinstance(schema, dict):
|
||||
raise ValueError(
|
||||
f"Tool {name!r}: schema must be a dict, got {type(schema).__name__}")
|
||||
params = schema.get("parameters")
|
||||
if params is not None and not isinstance(params, dict):
|
||||
raise ValueError(
|
||||
f"Tool {name!r}: schema['parameters'] must be an object (JSON Schema dict), "
|
||||
f"got {type(params).__name__}")
|
||||
handler_owner = self._plugin_owner_of(handler)
|
||||
caller_owner = self._plugin_namespace_of_module(self._caller_module())
|
||||
owner = caller_owner or handler_owner
|
||||
|
||||
Reference in New Issue
Block a user