Skip to content

Conversation

JackYPCOnline
Copy link
Contributor

@JackYPCOnline JackYPCOnline commented Jul 29, 2025

Description

Before fix:
'input': {'action': 'start_socket_mode', 'agent': <strands.agent.agent.Agent object at 0x1041d9a90>}}}
After fix:

[{'role': 'user', 'content': [{'text': 'agent.tool.slack direct tool call.\nInput parameters: {"action": "start_socket_mode"}\n'}]}, {'role': 'assistant', 'content': [{'toolUse': {'toolUseId': 'tooluse_slack_306896846', 'name': 'slack', 'input': {'action': 'start_socket_mode'}}}]}, {'role': 'user', 'content': [{'toolResult': {'toolUseId': 'tooluse_slack_306896846', 'status': 'success', 'content': [{'text': '✅ Socket Mode connection established and ready to receive real-time events'}]}}]}, {'role': 'assistant', 'content': [{'text': 'agent.tool.slack was called.'}]}]

Related Issues

#545

Documentation PR

Type of Change

Bug fix
New feature
Breaking change
Documentation update
Other (please describe):

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

  • [x ] I ran hatch run prepare

Checklist

  • [ x] I have read the CONTRIBUTING document
  • [ x] I have added any necessary tests that prove my fix is effective or my feature works
  • [ x] I have updated the documentation accordingly
  • [ x] I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • [ x] My changes generate no new warnings
  • [ x] 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.

@Unshure
Copy link
Member

Unshure commented Jul 29, 2025

In the example where this bug was exposed, the code that was written was:

agent.tool.slack(action="start_socket_mode", agent=agent)

The user should not need to pass in the agent field into the direct tool call function, and instead this should be handled automatically when invoking a tool this way. I would expect the user to just have to write the following:

agent.tool.slack(action="start_socket_mode")

Then the sdk will automatically add the agent parameter when calling the tool. Additionally, the agent parameter should not be shown in the direct tool call message.

It should be this:

 {'action': 'start_socket_mode'}

Not this:

 {'action': 'start_socket_mode', 'agent': '<<non-serializable: Agent>>'}

@JackYPCOnline
Copy link
Contributor Author

@cagataycali if nick's answer makes sense to you, I will close this PR.

@cagataycali
Copy link
Member

cagataycali commented Aug 8, 2025

I disagree following statement,

The user should not need to pass in the agent field into the direct tool call function, and instead this should be handled automatically when invoking a tool this way. I would expect the user to just have to write the following:

Users can pass any agent instance to direct tool call as agent instance. @Unshure

cagataycali
cagataycali previously approved these changes Aug 8, 2025
JackYPCOnline and others added 4 commits August 14, 2025 10:49
…rands-agents#643)

Previously (strands-agents#642) bedrock would hang during message conversion because the exception was not being caught and thus the queue was always empty. Now all exceptions during conversion are caught

Co-authored-by: Mackenzie Zastrow <[email protected]>
@JackYPCOnline
Copy link
Contributor Author

After discussion, team deceided to strip out any parameter that is not in tool spec.

Copy link
Member

@cagataycali cagataycali left a comment

Choose a reason for hiding this comment

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

lgtm

@JackYPCOnline JackYPCOnline merged commit c087f18 into strands-agents:main Aug 19, 2025
12 checks passed
dbschmigelski pushed a commit to dbavro19/sdk-python that referenced this pull request Aug 28, 2025
…nds-agents#568)

* fix: fix non-serializable parameter of agent from toolUse block

* feat: Add configuration option to MCP Client for server init timeout (strands-agents#657)

Co-authored-by: Harry Wilton <[email protected]>

* fix: Bedrock hang when exception occurs during message conversion (strands-agents#643)

Previously (strands-agents#642) bedrock would hang during message conversion because the exception was not being caught and thus the queue was always empty. Now all exceptions during conversion are caught

Co-authored-by: Mackenzie Zastrow <[email protected]>

* fix: only include parameters that defined in tool spec

---------

Co-authored-by: Jack Yuan <[email protected]>
Co-authored-by: fhwilton55 <[email protected]>
Co-authored-by: Harry Wilton <[email protected]>
Co-authored-by: Mackenzie Zastrow <[email protected]>
Co-authored-by: Mackenzie Zastrow <[email protected]>
@JackYPCOnline JackYPCOnline deleted the fix branch August 31, 2025 03:06
This was referenced Sep 17, 2025
Unshure pushed a commit to Unshure/sdk-python that referenced this pull request Sep 24, 2025
…nds-agents#568)

* fix: fix non-serializable parameter of agent from toolUse block

* feat: Add configuration option to MCP Client for server init timeout (strands-agents#657)

Co-authored-by: Harry Wilton <[email protected]>

* fix: Bedrock hang when exception occurs during message conversion (strands-agents#643)

Previously (strands-agents#642) bedrock would hang during message conversion because the exception was not being caught and thus the queue was always empty. Now all exceptions during conversion are caught

Co-authored-by: Mackenzie Zastrow <[email protected]>

* fix: only include parameters that defined in tool spec

---------

Co-authored-by: Jack Yuan <[email protected]>
Co-authored-by: fhwilton55 <[email protected]>
Co-authored-by: Harry Wilton <[email protected]>
Co-authored-by: Mackenzie Zastrow <[email protected]>
Co-authored-by: Mackenzie Zastrow <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants