summaryrefslogtreecommitdiff
path: root/packages/core/src
diff options
context:
space:
mode:
Diffstat (limited to 'packages/core/src')
-rw-r--r--packages/core/src/tools/mcp-client.test.ts336
-rw-r--r--packages/core/src/tools/mcp-client.ts68
2 files changed, 404 insertions, 0 deletions
diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts
index 9997d60e..1ccba76a 100644
--- a/packages/core/src/tools/mcp-client.test.ts
+++ b/packages/core/src/tools/mcp-client.test.ts
@@ -12,6 +12,7 @@ import {
isEnabled,
discoverTools,
discoverPrompts,
+ hasValidTypes,
} from './mcp-client.js';
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
import * as SdkClientStdioLib from '@modelcontextprotocol/sdk/client/stdio.js';
@@ -97,6 +98,182 @@ describe('mcp-client', () => {
`Error discovering tool: 'invalid tool name' from MCP server 'test-server': ${testError.message}`,
);
});
+
+ it('should skip tools if a parameter is missing a type', async () => {
+ const mockedClient = {} as unknown as ClientLib.Client;
+ const consoleWarnSpy = vi
+ .spyOn(console, 'warn')
+ .mockImplementation(() => {});
+ vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
+ tool: () =>
+ Promise.resolve({
+ functionDeclarations: [
+ {
+ name: 'validTool',
+ parametersJsonSchema: {
+ type: 'object',
+ properties: {
+ param1: { type: 'string' },
+ },
+ },
+ },
+ {
+ name: 'invalidTool',
+ parametersJsonSchema: {
+ type: 'object',
+ properties: {
+ param1: { description: 'a param with no type' },
+ },
+ },
+ },
+ ],
+ }),
+ } as unknown as GenAiLib.CallableTool);
+
+ const tools = await discoverTools('test-server', {}, mockedClient);
+
+ expect(tools.length).toBe(1);
+ expect(vi.mocked(DiscoveredMCPTool).mock.calls[0][2]).toBe('validTool');
+ expect(consoleWarnSpy).toHaveBeenCalledOnce();
+ expect(consoleWarnSpy).toHaveBeenCalledWith(
+ `Skipping tool 'invalidTool' from MCP server 'test-server' because it has ` +
+ `missing types in its parameter schema. Please file an issue with the owner of the MCP server.`,
+ );
+ consoleWarnSpy.mockRestore();
+ });
+
+ it('should skip tools if a nested parameter is missing a type', async () => {
+ const mockedClient = {} as unknown as ClientLib.Client;
+ const consoleWarnSpy = vi
+ .spyOn(console, 'warn')
+ .mockImplementation(() => {});
+ vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
+ tool: () =>
+ Promise.resolve({
+ functionDeclarations: [
+ {
+ name: 'invalidTool',
+ parametersJsonSchema: {
+ type: 'object',
+ properties: {
+ param1: {
+ type: 'object',
+ properties: {
+ nestedParam: {
+ description: 'a nested param with no type',
+ },
+ },
+ },
+ },
+ },
+ },
+ ],
+ }),
+ } as unknown as GenAiLib.CallableTool);
+
+ const tools = await discoverTools('test-server', {}, mockedClient);
+
+ expect(tools.length).toBe(0);
+ expect(consoleWarnSpy).toHaveBeenCalledOnce();
+ expect(consoleWarnSpy).toHaveBeenCalledWith(
+ `Skipping tool 'invalidTool' from MCP server 'test-server' because it has ` +
+ `missing types in its parameter schema. Please file an issue with the owner of the MCP server.`,
+ );
+ consoleWarnSpy.mockRestore();
+ });
+
+ it('should skip tool if an array item is missing a type', async () => {
+ const mockedClient = {} as unknown as ClientLib.Client;
+ const consoleWarnSpy = vi
+ .spyOn(console, 'warn')
+ .mockImplementation(() => {});
+ vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
+ tool: () =>
+ Promise.resolve({
+ functionDeclarations: [
+ {
+ name: 'invalidTool',
+ parametersJsonSchema: {
+ type: 'object',
+ properties: {
+ param1: {
+ type: 'array',
+ items: {
+ description: 'an array item with no type',
+ },
+ },
+ },
+ },
+ },
+ ],
+ }),
+ } as unknown as GenAiLib.CallableTool);
+
+ const tools = await discoverTools('test-server', {}, mockedClient);
+
+ expect(tools.length).toBe(0);
+ expect(consoleWarnSpy).toHaveBeenCalledOnce();
+ expect(consoleWarnSpy).toHaveBeenCalledWith(
+ `Skipping tool 'invalidTool' from MCP server 'test-server' because it has ` +
+ `missing types in its parameter schema. Please file an issue with the owner of the MCP server.`,
+ );
+ consoleWarnSpy.mockRestore();
+ });
+
+ it('should discover tool with no properties in schema', async () => {
+ const mockedClient = {} as unknown as ClientLib.Client;
+ const consoleWarnSpy = vi
+ .spyOn(console, 'warn')
+ .mockImplementation(() => {});
+ vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
+ tool: () =>
+ Promise.resolve({
+ functionDeclarations: [
+ {
+ name: 'validTool',
+ parametersJsonSchema: {
+ type: 'object',
+ },
+ },
+ ],
+ }),
+ } as unknown as GenAiLib.CallableTool);
+
+ const tools = await discoverTools('test-server', {}, mockedClient);
+
+ expect(tools.length).toBe(1);
+ expect(vi.mocked(DiscoveredMCPTool).mock.calls[0][2]).toBe('validTool');
+ expect(consoleWarnSpy).not.toHaveBeenCalled();
+ consoleWarnSpy.mockRestore();
+ });
+
+ it('should discover tool with empty properties object in schema', async () => {
+ const mockedClient = {} as unknown as ClientLib.Client;
+ const consoleWarnSpy = vi
+ .spyOn(console, 'warn')
+ .mockImplementation(() => {});
+ vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
+ tool: () =>
+ Promise.resolve({
+ functionDeclarations: [
+ {
+ name: 'validTool',
+ parametersJsonSchema: {
+ type: 'object',
+ properties: {},
+ },
+ },
+ ],
+ }),
+ } as unknown as GenAiLib.CallableTool);
+
+ const tools = await discoverTools('test-server', {}, mockedClient);
+
+ expect(tools.length).toBe(1);
+ expect(vi.mocked(DiscoveredMCPTool).mock.calls[0][2]).toBe('validTool');
+ expect(consoleWarnSpy).not.toHaveBeenCalled();
+ consoleWarnSpy.mockRestore();
+ });
});
describe('discoverPrompts', () => {
@@ -433,4 +610,163 @@ describe('mcp-client', () => {
);
});
});
+
+ describe('hasValidTypes', () => {
+ it('should return true for a valid schema with anyOf', () => {
+ const schema = {
+ anyOf: [{ type: 'string' }, { type: 'number' }],
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return false for an invalid schema with anyOf', () => {
+ const schema = {
+ anyOf: [{ type: 'string' }, { description: 'no type' }],
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return true for a valid schema with allOf', () => {
+ const schema = {
+ allOf: [
+ { type: 'string' },
+ { type: 'object', properties: { foo: { type: 'string' } } },
+ ],
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return false for an invalid schema with allOf', () => {
+ const schema = {
+ allOf: [{ type: 'string' }, { description: 'no type' }],
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return true for a valid schema with oneOf', () => {
+ const schema = {
+ oneOf: [{ type: 'string' }, { type: 'number' }],
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return false for an invalid schema with oneOf', () => {
+ const schema = {
+ oneOf: [{ type: 'string' }, { description: 'no type' }],
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return true for a valid schema with nested subschemas', () => {
+ const schema = {
+ anyOf: [
+ { type: 'string' },
+ {
+ allOf: [
+ { type: 'object', properties: { a: { type: 'string' } } },
+ { type: 'object', properties: { b: { type: 'number' } } },
+ ],
+ },
+ ],
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return false for an invalid schema with nested subschemas', () => {
+ const schema = {
+ anyOf: [
+ { type: 'string' },
+ {
+ allOf: [
+ { type: 'object', properties: { a: { type: 'string' } } },
+ { description: 'no type' },
+ ],
+ },
+ ],
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return true for a schema with a type and subschemas', () => {
+ const schema = {
+ type: 'string',
+ anyOf: [{ minLength: 1 }, { maxLength: 5 }],
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return false for a schema with no type and no subschemas', () => {
+ const schema = {
+ description: 'a schema with no type',
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return true for a valid schema', () => {
+ const schema = {
+ type: 'object',
+ properties: {
+ param1: { type: 'string' },
+ },
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return false if a parameter is missing a type', () => {
+ const schema = {
+ type: 'object',
+ properties: {
+ param1: { description: 'a param with no type' },
+ },
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return false if a nested parameter is missing a type', () => {
+ const schema = {
+ type: 'object',
+ properties: {
+ param1: {
+ type: 'object',
+ properties: {
+ nestedParam: {
+ description: 'a nested param with no type',
+ },
+ },
+ },
+ },
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return false if an array item is missing a type', () => {
+ const schema = {
+ type: 'object',
+ properties: {
+ param1: {
+ type: 'array',
+ items: {
+ description: 'an array item with no type',
+ },
+ },
+ },
+ };
+ expect(hasValidTypes(schema)).toBe(false);
+ });
+
+ it('should return true for a schema with no properties', () => {
+ const schema = {
+ type: 'object',
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+
+ it('should return true for a schema with an empty properties object', () => {
+ const schema = {
+ type: 'object',
+ properties: {},
+ };
+ expect(hasValidTypes(schema)).toBe(true);
+ });
+ });
});
diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts
index 26244d9e..9a35b84e 100644
--- a/packages/core/src/tools/mcp-client.ts
+++ b/packages/core/src/tools/mcp-client.ts
@@ -417,6 +417,65 @@ export async function connectAndDiscover(
}
/**
+ * Recursively validates that a JSON schema and all its nested properties and
+ * items have a `type` defined.
+ *
+ * @param schema The JSON schema to validate.
+ * @returns `true` if the schema is valid, `false` otherwise.
+ *
+ * @visiblefortesting
+ */
+export function hasValidTypes(schema: unknown): boolean {
+ if (typeof schema !== 'object' || schema === null) {
+ // Not a schema object we can validate, or not a schema at all.
+ // Treat as valid as it has no properties to be invalid.
+ return true;
+ }
+
+ const s = schema as Record<string, unknown>;
+
+ if (!s.type) {
+ // These keywords contain an array of schemas that should be validated.
+ //
+ // If no top level type was given, then they must each have a type.
+ let hasSubSchema = false;
+ const schemaArrayKeywords = ['anyOf', 'allOf', 'oneOf'];
+ for (const keyword of schemaArrayKeywords) {
+ const subSchemas = s[keyword];
+ if (Array.isArray(subSchemas)) {
+ hasSubSchema = true;
+ for (const subSchema of subSchemas) {
+ if (!hasValidTypes(subSchema)) {
+ return false;
+ }
+ }
+ }
+ }
+
+ // If the node itself is missing a type and had no subschemas, then it isn't valid.
+ if (!hasSubSchema) return false;
+ }
+
+ if (s.type === 'object' && s.properties) {
+ if (typeof s.properties === 'object' && s.properties !== null) {
+ for (const prop of Object.values(s.properties)) {
+ if (!hasValidTypes(prop)) {
+ return false;
+ }
+ }
+ }
+ }
+
+ if (s.type === 'array' && s.items) {
+ if (!hasValidTypes(s.items)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+/**
* Discovers and sanitizes tools from a connected MCP client.
* It retrieves function declarations from the client, filters out disabled tools,
* generates valid names for them, and wraps them in `DiscoveredMCPTool` instances.
@@ -448,6 +507,15 @@ export async function discoverTools(
continue;
}
+ if (!hasValidTypes(funcDecl.parametersJsonSchema)) {
+ console.warn(
+ `Skipping tool '${funcDecl.name}' from MCP server '${mcpServerName}' ` +
+ `because it has missing types in its parameter schema. Please file an ` +
+ `issue with the owner of the MCP server.`,
+ );
+ continue;
+ }
+
discoveredTools.push(
new DiscoveredMCPTool(
mcpCallableTool,
generated by cgit v1.2.3 (git 2.51.0) at 2025-11-12 09:02:24 +0000