Skip to content

Conversation

@nirinchev
Copy link
Collaborator

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 formatUntrustedData helper - 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.

@nirinchev nirinchev requested a review from a team as a code owner December 3, 2025 23:19
Copilot AI review requested due to automatic review settings December 3, 2025 23:19
@nirinchev nirinchev marked this pull request as draft December 3, 2025 23:19
Copy link
Contributor

Copilot AI left a 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 outputShape and ToolResult types to define structured responses
  • Modified insert-many tool to return structured content with success status, inserted count, and IDs
  • Updated tool registration to use new registerTool method 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

@coveralls
Copy link
Collaborator

coveralls commented Dec 4, 2025

Pull Request Test Coverage Report for Build 19923606969

Details

  • 18 of 18 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 80.63%

Totals Coverage Status
Change from base Build 19904428619: 0.02%
Covered Lines: 6741
Relevant Lines: 8279

💛 - Coveralls


return {
content,
structuredContent: {
Copy link
Collaborator

@fmenezes fmenezes Dec 8, 2025

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

Copy link
Collaborator Author

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).

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.

4 participants