-
Couldn't load subscription status.
- Fork 453
Add EmbeddedResource support to mcp (read GitHub file contents blocker) #726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add EmbeddedResource support to mcp (read GitHub file contents blocker) #726
Conversation
|
@notgitika, @pgrayy, @mehtarac - I saw ya'll have recently deployed PRs for testing - does anyone have time to approve this unit testing on this PR so I can make sure it ready for review? 🙏 Thanks in advance! |
|
Thanks for raising this. Left a few minor comments with the larger one being adding full resource support in one PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
|
@dbschmigelski - Do you think it's possible to move this forward? Lacking the ability to read GitHub file contents is a pretty big miss, and I think resolving that is worth not packaging the entire support for Resources in a single PR. Please let me know your thoughts. |
Hi @KyMidd, very sorry for the delay on this. I think I am still hesitant to go forward without resources first being explored in Strands. This is mostly a fear of unknown unknowns. I do not know if there will be significant differences but I want to avoid getting stuck in a sub-optimal place because of backwards compatibility issues when I believe resources will not require much more beyond this PR. Resources is still on our roadmap, and we'll certainly move forward with embedded after that. |
|
Hey @dbschmigelski, understood. I want to help if possible. Do you mean we should extend the EmbeddedResource support to also include binary/blob items, like images? I believe I know enough about the context and use case of these to support this, I'd just need to learn more about how binary files are stored and remembered (stored in memory? temporarily stored as binary files on disk?). If you mean supporting Resources for list/changed, and subscription, I don't believe I know enough. Is that what you mean? If so, how can I help? I can keep researching and iterating, or I could test out fixes when someone gets here from your team. Thanks Dean, appreciate it. |
Sorry, I mean supporting resources. But only list not changed or subscription. This would be almost the same as our prompt implementation which is not as "deep" in Strands as tools. So it is a lot of boilerplate but in practice quite simple
I really just want to see if there is anything that we need to do which overlaps with what you proposed here. I honestly don't think it is much but prefer to have all of the data in front of us. I can commit to getting looking at adding resources next week, raising the pr not necessarily merged or released, then we can compare if there is overlap here. Sorry again for the delay |
|
@dbschmigelski - Did you find any time to work on this last week? I'd love to compare when you get a chance :) Let me know if I should resolve the conflicts, I've done that once before but seems I should wait until we're closer. |
914e9c8 to
8d7aa66
Compare
|
It took quite some time, but finally got this in @KyMidd. Thanks for the contribution! |
Description
Hey team! Utilizing strands locally to connect to (remote github MCP), and saw that it was unable to download particular files using github MCP tool
get_file_contents.When I'd attempt I'd see logs like this:
The main issues appears to be that the file is not provided as text, it's provided as an EmbeddedResource. This PR provides Slack support for EmbeddedResources that match plaintext-readable types.
In my local testing, it fixes the behavior, and my strands agent can now read github files without any changes to the github MCP tools.
Related Issues
Partially fixes #535
Documentation PR
n/a
Type of Change
Bug fix
Testing
I made the changes to my local version of the sdk-python/src/strands/tools/mcp/mcp_client.py to add support for EmbeddedResource.
Here is my agent config file, it's an MVP of using github to fetch a file when I noticed it was unable to do so reliably.
When run, it is now able to fetch the file:
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.