Skip to content

Conversation

@afarntrog
Copy link
Contributor

@afarntrog afarntrog commented Dec 1, 2025

Description

This pull request improves the robustness of session message history handling by ensuring that orphaned tool use and tool result messages are properly paired, preventing broken conversation states. It introduces a new helper for generating missing tool use content, updates the session manager to handle both types of orphaned messages, and adds comprehensive unit tests for these helpers.

Session message history fixes:

  • Updated _fix_broken_tool_use in repository_session_manager.py to handle both orphaned toolUse messages (no toolResult) and orphaned toolResult messages (no toolUse), ensuring valid message history even after pagination or legacy bugs.
  • Added use of the new generate_missing_tool_use_content helper to retroactively insert dummy toolUse messages when needed.

Tool helper enhancements:

  • Added generate_missing_tool_use_content to _tool_helpers.py, which generates dummy toolUse content blocks for orphaned toolResult messages.

Testing improvements:

  • Added test_tool_helpers.py with thorough tests for both generate_missing_tool_result_content and the new generate_missing_tool_use_content, covering single, multiple, empty, and realistic ID scenarios.

Related Issues

#1272

Documentation PR

Type of Change

Bug fix

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/strands/session/repository_session_manager.py 33.33% 5 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Fixed list of messages with proper tool use/result pairs
"""
# First, check if the first message has orphaned toolResult (no preceding toolUse)
if messages:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the bug report - Isn't the problem the opposite?

It's not that the latest message is broken; it's that the oldest message is broken - that's because we have N messages:

  • 55
  • 54
  • 53
  • 52
  • 51
  • 50
  • 49
  • 48

And 48 is a toolResult with 47 being toolUse, but it's not included

In this case messages[0] is 55, while messages[-1] == 48

And I think the fix in this case is to just not include 48

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here's a full example:

from strands import Agent, tool


@tool
def get_user_location() -> str:
    """Get the user's location."""
    return "Seattle, USA"

MEM_ID='ComprehensiveAgentMemory_sep16-YDe9F76vbr'
ACTOR_ID='actor_id_test_20251201135435'
SESSION_ID='testing_session_id_20251201135435'


config = AgentCoreMemoryConfig(
    memory_id=MEM_ID,
    session_id=SESSION_ID,
    actor_id=ACTOR_ID,
)
session_manager = AgentCoreMemorySessionManager(config, region_name='us-east-1')

agent_1 = Agent(session_manager=session_manager, tools=[get_user_location])

Let's view the agent_1 messages:

agent_1.messages
[{'role': 'assistant',
  'content': [{'toolUse': {'toolUseId': 'tooluse_f09Y0LwyT2yteCYshTzb_Q',
     'name': 'unknown_tool',
     'input': {'error': 'Tool use was truncated due to message limit.'}}}]},
 {'role': 'user',
  'content': [{'toolResult': {'toolUseId': 'tooluse_f09Y0LwyT2yteCYshTzb_Q',
     'status': 'success',
     'content': [{'text': 'Seattle, USA'}]}}]},
 {'role': 'assistant', 'content': [{'text': 'You live in Seattle, USA.'}]},
 {'role': 'user', 'content': [{'text': 'I like pizza'}]},
 {'role': 'user', 'content': [{'text': 'where do I live?'}]},
 {'role': 'user', 'content': [{'text': 'where do I live?'}]},
 {'role': 'assistant',
  'content': [{'text': '\n\nI can see you mentioned that you like pizza again! As I mentioned before, you live in **Seattle, USA**. \n\nSeattle has a fantastic pizza scene! Are you looking for pizza recommendations in your area, or did you have a specific question about pizza places in Seattle?'}]},
 {'role': 'user', 'content': [{'text': 'I like pizza'}]}]

The session manager is replaying the conversation so the -1 message is the newest one (I like pizza) whereas the closer to 0 the further back we go.

Copy link
Member

@zastrowm zastrowm Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right; I was thinking of it backwards.

Still, can we remove the first message instead creating a fake one? IMHO it's better to trim than to synthesize

(and if you do ^, can we update the comment to indicate "check if the oldest message")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants