Skip to content

Make content in Tool Result required #524

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

Merged
merged 2 commits into from
May 21, 2025
Merged

Make content in Tool Result required #524

merged 2 commits into from
May 21, 2025

Conversation

ihrpr
Copy link
Contributor

@ihrpr ihrpr commented May 21, 2025

Right now we have a problem when the content field is optional; it breaks backwards compatibility for clients and they need to modify the core from:
result.content.forEach -> result.content?.forEach

This PR implements making the content field required:

  • Tools should be explicit if they want to return structured output and what type of content will be returned (json serialized structured content or something else).
  • Clients will check if structured output is present and if not, will just use content
  • Best practices should be provided to server and client implementations to always return content along with structured content. If clients choose to use structured content, they can start using it, but if not, no changes are needed after SDK version upgrade.

Tracks changes to draft spec in modelcontextprotocol/modelcontextprotocol#559

@bhosmer-ant bhosmer-ant force-pushed the ihrpr/output-types branch from 41e1176 to 08d5db7 Compare May 21, 2025 16:19
@bhosmer-ant bhosmer-ant force-pushed the ihrpr/output-types branch from 08d5db7 to 7636c63 Compare May 21, 2025 16:42
@ihrpr ihrpr marked this pull request as ready for review May 21, 2025 16:50
@bhosmer-ant bhosmer-ant self-requested a review May 21, 2025 16:52
Copy link
Contributor Author

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhosmer-ant changes LGTM

Copy link
Contributor

@bhosmer-ant bhosmer-ant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tunneling @ihrpr approval 😬

@bhosmer-ant bhosmer-ant merged commit a41b174 into main May 21, 2025
5 checks passed
@bhosmer-ant bhosmer-ant deleted the ihrpr/output-types branch May 21, 2025 16:54
ihrpr pushed a commit that referenced this pull request May 21, 2025
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.

2 participants