-
Notifications
You must be signed in to change notification settings - Fork 1.5k
registerTool: accept ZodType<object> for input and output schema
#816
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
|
+1 on merging this or at least getting feedback from the team. I ran into this problem myself just today. Some of the error conditions that the API I'm wrapping for LLM consumption are complex and the better I can hint the LLM in the response, the better we'll be able to properly recover. |
0845a57 to
c94ba4b
Compare
|
guys, can we get this merged |
|
It's disappointing that @ksinder went to the trouble of creating a PR for this and it's been ignored for many months. |
47678e4 to
d839335
Compare
pcarleton
left a comment
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.
LGTM, thanks!
|
Can I get help with this please? #1148 |
|
This change was released in const schema = z.strictObject({
key1: z.literal('value1'),
key2: z.literal('value2'),
});
const mcpTool: Tool = {
name: 'Test tool',
description: 'Test tool',
title: 'Test tool',
// just a dummy input and output schema
inputSchema: schema,
outputSchema: schema,
};InputSchema and outputSchema have the following error. TLDR: 'type' is missing I think this pr is supposed to support the schema mentioned above directly. Please correct me if I am wrong const mcpTool: Tool = {
name: 'Test tool',
description: 'Test tool',
title: 'Test tool',
// just a dummy input and output schema
inputSchema: { ...schema, type: 'object' },
outputSchema: { ...schema, type: 'object' },
}; |
The issue was due to a difference in exported types from the sdk. I was using low-level server, and the type I used for creating the tool was // GOOD
import { RegisteredTool, RegisteredResource } from '@modelcontextprotocol/sdk/server/mcp';
// BAD
import { Tool, Resource } from '@modelcontextprotocol/sdk/types'; |
Motivation and Context
See #588. This PR adds a backwards-compatible change to
registerToolonMcpServerto allow any Zod schema extendingZodType<object>instead of requiringinputSchemaandoutputSchemato conform toZodRawShape(:=Record<string, ZodType<any>>).The underlying schema validation and
zod-to-json-schemadependency already support this, so adding this plumbing expands SDK expressiveness and flexibility. Specifically, this unblocks MCP servers' ability to define tool schemas that usez.union(...)andz.intersection(...). Historically, some creators have had to wrap existing schemas in some wrapper object that has fixed keys, e.g.{bodyParams: z.union(...)}to work around this limitation.How Has This Been Tested?
Added automated tests in
mcp.test.tsand confirmed they pass locally withnpm run test.Also manually tried changing the test locally to use
inputSchemas for types that aren't objects at the top level -- e.g.z.number()andz.union([z.number(), z.object({})]), and confirmed typechecking fails (which is good, since I'm guessing we still want guardrails to push toward objects for params and responses. If this ever changes in the future it's easy enough to change allZodType<object>toZodTypeAny!)Breaking Changes
Should be backwards-compatible in the SDK with existing server code. This is purely additive since we can easily dynamically distinguish between existing
ZodRawShapes being passed in vs. directZodType<object>and branch accordingly.ZodType<object>is added as a union type variant forserver.registerTool, and isn't mandatory.Types of changes
Checklist
Additional context
N/A