Skip to content

Commit 45dd597

Browse files
authored
fix: fix broken tool spec with composition keywords (#1301)
1 parent 2944abf commit 45dd597

File tree

4 files changed

+149
-3
lines changed

4 files changed

+149
-3
lines changed

src/strands/tools/registry.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from ..tools.decorator import DecoratedFunctionTool
2323
from ..types.tools import AgentTool, ToolSpec
2424
from .loader import load_tool_from_string, load_tools_from_module
25-
from .tools import PythonAgentTool, normalize_schema, normalize_tool_spec
25+
from .tools import _COMPOSITION_KEYWORDS, PythonAgentTool, normalize_schema, normalize_tool_spec
2626

2727
logger = logging.getLogger(__name__)
2828

@@ -604,7 +604,8 @@ def validate_tool_spec(self, tool_spec: ToolSpec) -> None:
604604
if "$ref" in prop_def:
605605
continue
606606

607-
if "type" not in prop_def:
607+
has_composition = any(kw in prop_def for kw in _COMPOSITION_KEYWORDS)
608+
if "type" not in prop_def and not has_composition:
608609
prop_def["type"] = "string"
609610
if "description" not in prop_def:
610611
prop_def["description"] = f"Property {prop_name}"

src/strands/tools/tools.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717

1818
logger = logging.getLogger(__name__)
1919

20+
_COMPOSITION_KEYWORDS = ("anyOf", "oneOf", "allOf", "not")
21+
"""JSON Schema composition keywords that define type constraints.
22+
23+
Properties with these should not get a default type: "string" added.
24+
"""
25+
2026

2127
class InvalidToolUseNameException(Exception):
2228
"""Exception raised when a tool use has an invalid name."""
@@ -88,7 +94,9 @@ def _normalize_property(prop_name: str, prop_def: Any) -> dict[str, Any]:
8894
if "$ref" in normalized_prop:
8995
return normalized_prop
9096

91-
normalized_prop.setdefault("type", "string")
97+
has_composition = any(kw in normalized_prop for kw in _COMPOSITION_KEYWORDS)
98+
if not has_composition:
99+
normalized_prop.setdefault("type", "string")
92100
normalized_prop.setdefault("description", f"Property {prop_name}")
93101
return normalized_prop
94102

tests/strands/tools/test_registry.py

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,3 +389,125 @@ async def track_load_tools(*args, **kwargs):
389389

390390
# Verify add_consumer was called with the registry ID
391391
mock_provider.add_consumer.assert_called_once_with(registry._registry_id)
392+
393+
394+
def test_validate_tool_spec_with_anyof_property():
395+
"""Test that validate_tool_spec does not add type: 'string' to anyOf properties.
396+
397+
This is important for MCP tools that use anyOf for optional/union types like
398+
Optional[List[str]]. Adding type: 'string' causes models to return string-encoded
399+
JSON instead of proper arrays/objects.
400+
"""
401+
tool_spec = {
402+
"name": "test_tool",
403+
"description": "A test tool",
404+
"inputSchema": {
405+
"json": {
406+
"type": "object",
407+
"properties": {
408+
"regular_field": {}, # Should get type: "string"
409+
"anyof_field": {
410+
"anyOf": [
411+
{"type": "array", "items": {"type": "string"}},
412+
{"type": "null"},
413+
]
414+
},
415+
},
416+
}
417+
},
418+
}
419+
420+
registry = ToolRegistry()
421+
registry.validate_tool_spec(tool_spec)
422+
423+
props = tool_spec["inputSchema"]["json"]["properties"]
424+
425+
# Regular field should get default type: "string"
426+
assert props["regular_field"]["type"] == "string"
427+
assert props["regular_field"]["description"] == "Property regular_field"
428+
429+
# anyOf field should NOT get type: "string" added
430+
assert "type" not in props["anyof_field"], "anyOf property should not have type added"
431+
assert "anyOf" in props["anyof_field"], "anyOf should be preserved"
432+
assert props["anyof_field"]["description"] == "Property anyof_field"
433+
434+
435+
def test_validate_tool_spec_with_composition_keywords():
436+
"""Test that validate_tool_spec does not add type: 'string' to composition keyword properties.
437+
438+
JSON Schema composition keywords (anyOf, oneOf, allOf, not) define type constraints.
439+
Properties using these should not get a default type added.
440+
"""
441+
tool_spec = {
442+
"name": "test_tool",
443+
"description": "A test tool",
444+
"inputSchema": {
445+
"json": {
446+
"type": "object",
447+
"properties": {
448+
"regular_field": {}, # Should get type: "string"
449+
"oneof_field": {
450+
"oneOf": [
451+
{"type": "string"},
452+
{"type": "integer"},
453+
]
454+
},
455+
"allof_field": {
456+
"allOf": [
457+
{"minimum": 0},
458+
{"maximum": 100},
459+
]
460+
},
461+
"not_field": {"not": {"type": "null"}},
462+
},
463+
}
464+
},
465+
}
466+
467+
registry = ToolRegistry()
468+
registry.validate_tool_spec(tool_spec)
469+
470+
props = tool_spec["inputSchema"]["json"]["properties"]
471+
472+
# Regular field should get default type: "string"
473+
assert props["regular_field"]["type"] == "string"
474+
475+
# Composition keyword fields should NOT get type: "string" added
476+
assert "type" not in props["oneof_field"], "oneOf property should not have type added"
477+
assert "oneOf" in props["oneof_field"], "oneOf should be preserved"
478+
479+
assert "type" not in props["allof_field"], "allOf property should not have type added"
480+
assert "allOf" in props["allof_field"], "allOf should be preserved"
481+
482+
assert "type" not in props["not_field"], "not property should not have type added"
483+
assert "not" in props["not_field"], "not should be preserved"
484+
485+
# All should have descriptions
486+
for field in ["oneof_field", "allof_field", "not_field"]:
487+
assert props[field]["description"] == f"Property {field}"
488+
489+
490+
def test_validate_tool_spec_with_ref_property():
491+
"""Test that validate_tool_spec does not modify $ref properties."""
492+
tool_spec = {
493+
"name": "test_tool",
494+
"description": "A test tool",
495+
"inputSchema": {
496+
"json": {
497+
"type": "object",
498+
"properties": {
499+
"ref_field": {"$ref": "#/$defs/SomeType"},
500+
},
501+
}
502+
},
503+
}
504+
505+
registry = ToolRegistry()
506+
registry.validate_tool_spec(tool_spec)
507+
508+
props = tool_spec["inputSchema"]["json"]["properties"]
509+
510+
# $ref field should not be modified
511+
assert props["ref_field"] == {"$ref": "#/$defs/SomeType"}
512+
assert "type" not in props["ref_field"]
513+
assert "description" not in props["ref_field"]

tests/strands/tools/test_tools.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,3 +509,18 @@ async def test_stream(identity_tool, alist):
509509
tru_events = await alist(stream)
510510
exp_events = [ToolResultEvent(({"tool_use": 1}, 2))]
511511
assert tru_events == exp_events
512+
513+
514+
def test_normalize_schema_with_anyof():
515+
"""Test that anyOf properties don't get default type."""
516+
schema = {
517+
"type": "object",
518+
"properties": {
519+
"optional_field": {"anyOf": [{"items": {"type": "string"}, "type": "array"}, {"type": "null"}]},
520+
"regular_field": {},
521+
},
522+
}
523+
normalized = normalize_schema(schema)
524+
525+
assert "type" not in normalized["properties"]["optional_field"]
526+
assert normalized["properties"]["regular_field"]["type"] == "string"

0 commit comments

Comments
 (0)