-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Type generics for CallToolResult - Require 'structuredContent' if 'outputSchema' defined. #859
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
…urn if outputSchema defined
Please review, but this could be further improved by not allowing "structuredContent" at all when "outputSchema" is defined. Currently, it'll only generate a typecheck error if "outputSchema" is defined but "structuredContent" is missing, but it will not error when "structuredContent" is returned and "outputSchema" wasn't defined in the first place. (e.g. if the user forgot by accident). cc @ihrpr |
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.
Suggestion for changes to mcpServerOutputSchema.ts
example.
Would be good to see explicit tests for CallToolResultUnstructuredSchema
and CallToolResultStructuredSchema
.
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 can't make code suggestions on lines not in the PR, but I would refactor this entire file to remove the duplication of weather condition literals (repeated in three places) and also ensure that wind direction value is similarly restricted (it's oddly just a string in the schema but selected randomly from an array of literals when set in the structured output).
#!/usr/bin/env node
/**
* Example MCP server using the high-level McpServer API with outputSchema
* This demonstrates how to easily create tools with structured output
*/
import { McpServer } from "../../server/mcp.js";
import { StdioServerTransport } from "../../server/stdio.js";
import { z } from "zod";
const server = new McpServer(
{
name: "mcp-output-schema-high-level-example",
version: "1.0.0",
}
);
const weatherConditions = ["sunny", "cloudy", "rainy", "stormy", "snowy"] as const;
const windDirections = ["N", "NE", "E", "SE", "S", "SW", "W", "NW"] as const;
// Define a tool with structured output - Weather data
server.registerTool(
"get_weather",
{
description: "Get weather information for a city",
inputSchema: {
city: z.string().describe("City name"),
country: z.string().describe("Country code (e.g., US, UK)")
},
outputSchema: {
temperature: z.object({
celsius: z.number(),
fahrenheit: z.number()
}),
conditions: z.enum(weatherConditions),
humidity: z.number().min(0).max(100),
wind: z.object({
speed_kmh: z.number(),
direction: z.enum(windDirections)
})
},
},
async ({ city, country }) => {
// Parameters are available but not used in this example
void city;
void country;
// Simulate weather API call
const temp_c = Math.round((Math.random() * 35 - 5) * 10) / 10;
const conditions =
weatherConditions[Math.floor(Math.random() * weatherConditions.length)];
const structuredContent = {
temperature: {
celsius: temp_c,
fahrenheit: Math.round((temp_c * 9 / 5 + 32) * 10) / 10
},
conditions,
humidity: Math.round(Math.random() * 100),
wind: {
speed_kmh: Math.round(Math.random() * 50),
direction:
windDirections[Math.floor(Math.random() * windDirections.length)]
}
};
return {
content: [{
type: "text",
text: JSON.stringify(structuredContent, null, 2)
}],
structuredContent
};
}
);
async function main() {
const transport = new StdioServerTransport();
await server.connect(transport);
console.error("High-level Output Schema Example Server running on stdio");
}
main().catch((error) => {
console.error("Server error:", error);
process.exit(1);
});
The structuredContent tool output is currently not utilizing Typescript's capabilities and the check is performed only runtime.
If outputSchema is defined, as per the implementation, structuredContent is required in the response.
Motivation and Context
Mitigating runtime-only handling and proper type checking when outputSchema is defined.
How Has This Been Tested?
The changes are in types only - the results of the PR can be observed in tests, //@ts-expect-error has to be added now when testing for tools defined with outputSchema and not returning structuredContent.
Breaking Changes
No need, only if they had a runtime error anyways will they get a type error.
Types of changes
Checklist