-
Notifications
You must be signed in to change notification settings - Fork 73
Support structuredContent in tool response #147
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
Support structuredContent in tool response #147
Conversation
lib/mcp/tool/response.rb
Outdated
|
||
if structured_content | ||
@structured_content = structured_content | ||
@content ||= [{ type: "text", text: structured_content.to_json }] |
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.
I'm not entirely sure if it should be responsibility of the gem to add the backwards-compatibility content chunk here. @koic @topherbullock thoughts?
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.
That was in response to this section of the spec:
5.2.6 Structured Content
Structured content is returned as a JSON object in the structuredContent field of a result.
For backwards compatibility, a tool that returns structured content SHOULD also return the serialized JSON in a TextContent block [emphasis added]
Without L21 the user is obligated to redundantly and possibly incorrectly supply the content
block themselves.
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.
But if we make the gem opinionated like this, there's no way for a gem user to not redundantly include the content chunk. I feel like the gem should let people do whatever they want.
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.
This PR should just add support for structured content, and leave the potentially-helpful opinion bit as a follow-up.
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.
OK, fair enough. Given that it is a SHOULD and not a MUST, I think that is reasonable.
I've updated the branch:
- removed the automatic
content
chunk - updated the README with sample usage of the new
structured_content
param - tested that everything still works in the downstream environments mentioned in the description
04da1d8
to
b629b58
Compare
Follow-up to #147. According to the specification schema, `content` is not optional and therefore cannot be omitted: ```typescript interface CallToolResult { _meta?: { [key: string]: unknown }; content: ContentBlock[]; isError?: boolean; structuredContent?: { [key: string]: unknown }; [key: string]: unknown; } ``` https://modelcontextprotocol.io/specification/2025-06-18/schema#calltoolresult Instead of `nil`, an empty array is set as the default value. There may be a better value for `content`, but at the very least it should not be missing.
This PR enables the inclusion of
structuredContent
within tool call responses.Or, to conform with the spec's suggestion about backward compatibility, both
content
andstructured_content
may be provided:Motivation and Context
Currently an
MCP::Tool
which declares anoutput_schema
but does not providestructuredContent
in its response is entirely incompatible with the many MCP clients that are built with the official Python SDK:See: modelcontextprotocol/python-sdk/src/mcp/client/session.py
In order to make Ruby SDK MCP servers useable with such clients we need to support
structuredContent
in conformance with the spec forCallToolResult
:See: https://modelcontextprotocol.io/specification/2025-06-18/schema#calltoolresult
See: https://modelcontextprotocol.io/specification/2025-06-18/server/tools#structured-content
@honzasterba had kindly submitted a similar PR #81 — this PR follows @koic's suggestion there.
How Has This Been Tested?
In addition to the unit tests, I have field tested this in an MCP server that utilizes this change in:
structuredContent
against theoutput_schema
and confirms that thestructuredContent
matches the serializedcontent
structuredContent
, so was fine before and after this change 🤷🏽Breaking Changes
None.
The tool response can be constructed as before, or in the new style:
Types of changes
Checklist
Additional context
I have not added documentation yet, but if this PR seems likely to be accepted I can definitely do so!Added now