feat: switch to using MCPToolResult strategy

Author: dbschmigelski

src/strands/models/bedrock.py

@@ -7,7 +7,7 @@
 import json
 import logging
 import os
-from typing import Any, AsyncGenerator, Callable, Iterable, Literal, Optional, Type, TypeVar, Union
+from typing import Any, AsyncGenerator, Callable, Iterable, List, Literal, Optional, Type, TypeVar, Union
 
 import boto3
 from botocore.config import Config as BotocoreConfig
@@ -17,10 +17,10 @@
 
 from ..event_loop import streaming
 from ..tools import convert_pydantic_to_tool_spec
-from ..types.content import Messages
+from ..types.content import ContentBlock, Message, Messages
 from ..types.exceptions import ContextWindowOverflowException, ModelThrottledException
 from ..types.streaming import StreamEvent
-from ..types.tools import ToolSpec
+from ..types.tools import ToolResult, ToolSpec
 from .model import Model
 
 logger = logging.getLogger(__name__)
@@ -181,7 +181,7 @@ def format_request(
         """
         return {
             "modelId": self.config["model_id"],
-            "messages": messages,
+            "messages": self._clean_tool_result_content_blocks(messages),
             "system": [
                 *([{"text": system_prompt}] if system_prompt else []),
                 *([{"cachePoint": {"type": self.config["cache_prompt"]}}] if self.config.get("cache_prompt") else []),
@@ -246,6 +246,42 @@ def format_request(
             ),
         }
 
+    def _clean_tool_result_content_blocks(self, messages: Messages) -> Messages:
+        """Additional fields may be added to ToolResult, like MCPToolResult. These can be useful for retaining
+        information to be used later in hooks.
+
+        However, Bedrock will throw validation exceptions when presented with additional unexpected fields.
+        https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_ToolResultBlock.html
+        """
+
+        cleaned_messages = []
+
+        for message in messages:
+            cleaned_content: List[ContentBlock] = []
+
+            for content_block in message["content"]:
+                if "toolResult" in content_block:
+                    # Create a new content block with only the cleaned toolResult
+                    cleaned_block: ContentBlock = content_block.copy()
+                    tool_result: ToolResult = content_block["toolResult"]
+
+                    # Keep only the required fields for Bedrock
+                    cleaned_tool_result = ToolResult(
+                        content=tool_result["content"], toolUseId=tool_result["toolUseId"], status=tool_result["status"]
+                    )
+
+                    cleaned_block["toolResult"] = cleaned_tool_result
+                    cleaned_content.append(cleaned_block)
+                else:
+                    # Keep other content blocks as-is
+                    cleaned_content.append(content_block)
+
+            # Create new message with cleaned content
+            cleaned_message: Message = Message(content=cleaned_content, role=message["role"])
+            cleaned_messages.append(cleaned_message)
+
+        return cleaned_messages
+
     def _has_blocked_guardrail(self, guardrail_data: dict[str, Any]) -> bool:
         """Check if guardrail data contains any blocked policies.
 

src/strands/tools/mcp/mcp_client.py

@@ -26,9 +26,9 @@
 from ...types import PaginatedList
 from ...types.exceptions import MCPClientInitializationError
 from ...types.media import ImageFormat
-from ...types.tools import ToolResult, ToolResultContent, ToolResultStatus
+from ...types.tools import ToolResultContent, ToolResultStatus
 from .mcp_agent_tool import MCPAgentTool
-from .mcp_types import MCPTransport
+from .mcp_types import MCPToolResult, MCPTransport
 
 logger = logging.getLogger(__name__)
 
@@ -171,7 +171,7 @@ def call_tool_sync(
         name: str,
         arguments: dict[str, Any] | None = None,
         read_timeout_seconds: timedelta | None = None,
-    ) -> ToolResult:
+    ) -> MCPToolResult:
         """Synchronously calls a tool on the MCP server.
 
         This method calls the asynchronous call_tool method on the MCP session
@@ -186,7 +186,7 @@ def call_tool_sync(
             read_timeout_seconds: Optional timeout for the tool call
 
         Returns:
-            ToolResult: The result of the tool call
+            MCPToolResult: The result of the tool call
         """
         self._log_debug_with_thread("calling MCP tool '%s' synchronously with tool_use_id=%s", name, tool_use_id)
         if not self._is_session_active():
@@ -208,13 +208,11 @@ async def call_tool_async(
         name: str,
         arguments: dict[str, Any] | None = None,
         read_timeout_seconds: timedelta | None = None,
-    ) -> ToolResult:
+    ) -> MCPToolResult:
         """Asynchronously calls a tool on the MCP server.
 
         This method calls the asynchronous call_tool method on the MCP session
-        and converts the result to the ToolResult format. If the MCP tool returns
-        structured content, it will be included as the last item in the content array
-        of the returned ToolResult.
+        and converts the result to the MCPToolResult format.
 
         Args:
             tool_use_id: Unique identifier for this tool use
@@ -223,7 +221,7 @@ async def call_tool_async(
             read_timeout_seconds: Optional timeout for the tool call
 
         Returns:
-            ToolResult: The result of the tool call
+            MCPToolResult: The result of the tool call
         """
         self._log_debug_with_thread("calling MCP tool '%s' asynchronously with tool_use_id=%s", name, tool_use_id)
         if not self._is_session_active():
@@ -240,27 +238,26 @@ async def _call_tool_async() -> MCPCallToolResult:
             logger.exception("tool execution failed")
             return self._handle_tool_execution_error(tool_use_id, e)
 
-    def _handle_tool_execution_error(self, tool_use_id: str, exception: Exception) -> ToolResult:
+    def _handle_tool_execution_error(self, tool_use_id: str, exception: Exception) -> MCPToolResult:
         """Create error ToolResult with consistent logging."""
-        return ToolResult(
+        return MCPToolResult(
             status="error",
             toolUseId=tool_use_id,
             content=[{"text": f"Tool execution failed: {str(exception)}"}],
         )
 
-    def _handle_tool_result(self, tool_use_id: str, call_tool_result: MCPCallToolResult) -> ToolResult:
-        """Maps MCP tool result to the agent's ToolResult format.
+    def _handle_tool_result(self, tool_use_id: str, call_tool_result: MCPCallToolResult) -> MCPToolResult:
+        """Maps MCP tool result to the agent's MCPToolResult format.
 
         This method processes the content from the MCP tool call result and converts it to the format
-        expected by the agent framework. If structured content is available in the MCP tool result,
-        it will be appended as the last item in the content array of the returned ToolResult.
+        expected by the framework.
 
         Args:
             tool_use_id: Unique identifier for this tool use
             call_tool_result: The result from the MCP tool call
 
         Returns:
-            ToolResult: The converted tool result
+            MCPToolResult: The converted tool result
         """
         self._log_debug_with_thread("received tool result with %d content items", len(call_tool_result.content))
 
@@ -270,12 +267,14 @@ def _handle_tool_result(self, tool_use_id: str, call_tool_result: MCPCallToolRes
             if (mapped_content := self._map_mcp_content_to_tool_result_content(content)) is not None
         ]
 
-        if call_tool_result.structuredContent:
-            mapped_content.append({"json": call_tool_result.structuredContent})
-
         status: ToolResultStatus = "error" if call_tool_result.isError else "success"
         self._log_debug_with_thread("tool execution completed with status: %s", status)
-        return ToolResult(status=status, toolUseId=tool_use_id, content=mapped_content)
+        return MCPToolResult(
+            status=status,
+            toolUseId=tool_use_id,
+            content=mapped_content,
+            structuredContent=call_tool_result.structuredContent,
+        )
 
     async def _async_background_thread(self) -> None:
         """Asynchronous method that runs in the background thread to manage the MCP connection.

src/strands/tools/mcp/mcp_types.py

@@ -1,11 +1,15 @@
 """Type definitions for MCP integration."""
 
 from contextlib import AbstractAsyncContextManager
+from typing import Any, Dict
 
 from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
 from mcp.client.streamable_http import GetSessionIdCallback
 from mcp.shared.memory import MessageStream
 from mcp.shared.message import SessionMessage
+from typing_extensions import NotRequired
+
+from strands.types.tools import ToolResult
 
 """
 MCPTransport defines the interface for MCP transport implementations. This abstracts
@@ -41,3 +45,19 @@ async def my_transport_implementation():
     MemoryObjectReceiveStream[SessionMessage | Exception], MemoryObjectSendStream[SessionMessage], GetSessionIdCallback
 ]
 MCPTransport = AbstractAsyncContextManager[MessageStream | _MessageStreamWithGetSessionIdCallback]
+
+
+class MCPToolResult(ToolResult):
+    """Result of an MCP tool execution.
+
+    Extends the base ToolResult with MCP-specific structured content support.
+    The structuredContent field contains optional JSON data returned by MCP tools
+    that provides structured results beyond the standard text/image/document content.
+
+    Attributes:
+        structuredContent: Optional JSON object containing structured data returned
+            by the MCP tool. This allows MCP tools to return complex data structures
+            that can be processed programmatically by agents or other tools.
+    """
+
+    structuredContent: NotRequired[Dict[str, Any]]

tests/strands/models/test_bedrock.py

@@ -51,7 +51,7 @@ def model(bedrock_client, model_id):
 
 @pytest.fixture
 def messages():
-    return [{"role": "user", "content": {"text": "test"}}]
+    return [{"role": "user", "content": [{"text": "test"}]}]
 
 
 @pytest.fixture
@@ -1202,3 +1202,37 @@ async def test_stream_logging(bedrock_client, model, messages, caplog, alist):
     assert "invoking model" in log_text
     assert "got response from model" in log_text
     assert "finished streaming response from model" in log_text
+
+
+def test_format_request_cleans_tool_result_content_blocks(model, model_id):
+    """Test that format_request cleans toolResult blocks by removing extra fields."""
+    messages = [
+        {
+            "role": "user",
+            "content": [
+                {"text": "Hello"},
+                {
+                    "toolResult": {
+                        "content": [{"text": "Tool output"}],
+                        "toolUseId": "tool-123",
+                        "status": "success",
+                        "extraField": "should be removed",
+                        "mcpMetadata": {"server": "test"},
+                    }
+                },
+            ],
+        }
+    ]
+
+    request = model.format_request(messages)
+
+    # Verify the request structure
+    assert request["modelId"] == model_id
+    assert "messages" in request
+
+    # Verify toolResult only contains allowed fields in the formatted request
+    tool_result = request["messages"][0]["content"][1]["toolResult"]
+    expected = {"content": [{"text": "Tool output"}], "toolUseId": "tool-123", "status": "success"}
+    assert tool_result == expected
+    assert "extraField" not in tool_result
+    assert "mcpMetadata" not in tool_result