address comments: 1. validate schemas w/o codegen, 2. relax no-structured-output restriction
This commit is contained in:
Basil Hosmer
parent
0fa3277550
commit
18c7e795a0
4 changed files with 106 additions and 119 deletions
113
package-lock.json
generated
113
package-lock.json
generated
|
|
@ -9,13 +9,13 @@
|
|||
"version": "1.11.3",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ajv": "^8.17.1",
|
||||
"content-type": "^1.0.5",
|
||||
"cors": "^2.8.5",
|
||||
"cross-spawn": "^7.0.5",
|
||||
"eventsource": "^3.0.2",
|
||||
"express": "^5.0.1",
|
||||
"express-rate-limit": "^7.5.0",
|
||||
"json-schema-to-zod": "^2.6.1",
|
||||
"pkce-challenge": "^5.0.0",
|
||||
"raw-body": "^3.0.0",
|
||||
"zod": "^3.23.8",
|
||||
|
|
@ -1036,6 +1036,30 @@
|
|||
"url": "https://opencollective.com/eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@eslint/eslintrc/node_modules/ajv": {
|
||||
"version": "6.12.6",
|
||||
"resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz",
|
||||
"integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"fast-deep-equal": "^3.1.1",
|
||||
"fast-json-stable-stringify": "^2.0.0",
|
||||
"json-schema-traverse": "^0.4.1",
|
||||
"uri-js": "^4.2.2"
|
||||
},
|
||||
"funding": {
|
||||
"type": "github",
|
||||
"url": "https://github.com/sponsors/epoberezkin"
|
||||
}
|
||||
},
|
||||
"node_modules/@eslint/eslintrc/node_modules/json-schema-traverse": {
|
||||
"version": "0.4.1",
|
||||
"resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz",
|
||||
"integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@eslint/js": {
|
||||
"version": "9.13.0",
|
||||
"resolved": "https://registry.npmjs.org/@eslint/js/-/js-9.13.0.tgz",
|
||||
|
|
@ -2199,15 +2223,15 @@
|
|||
}
|
||||
},
|
||||
"node_modules/ajv": {
|
||||
"version": "6.12.6",
|
||||
"resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz",
|
||||
"integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==",
|
||||
"dev": true,
|
||||
"version": "8.17.1",
|
||||
"resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz",
|
||||
"integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"fast-deep-equal": "^3.1.1",
|
||||
"fast-json-stable-stringify": "^2.0.0",
|
||||
"json-schema-traverse": "^0.4.1",
|
||||
"uri-js": "^4.2.2"
|
||||
"fast-deep-equal": "^3.1.3",
|
||||
"fast-uri": "^3.0.1",
|
||||
"json-schema-traverse": "^1.0.0",
|
||||
"require-from-string": "^2.0.2"
|
||||
},
|
||||
"funding": {
|
||||
"type": "github",
|
||||
|
|
@ -3217,6 +3241,30 @@
|
|||
"url": "https://opencollective.com/eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/eslint/node_modules/ajv": {
|
||||
"version": "6.12.6",
|
||||
"resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz",
|
||||
"integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"fast-deep-equal": "^3.1.1",
|
||||
"fast-json-stable-stringify": "^2.0.0",
|
||||
"json-schema-traverse": "^0.4.1",
|
||||
"uri-js": "^4.2.2"
|
||||
},
|
||||
"funding": {
|
||||
"type": "github",
|
||||
"url": "https://github.com/sponsors/epoberezkin"
|
||||
}
|
||||
},
|
||||
"node_modules/eslint/node_modules/json-schema-traverse": {
|
||||
"version": "0.4.1",
|
||||
"resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz",
|
||||
"integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/espree": {
|
||||
"version": "10.2.0",
|
||||
"resolved": "https://registry.npmjs.org/espree/-/espree-10.2.0.tgz",
|
||||
|
|
@ -3451,8 +3499,7 @@
|
|||
"node_modules/fast-deep-equal": {
|
||||
"version": "3.1.3",
|
||||
"resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz",
|
||||
"integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==",
|
||||
"dev": true
|
||||
"integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q=="
|
||||
},
|
||||
"node_modules/fast-glob": {
|
||||
"version": "3.3.2",
|
||||
|
|
@ -3501,6 +3548,22 @@
|
|||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/fast-uri": {
|
||||
"version": "3.0.6",
|
||||
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.6.tgz",
|
||||
"integrity": "sha512-Atfo14OibSv5wAp4VWNsFYE1AchQRTv9cBGWET4pZWHzYshFSS9NQI6I57rdKn9croWVMbYFbLhJ+yJvmZIIHw==",
|
||||
"funding": [
|
||||
{
|
||||
"type": "github",
|
||||
"url": "https://github.com/sponsors/fastify"
|
||||
},
|
||||
{
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/fastify"
|
||||
}
|
||||
],
|
||||
"license": "BSD-3-Clause"
|
||||
},
|
||||
"node_modules/fastq": {
|
||||
"version": "1.17.1",
|
||||
"resolved": "https://registry.npmjs.org/fastq/-/fastq-1.17.1.tgz",
|
||||
|
|
@ -4880,20 +4943,11 @@
|
|||
"integrity": "sha512-xyFwyhro/JEof6Ghe2iz2NcXoj2sloNsWr/XsERDK/oiPCfaNhl5ONfp+jQdAZRQQ0IJWNzH9zIZF7li91kh2w==",
|
||||
"dev": true
|
||||
},
|
||||
"node_modules/json-schema-to-zod": {
|
||||
"version": "2.6.1",
|
||||
"resolved": "https://registry.npmjs.org/json-schema-to-zod/-/json-schema-to-zod-2.6.1.tgz",
|
||||
"integrity": "sha512-uiHmWH21h9FjKJkRBntfVGTLpYlCZ1n98D0izIlByqQLqpmkQpNTBtfbdP04Na6+43lgsvrShFh2uWLkQDKJuQ==",
|
||||
"license": "ISC",
|
||||
"bin": {
|
||||
"json-schema-to-zod": "dist/cjs/cli.js"
|
||||
}
|
||||
},
|
||||
"node_modules/json-schema-traverse": {
|
||||
"version": "0.4.1",
|
||||
"resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz",
|
||||
"integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==",
|
||||
"dev": true
|
||||
"version": "1.0.0",
|
||||
"resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz",
|
||||
"integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==",
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/json-stable-stringify-without-jsonify": {
|
||||
"version": "1.0.1",
|
||||
|
|
@ -5568,6 +5622,7 @@
|
|||
"resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz",
|
||||
"integrity": "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=6"
|
||||
}
|
||||
|
|
@ -5672,6 +5727,15 @@
|
|||
"node": ">=0.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/require-from-string": {
|
||||
"version": "2.0.2",
|
||||
"resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz",
|
||||
"integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=0.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/resolve": {
|
||||
"version": "1.22.8",
|
||||
"resolved": "https://registry.npmjs.org/resolve/-/resolve-1.22.8.tgz",
|
||||
|
|
@ -6455,6 +6519,7 @@
|
|||
"resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.4.1.tgz",
|
||||
"integrity": "sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==",
|
||||
"dev": true,
|
||||
"license": "BSD-2-Clause",
|
||||
"dependencies": {
|
||||
"punycode": "^2.1.0"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -46,13 +46,13 @@
|
|||
"client": "tsx src/cli.ts client"
|
||||
},
|
||||
"dependencies": {
|
||||
"ajv": "^8.17.1",
|
||||
"content-type": "^1.0.5",
|
||||
"cors": "^2.8.5",
|
||||
"cross-spawn": "^7.0.5",
|
||||
"eventsource": "^3.0.2",
|
||||
"express": "^5.0.1",
|
||||
"express-rate-limit": "^7.5.0",
|
||||
"json-schema-to-zod": "^2.6.1",
|
||||
"pkce-challenge": "^5.0.0",
|
||||
"raw-body": "^3.0.0",
|
||||
"zod": "^3.23.8",
|
||||
|
|
|
|||
|
|
@ -1261,72 +1261,5 @@ describe('outputSchema validation', () => {
|
|||
);
|
||||
});
|
||||
|
||||
/***
|
||||
* Test: Throw Error when Tool Without outputSchema Returns structuredContent
|
||||
*/
|
||||
test('should throw error when tool without outputSchema returns structuredContent', async () => {
|
||||
const server = new Server({
|
||||
name: 'test-server',
|
||||
version: '1.0.0',
|
||||
}, {
|
||||
capabilities: {
|
||||
tools: {},
|
||||
},
|
||||
});
|
||||
|
||||
// Set up server handlers
|
||||
server.setRequestHandler(InitializeRequestSchema, async (request) => ({
|
||||
protocolVersion: request.params.protocolVersion,
|
||||
capabilities: {},
|
||||
serverInfo: {
|
||||
name: 'test-server',
|
||||
version: '1.0.0',
|
||||
}
|
||||
}));
|
||||
|
||||
server.setRequestHandler(ListToolsRequestSchema, async () => ({
|
||||
tools: [
|
||||
{
|
||||
name: 'test-tool',
|
||||
description: 'A test tool',
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {},
|
||||
},
|
||||
// No outputSchema defined
|
||||
},
|
||||
],
|
||||
}));
|
||||
|
||||
server.setRequestHandler(CallToolRequestSchema, async (request) => {
|
||||
if (request.params.name === 'test-tool') {
|
||||
// Incorrectly return structuredContent for a tool without outputSchema
|
||||
return {
|
||||
structuredContent: { result: 'This should not be allowed' },
|
||||
};
|
||||
}
|
||||
throw new Error('Unknown tool');
|
||||
});
|
||||
|
||||
const client = new Client({
|
||||
name: 'test-client',
|
||||
version: '1.0.0',
|
||||
});
|
||||
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
|
||||
await Promise.all([
|
||||
client.connect(clientTransport),
|
||||
server.connect(serverTransport),
|
||||
]);
|
||||
|
||||
// List tools to cache the schemas
|
||||
await client.listTools();
|
||||
|
||||
// Call the tool - should throw validation error
|
||||
await expect(client.callTool({ name: 'test-tool' })).rejects.toThrow(
|
||||
/Tool without outputSchema cannot return structuredContent/
|
||||
);
|
||||
});
|
||||
|
||||
});
|
||||
|
|
|
|||
|
|
@ -43,8 +43,7 @@ import {
|
|||
ErrorCode,
|
||||
McpError,
|
||||
} from "../types.js";
|
||||
import { z } from "zod";
|
||||
import { JsonSchema, parseSchema } from "json-schema-to-zod";
|
||||
import { Ajv, type ValidateFunction } from "ajv";
|
||||
|
||||
export type ClientOptions = ProtocolOptions & {
|
||||
/**
|
||||
|
|
@ -91,7 +90,8 @@ export class Client<
|
|||
private _serverVersion?: Implementation;
|
||||
private _capabilities: ClientCapabilities;
|
||||
private _instructions?: string;
|
||||
private _cachedToolOutputSchemas: Map<string, z.ZodTypeAny> = new Map();
|
||||
private _cachedToolOutputValidators: Map<string, ValidateFunction> = new Map();
|
||||
private _ajv: InstanceType<typeof Ajv>;
|
||||
|
||||
/**
|
||||
* Initializes this client with the given name and version information.
|
||||
|
|
@ -102,6 +102,7 @@ export class Client<
|
|||
) {
|
||||
super(options);
|
||||
this._capabilities = options?.capabilities ?? {};
|
||||
this._ajv = new Ajv({ strict: false, validateFormats: true });
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -426,8 +427,8 @@ export class Client<
|
|||
);
|
||||
|
||||
// Check if the tool has an outputSchema
|
||||
const outputSchema = this.getToolOutputSchema(params.name);
|
||||
if (outputSchema) {
|
||||
const validator = this.getToolOutputValidator(params.name);
|
||||
if (validator) {
|
||||
// If tool has outputSchema, it MUST return structuredContent (unless it's an error)
|
||||
if (!result.structuredContent && !result.isError) {
|
||||
throw new McpError(
|
||||
|
|
@ -440,12 +441,12 @@ export class Client<
|
|||
if (result.structuredContent) {
|
||||
try {
|
||||
// Validate the structured content (which is already an object) against the schema
|
||||
const validationResult = outputSchema.safeParse(result.structuredContent);
|
||||
const isValid = validator(result.structuredContent);
|
||||
|
||||
if (!validationResult.success) {
|
||||
if (!isValid) {
|
||||
throw new McpError(
|
||||
ErrorCode.InvalidParams,
|
||||
`Structured content does not match the tool's output schema: ${validationResult.error.message}`
|
||||
`Structured content does not match the tool's output schema: ${this._ajv.errorsText(validator.errors)}`
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
|
|
@ -458,41 +459,29 @@ export class Client<
|
|||
);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// If tool doesn't have outputSchema, it MUST NOT return structuredContent
|
||||
if (result.structuredContent) {
|
||||
throw new McpError(
|
||||
ErrorCode.InvalidRequest,
|
||||
`Tool without outputSchema cannot return structuredContent`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private cacheToolOutputSchemas(tools: Tool[]) {
|
||||
this._cachedToolOutputSchemas.clear();
|
||||
this._cachedToolOutputValidators.clear();
|
||||
|
||||
for (const tool of tools) {
|
||||
// If the tool has an outputSchema, create and cache the Zod schema
|
||||
// If the tool has an outputSchema, create and cache the Ajv validator
|
||||
if (tool.outputSchema) {
|
||||
try {
|
||||
const zodSchemaCode = parseSchema(tool.outputSchema as JsonSchema);
|
||||
// The library returns a string of Zod code, we need to evaluate it
|
||||
// Using Function constructor to safely evaluate the Zod schema
|
||||
const createSchema = new Function('z', `return ${zodSchemaCode}`);
|
||||
const zodSchema = createSchema(z);
|
||||
this._cachedToolOutputSchemas.set(tool.name, zodSchema);
|
||||
const validator = this._ajv.compile(tool.outputSchema);
|
||||
this._cachedToolOutputValidators.set(tool.name, validator);
|
||||
} catch (error) {
|
||||
console.warn(`Failed to create Zod schema for tool ${tool.name}: ${error}`);
|
||||
console.warn(`Failed to compile output schema for tool ${tool.name}: ${error}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private getToolOutputSchema(toolName: string): z.ZodTypeAny | undefined {
|
||||
return this._cachedToolOutputSchemas.get(toolName);
|
||||
private getToolOutputValidator(toolName: string): ValidateFunction | undefined {
|
||||
return this._cachedToolOutputValidators.get(toolName);
|
||||
}
|
||||
|
||||
async listTools(
|
||||
|
|
|
|||
Loading…
Reference in a new issue