Skip to content

Commit 7ff6b09

Browse files
committed
address comments: 1. validate schemas w/o codegen, 2. relax no-structured-output restriction
1 parent 0fcb0cb commit 7ff6b09

File tree

4 files changed

+106
-119
lines changed

4 files changed

+106
-119
lines changed

package-lock.json

Lines changed: 89 additions & 24 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@
4646
"client": "tsx src/cli.ts client"
4747
},
4848
"dependencies": {
49+
"ajv": "^8.17.1",
4950
"content-type": "^1.0.5",
5051
"cors": "^2.8.5",
5152
"cross-spawn": "^7.0.3",
5253
"eventsource": "^3.0.2",
5354
"express": "^5.0.1",
5455
"express-rate-limit": "^7.5.0",
55-
"json-schema-to-zod": "^2.6.1",
5656
"pkce-challenge": "^5.0.0",
5757
"raw-body": "^3.0.0",
5858
"zod": "^3.23.8",

src/client/index.test.ts

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,72 +1261,5 @@ describe('outputSchema validation', () => {
12611261
);
12621262
});
12631263

1264-
/***
1265-
* Test: Throw Error when Tool Without outputSchema Returns structuredContent
1266-
*/
1267-
test('should throw error when tool without outputSchema returns structuredContent', async () => {
1268-
const server = new Server({
1269-
name: 'test-server',
1270-
version: '1.0.0',
1271-
}, {
1272-
capabilities: {
1273-
tools: {},
1274-
},
1275-
});
1276-
1277-
// Set up server handlers
1278-
server.setRequestHandler(InitializeRequestSchema, async (request) => ({
1279-
protocolVersion: request.params.protocolVersion,
1280-
capabilities: {},
1281-
serverInfo: {
1282-
name: 'test-server',
1283-
version: '1.0.0',
1284-
}
1285-
}));
1286-
1287-
server.setRequestHandler(ListToolsRequestSchema, async () => ({
1288-
tools: [
1289-
{
1290-
name: 'test-tool',
1291-
description: 'A test tool',
1292-
inputSchema: {
1293-
type: 'object',
1294-
properties: {},
1295-
},
1296-
// No outputSchema defined
1297-
},
1298-
],
1299-
}));
1300-
1301-
server.setRequestHandler(CallToolRequestSchema, async (request) => {
1302-
if (request.params.name === 'test-tool') {
1303-
// Incorrectly return structuredContent for a tool without outputSchema
1304-
return {
1305-
structuredContent: { result: 'This should not be allowed' },
1306-
};
1307-
}
1308-
throw new Error('Unknown tool');
1309-
});
1310-
1311-
const client = new Client({
1312-
name: 'test-client',
1313-
version: '1.0.0',
1314-
});
1315-
1316-
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
1317-
1318-
await Promise.all([
1319-
client.connect(clientTransport),
1320-
server.connect(serverTransport),
1321-
]);
1322-
1323-
// List tools to cache the schemas
1324-
await client.listTools();
1325-
1326-
// Call the tool - should throw validation error
1327-
await expect(client.callTool({ name: 'test-tool' })).rejects.toThrow(
1328-
/Tool without outputSchema cannot return structuredContent/
1329-
);
1330-
});
13311264

13321265
});

src/client/index.ts

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ import {
4343
ErrorCode,
4444
McpError,
4545
} from "../types.js";
46-
import { z } from "zod";
47-
import { JsonSchema, parseSchema } from "json-schema-to-zod";
46+
import { Ajv, type ValidateFunction } from "ajv";
4847

4948
export type ClientOptions = ProtocolOptions & {
5049
/**
@@ -91,7 +90,8 @@ export class Client<
9190
private _serverVersion?: Implementation;
9291
private _capabilities: ClientCapabilities;
9392
private _instructions?: string;
94-
private _cachedToolOutputSchemas: Map<string, z.ZodTypeAny> = new Map();
93+
private _cachedToolOutputValidators: Map<string, ValidateFunction> = new Map();
94+
private _ajv: InstanceType<typeof Ajv>;
9595

9696
/**
9797
* Initializes this client with the given name and version information.
@@ -102,6 +102,7 @@ export class Client<
102102
) {
103103
super(options);
104104
this._capabilities = options?.capabilities ?? {};
105+
this._ajv = new Ajv({ strict: false, validateFormats: true });
105106
}
106107

107108
/**
@@ -426,8 +427,8 @@ export class Client<
426427
);
427428

428429
// Check if the tool has an outputSchema
429-
const outputSchema = this.getToolOutputSchema(params.name);
430-
if (outputSchema) {
430+
const validator = this.getToolOutputValidator(params.name);
431+
if (validator) {
431432
// If tool has outputSchema, it MUST return structuredContent (unless it's an error)
432433
if (!result.structuredContent && !result.isError) {
433434
throw new McpError(
@@ -440,12 +441,12 @@ export class Client<
440441
if (result.structuredContent) {
441442
try {
442443
// Validate the structured content (which is already an object) against the schema
443-
const validationResult = outputSchema.safeParse(result.structuredContent);
444+
const isValid = validator(result.structuredContent);
444445

445-
if (!validationResult.success) {
446+
if (!isValid) {
446447
throw new McpError(
447448
ErrorCode.InvalidParams,
448-
`Structured content does not match the tool's output schema: ${validationResult.error.message}`
449+
`Structured content does not match the tool's output schema: ${this._ajv.errorsText(validator.errors)}`
449450
);
450451
}
451452
} catch (error) {
@@ -458,41 +459,29 @@ export class Client<
458459
);
459460
}
460461
}
461-
} else {
462-
// If tool doesn't have outputSchema, it MUST NOT return structuredContent
463-
if (result.structuredContent) {
464-
throw new McpError(
465-
ErrorCode.InvalidRequest,
466-
`Tool without outputSchema cannot return structuredContent`
467-
);
468-
}
469462
}
470463

471464
return result;
472465
}
473466

474467
private cacheToolOutputSchemas(tools: Tool[]) {
475-
this._cachedToolOutputSchemas.clear();
468+
this._cachedToolOutputValidators.clear();
476469

477470
for (const tool of tools) {
478-
// If the tool has an outputSchema, create and cache the Zod schema
471+
// If the tool has an outputSchema, create and cache the Ajv validator
479472
if (tool.outputSchema) {
480473
try {
481-
const zodSchemaCode = parseSchema(tool.outputSchema as JsonSchema);
482-
// The library returns a string of Zod code, we need to evaluate it
483-
// Using Function constructor to safely evaluate the Zod schema
484-
const createSchema = new Function('z', `return ${zodSchemaCode}`);
485-
const zodSchema = createSchema(z);
486-
this._cachedToolOutputSchemas.set(tool.name, zodSchema);
474+
const validator = this._ajv.compile(tool.outputSchema);
475+
this._cachedToolOutputValidators.set(tool.name, validator);
487476
} catch (error) {
488-
console.warn(`Failed to create Zod schema for tool ${tool.name}: ${error}`);
477+
console.warn(`Failed to compile output schema for tool ${tool.name}: ${error}`);
489478
}
490479
}
491480
}
492481
}
493482

494-
private getToolOutputSchema(toolName: string): z.ZodTypeAny | undefined {
495-
return this._cachedToolOutputSchemas.get(toolName);
483+
private getToolOutputValidator(toolName: string): ValidateFunction | undefined {
484+
return this._cachedToolOutputValidators.get(toolName);
496485
}
497486

498487
async listTools(

0 commit comments

Comments
 (0)