Skip to content

Commit d72c1fc

Browse files
committed
fix: Fix broken converstaion with orphaned toolUse
1 parent db671ba commit d72c1fc

File tree

4 files changed

+343
-2
lines changed

4 files changed

+343
-2
lines changed

src/strands/agent/agent.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ def __init__(
280280
Defaults to None.
281281
session_manager: Manager for handling agent sessions including conversation history and state.
282282
If provided, enables session-based persistence and state management.
283-
tool_executor: Definition of tool execution stragety (e.g., sequential, concurrent, etc.).
283+
tool_executor: Definition of tool execution strategy (e.g., sequential, concurrent, etc.).
284284
285285
Raises:
286286
ValueError: If agent id contains path separators.
@@ -816,6 +816,27 @@ def _convert_prompt_to_messages(self, prompt: AgentInput) -> Messages:
816816

817817
messages: Messages | None = None
818818
if prompt is not None:
819+
# Check if the latest message is toolUse
820+
if len(self.messages) > 0 and any("toolUse" in content for content in self.messages[-1]["content"]):
821+
# Add toolResult message after to have a valid conversation
822+
tool_use_ids = [
823+
content["toolUse"]["toolUseId"] for content in self.messages[-1]["content"] if "toolUse" in content
824+
]
825+
self._append_message(
826+
{
827+
"role": "user",
828+
"content": [
829+
{
830+
"toolResult": {
831+
"toolUseId": tool_use_id,
832+
"status": "error",
833+
"content": [{"text": "Tool was interrupted."}],
834+
}
835+
}
836+
for tool_use_id in tool_use_ids
837+
],
838+
}
839+
)
819840
if isinstance(prompt, str):
820841
# String input - convert to user message
821842
messages = [{"role": "user", "content": [{"text": prompt}]}]

src/strands/session/repository_session_manager.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from typing import TYPE_CHECKING, Any, Optional
55

66
from ..agent.state import AgentState
7-
from ..types.content import Message
7+
from ..types.content import ContentBlock, Message
88
from ..types.exceptions import SessionException
99
from ..types.session import (
1010
Session,
@@ -159,6 +159,54 @@ def initialize(self, agent: "Agent", **kwargs: Any) -> None:
159159
# Restore the agents messages array including the optional prepend messages
160160
agent.messages = prepend_messages + [session_message.to_message() for session_message in session_messages]
161161

162+
# Fix broken session histories: https://github.com/strands-agents/sdk-python/issues/859
163+
agent.messages = self._fix_broken_tool_use(agent.messages)
164+
165+
def _fix_broken_tool_use(self, messages: list[Message]) -> list[Message]:
166+
"""Add tool_result after orphaned tool_use messages.
167+
168+
Before 1.15.0, strands had a bug where they persisted sessions with a potentially broken messages array.
169+
This method retroactively fixes that issue by adding a tool_result outside of session management. After 1.15.0,
170+
this bug is no longer present.
171+
"""
172+
for index, message in enumerate(messages):
173+
# Check all but the latest message in the messages array
174+
if index + 1 < len(messages):
175+
if any("toolUse" in content for content in message["content"]):
176+
tool_use_ids = [
177+
content["toolUse"]["toolUseId"] for content in message["content"] if "toolUse" in content
178+
]
179+
180+
# Check if there are more messages after the current toolUse message
181+
tool_result_ids = [
182+
content["toolResult"]["toolUseId"]
183+
for content in messages[index + 1]["content"]
184+
if "toolResult" in content
185+
]
186+
187+
missing_tool_use_ids = list(set(tool_use_ids) - set(tool_result_ids))
188+
# If there area missing tool use ids, that means the messages history is broken
189+
if missing_tool_use_ids:
190+
# Create the missing toolResult content blocks
191+
missing_content_blocks: list[ContentBlock] = [
192+
{
193+
"toolResult": {
194+
"toolUseId": missing_tool_use_id,
195+
"content": [{"text": "Tool execution interrupted."}],
196+
"status": "error",
197+
}
198+
}
199+
for missing_tool_use_id in missing_tool_use_ids
200+
]
201+
202+
if tool_result_ids:
203+
# If there were any toolResult ids, that means only some of the content blocks are missing
204+
messages[index + 1]["content"].extend(missing_content_blocks)
205+
else:
206+
# The message following the toolUse was not a toolResult, so lets insert it
207+
messages.insert(index + 1, {"role": "user", "content": missing_content_blocks})
208+
return messages
209+
162210
def sync_multi_agent(self, source: "MultiAgentBase", **kwargs: Any) -> None:
163211
"""Serialize and update the multi-agent state into the session repository.
164212

tests/strands/agent/test_agent.py

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,3 +2215,125 @@ def test_redact_user_content(content, expected):
22152215
agent = Agent()
22162216
result = agent._redact_user_content(content, "REDACTED")
22172217
assert result == expected
2218+
2219+
2220+
def test_agent_fixes_orphaned_tool_use_on_new_prompt(mock_model, agenerator):
2221+
"""Test that agent adds toolResult for orphaned toolUse when called with new prompt."""
2222+
mock_model.mock_stream.return_value = agenerator(
2223+
[
2224+
{"messageStart": {"role": "assistant"}},
2225+
{"contentBlockStart": {"start": {"text": ""}}},
2226+
{"contentBlockDelta": {"delta": {"text": "Fixed!"}}},
2227+
{"contentBlockStop": {}},
2228+
{"messageStop": {"stopReason": "end_turn"}},
2229+
]
2230+
)
2231+
2232+
# Start with orphaned toolUse message
2233+
messages = [
2234+
{
2235+
"role": "assistant",
2236+
"content": [
2237+
{"toolUse": {"toolUseId": "orphaned-123", "name": "tool_decorated", "input": {"random_string": "test"}}}
2238+
],
2239+
}
2240+
]
2241+
2242+
agent = Agent(model=mock_model, messages=messages, tools=[tool_decorated])
2243+
2244+
# Call with new prompt should fix orphaned toolUse
2245+
agent("Continue conversation")
2246+
2247+
# Should have added toolResult message
2248+
assert len(agent.messages) >= 3
2249+
assert agent.messages[1]["role"] == "user"
2250+
assert "toolResult" in agent.messages[1]["content"][0]
2251+
assert agent.messages[1]["content"][0]["toolResult"]["toolUseId"] == "orphaned-123"
2252+
assert agent.messages[1]["content"][0]["toolResult"]["status"] == "error"
2253+
assert agent.messages[1]["content"][0]["toolResult"]["content"][0]["text"] == "Tool was interrupted."
2254+
2255+
2256+
def test_agent_fixes_multiple_orphaned_tool_uses(mock_model, agenerator):
2257+
"""Test that agent handles multiple orphaned toolUse messages."""
2258+
mock_model.mock_stream.return_value = agenerator(
2259+
[
2260+
{"messageStart": {"role": "assistant"}},
2261+
{"contentBlockStart": {"start": {"text": ""}}},
2262+
{"contentBlockDelta": {"delta": {"text": "Fixed multiple!"}}},
2263+
{"contentBlockStop": {}},
2264+
{"messageStop": {"stopReason": "end_turn"}},
2265+
]
2266+
)
2267+
2268+
messages = [
2269+
{
2270+
"role": "assistant",
2271+
"content": [
2272+
{
2273+
"toolUse": {
2274+
"toolUseId": "orphaned-123",
2275+
"name": "tool_decorated",
2276+
"input": {"random_string": "test1"},
2277+
}
2278+
},
2279+
{
2280+
"toolUse": {
2281+
"toolUseId": "orphaned-456",
2282+
"name": "tool_decorated",
2283+
"input": {"random_string": "test2"},
2284+
}
2285+
},
2286+
],
2287+
}
2288+
]
2289+
2290+
agent = Agent(model=mock_model, messages=messages, tools=[tool_decorated])
2291+
agent("Continue")
2292+
2293+
# Should have toolResult for both toolUse IDs
2294+
tool_results = agent.messages[1]["content"]
2295+
assert len(tool_results) == 2
2296+
tool_use_ids = {tr["toolResult"]["toolUseId"] for tr in tool_results}
2297+
assert tool_use_ids == {"orphaned-123", "orphaned-456"}
2298+
2299+
for tool_result in tool_results:
2300+
assert tool_result["toolResult"]["status"] == "error"
2301+
assert tool_result["toolResult"]["content"][0]["text"] == "Tool was interrupted."
2302+
2303+
2304+
def test_agent_skips_fix_for_valid_conversation(mock_model, agenerator):
2305+
"""Test that agent doesn't modify valid toolUse/toolResult pairs."""
2306+
mock_model.mock_stream.return_value = agenerator(
2307+
[
2308+
{"messageStart": {"role": "assistant"}},
2309+
{"contentBlockStart": {"start": {"text": ""}}},
2310+
{"contentBlockDelta": {"delta": {"text": "No fix needed!"}}},
2311+
{"contentBlockStop": {}},
2312+
{"messageStop": {"stopReason": "end_turn"}},
2313+
]
2314+
)
2315+
2316+
# Valid conversation with toolUse followed by toolResult
2317+
messages = [
2318+
{
2319+
"role": "assistant",
2320+
"content": [
2321+
{"toolUse": {"toolUseId": "valid-123", "name": "tool_decorated", "input": {"random_string": "test"}}}
2322+
],
2323+
},
2324+
{
2325+
"role": "user",
2326+
"content": [
2327+
{"toolResult": {"toolUseId": "valid-123", "status": "success", "content": [{"text": "result"}]}}
2328+
],
2329+
},
2330+
]
2331+
2332+
agent = Agent(model=mock_model, messages=messages, tools=[tool_decorated])
2333+
original_length = len(agent.messages)
2334+
2335+
agent("Continue")
2336+
2337+
# Should not have added any toolResult messages
2338+
# Only the new user message and assistant response should be added
2339+
assert len(agent.messages) == original_length + 2

tests/strands/session/test_repository_session_manager.py

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,153 @@ def test_initialize_multi_agent_existing(session_manager, mock_multi_agent):
233233

234234
# Verify deserialize_state was called with existing state
235235
mock_multi_agent.deserialize_state.assert_called_once_with(existing_state)
236+
237+
238+
def test_fix_broken_tool_use_adds_missing_tool_results(session_manager):
239+
"""Test that _fix_broken_tool_use adds missing toolResult messages."""
240+
# Messages with orphaned toolUse
241+
broken_messages = [
242+
{
243+
"role": "assistant",
244+
"content": [{"toolUse": {"toolUseId": "orphaned-123", "name": "test_tool", "input": {"input": "test"}}}],
245+
},
246+
{"role": "user", "content": [{"text": "Some other message"}]},
247+
]
248+
249+
fixed_messages = session_manager._fix_broken_tool_use(broken_messages)
250+
251+
# Should insert toolResult message between toolUse and other message
252+
assert len(fixed_messages) == 3
253+
assert fixed_messages[1]["role"] == "user"
254+
assert "toolResult" in fixed_messages[1]["content"][0]
255+
assert fixed_messages[1]["content"][0]["toolResult"]["toolUseId"] == "orphaned-123"
256+
assert fixed_messages[1]["content"][0]["toolResult"]["status"] == "error"
257+
assert fixed_messages[1]["content"][0]["toolResult"]["content"][0]["text"] == "Tool execution interrupted."
258+
259+
260+
def test_fix_broken_tool_use_extends_partial_tool_results(session_manager):
261+
"""Test fixing messages where some toolResults are missing."""
262+
messages = [
263+
{
264+
"role": "assistant",
265+
"content": [
266+
{"toolUse": {"toolUseId": "complete-123", "name": "test_tool", "input": {"input": "test1"}}},
267+
{"toolUse": {"toolUseId": "missing-456", "name": "test_tool", "input": {"input": "test2"}}},
268+
],
269+
},
270+
{
271+
"role": "user",
272+
"content": [
273+
{"toolResult": {"toolUseId": "complete-123", "status": "success", "content": [{"text": "result"}]}}
274+
],
275+
},
276+
]
277+
278+
fixed_messages = session_manager._fix_broken_tool_use(messages)
279+
280+
# Should add missing toolResult to existing message
281+
assert len(fixed_messages) == 2
282+
assert len(fixed_messages[1]["content"]) == 2
283+
284+
tool_use_ids = {tr["toolResult"]["toolUseId"] for tr in fixed_messages[1]["content"]}
285+
assert tool_use_ids == {"complete-123", "missing-456"}
286+
287+
# Check the added toolResult has correct properties
288+
missing_result = next(tr for tr in fixed_messages[1]["content"] if tr["toolResult"]["toolUseId"] == "missing-456")
289+
assert missing_result["toolResult"]["status"] == "error"
290+
assert missing_result["toolResult"]["content"][0]["text"] == "Tool execution interrupted."
291+
292+
293+
def test_fix_broken_tool_use_handles_multiple_orphaned_tools(session_manager):
294+
"""Test fixing multiple orphaned toolUse messages."""
295+
messages = [
296+
{
297+
"role": "assistant",
298+
"content": [
299+
{"toolUse": {"toolUseId": "orphaned-123", "name": "test_tool", "input": {"input": "test1"}}},
300+
{"toolUse": {"toolUseId": "orphaned-456", "name": "test_tool", "input": {"input": "test2"}}},
301+
],
302+
},
303+
{"role": "user", "content": [{"text": "Next message"}]},
304+
]
305+
306+
fixed_messages = session_manager._fix_broken_tool_use(messages)
307+
308+
# Should insert message with both toolResults
309+
assert len(fixed_messages) == 3
310+
assert len(fixed_messages[1]["content"]) == 2
311+
312+
tool_use_ids = {tr["toolResult"]["toolUseId"] for tr in fixed_messages[1]["content"]}
313+
assert tool_use_ids == {"orphaned-123", "orphaned-456"}
314+
315+
316+
def test_fix_broken_tool_use_preserves_valid_messages(session_manager):
317+
"""Test that valid message sequences are not modified."""
318+
valid_messages = [
319+
{
320+
"role": "assistant",
321+
"content": [{"toolUse": {"toolUseId": "valid-123", "name": "test_tool", "input": {"input": "test"}}}],
322+
},
323+
{
324+
"role": "user",
325+
"content": [
326+
{"toolResult": {"toolUseId": "valid-123", "status": "success", "content": [{"text": "result"}]}}
327+
],
328+
},
329+
]
330+
331+
fixed_messages = session_manager._fix_broken_tool_use(valid_messages)
332+
333+
# Should remain unchanged
334+
assert fixed_messages == valid_messages
335+
336+
337+
def test_fix_broken_tool_use_ignores_last_message(session_manager):
338+
"""Test that orphaned toolUse in the last message is not fixed."""
339+
messages = [
340+
{"role": "user", "content": [{"text": "Hello"}]},
341+
{
342+
"role": "assistant",
343+
"content": [
344+
{"toolUse": {"toolUseId": "last-message-123", "name": "test_tool", "input": {"input": "test"}}}
345+
],
346+
},
347+
]
348+
349+
fixed_messages = session_manager._fix_broken_tool_use(messages)
350+
351+
# Should remain unchanged since toolUse is in last message
352+
assert fixed_messages == messages
353+
354+
355+
def test_initialize_agent_applies_fix_broken_tool_use(session_manager):
356+
"""Test that initialize applies the fix_broken_tool_use method."""
357+
agent = Agent(agent_id="test-agent")
358+
359+
# Create session agent first
360+
session_agent = SessionAgent(
361+
agent_id="test-agent", state={}, conversation_manager_state=SlidingWindowConversationManager().get_state()
362+
)
363+
session_manager.session_repository.create_agent("test-session", session_agent)
364+
365+
# Add broken messages to session
366+
broken_message = SessionMessage(
367+
message={
368+
"role": "assistant",
369+
"content": [{"toolUse": {"toolUseId": "broken-123", "name": "test_tool", "input": {"input": "test"}}}],
370+
},
371+
message_id=0,
372+
)
373+
session_manager.session_repository.create_message("test-session", "test-agent", broken_message)
374+
375+
follow_up_message = SessionMessage(message={"role": "user", "content": [{"text": "Some message"}]}, message_id=1)
376+
session_manager.session_repository.create_message("test-session", "test-agent", follow_up_message)
377+
378+
# Initialize agent - should apply fix
379+
session_manager.initialize(agent)
380+
381+
# Check that fix was applied
382+
assert len(agent.messages) == 3
383+
assert agent.messages[1]["role"] == "user"
384+
assert "toolResult" in agent.messages[1]["content"][0]
385+
assert agent.messages[1]["content"][0]["toolResult"]["toolUseId"] == "broken-123"

0 commit comments

Comments
 (0)