diff --git a/strix/agents/factory.py b/strix/agents/factory.py index 9d599a54f..1846267e6 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -16,6 +16,7 @@ from agents.tool import CustomTool, FunctionTool, Tool from pydantic import ValidationError from strix.agents.prompt import render_system_prompt +from strix.agents.tool_schema import sanitize_tool_schema from strix.config import load_settings from strix.tools.agents_graph.tools import ( agent_finish, @@ -604,7 +605,7 @@ def build_strix_agent( tools = [*_BASE_TOOLS, *agent_tools, agent_finish] _ensure_unique_tool_names(tools) tools = [ - _with_bounded_result(_with_coerced_arguments(tool)) + _with_bounded_result(_with_coerced_arguments(sanitize_tool_schema(tool))) if isinstance(tool, FunctionTool) else tool for tool in tools diff --git a/strix/agents/tool_schema.py b/strix/agents/tool_schema.py new file mode 100644 index 000000000..400e25d4c --- /dev/null +++ b/strix/agents/tool_schema.py @@ -0,0 +1,51 @@ +"""Tool-schema normalization utilities. + +Kept in a standalone module so it can be imported without pulling in the full +Strix runtime (docker, sandbox, etc.). ``factory.py`` re-exports the public +helpers from here. +""" + +from __future__ import annotations + +from typing import Any + +from agents.tool import FunctionTool + + +def sanitize_tool_schema(tool: FunctionTool) -> FunctionTool: + """Remove schema fields that strict providers (e.g. Groq) reject. + + The ``@function_tool`` decorator generates a ``params_json_schema`` of:: + + {"type": "object", "properties": {}, "required": [], "additionalProperties": false} + + for zero-argument tools such as ``view_agent_graph``. Groq's API + validation rejects this with:: + + 'required' present but 'properties' is missing + + (It interprets an empty ``properties`` object as absent.) An empty + ``required`` array is also semantically meaningless: it constrains nothing. + Strip it — and the now-redundant ``additionalProperties`` flag — so the + schema degenerates to the universally-accepted ``{"type": "object"}`` form + when there are truly no parameters. Tools that do have parameters are + left unchanged. + + This mutates ``params_json_schema`` in place (idempotent). + """ + schema: dict[str, Any] = tool.params_json_schema + properties = schema.get("properties") + required = schema.get("required") + + # Only sanitize when the tool genuinely has no user-visible parameters. + if ( + isinstance(properties, dict) + and not properties + and isinstance(required, list) + and not required + ): + schema.pop("required", None) + schema.pop("additionalProperties", None) + schema.pop("properties", None) + + return tool diff --git a/tests/test_groq_tool_schema.py b/tests/test_groq_tool_schema.py new file mode 100644 index 000000000..461f90854 --- /dev/null +++ b/tests/test_groq_tool_schema.py @@ -0,0 +1,184 @@ +"""Regression tests for issue #1088. + +Groq provider: view_agent_graph tool schema (required without properties) +causes 400 BadRequestError, aborts scan. + +Groq's strict JSON-schema validator rejects tool parameters that contain +a ``required`` key alongside an empty (or absent) ``properties`` object: + + GroqException - invalid JSON schema for tool view_agent_graph, + tools[24].function.parameters: 'required' present but 'properties' + is missing + +The ``@function_tool`` decorator generates this shape for every zero-argument +tool (e.g. ``view_agent_graph``):: + + {"type": "object", "properties": {}, "required": [], "additionalProperties": false} + +The fix: ``sanitize_tool_schema`` strips ``required``, ``properties``, and +``additionalProperties`` when all three are empty/false, leaving the +provider-agnostic ``{"type": "object"}`` form. +""" + +from __future__ import annotations + +import json +from typing import Any + +import pytest +from agents import RunContextWrapper, function_tool +from agents.tool import FunctionTool + +from strix.agents.tool_schema import sanitize_tool_schema + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_no_arg_tool() -> FunctionTool: + """Simulate a @function_tool with no user-visible parameters.""" + + @function_tool(timeout=30) + async def view_agent_graph(ctx: RunContextWrapper) -> str: + """Print the multi-agent tree.""" + return "" + + return view_agent_graph # type: ignore[return-value] + + +def _make_tool_with_args() -> FunctionTool: + """Simulate a @function_tool with real parameters.""" + + @function_tool(timeout=30) + async def send_message(ctx: RunContextWrapper, target_agent_id: str, message: str) -> str: + """Send a message to another agent.""" + return "" + + return send_message # type: ignore[return-value] + + +# --------------------------------------------------------------------------- +# Core regression: empty required/properties must be stripped +# --------------------------------------------------------------------------- + + +def test_sanitize_removes_empty_required_from_no_arg_tool() -> None: + """Issue #1088: 'required: []' must be removed so Groq doesn't reject it.""" + tool = _make_no_arg_tool() + + # Confirm the decorator generates the problematic schema + raw = tool.params_json_schema + assert isinstance(raw.get("required"), list), ( + "Expected @function_tool to generate a 'required' key for a no-arg tool" + ) + + sanitize_tool_schema(tool) + + schema = tool.params_json_schema + assert "required" not in schema, ( + "After sanitize_tool_schema, 'required' must be removed from a no-arg " + "tool schema. Groq rejects schemas where 'required' is present alongside " + "an empty/absent 'properties' (issue #1088)." + ) + + +def test_sanitize_removes_empty_properties_from_no_arg_tool() -> None: + """Issue #1088: 'properties: {}' alongside empty required must be stripped.""" + tool = _make_no_arg_tool() + sanitize_tool_schema(tool) + + schema = tool.params_json_schema + # After sanitization the schema should be minimal: just {"type": "object"} + assert schema.get("type") == "object" + assert "properties" not in schema, ( + "Empty 'properties' should be stripped along with empty 'required'." + ) + + +def test_sanitize_removes_additionalproperties_from_no_arg_tool() -> None: + """Groq may also choke on additionalProperties on an empty schema.""" + tool = _make_no_arg_tool() + sanitize_tool_schema(tool) + + schema = tool.params_json_schema + assert "additionalProperties" not in schema, ( + "Redundant 'additionalProperties: false' should be removed from a " + "no-arg tool schema." + ) + + +def test_sanitize_resulting_schema_is_minimal_object() -> None: + """After sanitization, view_agent_graph-style tools emit only {type: object}.""" + tool = _make_no_arg_tool() + sanitize_tool_schema(tool) + + schema = tool.params_json_schema + # Only 'type' (and optionally 'title') should remain. + allowed_keys = {"type", "title"} + unexpected = set(schema) - allowed_keys + assert not unexpected, ( + f"Unexpected keys remain in sanitized no-arg tool schema: {unexpected}" + ) + + +# --------------------------------------------------------------------------- +# Safety: tools WITH parameters must not be modified +# --------------------------------------------------------------------------- + + +def test_sanitize_does_not_modify_tool_with_parameters() -> None: + """sanitize_tool_schema must be a no-op for tools that have real params.""" + tool = _make_tool_with_args() + original_schema = json.loads(json.dumps(tool.params_json_schema)) # deep copy + + sanitize_tool_schema(tool) + + assert tool.params_json_schema == original_schema, ( + "sanitize_tool_schema must not alter schemas that have actual parameters." + ) + + +def test_sanitize_is_idempotent() -> None: + """Calling sanitize_tool_schema twice must produce the same result as once.""" + tool = _make_no_arg_tool() + sanitize_tool_schema(tool) + schema_after_first = json.loads(json.dumps(tool.params_json_schema)) + sanitize_tool_schema(tool) + assert tool.params_json_schema == schema_after_first, ( + "sanitize_tool_schema must be idempotent." + ) + + +# --------------------------------------------------------------------------- +# Integration: the real view_agent_graph schema passes Groq validation rules +# --------------------------------------------------------------------------- + + +def test_view_agent_graph_schema_valid_for_groq() -> None: + """Full integration: the real view_agent_graph schema must pass Groq's rules + after being piped through sanitize_tool_schema. + + Groq's rule: a tool parameters object must NOT have 'required' present + without non-empty 'properties'. + """ + # Import the real tool (without docker/runtime deps) + from agents import RunContextWrapper, function_tool # noqa: F811 – re-import for clarity + + @function_tool(timeout=30) + async def view_agent_graph(ctx: RunContextWrapper) -> str: # type: ignore[no-redef] + """Print the multi-agent tree.""" + return "" + + sanitize_tool_schema(view_agent_graph) # type: ignore[arg-type] + schema: dict[str, Any] = view_agent_graph.params_json_schema # type: ignore[attr-defined] + + # Groq's rule: if 'required' is present, 'properties' must also be present + # and non-empty. + if "required" in schema: + props = schema.get("properties") + assert isinstance(props, dict) and props, ( + "Groq validation failure: 'required' is present but 'properties' " + "is missing or empty. This causes a 400 BadRequestError (issue #1088)." + )