From 03b88434ca27b1fb7240668e2d34df4e5ad572cb Mon Sep 17 00:00:00 2001 From: Stanley Shen Date: Wed, 7 Oct 2026 12:17:36 -0700 Subject: [PATCH 1/2] fix(agents): give every tool parameter a type on chat-completions routes Ollama tool templates index the type of each parameter and fail with a 500 on a bare anyOf. Collapse unions of simple types into a type list on chat-completions routes, and declare call_mcp.arguments as an object. Co-Authored-By: Claude Sonnet 5.5 --- strix/agents/factory.py | 56 ++++++++++++++++++--- strix/tools/mcp/agent_tools.py | 13 ++--- tests/test_agent_factory_shell.py | 22 +++++++++ tests/test_agent_tool_registration.py | 71 +++++++++++++++++++++++++++ tests/test_mcp_client.py | 4 ++ 5 files changed, 152 insertions(+), 14 deletions(-) diff --git a/strix/agents/factory.py b/strix/agents/factory.py index 6622d3c0..c6e48fd9 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -285,6 +285,42 @@ 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] = {} + for variant in variants: + if not isinstance(variant.get("type"), str): + return None + if merged.keys() & (variant.keys() - {"type"}) or variant.keys() & {"enum", "const"}: + return None + merged |= variant + merged["type"] = [variant["type"] for variant in variants] + 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 +396,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): @@ -495,7 +533,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) @@ -705,12 +743,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 ab068c64..59c7ab68 100644 --- a/strix/tools/mcp/agent_tools.py +++ b/strix/tools/mcp/agent_tools.py @@ -25,9 +25,10 @@ automatically. from __future__ import annotations 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 @@ -238,7 +239,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. @@ -266,10 +267,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 6bef1211..0455fa62 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 489fea70..b9f6474a 100644 --- a/tests/test_agent_tool_registration.py +++ b/tests/test_agent_tool_registration.py @@ -130,6 +130,77 @@ 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"}]}, + ], +) +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 54f49f54..0169f277 100644 --- a/tests/test_mcp_client.py +++ b/tests/test_mcp_client.py @@ -786,6 +786,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() From 0b9af25ae0d7ca261362cb4a5935026fa14f33c3 Mon Sep 17 00:00:00 2001 From: Stanley Shen Date: Wed, 7 Oct 2026 16:26:13 -0700 Subject: [PATCH 2/2] fix(agents): leave anyOf unions with repeated types untouched Merging two branches of the same type applied one branch's limits to both, so the rewritten schema rejected values the original accepted. Co-Authored-By: Claude Opus 5.5 --- strix/agents/factory.py | 6 ++++-- tests/test_agent_tool_registration.py | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/strix/agents/factory.py b/strix/agents/factory.py index c6e48fd9..5de2cbcb 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -287,13 +287,15 @@ def _with_strictness(tool: FunctionTool, strict_schemas: bool) -> FunctionTool: 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): + 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 - merged["type"] = [variant["type"] for variant in variants] + types.append(variant["type"]) + merged["type"] = types return merged diff --git a/tests/test_agent_tool_registration.py b/tests/test_agent_tool_registration.py index b9f6474a..069f8c57 100644 --- a/tests/test_agent_tool_registration.py +++ b/tests/test_agent_tool_registration.py @@ -184,6 +184,7 @@ def test_nullable_parameter_keeps_its_variant_schema() -> None: {"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: