mirror of
https://github.com/usestrix/strix.git
synced 2026-11-09 08:17:19 +00:00
fix(agents): strip empty required/properties from tool schemas for Groq (#1088)
Groq's API strictly validates tool parameter schemas and rejects any
tool where 'required' is present alongside an absent or empty
'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 exact shape for every
zero-argument tool (e.g. view_agent_graph):
{'type': 'object', 'properties': {}, 'required': [], 'additionalProperties': false}
Fix: add sanitize_tool_schema() (strix/agents/tool_schema.py) that
strips 'required', 'properties', and 'additionalProperties' when all
are empty, reducing the schema to the universally-accepted
{'type': 'object'} form. Applied to every FunctionTool in
build_strix_agent() so all current and future zero-arg tools are
covered automatically. Tools with real parameters are not modified.
Fixes #1088
This commit is contained in:
parent
7e52482c0b
commit
8ecf6067bc
3 changed files with 237 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
51
strix/agents/tool_schema.py
Normal file
51
strix/agents/tool_schema.py
Normal file
|
|
@ -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
|
||||
184
tests/test_groq_tool_schema.py
Normal file
184
tests/test_groq_tool_schema.py
Normal file
|
|
@ -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)."
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue