diff options
| author | Taylor Mullen <[email protected]> | 2025-05-16 22:09:24 -0700 |
|---|---|---|
| committer | N. Taylor Mullen <[email protected]> | 2025-05-16 22:14:51 -0700 |
| commit | e0b88dc8da3667bf4ba172a334a176d08fb0129d (patch) | |
| tree | 8fec045eb4277a7b1361aca45f0c8cbc1925f690 /packages/server/src | |
| parent | 0e25fdd56e99aaa51f805a075e45634cb7f2c02a (diff) | |
feat: Strip schema props from MCP tool definitions
- This change modifies the tool discovery process for MCP (Model Context Protocol) tools.
- When tools are fetched from an MCP server, the `additionalProperties` and `$schema` fields are now recursively removed from their input schemas. This ensures cleaner and more concise tool definitions within the CLI, aligning with the expected schema structure and preventing potential conflicts or verbose outputs.
- The corresponding tests in `tool-registry.test.ts` have been updated to reflect this new behavior and verify the correct stripping of these properties.
Workaround for https://github.com/google-gemini/gemini-cli/issues/398
Diffstat (limited to 'packages/server/src')
| -rw-r--r-- | packages/server/src/tools/tool-registry.test.ts | 41 | ||||
| -rw-r--r-- | packages/server/src/tools/tool-registry.ts | 16 |
2 files changed, 49 insertions, 8 deletions
diff --git a/packages/server/src/tools/tool-registry.test.ts b/packages/server/src/tools/tool-registry.test.ts index 2907798c..d43eaff9 100644 --- a/packages/server/src/tools/tool-registry.test.ts +++ b/packages/server/src/tools/tool-registry.test.ts @@ -263,7 +263,7 @@ describe('ToolRegistry', () => { ); }); - it('should discover tools using MCP servers defined in getMcpServers', async () => { + it('should discover tools using MCP servers defined in getMcpServers and strip schema properties', async () => { mockConfigGetToolDiscoveryCommand.mockReturnValue(undefined); // No regular discovery mockConfigGetMcpServerCommand.mockReturnValue(undefined); // No command-based MCP mockConfigGetMcpServers.mockReturnValue({ @@ -279,16 +279,26 @@ describe('ToolRegistry', () => { { name: 'mcp-tool-1', description: 'An MCP tool', - inputSchema: { type: 'object' }, - }, // Corrected: Add type: 'object' + inputSchema: { + type: 'object', + properties: { + param1: { type: 'string', $schema: 'remove-me' }, + param2: { + type: 'object', + additionalProperties: false, + properties: { + nested: { type: 'number' }, + }, + }, + }, + additionalProperties: true, + $schema: 'http://json-schema.org/draft-07/schema#', + }, + }, ], }); mockMcpClientInstance.connect.mockResolvedValue(undefined); - // discoverTools has an async IIFE, so we need to wait for it. - // A simple way is to use a short timeout, but a more robust way would be to - // have discoverTools return a promise that resolves when all async operations are done. - // For now, using a timeout. toolRegistry.discoverTools(); await new Promise((resolve) => setTimeout(resolve, 100)); // Wait for async operations @@ -302,11 +312,26 @@ describe('ToolRegistry', () => { expect(mockMcpClientInstance.connect).toHaveBeenCalled(); expect(mockMcpClientInstance.listTools).toHaveBeenCalled(); - const discoveredTool = toolRegistry.getTool('mcp-tool-1'); // Name is not prefixed if only one MCP server from getMcpServers + const discoveredTool = toolRegistry.getTool('mcp-tool-1'); expect(discoveredTool).toBeInstanceOf(DiscoveredMCPTool); expect(discoveredTool?.name).toBe('mcp-tool-1'); expect(discoveredTool?.description).toContain('An MCP tool'); expect(discoveredTool?.description).toContain('mcp-tool-1'); + + // Verify that $schema and additionalProperties are removed + const cleanedSchema = discoveredTool?.schema.parameters; + expect(cleanedSchema).not.toHaveProperty('$schema'); + expect(cleanedSchema).not.toHaveProperty('additionalProperties'); + expect(cleanedSchema?.properties?.param1).not.toHaveProperty('$schema'); + expect(cleanedSchema?.properties?.param2).not.toHaveProperty( + 'additionalProperties', + ); + expect( + cleanedSchema?.properties?.param2?.properties?.nested, + ).not.toHaveProperty('$schema'); + expect( + cleanedSchema?.properties?.param2?.properties?.nested, + ).not.toHaveProperty('additionalProperties'); }); it('should discover tools using MCP server command from getMcpServerCommand', async () => { diff --git a/packages/server/src/tools/tool-registry.ts b/packages/server/src/tools/tool-registry.ts index 6a737482..7bc04432 100644 --- a/packages/server/src/tools/tool-registry.ts +++ b/packages/server/src/tools/tool-registry.ts @@ -226,6 +226,22 @@ export class ToolRegistry { }); const result = await mcpClient.listTools(); for (const tool of result.tools) { + // Recursively remove additionalProperties and $schema from the inputSchema + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- This function recursively navigates a deeply nested and potentially heterogeneous JSON schema object. Using 'any' is a pragmatic choice here to avoid overly complex type definitions for all possible schema variations. + const removeSchemaProps = (obj: any) => { + if (typeof obj !== 'object' || obj === null) { + return; + } + if (Array.isArray(obj)) { + obj.forEach(removeSchemaProps); + } else { + delete obj.additionalProperties; + delete obj.$schema; + Object.values(obj).forEach(removeSchemaProps); + } + }; + removeSchemaProps(tool.inputSchema); + this.registerTool( new DiscoveredMCPTool( mcpClient, |
