-
Notifications
You must be signed in to change notification settings - Fork 170
feat: add structured content #780
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR introduces structured content support for MCP tools, specifically implementing it for the insert-many tool. The change allows tools to return both human-readable text content and machine-parseable structured output.
Key changes:
- Added
outputShapeandToolResulttypes to define structured responses - Modified
insert-manytool to return structured content with success status, inserted count, and IDs - Updated tool registration to use new
registerToolmethod that accepts output schema
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/tool.ts | Defines ToolResult type and adds outputShape field to ToolBase class, updates tool registration to include output schema |
| src/tools/mongodb/create/insertMany.ts | Implements structured content for insert-many tool, defining output shape and returning structured response |
| tests/integration/tools/mongodb/create/insertMany.test.ts | Adds validation for structured content in all insert-many test cases |
| tests/integration/helpers.ts | Updates getResponseContent signature to handle structured content |
| package.json | Changes preview feature flag from vectorSearch to search |
Pull Request Test Coverage Report for Build 19923606969Details
💛 - Coveralls |
|
|
||
| return { | ||
| content, | ||
| structuredContent: { |
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.
FYI the spec says we should return the json serialised instead of formatted text at https://modelcontextprotocol.io/specification/2025-06-18/server/tools#structured-content, I don't think if you would be thinking about automating it as well
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 think we touched on it at some point in Slack, but in my opinion the target of the "SHOULD" here is the backwards compatibility part, not the exact shape of the response. That is, we want to make sure clients that don't support structured content still work as expected, but I don't believe those matching precisely is critical.
I've decided to keep the prompt injection mitigation in place as, if a client does not support structured content, returning json with potentially malicious content is risky. Technically, automating it further is feasible - we can have formatUntrustedData both construct the content with the injection mitigation and returned the structured content as is, but it seemed like we're not winning a lot by doing that (the code would get more complex with a bunch of generics to enforce the output shape).
Proposed changes
This is mostly a POC for how adding structured content to tools could look like. I'm still torn between adding it as is and adding it also to the
formatUntrustedDatahelper - I've decided to keep them separate because I'm hoping we will eventually be able to remove the helper, though I'm very open to being convinced either way.