Skip to content

fix: add type safety for tool output schemas in ToolCallback #670

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

sushichan044
Copy link
Contributor

@sushichan044 sushichan044 commented Jun 20, 2025

Motivation and Context

Enhances ToolCallback to support type-safe structured outputs
by introducing a second generic parameter for the output schema.
fixes: #669

How Has This Been Tested?

Passing all existing tests.
No need for adding new tests as no change for runtime behavior.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@sushichan044 sushichan044 force-pushed the feat/structured-content-with-type-safety branch from 3f68c37 to 08808a4 Compare June 20, 2025 08:24
@sushichan044 sushichan044 marked this pull request as ready for review June 20, 2025 12:25
@ihrpr ihrpr added this to the twr milestone Jun 27, 2025
@ihrpr ihrpr added this to 🐛 🛠 Jun 27, 2025
@github-project-automation github-project-automation bot moved this to To triage in 🐛 🛠 Jun 27, 2025
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.

@sushichan044 many thanks for doing this! One minor suggestion inline for the server example.

@@ -43,7 +43,7 @@ server.registerTool(
void country;
// Simulate weather API call
const temp_c = Math.round((Math.random() * 35 - 5) * 10) / 10;
const conditions = ["sunny", "cloudy", "rainy", "stormy", "snowy"][Math.floor(Math.random() * 5)];
const conditions = ["sunny", "cloudy", "rainy", "stormy", "snowy"][Math.floor(Math.random() * 5)] as unknown as "sunny" | "cloudy" | "rainy" | "stormy" | "snowy";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const conditions = ["sunny", "cloudy", "rainy", "stormy", "snowy"][Math.floor(Math.random() * 5)] as unknown as "sunny" | "cloudy" | "rainy" | "stormy" | "snowy";
const weatherConditions = ["sunny", "cloudy", "rainy", "stormy", "snowy"] as const;
const conditions = weatherConditions[Math.floor(Math.random() * 5)];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bhosmer-ant yes, that's more concise 🙏
fixed: fb303d7

@sushichan044 sushichan044 requested a review from bhosmer-ant June 28, 2025 06:30
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.

@sushichan044 excellent, thank you!

@bhosmer-ant bhosmer-ant enabled auto-merge June 30, 2025 14:09
@bhosmer-ant bhosmer-ant merged commit 9533972 into modelcontextprotocol:main Jun 30, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from To triage to Done in 🐛 🛠 Jun 30, 2025
@alasano
Copy link

alasano commented Jul 2, 2025

I think this PR caused a breaking change for TypeScript users despite being a patch release

While the runtime still works, MCP SDK 1.13.3 introduced TypeScript compilation errors that require code changes to fix. The ToolCallback type now mandates an extra parameter and enforces strict structuredContent typing.

Before (1.13.2 - worked):

mcp.registerTool(
    'send_email',
    {
      description: "Sends an email.",
      inputSchema: SendEmailInputSchema.shape,
      outputSchema: SendEmailOutputSchema.shape,
      annotations: {
        title: "Send Email",
        readOnlyHint: false,
        destructiveHint: false,
        idempotentHint: false,
        openWorldHint: true,
      } as ToolAnnotations,
   },
  async (args): Promise<CallToolResult> => {
    // ... implementation
    return {
      content: [{ type: 'text', text: 'Email sent successfully' }],
      structuredContent: {
        success: true,
        messageId: res.data.id || '',
        threadId: res.data.threadId || undefined,
      },
    };
  }

After (1.13.3 - requires changes):

  async (args, _extra): Promise<CallToolResult & { structuredContent?: z.infer<typeof SendEmailOutputSchema> }> => {
    // ... implementation  
    return {
      content: [{ type: 'text', text: 'Email sent successfully' }],
      structuredContent: {
        success: true,
        messageId: res.data.id || '',
        threadId: res.data.threadId || undefined,
      },
    };
  }

Every tool handler now needs:

  1. Added _extra parameter (was introduced in 1.13.2 but was optional)
  2. Complex return type with z.infer pattern
  3. Null-to-undefined conversions for API responses

Is this really the intended way to type this now @sushichan044 ?

The existing CallToolResult.structuredContent wasn't strictly typed, but now requires exact schema compliance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add type safety for structuredContent in ToolCallback with outputSchema
4 participants