diff --git a/strix/agents/factory.py b/strix/agents/factory.py index 4cd9e037..b18d8541 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -285,6 +285,44 @@ def _with_strictness(tool: FunctionTool, strict_schemas: bool) -> FunctionTool: return dataclasses.replace(tool, strict_json_schema=False) +def _merged_variants(variants: list[dict[str, Any]]) -> dict[str, Any] | None: + merged: dict[str, Any] = {} + types: list[str] = [] + for variant in variants: + if not isinstance(variant.get("type"), str) or variant["type"] in types: + return None + if merged.keys() & (variant.keys() - {"type"}) or variant.keys() & {"enum", "const"}: + return None + merged |= variant + types.append(variant["type"]) + merged["type"] = types + return merged + + +def _typed_schema(spec: dict[str, Any]) -> dict[str, Any]: + variants: list[dict[str, Any]] | None = spec.get("anyOf") + if "type" not in spec and variants: + merged = _merged_variants(variants) + if merged is not None: + rest = {key: value for key, value in spec.items() if key != "anyOf"} + spec = {**merged, **rest} + typed = dict(spec) + properties: dict[str, dict[str, Any]] | None = spec.get("properties") + if properties: + typed["properties"] = {key: _typed_schema(value) for key, value in properties.items()} + items: dict[str, Any] | bool | None = spec.get("items") + if isinstance(items, dict): + typed["items"] = _typed_schema(items) + return typed + + +def _with_typed_parameters(tool: FunctionTool) -> FunctionTool: + # Ollama's tool templates fail with ``index $prop.Type 0`` on a bare ``anyOf``. + # Rewrites in place, so callers pass a copy of a shared tool singleton. + tool.params_json_schema = _typed_schema(tool.params_json_schema) + return tool + + def _function_tool_with_error_result(tool: FunctionTool) -> FunctionTool: invoke_tool = tool.on_invoke_tool @@ -360,7 +398,9 @@ def _configure_filesystem_tools( toolset, name, _function_tool_with_error_result( - _with_strictness(_with_coerced_arguments(tool), strict_schemas) + _with_typed_parameters( + _with_strictness(_with_coerced_arguments(tool), strict_schemas) + ) ), ) elif isinstance(tool, CustomTool): @@ -499,7 +539,7 @@ def _configure_shell_tools( elif tool.name == "write_stdin": wrapped = _wrap_write_stdin(wrapped) if chat_completions: - wrapped = _function_tool_with_error_result(wrapped) + wrapped = _function_tool_with_error_result(_with_typed_parameters(wrapped)) setattr(toolset, name, wrapped) @@ -713,12 +753,14 @@ def build_strix_agent( else: tools = [*_BASE_TOOLS, *agent_tools, agent_finish] _ensure_unique_tool_names(tools) - tools = [ - _with_bounded_result(_with_strictness(_with_coerced_arguments(tool), strict_tool_schemas)) - if isinstance(tool, FunctionTool) - else tool - for tool in tools - ] + + def wrap(tool: FunctionTool) -> FunctionTool: + wrapped = _with_strictness(_with_coerced_arguments(tool), strict_tool_schemas) + if chat_completions_tools: + wrapped = _with_typed_parameters(dataclasses.replace(wrapped)) + return _with_bounded_result(wrapped) + + tools = [wrap(tool) if isinstance(tool, FunctionTool) else tool for tool in tools] logger.info( "Built %s agent '%s' (skills=%d, tools=%d, scan_mode=%s, whitebox=%s)", diff --git a/strix/tools/mcp/agent_tools.py b/strix/tools/mcp/agent_tools.py index 1033d334..ef2d718e 100644 --- a/strix/tools/mcp/agent_tools.py +++ b/strix/tools/mcp/agent_tools.py @@ -26,9 +26,10 @@ from __future__ import annotations import asyncio import json -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Annotated, Any from agents import RunContextWrapper, function_tool +from pydantic import Field from strix.tools.mcp.client import _errored_tool_output from strix.tools.mcp.naming import namespaced_tool_name @@ -259,7 +260,7 @@ async def call_mcp( ctx: RunContextWrapper, connection: str, tool: str, - arguments: Any = None, + arguments: Annotated[Any, Field(json_schema_extra={"type": "object"})] = None, ) -> Any: """Call one tool on one MCP connection and return its result. @@ -287,10 +288,10 @@ async def call_mcp( "argument names to values, or none. Call get_mcp_tool_schema for the schema." ) if isinstance(arguments, str): - # The ``arguments`` parameter is schema-less (an open object is not - # expressible as a strict tool schema), so some models serialize it as a - # JSON string instead of a bare object. Accept a string that decodes to an - # object so a correct call is not rejected over its encoding. + # The ``arguments`` parameter is an open object with no declared + # properties, so some models serialize it as a JSON string instead of a + # bare object. Accept a string that decodes to an object so a correct + # call is not rejected over its encoding. stripped = arguments.strip() try: arguments = json.loads(stripped) if stripped else {} diff --git a/tests/test_agent_factory_shell.py b/tests/test_agent_factory_shell.py index 0109bf52..877efc61 100644 --- a/tests/test_agent_factory_shell.py +++ b/tests/test_agent_factory_shell.py @@ -110,6 +110,28 @@ async def test_chat_completions_filesystem_custom_tool_becomes_function_tool() - assert isinstance(toolset.read_file, FunctionTool) +def test_chat_completions_shell_tool_nullable_parameters_get_a_type() -> None: + async def invoke(_ctx: Any, _inp: str) -> str: + return "ok" + + toolset = SimpleNamespace( + exec_command=FunctionTool( + name="exec_command", + description="test tool", + params_json_schema={ + "type": "object", + "properties": {"workdir": {"anyOf": [{"type": "string"}, {"type": "null"}]}}, + }, + on_invoke_tool=invoke, + strict_json_schema=False, + ) + ) + factory._configure_shell_tools(toolset, chat_completions=True) + + properties = toolset.exec_command.params_json_schema["properties"] + assert properties["workdir"] == {"type": ["string", "null"]} + + def test_function_tools_are_result_bounded() -> None: agent = factory.build_strix_agent(is_root=True) by_name = {t.name: t for t in agent.tools} diff --git a/tests/test_agent_tool_registration.py b/tests/test_agent_tool_registration.py index e459670b..04224502 100644 --- a/tests/test_agent_tool_registration.py +++ b/tests/test_agent_tool_registration.py @@ -139,6 +139,78 @@ def test_disabling_strict_leaves_shared_tools_untouched() -> None: assert any(t.strict_json_schema for t in agent.tools if isinstance(t, FunctionTool)) +def _untyped_properties(schema: dict[str, Any], path: str) -> list[str]: + untyped = [] + for key, spec in (schema.get("properties") or {}).items(): + if "type" not in spec: + untyped.append(f"{path}.{key}") + untyped += _untyped_properties(spec, f"{path}.{key}") + if isinstance(spec.get("items"), dict): + untyped += _untyped_properties(spec["items"], f"{path}.{key}[]") + return untyped + + +@pytest.mark.parametrize("strict", [True, False]) +def test_chat_completions_tool_parameters_all_declare_a_type(strict: bool) -> None: + """Ollama's tool templates index ``$prop.Type`` and fail on a bare ``anyOf``.""" + agent = factory.build_strix_agent( + is_root=True, chat_completions_tools=True, strict_tool_schemas=strict, interactive=True + ) + + untyped = [ + path + for t in agent.tools + if isinstance(t, FunctionTool) + for path in _untyped_properties(t.params_json_schema, t.name) + ] + assert untyped == [] + + +def test_nullable_parameter_keeps_its_variant_schema() -> None: + spec = { + "anyOf": [{"type": "array", "items": {"type": "string"}}, {"type": "null"}], + "default": None, + "description": "Filter by tag.", + } + + assert factory._typed_schema({"type": "object", "properties": {"tags": spec}}) == { + "type": "object", + "properties": { + "tags": { + "type": ["array", "null"], + "items": {"type": "string"}, + "default": None, + "description": "Filter by tag.", + } + }, + } + + +@pytest.mark.parametrize( + "spec", + [ + {"anyOf": [{"type": "string", "enum": ["a", "b"]}, {"type": "null"}]}, + {"anyOf": [{"type": "string", "const": "a"}, {"type": "null"}]}, + {"anyOf": [{"type": "string", "format": "uri"}, {"type": "array", "format": "x"}]}, + {"anyOf": [{"$ref": "#/$defs/Mode"}, {"type": "null"}]}, + {"anyOf": [{"type": "string", "minLength": 2}, {"type": "string"}]}, + ], +) +def test_unmergeable_unions_are_left_untouched(spec: dict[str, Any]) -> None: + schema = {"type": "object", "properties": {"value": spec}} + + assert factory._typed_schema(schema) == schema + + +def test_typing_parameters_leaves_shared_tools_untouched() -> None: + factory.build_strix_agent(is_root=True, chat_completions_tools=True) + agent = factory.build_strix_agent(is_root=True) + list_notes = next(t for t in agent.tools if t.name == "list_notes") + + assert isinstance(list_notes, FunctionTool) + assert "anyOf" in list_notes.params_json_schema["properties"]["tags"] + + @pytest.mark.parametrize( ("tool_name", "output", "completed"), [ diff --git a/tests/test_mcp_client.py b/tests/test_mcp_client.py index fcbac374..4991efe7 100644 --- a/tests/test_mcp_client.py +++ b/tests/test_mcp_client.py @@ -960,6 +960,10 @@ async def test_call_mcp_coerces_json_string_arguments() -> None: assert out == {"type": "text", "text": "routed:read_file"} +def test_call_mcp_arguments_declare_an_object_type() -> None: + assert call_mcp.params_json_schema["properties"]["arguments"]["type"] == "object" + + @pytest.mark.asyncio async def test_call_mcp_errors_on_unparseable_string_arguments() -> None: registry = McpRegistry()