-
Notifications
You must be signed in to change notification settings - Fork 448
fix(mcp): add meta field to MCPToolResult for MCP _meta support #885
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
Conversation
…ously incorrectly written as meta. This did not comply with the MCP specification.
| document: DocumentContent | ||
| image: ImageContent | ||
| json: Any | ||
| _meta: NotRequired[Dict[str, Any]] |
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.
Hi, in the current state, we are going to need to do something similar to https://github.com/strands-agents/sdk-python/pull/818/files since if we try to pass _meta to certain models we will get a validation exception.
After #836 this may change. So let's wait until 836 is merged and then I'll re review.
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.
So #818 has since been merged as has #836.
I want to understand your needs though.
I see that you stated the following, indicating that you are aware that MCPToolResult has optional fields like structuredContent.
# strands
class ToolResultContent(TypedDict, total=False):
document: DocumentContent
image: ImageContent
json: Any
meta: NotRequired[Dict[str, Any]] # new field
text: str
class MCPToolResult(ToolResult):
content: list[ToolResultContent]
status: ToolResultStatus
toolUseId: str
structuredContent: NotRequired[Dict[str, Any]]
Does adding meta to MCPToolResult not satisfy your needs? I could entertain meta being added more generally to the base ToolResult but I'm trying to understand the desire for meta to be a ContentType.
This comes down to the following question: Is meta used by the application, in things like Hooks or other AgentTools, or do we want this exposed to the LLM?
class MCPToolResult(ToolResult):
content: list[ToolResultContent]
status: ToolResultStatus
toolUseId: str
structuredContent: NotRequired[Dict[str, Any]]
meta: NotRequired[Dict[str, Any]]
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.
Additionally, our ContentType is a discriminated union. We could change that with a meta special case - would need some convincing. But lets chat more about your needs and we can work back from that
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.
So #818 has since been merged as has #836.
I want to understand your needs though.
I see that you stated the following, indicating that you are aware that MCPToolResult has optional fields like
structuredContent.# strands class ToolResultContent(TypedDict, total=False): document: DocumentContent image: ImageContent json: Any meta: NotRequired[Dict[str, Any]] # new field text: str class MCPToolResult(ToolResult): content: list[ToolResultContent] status: ToolResultStatus toolUseId: str structuredContent: NotRequired[Dict[str, Any]]Does adding meta to MCPToolResult not satisfy your needs? I could entertain meta being added more generally to the base
ToolResultbut I'm trying to understand the desire for meta to be a ContentType.This comes down to the following question: Is meta used by the application, in things like Hooks or other AgentTools, or do we want this exposed to the LLM?
class MCPToolResult(ToolResult): content: list[ToolResultContent] status: ToolResultStatus toolUseId: str structuredContent: NotRequired[Dict[str, Any]] meta: NotRequired[Dict[str, Any]]
-
Here’s the situation: I found that the official MCP’s
CallToolResultcontent is a list, where each item is aTextContentobject that contains a_metafield. This indicates that the tool results in MCP can be a list, and each item in the list may include its own meta information. So my idea is to implement it according to the MCP protocol. -
Putting it into MCPToolResult would work, but I think it should be a list:
NotRequired[List[Dict[str, Any]]]. -
I don’t think this should be exposed to the LLM, since it’s an MCP reserved field 🤔 meant to carry business parameters like
token_usage. I believe a hook can carry such business-specific information in a non-intrusive way, so there’s no need for the LLM to be aware of it.
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.
@dbschmigelski I’d like to know the latest updates about this _meta. I think my idea aligns with the MCP standards.
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.
Does adding meta to MCPToolResult not satisfy your needs? I could entertain meta being added more generally to the base ToolResult but I'm trying to understand the desire for meta to be a ContentType.
I believe my business needs have already been met, because CallToolResult also has a meta field.
The meta field in CallToolResult corresponds to the one you added:
class MCPToolResult(ToolResult):
content: list[ToolResultContent]
status: ToolResultStatus
toolUseId: str
structuredContent: NotRequired[Dict[str, Any]]
meta: NotRequired[Dict[str, Any]]
Description
#881
Add a new
metafield to ToolResultContentInclude the new MCP feature field
_metain the returned fields of _map_mcp_content_to_tool_result_contentI believe adding the
metafield to ToolResultContent aligns with MCP's design principles.This is the official MCP SDK's return for a tool call, where
contentis a list that stores ContentBlocks, and ContentBlock includes a_metafield.The
contentattribute of ToolResult is of typelist[ToolResultContent]. Previously, ToolResultContent lacked themetafield, so this update introduces that change.Do not include this in the LLM's response, as it is an MCP business parameter that can be used to carry the token usage for tool execution.
I handle the business parameter in the meta field by implementing
AfterToolInvocationEventand using it in the post hook.Related Issues
Issues #881
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
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.