diff --git a/src/strands/session/repository_session_manager.py b/src/strands/session/repository_session_manager.py index d23c4a94f..753a5f159 100644 --- a/src/strands/session/repository_session_manager.py +++ b/src/strands/session/repository_session_manager.py @@ -172,28 +172,24 @@ def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]: 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) + 2. Orphaned toolResult messages without corresponding toolUse (e.g., when pagination truncates messages, + context is summarized, or messages are removed by conversation managers) 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 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"]): - 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 + # First, remove orphaned toolResult blocks throughout the conversation. + # This handles cases where toolUse blocks were removed due to: + # - Pagination/truncation of conversation history + # - Context summarization + # - Conversation manager pruning + # Related: https://github.com/strands-agents/sdk-python/issues/1610 + messages = self._remove_orphaned_tool_results(messages) + + # Then check for orphaned toolUse messages and add missing toolResults 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 @@ -228,6 +224,78 @@ def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]: messages.insert(index + 1, {"role": "user", "content": missing_content_blocks}) return messages + def _remove_orphaned_tool_results(self, messages: list[Message]) -> list[Message]: + """Remove orphaned toolResult blocks that have no matching toolUse. + + This scans through all messages and removes any toolResult blocks whose + toolUseId doesn't have a corresponding toolUse block in preceding messages. + + This is necessary because: + - Context truncation/summarization may remove toolUse blocks but keep toolResults + - Pagination may load partial conversation history + - Conversation managers may prune messages creating orphaned results + + Related issues: + - https://github.com/strands-agents/sdk-python/issues/1610 + - Anthropic Claude Code #13964: orphaned tool_result blocks after context truncation + - Pydantic AI, OpenHands, LiteLLM: similar ValidationException issues + + Args: + messages: The list of messages to scan + + Returns: + Messages with orphaned toolResult blocks removed + """ + if not messages: + return messages + + # Collect all toolUse IDs from all messages + all_tool_use_ids: set[str] = set() + for message in messages: + for content in message.get("content", []): + if "toolUse" in content: + all_tool_use_ids.add(content["toolUse"]["toolUseId"]) + + # Track if we removed any orphaned results for logging + removed_any = False + + # Filter out orphaned toolResult blocks from all messages + filtered_messages: list[Message] = [] + for message in messages: + if message["role"] == "user": + filtered_content = [] + for content in message.get("content", []): + if "toolResult" in content: + tool_use_id = content["toolResult"].get("toolUseId") + if tool_use_id in all_tool_use_ids: + # Keep this toolResult - it has a matching toolUse + filtered_content.append(content) + else: + # This is an orphaned toolResult - skip it + removed_any = True + else: + # Not a toolResult, keep it + filtered_content.append(content) + + # Only include the message if it still has content + if filtered_content: + filtered_messages.append({"role": message["role"], "content": filtered_content}) + # If message is now empty (was only orphaned toolResults), skip it entirely + elif removed_any: + pass # Message dropped + else: + # Non-user messages (assistant) - keep as-is + filtered_messages.append(message) + + if removed_any: + logger.warning( + "Session message history had orphaned toolResult blocks with no matching toolUse. " + "This typically happens when context is truncated or summarized. " + "Removed orphaned toolResult blocks to maintain valid conversation structure." + ) + + return filtered_messages + def sync_multi_agent(self, source: "MultiAgentBase", **kwargs: Any) -> None: """Serialize and update the multi-agent state into the session repository. diff --git a/tests/strands/session/test_repository_session_manager.py b/tests/strands/session/test_repository_session_manager.py index 22de9f964..84da3cc98 100644 --- a/tests/strands/session/test_repository_session_manager.py +++ b/tests/strands/session/test_repository_session_manager.py @@ -595,3 +595,242 @@ def test_fix_broken_tool_use_does_not_affect_normal_conversations(session_manage # Should remain unchanged assert fixed_messages == messages + + +# ============================================================================ +# Orphaned toolResult in Middle of Conversation Tests (Issue #1610) +# ============================================================================ + + +def test_fix_broken_tool_use_removes_orphaned_tool_result_in_middle(session_manager): + """Test that orphaned toolResult in the middle of conversation is removed. + + This is the core fix for issue #1610 - when context is truncated or summarized, + toolUse blocks may be removed while their corresponding toolResult blocks remain, + causing ValidationException on subsequent API calls. + """ + messages = [ + {"role": "user", "content": [{"text": "Hello"}]}, + {"role": "assistant", "content": [{"text": "Hi there!"}]}, + # This toolResult is orphaned - no toolUse exists for this ID + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "orphaned-middle-123", + "status": "success", + "content": [{"text": "Some result from a truncated tool call"}], + } + } + ], + }, + {"role": "assistant", "content": [{"text": "I processed that result."}]}, + {"role": "user", "content": [{"text": "Thanks!"}]}, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should remove the orphaned toolResult message entirely + assert len(fixed_messages) == 4 + assert fixed_messages[0]["content"][0]["text"] == "Hello" + assert fixed_messages[1]["content"][0]["text"] == "Hi there!" + assert fixed_messages[2]["content"][0]["text"] == "I processed that result." + assert fixed_messages[3]["content"][0]["text"] == "Thanks!" + + +def test_fix_broken_tool_use_removes_multiple_orphaned_tool_results(session_manager): + """Test that multiple orphaned toolResults throughout conversation are all removed.""" + messages = [ + {"role": "user", "content": [{"text": "Start"}]}, + # First orphaned toolResult + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "orphaned-1", + "status": "success", + "content": [{"text": "Result 1"}], + } + } + ], + }, + {"role": "assistant", "content": [{"text": "Response 1"}]}, + # Second orphaned toolResult + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "orphaned-2", + "status": "success", + "content": [{"text": "Result 2"}], + } + } + ], + }, + {"role": "assistant", "content": [{"text": "Response 2"}]}, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should remove both orphaned toolResult messages + assert len(fixed_messages) == 3 + assert fixed_messages[0]["content"][0]["text"] == "Start" + assert fixed_messages[1]["content"][0]["text"] == "Response 1" + assert fixed_messages[2]["content"][0]["text"] == "Response 2" + + +def test_fix_broken_tool_use_keeps_valid_tool_results_removes_orphaned(session_manager): + """Test that valid toolResults are kept while orphaned ones are removed.""" + messages = [ + {"role": "user", "content": [{"text": "Hello"}]}, + # Valid toolUse/toolResult pair + { + "role": "assistant", + "content": [ + {"toolUse": {"toolUseId": "valid-123", "name": "test_tool", "input": {"x": 1}}} + ], + }, + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "valid-123", + "status": "success", + "content": [{"text": "Valid result"}], + } + } + ], + }, + {"role": "assistant", "content": [{"text": "Got the valid result."}]}, + # Orphaned toolResult (no matching toolUse) + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "orphaned-456", + "status": "success", + "content": [{"text": "Orphaned result"}], + } + } + ], + }, + {"role": "assistant", "content": [{"text": "Final response."}]}, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should keep the valid pair but remove the orphaned toolResult + assert len(fixed_messages) == 5 + assert fixed_messages[0]["content"][0]["text"] == "Hello" + assert "toolUse" in fixed_messages[1]["content"][0] + assert fixed_messages[2]["content"][0]["toolResult"]["toolUseId"] == "valid-123" + assert fixed_messages[3]["content"][0]["text"] == "Got the valid result." + assert fixed_messages[4]["content"][0]["text"] == "Final response." + + +def test_fix_broken_tool_use_removes_orphaned_from_mixed_content_message(session_manager): + """Test that orphaned toolResults are removed from messages with mixed content.""" + messages = [ + {"role": "user", "content": [{"text": "Hello"}]}, + # Valid toolUse + { + "role": "assistant", + "content": [ + {"toolUse": {"toolUseId": "valid-tool", "name": "test", "input": {}}} + ], + }, + # Message with valid toolResult and text + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "valid-tool", + "status": "success", + "content": [{"text": "Valid"}], + } + }, + {"text": "Additional user text"}, + # Orphaned toolResult in the same message + { + "toolResult": { + "toolUseId": "orphaned-tool", + "status": "success", + "content": [{"text": "Orphaned"}], + } + }, + ], + }, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should keep the valid toolResult and text, but remove the orphaned toolResult + assert len(fixed_messages) == 3 + assert len(fixed_messages[2]["content"]) == 2 # Only valid toolResult and text + assert fixed_messages[2]["content"][0]["toolResult"]["toolUseId"] == "valid-tool" + assert fixed_messages[2]["content"][1]["text"] == "Additional user text" + + +def test_fix_broken_tool_use_browser_tool_memory_scenario(session_manager): + """Test the exact scenario from issue #1610 - Browser Tool + Memory. + + Simulates what happens when: + 1. Browser tool creates tool_use blocks + 2. Memory saves conversation + 3. On restore, context truncation removes tool_use but keeps tool_result + 4. This causes ValidationException + """ + messages = [ + {"role": "user", "content": [{"text": "Navigate to example.com"}]}, + # This simulates truncated history - toolUse was removed but toolResult remains + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "browser-nav-123", # No matching toolUse - it was truncated + "status": "success", + "content": [{"text": "Navigated to example.com"}], + } + } + ], + }, + {"role": "assistant", "content": [{"text": "I've navigated to example.com."}]}, + {"role": "user", "content": [{"text": "Now click the login button"}]}, + # Current valid tool use/result pair + { + "role": "assistant", + "content": [ + {"toolUse": {"toolUseId": "browser-click-456", "name": "browser_click", "input": {"selector": "#login"}}} + ], + }, + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "browser-click-456", + "status": "success", + "content": [{"text": "Clicked login button"}], + } + } + ], + }, + ] + + fixed_messages = session_manager._fix_broken_tool_use(messages) + + # Should remove the orphaned browser navigation toolResult + # but keep the valid browser click tool use/result pair + assert len(fixed_messages) == 5 + assert fixed_messages[0]["content"][0]["text"] == "Navigate to example.com" + assert fixed_messages[1]["content"][0]["text"] == "I've navigated to example.com." + assert fixed_messages[2]["content"][0]["text"] == "Now click the login button" + assert "toolUse" in fixed_messages[3]["content"][0] + assert fixed_messages[4]["content"][0]["toolResult"]["toolUseId"] == "browser-click-456"