From 828988cfcb74563ac391449047dcbac3c6a49243 Mon Sep 17 00:00:00 2001 From: Aaron Farntrog Date: Mon, 1 Dec 2025 15:41:39 -0500 Subject: [PATCH 1/3] Add dummy toolUse message when its missing due to pagination in session manager --- .../session/repository_session_manager.py | 43 +++++++++-- src/strands/tools/_tool_helpers.py | 21 +++++ tests/strands/tools/test_tool_helpers.py | 77 +++++++++++++++++++ 3 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 tests/strands/tools/test_tool_helpers.py diff --git a/src/strands/session/repository_session_manager.py b/src/strands/session/repository_session_manager.py index a042452d3..7cf534eeb 100644 --- a/src/strands/session/repository_session_manager.py +++ b/src/strands/session/repository_session_manager.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING, Any, Optional from ..agent.state import AgentState -from ..tools._tool_helpers import generate_missing_tool_result_content +from ..tools._tool_helpers import generate_missing_tool_result_content, generate_missing_tool_use_content from ..types.content import Message from ..types.exceptions import SessionException from ..types.session import ( @@ -164,12 +164,43 @@ def initialize(self, agent: "Agent", **kwargs: Any) -> None: agent.messages = self._fix_broken_tool_use(agent.messages) def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]: - """Add tool_result after orphaned tool_use messages. + """Fix broken tool use/result pairs in message history. - Before 1.15.0, strands had a bug where they persisted sessions with a potentially broken messages array. - This method retroactively fixes that issue by adding a tool_result outside of session management. After 1.15.0, - this bug is no longer present. + This method fixes two issues: + 1. Orphaned toolUse messages without corresponding toolResult. + Before 1.15.0, strands had a bug where they persisted sessions with a potentially broken messages array. + This method retroactively fixes that issue by adding a tool_result outside of session management. + After 1.15.0, this bug is no longer present. + 2. Orphaned toolResult messages without corresponding toolUse (e.g., when pagination truncates messages) + + Args: + messages: The list of messages to fix + agent_id: The agent ID for fetching previous messages + removed_message_count: Number of messages removed by the conversation manager + + Returns: + Fixed list of messages with proper tool use/result pairs """ + # First, check if the first message has orphaned toolResult (no preceding toolUse) + if messages: + first_message = messages[0] + if first_message["role"] == "user" and any("toolResult" in content for content in first_message["content"]): + orphaned_tool_result_ids = [ + content["toolResult"]["toolUseId"] + for content in first_message["content"] + if "toolResult" in content + ] + + if orphaned_tool_result_ids: + logger.warning( + "Session message history starts with orphaned toolResult(s) with no preceding toolUse. " + "This typically happens when messages are truncated due to pagination limits. " + "Adding dummy toolUse message to create valid conversation." + ) + missing_tool_use_blocks = generate_missing_tool_use_content(orphaned_tool_result_ids) + messages.insert(0, {"role": "assistant", "content": missing_tool_use_blocks}) + + # Then check for orphaned toolUse messages for index, message in enumerate(messages): # Check all but the latest message in the messages array # The latest message being orphaned is handled in the agent class @@ -187,7 +218,7 @@ def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]: ] missing_tool_use_ids = list(set(tool_use_ids) - set(tool_result_ids)) - # If there area missing tool use ids, that means the messages history is broken + # If there are missing tool use ids, that means the messages history is broken if missing_tool_use_ids: logger.warning( "Session message history has an orphaned toolUse with no toolResult. " diff --git a/src/strands/tools/_tool_helpers.py b/src/strands/tools/_tool_helpers.py index d023caeec..8201083f5 100644 --- a/src/strands/tools/_tool_helpers.py +++ b/src/strands/tools/_tool_helpers.py @@ -28,3 +28,24 @@ def generate_missing_tool_result_content(tool_use_ids: list[str]) -> list[Conten } for tool_use_id in tool_use_ids ] + + +def generate_missing_tool_use_content(tool_result_ids: list[str]) -> list[ContentBlock]: + """Generate ToolUse content blocks for orphaned ToolResult message. + + Args: + tool_result_ids: List of toolUseIds from orphaned toolResult blocks + + Returns: + List of ContentBlock dictionaries containing dummy toolUse blocks + """ + return [ + { + "toolUse": { + "toolUseId": tool_use_id, + "name": "unknown_tool", + "input": {"error": "toolUse is missing. Ignore."}, + } + } + for tool_use_id in tool_result_ids + ] diff --git a/tests/strands/tools/test_tool_helpers.py b/tests/strands/tools/test_tool_helpers.py new file mode 100644 index 000000000..1fefbcfa3 --- /dev/null +++ b/tests/strands/tools/test_tool_helpers.py @@ -0,0 +1,77 @@ +"""Tests for tool helper functions.""" + +from strands.tools._tool_helpers import ( + generate_missing_tool_result_content, + generate_missing_tool_use_content, +) + + +class TestGenerateMissingToolResultContent: + """Tests for generate_missing_tool_result_content function.""" + + def test_single_tool_use_id(self): + """Test generating content for a single tool use ID.""" + tool_use_ids = ["tool_123"] + result = generate_missing_tool_result_content(tool_use_ids) + + assert len(result) == 1 + assert "toolResult" in result[0] + assert result[0]["toolResult"]["toolUseId"] == "tool_123" + assert result[0]["toolResult"]["status"] == "error" + assert result[0]["toolResult"]["content"] == [{"text": "Tool was interrupted."}] + + def test_multiple_tool_use_ids(self): + """Test generating content for multiple tool use IDs.""" + tool_use_ids = ["tool_123", "tool_456", "tool_789"] + result = generate_missing_tool_result_content(tool_use_ids) + + assert len(result) == 3 + for i, tool_id in enumerate(tool_use_ids): + assert "toolResult" in result[i] + assert result[i]["toolResult"]["toolUseId"] == tool_id + assert result[i]["toolResult"]["status"] == "error" + + def test_empty_list(self): + """Test generating content for empty list.""" + result = generate_missing_tool_result_content([]) + assert result == [] + + +class TestGenerateMissingToolUseContent: + """Tests for generate_missing_tool_use_content function.""" + + def test_single_tool_result_id(self): + """Test generating content for a single tool result ID.""" + tool_result_ids = ["tooluse_abc123"] + result = generate_missing_tool_use_content(tool_result_ids) + + assert len(result) == 1 + assert "toolUse" in result[0] + assert result[0]["toolUse"]["toolUseId"] == "tooluse_abc123" + assert result[0]["toolUse"]["name"] == "unknown_tool" + assert result[0]["toolUse"]["input"] == {"error": "toolUse is missing. Ignore."} + + def test_multiple_tool_result_ids(self): + """Test generating content for multiple tool result IDs.""" + tool_result_ids = ["tooluse_abc123", "tooluse_def456", "tooluse_ghi789"] + result = generate_missing_tool_use_content(tool_result_ids) + + assert len(result) == 3 + for i, tool_id in enumerate(tool_result_ids): + assert "toolUse" in result[i] + assert result[i]["toolUse"]["toolUseId"] == tool_id + assert result[i]["toolUse"]["name"] == "unknown_tool" + assert result[i]["toolUse"]["input"] == {"error": "toolUse is missing. Ignore."} + + def test_empty_list(self): + """Test generating content for empty list.""" + result = generate_missing_tool_use_content([]) + assert result == [] + + def test_realistic_tool_use_id_format(self): + """Test with realistic tool use ID format (like those from Bedrock).""" + tool_result_ids = ["tooluse_f09Y0LwyT2yteCYshTzb_Q"] + result = generate_missing_tool_use_content(tool_result_ids) + + assert len(result) == 1 + assert result[0]["toolUse"]["toolUseId"] == "tooluse_f09Y0LwyT2yteCYshTzb_Q" From 47ecdeb844fa1be5649dd5edab9281e46637cddb Mon Sep 17 00:00:00 2001 From: Aaron Farntrog Date: Thu, 4 Dec 2025 13:14:04 -0500 Subject: [PATCH 2/3] Remove toolUse message when its missing due to pagination in session manager --- .../session/repository_session_manager.py | 26 ++++------- src/strands/tools/_tool_helpers.py | 21 --------- .../test_repository_session_manager.py | 37 +++++++++++++++ tests/strands/tools/test_tool_helpers.py | 45 +------------------ 4 files changed, 47 insertions(+), 82 deletions(-) diff --git a/src/strands/session/repository_session_manager.py b/src/strands/session/repository_session_manager.py index 7cf534eeb..4f62696fa 100644 --- a/src/strands/session/repository_session_manager.py +++ b/src/strands/session/repository_session_manager.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING, Any, Optional from ..agent.state import AgentState -from ..tools._tool_helpers import generate_missing_tool_result_content, generate_missing_tool_use_content +from ..tools._tool_helpers import generate_missing_tool_result_content from ..types.content import Message from ..types.exceptions import SessionException from ..types.session import ( @@ -166,7 +166,7 @@ def initialize(self, agent: "Agent", **kwargs: Any) -> None: def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]: """Fix broken tool use/result pairs in message history. - This method fixes two issues: + This method handles two issues: 1. Orphaned toolUse messages without corresponding toolResult. Before 1.15.0, strands had a bug where they persisted sessions with a potentially broken messages array. This method retroactively fixes that issue by adding a tool_result outside of session management. @@ -181,24 +181,16 @@ def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]: Returns: Fixed list of messages with proper tool use/result pairs """ - # First, check if the first message has orphaned toolResult (no preceding toolUse) + # First, check if the oldest message has orphaned toolResult (no preceding toolUse) and remove it. if messages: first_message = messages[0] if first_message["role"] == "user" and any("toolResult" in content for content in first_message["content"]): - orphaned_tool_result_ids = [ - content["toolResult"]["toolUseId"] - for content in first_message["content"] - if "toolResult" in content - ] - - if orphaned_tool_result_ids: - logger.warning( - "Session message history starts with orphaned toolResult(s) with no preceding toolUse. " - "This typically happens when messages are truncated due to pagination limits. " - "Adding dummy toolUse message to create valid conversation." - ) - missing_tool_use_blocks = generate_missing_tool_use_content(orphaned_tool_result_ids) - messages.insert(0, {"role": "assistant", "content": missing_tool_use_blocks}) + logger.warning( + "Session message history starts with orphaned toolResult with no preceding toolUse. " + "This typically happens when messages are truncated due to pagination limits. " + "Removing orphaned toolResult message to maintain valid conversation structure." + ) + messages.pop(0) # Then check for orphaned toolUse messages for index, message in enumerate(messages): diff --git a/src/strands/tools/_tool_helpers.py b/src/strands/tools/_tool_helpers.py index 8201083f5..d023caeec 100644 --- a/src/strands/tools/_tool_helpers.py +++ b/src/strands/tools/_tool_helpers.py @@ -28,24 +28,3 @@ def generate_missing_tool_result_content(tool_use_ids: list[str]) -> list[Conten } for tool_use_id in tool_use_ids ] - - -def generate_missing_tool_use_content(tool_result_ids: list[str]) -> list[ContentBlock]: - """Generate ToolUse content blocks for orphaned ToolResult message. - - Args: - tool_result_ids: List of toolUseIds from orphaned toolResult blocks - - Returns: - List of ContentBlock dictionaries containing dummy toolUse blocks - """ - return [ - { - "toolUse": { - "toolUseId": tool_use_id, - "name": "unknown_tool", - "input": {"error": "toolUse is missing. Ignore."}, - } - } - for tool_use_id in tool_result_ids - ] diff --git a/tests/strands/session/test_repository_session_manager.py b/tests/strands/session/test_repository_session_manager.py index 451d0dd09..f5189308b 100644 --- a/tests/strands/session/test_repository_session_manager.py +++ b/tests/strands/session/test_repository_session_manager.py @@ -413,3 +413,40 @@ def test_fix_broken_tool_use_does_not_change_valid_message(session_manager): # Should remain unchanged since toolUse is in last message assert fixed_messages == messages + + +def test_fix_broken_tool_use_removes_orphaned_tool_result_at_start(session_manager): + """Test that orphaned toolResult at the start of conversation is removed.""" + messages = [ + { + "role": "user", + "content": [ + {"toolResult": {"toolUseId": "orphaned-result-123", "status": "success", "content": [{"text": "Seattle, USA"}]}} + ], + }, + {"role": "assistant", "content": [{"text": "You live in Seattle, USA."}]}, + {"role": "user", "content": [{"text": "I like pizza"}]}, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should remove the first message with orphaned toolResult + assert len(fixed_messages) == 2 + assert fixed_messages[0]["role"] == "assistant" + assert fixed_messages[0]["content"][0]["text"] == "You live in Seattle, USA." + assert fixed_messages[1]["role"] == "user" + assert fixed_messages[1]["content"][0]["text"] == "I like pizza" + + +def test_fix_broken_tool_use_does_not_affect_normal_conversations(session_manager): + """Test that normal conversations without orphaned toolResults are unaffected.""" + messages = [ + {"role": "user", "content": [{"text": "Hello"}]}, + {"role": "assistant", "content": [{"text": "Hi there!"}]}, + {"role": "user", "content": [{"text": "How are you?"}]}, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should remain unchanged + assert fixed_messages == messages diff --git a/tests/strands/tools/test_tool_helpers.py b/tests/strands/tools/test_tool_helpers.py index 1fefbcfa3..2fb2201f4 100644 --- a/tests/strands/tools/test_tool_helpers.py +++ b/tests/strands/tools/test_tool_helpers.py @@ -1,9 +1,6 @@ """Tests for tool helper functions.""" -from strands.tools._tool_helpers import ( - generate_missing_tool_result_content, - generate_missing_tool_use_content, -) +from strands.tools._tool_helpers import generate_missing_tool_result_content class TestGenerateMissingToolResultContent: @@ -35,43 +32,3 @@ def test_empty_list(self): """Test generating content for empty list.""" result = generate_missing_tool_result_content([]) assert result == [] - - -class TestGenerateMissingToolUseContent: - """Tests for generate_missing_tool_use_content function.""" - - def test_single_tool_result_id(self): - """Test generating content for a single tool result ID.""" - tool_result_ids = ["tooluse_abc123"] - result = generate_missing_tool_use_content(tool_result_ids) - - assert len(result) == 1 - assert "toolUse" in result[0] - assert result[0]["toolUse"]["toolUseId"] == "tooluse_abc123" - assert result[0]["toolUse"]["name"] == "unknown_tool" - assert result[0]["toolUse"]["input"] == {"error": "toolUse is missing. Ignore."} - - def test_multiple_tool_result_ids(self): - """Test generating content for multiple tool result IDs.""" - tool_result_ids = ["tooluse_abc123", "tooluse_def456", "tooluse_ghi789"] - result = generate_missing_tool_use_content(tool_result_ids) - - assert len(result) == 3 - for i, tool_id in enumerate(tool_result_ids): - assert "toolUse" in result[i] - assert result[i]["toolUse"]["toolUseId"] == tool_id - assert result[i]["toolUse"]["name"] == "unknown_tool" - assert result[i]["toolUse"]["input"] == {"error": "toolUse is missing. Ignore."} - - def test_empty_list(self): - """Test generating content for empty list.""" - result = generate_missing_tool_use_content([]) - assert result == [] - - def test_realistic_tool_use_id_format(self): - """Test with realistic tool use ID format (like those from Bedrock).""" - tool_result_ids = ["tooluse_f09Y0LwyT2yteCYshTzb_Q"] - result = generate_missing_tool_use_content(tool_result_ids) - - assert len(result) == 1 - assert result[0]["toolUse"]["toolUseId"] == "tooluse_f09Y0LwyT2yteCYshTzb_Q" From bac114be7762bfff2badaa9d64d1fdca84ec2b6c Mon Sep 17 00:00:00 2001 From: Aaron Farntrog Date: Thu, 4 Dec 2025 13:17:53 -0500 Subject: [PATCH 3/3] format --- tests/strands/session/test_repository_session_manager.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/strands/session/test_repository_session_manager.py b/tests/strands/session/test_repository_session_manager.py index 28f1bbcda..22de9f964 100644 --- a/tests/strands/session/test_repository_session_manager.py +++ b/tests/strands/session/test_repository_session_manager.py @@ -560,7 +560,13 @@ def test_fix_broken_tool_use_removes_orphaned_tool_result_at_start(session_manag { "role": "user", "content": [ - {"toolResult": {"toolUseId": "orphaned-result-123", "status": "success", "content": [{"text": "Seattle, USA"}]}} + { + "toolResult": { + "toolUseId": "orphaned-result-123", + "status": "success", + "content": [{"text": "Seattle, USA"}], + } + } ], }, {"role": "assistant", "content": [{"text": "You live in Seattle, USA."}]},