diff --git a/strix/agents/factory.py b/strix/agents/factory.py index 9d599a54f..12867944d 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import inspect import json import logging @@ -17,6 +18,7 @@ from pydantic import ValidationError from strix.agents.prompt import render_system_prompt from strix.config import load_settings +from strix.config.models import is_bedrock_route, is_claude_model from strix.tools.agents_graph.tools import ( agent_finish, create_agent, @@ -535,6 +537,32 @@ def _ensure_unique_tool_names(tools: Sequence[Tool]) -> None: raise ValueError(msg) +def _bedrock_claude_strict_tool_limit_exceeded(tool_count: int) -> bool: + """Whether ``tool_count`` strict-schema tools would exceed Bedrock's cap. + + AWS Bedrock's Converse API rejects a request with more than 20 + strict-mode tool schemas (``ValidationException: Too many strict + tools``). Strix's own ``_BASE_TOOLS`` alone already exceeds 20 once + ``_EXTRA_TOOLS``/agent-browser/shell/filesystem tools are added, so any + Bedrock Claude scan with more than a couple of extra tools hits this. + """ + return tool_count > 20 + + +def _disable_strict_schema(tool: Tool) -> Tool: + """Return a non-strict copy, never mutating the shared tool singleton. + + ``_BASE_TOOLS``/``_EXTRA_TOOLS`` entries are shared module-level objects + reused by every ``build_strix_agent`` call; flipping ``strict_json_schema`` + in place would leak across builds (e.g. a Bedrock root agent's tools + would stay non-strict for a later non-Bedrock child agent in the same + process). + """ + if isinstance(tool, FunctionTool) and tool.strict_json_schema: + return dataclasses.replace(tool, strict_json_schema=False) + return tool + + def register_agent_tools(*tools: Tool) -> None: """Register tools for every scan agent built afterwards. @@ -610,6 +638,16 @@ def build_strix_agent( for tool in tools ] + model_name = load_settings().llm.model + if ( + is_claude_model(model_name) + and is_bedrock_route(model_name) + and _bedrock_claude_strict_tool_limit_exceeded(len(tools)) + ): + # Bedrock rejects the whole request over the 20-strict-tool cap, not + # just the tools past #20, so every tool must drop strict mode here. + tools = [_disable_strict_schema(tool) for tool in tools] + logger.info( "Built %s agent '%s' (skills=%d, tools=%d, scan_mode=%s, whitebox=%s)", "root" if is_root else "child", diff --git a/tests/test_agent_tool_registration.py b/tests/test_agent_tool_registration.py index 7d002bdc8..a6c9ff194 100644 --- a/tests/test_agent_tool_registration.py +++ b/tests/test_agent_tool_registration.py @@ -8,9 +8,12 @@ import pytest from agents.tool import FunctionTool from strix.agents import factory +from strix.config import loader if TYPE_CHECKING: + from collections.abc import Iterator + from agents.tool_context import ToolContext @@ -38,6 +41,16 @@ def _reset_registry() -> object: factory._EXTRA_TOOLS[:] = saved +@pytest.fixture +def _bedrock_claude_model(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: + """Point STRIX_LLM at a Bedrock Claude route for the duration of a test.""" + monkeypatch.setenv("STRIX_LLM", "bedrock/anthropic.claude-4-6-sonnet") + monkeypatch.setattr(loader, "_cached", None) + monkeypatch.setattr(loader, "_override", None) + yield + monkeypatch.setattr(loader, "_cached", None) + + def test_register_agent_tools_is_deduped() -> None: tool = _tool("dup") factory.register_agent_tools(tool) @@ -112,3 +125,82 @@ def test_wait_for_agents_is_available_in_both_modes() -> None: for interactive in (True, False): agent = factory.build_strix_agent(is_root=True, interactive=interactive) assert "wait_for_agents" in [t.name for t in agent.tools] + + +def test_bedrock_claude_disables_strict_schema_past_the_tool_cap( + _bedrock_claude_model: None, +) -> None: + """Bedrock's Converse API 400s a request with >20 strict-schema tools. + + ``_BASE_TOOLS`` alone is already past 20, so every Bedrock Claude agent + must drop strict mode on every tool, not just the ones past #20 (Bedrock + rejects the whole request, not a subset of tools). + """ + agent = factory.build_strix_agent(is_root=True) + + assert len(agent.tools) > 20 + function_tools = [t for t in agent.tools if isinstance(t, FunctionTool)] + assert function_tools + assert all(not t.strict_json_schema for t in function_tools) + + +def test_non_bedrock_model_keeps_default_strict_schema(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("STRIX_LLM", "anthropic/claude-4-6-sonnet") + monkeypatch.setattr(loader, "_cached", None) + monkeypatch.setattr(loader, "_override", None) + + agent = factory.build_strix_agent(is_root=True) + + function_tools = [t for t in agent.tools if isinstance(t, FunctionTool)] + assert any(t.strict_json_schema for t in function_tools) + + +def test_bedrock_claude_under_the_cap_keeps_strict_schema( + monkeypatch: pytest.MonkeyPatch, + _bedrock_claude_model: None, +) -> None: + """A tool set at/under the cap should keep the SDK's strict-mode default.""" + monkeypatch.setattr(factory, "_BASE_TOOLS", (_tool("only_base"),)) + + agent = factory.build_strix_agent(is_root=True) + + assert len(agent.tools) <= 20 + function_tools = [t for t in agent.tools if isinstance(t, FunctionTool)] + assert any(t.strict_json_schema for t in function_tools) + + +def test_disabling_strict_schema_does_not_leak_across_builds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A Bedrock build must not mutate the shared tool singletons. + + ``_BASE_TOOLS``/``_EXTRA_TOOLS`` entries are the same objects reused by + every ``build_strix_agent`` call; if a Bedrock build flipped + ``strict_json_schema`` in place, a later non-Bedrock build in the same + process would incorrectly inherit non-strict tools. + """ + originally_strict = { + tool.name + for tool in factory._BASE_TOOLS + if isinstance(tool, FunctionTool) and tool.strict_json_schema + } + assert originally_strict, "expected at least one strict tool in _BASE_TOOLS to test against" + + monkeypatch.setenv("STRIX_LLM", "bedrock/anthropic.claude-4-6-sonnet") + monkeypatch.setattr(loader, "_cached", None) + monkeypatch.setattr(loader, "_override", None) + bedrock_agent = factory.build_strix_agent(is_root=True) + bedrock_tools = [t for t in bedrock_agent.tools if isinstance(t, FunctionTool)] + assert all(not t.strict_json_schema for t in bedrock_tools) + + monkeypatch.setenv("STRIX_LLM", "anthropic/claude-4-6-sonnet") + monkeypatch.setattr(loader, "_cached", None) + non_bedrock_agent = factory.build_strix_agent(is_root=True) + non_bedrock_tools = {t.name: t for t in non_bedrock_agent.tools if isinstance(t, FunctionTool)} + assert all(non_bedrock_tools[name].strict_json_schema for name in originally_strict) + + for shared in factory._BASE_TOOLS: + if isinstance(shared, FunctionTool) and shared.name in originally_strict: + assert shared.strict_json_schema, ( + f"{shared.name} singleton was mutated by the Bedrock build" + )