From 3bf8e32b8c52bdba51cc2cffdaf468dc2762a38b Mon Sep 17 00:00:00 2001 From: JamesDAdams Date: Wed, 2 Sep 2026 15:22:08 +0200 Subject: [PATCH 1/4] fix(llm): sanitize tool schemas and fix invalid built-in tool definitions --- src/server/llm/client-pure.test.ts | 32 ++++++++- src/server/llm/client-pure.ts | 3 +- src/server/llm/schema-sanitizer.test.ts | 89 ++++++++++++++++++++++++ src/server/llm/schema-sanitizer.ts | 91 +++++++++++++++++++++++++ src/server/tools/ask.ts | 19 ++---- src/server/tools/project-tasks.ts | 10 ++- 6 files changed, 227 insertions(+), 17 deletions(-) create mode 100644 src/server/llm/schema-sanitizer.test.ts create mode 100644 src/server/llm/schema-sanitizer.ts diff --git a/src/server/llm/client-pure.test.ts b/src/server/llm/client-pure.test.ts index 8727ef38..e3b9bb23 100644 --- a/src/server/llm/client-pure.test.ts +++ b/src/server/llm/client-pure.test.ts @@ -377,12 +377,38 @@ describe('llm client pure helpers', () => { expect(withoutFlag).toEqual([{ role: 'user', content: 'hi' }]) }) - it('converts tool definitions to openai function schema', () => { + it('converts tool definitions to openai function schema and sanitizes invalid schema fields', () => { expect( convertTools([ - { type: 'function', function: { name: 'grep', description: 'Search', parameters: { type: 'object' } } }, + { + type: 'function', + function: { + name: 'grep', + description: 'Search', + parameters: { + type: 'object', + properties: { + tags: { type: 'array', items: {} }, + }, + }, + }, + }, ]), - ).toEqual([{ type: 'function', function: { name: 'grep', description: 'Search', parameters: { type: 'object' } } }]) + ).toEqual([ + { + type: 'function', + function: { + name: 'grep', + description: 'Search', + parameters: { + type: 'object', + properties: { + tags: { type: 'array', items: { type: 'string' } }, + }, + }, + }, + }, + ]) }) it('maps finish reasons', () => { diff --git a/src/server/llm/client-pure.ts b/src/server/llm/client-pure.ts index 8e7a6637..403d856c 100644 --- a/src/server/llm/client-pure.ts +++ b/src/server/llm/client-pure.ts @@ -22,6 +22,7 @@ import { extractPdfBlocksFromDataUrl, formatVisionFallbackDescription, } from './resolve-attachments.js' +import { sanitizeToolSchema } from './schema-sanitizer.js' import type { ContentPart } from './resolve-attachments.js' export { resolveAttachmentsInMessages } from './resolve-attachments.js' @@ -255,7 +256,7 @@ export function convertTools(tools: LLMToolDefinition[]): ChatCompletionTool[] { function: { name: tool.function.name, description: tool.function.description, - parameters: tool.function.parameters, + parameters: sanitizeToolSchema(tool.function.parameters), }, })) } diff --git a/src/server/llm/schema-sanitizer.test.ts b/src/server/llm/schema-sanitizer.test.ts new file mode 100644 index 00000000..945ad5f1 --- /dev/null +++ b/src/server/llm/schema-sanitizer.test.ts @@ -0,0 +1,89 @@ +import { describe, it, expect } from 'vitest' +import { sanitizeToolSchema } from './schema-sanitizer.js' + +describe('sanitizeToolSchema', () => { + it('returns valid default schema for non-object inputs', () => { + expect(sanitizeToolSchema(null)).toEqual({ type: 'object', properties: {} }) + expect(sanitizeToolSchema(undefined)).toEqual({ type: 'object', properties: {} }) + expect(sanitizeToolSchema('string')).toEqual({ type: 'object', properties: {} }) + expect(sanitizeToolSchema([])).toEqual({ type: 'object', properties: {} }) + }) + + it('strips unsupported JSON schema keywords', () => { + const input = { + $schema: 'http://json-schema.org/draft-07/schema#', + $id: 'http://example.com/schema.json', + type: 'object', + properties: { + name: { type: 'string', patternProperties: {} }, + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + name: { type: 'string' }, + }, + }) + }) + + it('converts const to enum', () => { + const input = { + type: 'object', + properties: { + mode: { const: 'exact' }, + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + mode: { enum: ['exact'] }, + }, + }) + }) + + it('normalizes empty or missing array items to { type: "string" }', () => { + const input = { + type: 'object', + properties: { + tags: { type: 'array', items: {} }, + labels: { type: 'array' }, + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + tags: { type: 'array', items: { type: 'string' } }, + labels: { type: 'array', items: { type: 'string' } }, + }, + }) + }) + + it('recursively sanitizes nested properties and oneOf/anyOf branches', () => { + const input = { + type: 'object', + properties: { + config: { + type: 'object', + properties: { + subOptions: { + oneOf: [{ const: 'auto' }, { type: 'array', items: {} }], + }, + }, + }, + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + config: { + type: 'object', + properties: { + subOptions: { + oneOf: [{ enum: ['auto'] }, { type: 'array', items: { type: 'string' } }], + }, + }, + }, + }, + }) + }) +}) diff --git a/src/server/llm/schema-sanitizer.ts b/src/server/llm/schema-sanitizer.ts new file mode 100644 index 00000000..332311b4 --- /dev/null +++ b/src/server/llm/schema-sanitizer.ts @@ -0,0 +1,91 @@ +/** + * Schema Sanitizer + * + * Sanitizes JSON Schema tool parameters for broad LLM provider compatibility + * (OpenAI, Gemini/Vertex AI/Antigravity, Anthropic, Ollama, etc.). + * + * Normalizes invalid or unsupported constructs: + * - Empty or missing `items` in array schemas (`items: {}` -> `items: { type: 'string' }`) + * - Strips meta keywords ($schema, $id, $vocabulary, etc.) + * - Converts `const` values to `enum: [val]` + * - Recursively processes properties, array items, and union branches (anyOf, oneOf, allOf) + */ + +export function sanitizeToolSchema(schema: unknown): Record { + if (!schema || typeof schema !== 'object' || Array.isArray(schema)) { + return { type: 'object', properties: {} } + } + + return cleanSchemaNode(schema as Record) +} + +function cleanSchemaNode(node: Record): Record { + const result: Record = {} + + for (const [key, val] of Object.entries(node)) { + // Strip meta keywords unsupported across providers + if ( + key === '$schema' || + key === '$id' || + key === '$vocabulary' || + key === '$anchor' || + key === 'dependentRequired' || + key === 'dependentSchemas' || + key === 'unevaluatedProperties' || + key === 'unevaluatedItems' || + key === 'patternProperties' + ) { + continue + } + + if (key === 'const') { + result['enum'] = [val] + continue + } + + if (key === 'properties' && val && typeof val === 'object' && !Array.isArray(val)) { + const sanitizedProps: Record = {} + for (const [propKey, propVal] of Object.entries(val as Record)) { + if (propVal && typeof propVal === 'object' && !Array.isArray(propVal)) { + sanitizedProps[propKey] = cleanSchemaNode(propVal as Record) + } else { + sanitizedProps[propKey] = propVal + } + } + result['properties'] = sanitizedProps + continue + } + + if (key === 'items') { + if (!val || typeof val !== 'object' || Object.keys(val).length === 0) { + result['items'] = { type: 'string' } + } else if (Array.isArray(val)) { + result['items'] = val.map((item) => + item && typeof item === 'object' ? cleanSchemaNode(item as Record) : item, + ) + } else { + result['items'] = cleanSchemaNode(val as Record) + } + continue + } + + if ((key === 'anyOf' || key === 'oneOf' || key === 'allOf') && Array.isArray(val)) { + result[key] = val.map((item) => + item && typeof item === 'object' ? cleanSchemaNode(item as Record) : item, + ) + continue + } + + result[key] = val + } + + // If type is array but items is missing or empty, supply a default string item schema + if ( + result['type'] === 'array' && + (!result['items'] || (typeof result['items'] === 'object' && Object.keys(result['items'] as object).length === 0)) + ) { + result['items'] = { type: 'string' } + } + + return result +} diff --git a/src/server/tools/ask.ts b/src/server/tools/ask.ts index 3e78737f..f61bad90 100644 --- a/src/server/tools/ask.ts +++ b/src/server/tools/ask.ts @@ -43,18 +43,13 @@ export const askUserTool: Tool = { description: 'Options for choice-type questions. Each entry may be a plain string or an object {value, label, description?} (or legacy {label, description?}). The server normalizes everything to {value, label, description?}.', items: { - oneOf: [ - { type: 'string' }, - { - type: 'object', - properties: { - value: { type: 'string' }, - label: { type: 'string' }, - description: { type: 'string' }, - }, - required: ['label'], - }, - ], + type: 'object', + properties: { + value: { type: 'string' }, + label: { type: 'string' }, + description: { type: 'string' }, + }, + required: ['label'], }, }, }, diff --git a/src/server/tools/project-tasks.ts b/src/server/tools/project-tasks.ts index 9a87d701..4de93bf8 100644 --- a/src/server/tools/project-tasks.ts +++ b/src/server/tools/project-tasks.ts @@ -94,7 +94,15 @@ export const projectTasksTool = createTool( attachments: { type: 'array', description: 'Optional attachments (same shape as chat attachments)', - items: {}, + items: { + type: 'object', + properties: { + id: { type: 'string', description: 'Attachment ID' }, + filename: { type: 'string', description: 'File name' }, + mimeType: { type: 'string', description: 'MIME type' }, + size: { type: 'number', description: 'File size in bytes' }, + }, + }, }, agentId: { type: 'string', description: 'Selected agent id' }, providerId: { type: 'string', description: 'Provider id used when a session is spawned' }, From cc08cbfb9f67f886c8b61cbf7158dbf1d1e911c3 Mon Sep 17 00:00:00 2001 From: JamesDAdams Date: Wed, 2 Sep 2026 15:35:20 +0200 Subject: [PATCH 2/4] fix(llm): strip additionalProperties and null defaults, and sanitize MCP tool schemas --- src/server/llm/client-pure.test.ts | 25 +++- src/server/llm/schema-sanitizer.test.ts | 112 +++++++++++++++++- src/server/llm/schema-sanitizer.ts | 23 +++- src/server/mcp/manager.test.ts | 21 ++++ src/server/mcp/tool-adapter.ts | 3 +- .../adapters/transport-client.test.ts | 61 ++++++++++ .../providers/adapters/transport-client.ts | 12 ++ 7 files changed, 247 insertions(+), 10 deletions(-) diff --git a/src/server/llm/client-pure.test.ts b/src/server/llm/client-pure.test.ts index e3b9bb23..53483080 100644 --- a/src/server/llm/client-pure.test.ts +++ b/src/server/llm/client-pure.test.ts @@ -614,7 +614,10 @@ describe('llm client pure helpers', () => { model: 'test-model', messages: [{ role: 'user', content: 'hello' }], tools: [ - { type: 'function', function: { name: 'glob', description: 'Search', parameters: { type: 'object' } } }, + { + type: 'function', + function: { name: 'glob', description: 'Search', parameters: { type: 'object', properties: {} } }, + }, ], tool_choice: 'auto', temperature: 0.2, @@ -655,7 +658,10 @@ describe('llm client pure helpers', () => { model: 'test-model', messages: [{ role: 'user', content: 'hello' }], tools: [ - { type: 'function', function: { name: 'glob', description: 'Search', parameters: { type: 'object' } } }, + { + type: 'function', + function: { name: 'glob', description: 'Search', parameters: { type: 'object', properties: {} } }, + }, ], tool_choice: 'auto', temperature: 0.2, @@ -696,7 +702,10 @@ describe('llm client pure helpers', () => { model: 'test-model', messages: [{ role: 'user', content: 'hello' }], tools: [ - { type: 'function', function: { name: 'glob', description: 'Search', parameters: { type: 'object' } } }, + { + type: 'function', + function: { name: 'glob', description: 'Search', parameters: { type: 'object', properties: {} } }, + }, ], tool_choice: 'auto', temperature: 0.2, @@ -737,7 +746,10 @@ describe('llm client pure helpers', () => { model: 'test-model', messages: [{ role: 'user', content: 'hello' }], tools: [ - { type: 'function', function: { name: 'glob', description: 'Search', parameters: { type: 'object' } } }, + { + type: 'function', + function: { name: 'glob', description: 'Search', parameters: { type: 'object', properties: {} } }, + }, ], tool_choice: 'auto', temperature: 0.2, @@ -779,7 +791,10 @@ describe('llm client pure helpers', () => { model: 'test-model', messages: [{ role: 'user', content: 'hello' }], tools: [ - { type: 'function', function: { name: 'glob', description: 'Search', parameters: { type: 'object' } } }, + { + type: 'function', + function: { name: 'glob', description: 'Search', parameters: { type: 'object', properties: {} } }, + }, ], tool_choice: 'auto', temperature: 0.2, diff --git a/src/server/llm/schema-sanitizer.test.ts b/src/server/llm/schema-sanitizer.test.ts index 945ad5f1..15a8e53f 100644 --- a/src/server/llm/schema-sanitizer.test.ts +++ b/src/server/llm/schema-sanitizer.test.ts @@ -9,11 +9,12 @@ describe('sanitizeToolSchema', () => { expect(sanitizeToolSchema([])).toEqual({ type: 'object', properties: {} }) }) - it('strips unsupported JSON schema keywords', () => { + it('strips unsupported JSON schema keywords and additionalProperties', () => { const input = { $schema: 'http://json-schema.org/draft-07/schema#', $id: 'http://example.com/schema.json', type: 'object', + additionalProperties: false, properties: { name: { type: 'string', patternProperties: {} }, }, @@ -26,6 +27,25 @@ describe('sanitizeToolSchema', () => { }) }) + it('strips null and undefined values such as default: null', () => { + const input = { + type: 'object', + properties: { + expand: { type: 'string', default: null }, + include: { type: 'string', default: null }, + count: { type: 'number', default: 10 }, + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + expand: { type: 'string' }, + include: { type: 'string' }, + count: { type: 'number', default: 10 }, + }, + }) + }) + it('converts const to enum', () => { const input = { type: 'object', @@ -58,6 +78,96 @@ describe('sanitizeToolSchema', () => { }) }) + it('ensures type: object always has a properties map', () => { + const input = { + type: 'object', + properties: { + env: { type: 'object' }, + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + env: { type: 'object', properties: {} }, + }, + }) + }) + + it('normalizes string property values in properties map', () => { + const input = { + type: 'object', + properties: { + env: 'object', + name: 'string', + }, + } + expect(sanitizeToolSchema(input)).toEqual({ + type: 'object', + properties: { + env: { type: 'object', properties: {} }, + name: { type: 'string' }, + }, + }) + }) + + it('sanitizes real Jira MCP tool schema containing additionalProperties and null defaults', () => { + const jiraSchema = { + type: 'object', + properties: { + issue_key: { + description: "Jira issue key (e.g., 'PROJ-123')", + type: 'string', + }, + expand: { + default: null, + description: '(Optional) Fields to expand', + type: 'string', + }, + properties: { + description: '(Optional) A comma-separated list of issue properties to return', + default: null, + type: 'string', + }, + comment_limit: { + default: 10, + maximum: 100, + minimum: 0, + type: 'integer', + }, + }, + required: ['issue_key'], + additionalProperties: false, + } + + const sanitized = sanitizeToolSchema(jiraSchema) + + expect(sanitized).toEqual({ + type: 'object', + properties: { + issue_key: { + description: "Jira issue key (e.g., 'PROJ-123')", + type: 'string', + }, + expand: { + description: '(Optional) Fields to expand', + type: 'string', + }, + properties: { + description: '(Optional) A comma-separated list of issue properties to return', + type: 'string', + }, + comment_limit: { + default: 10, + maximum: 100, + minimum: 0, + type: 'integer', + }, + }, + required: ['issue_key'], + }) + expect(sanitized).not.toHaveProperty('additionalProperties') + }) + it('recursively sanitizes nested properties and oneOf/anyOf branches', () => { const input = { type: 'object', diff --git a/src/server/llm/schema-sanitizer.ts b/src/server/llm/schema-sanitizer.ts index 332311b4..d6e81a17 100644 --- a/src/server/llm/schema-sanitizer.ts +++ b/src/server/llm/schema-sanitizer.ts @@ -5,6 +5,9 @@ * (OpenAI, Gemini/Vertex AI/Antigravity, Anthropic, Ollama, etc.). * * Normalizes invalid or unsupported constructs: + * - Strips `additionalProperties` (unsupported by Vertex AI / Antigravity protobuf Schema) + * - Strips `null` values (e.g. `default: null`) + * - Ensures `type: 'object'` always defines a valid `properties` map * - Empty or missing `items` in array schemas (`items: {}` -> `items: { type: 'string' }`) * - Strips meta keywords ($schema, $id, $vocabulary, etc.) * - Converts `const` values to `enum: [val]` @@ -23,7 +26,12 @@ function cleanSchemaNode(node: Record): Record const result: Record = {} for (const [key, val] of Object.entries(node)) { - // Strip meta keywords unsupported across providers + // Strip null / undefined values (e.g. default: null) + if (val === null || val === undefined) { + continue + } + + // Strip meta keywords & additionalProperties unsupported across providers if ( key === '$schema' || key === '$id' || @@ -33,7 +41,8 @@ function cleanSchemaNode(node: Record): Record key === 'dependentSchemas' || key === 'unevaluatedProperties' || key === 'unevaluatedItems' || - key === 'patternProperties' + key === 'patternProperties' || + key === 'additionalProperties' ) { continue } @@ -46,7 +55,10 @@ function cleanSchemaNode(node: Record): Record if (key === 'properties' && val && typeof val === 'object' && !Array.isArray(val)) { const sanitizedProps: Record = {} for (const [propKey, propVal] of Object.entries(val as Record)) { - if (propVal && typeof propVal === 'object' && !Array.isArray(propVal)) { + if (typeof propVal === 'string') { + sanitizedProps[propKey] = + propVal === 'object' ? { type: 'object', properties: {} } : { type: propVal } + } else if (propVal && typeof propVal === 'object' && !Array.isArray(propVal)) { sanitizedProps[propKey] = cleanSchemaNode(propVal as Record) } else { sanitizedProps[propKey] = propVal @@ -79,6 +91,11 @@ function cleanSchemaNode(node: Record): Record result[key] = val } + // If type is object but properties is missing, provide empty properties map + if (result['type'] === 'object' && !result['properties']) { + result['properties'] = {} + } + // If type is array but items is missing or empty, supply a default string item schema if ( result['type'] === 'array' && diff --git a/src/server/mcp/manager.test.ts b/src/server/mcp/manager.test.ts index 153a1b3f..86130f16 100644 --- a/src/server/mcp/manager.test.ts +++ b/src/server/mcp/manager.test.ts @@ -617,4 +617,25 @@ describe('McpManager token estimation', () => { expect(betaRe.status).toBe('connected') expect(betaRe.tools.length).toBe(2) }) + + it('creates MCP tools with sanitized parameters schema', async () => { + mockClientInstance.listTools.mockResolvedValueOnce({ + tools: [ + { + name: 'get_weather', + description: 'Get weather', + inputSchema: { type: 'object', properties: { location: { type: 'string' } } }, + }, + ], + }) + const manager = new McpManager() + await manager.addServer('test', { transport: 'stdio', command: 'node' }) + const tools = createMcpTools(manager) + expect(tools).toHaveLength(1) + expect(tools[0]!.name).toBe('test_get_weather') + expect(tools[0]!.definition.function.parameters).toEqual({ + type: 'object', + properties: { location: { type: 'string' } }, + }) + }) }) diff --git a/src/server/mcp/tool-adapter.ts b/src/server/mcp/tool-adapter.ts index 5b724d21..61b76c0c 100644 --- a/src/server/mcp/tool-adapter.ts +++ b/src/server/mcp/tool-adapter.ts @@ -2,6 +2,7 @@ import type { Tool, ToolContext } from '../tools/types.js' import type { LLMToolDefinition } from '../llm/types.js' import type { McpManager } from './manager.js' import type { ToolResult } from '../../shared/types.js' +import { sanitizeToolSchema } from '../llm/schema-sanitizer.js' export function createMcpTools(mcpManager: McpManager): Tool[] { const tools: Tool[] = [] @@ -17,7 +18,7 @@ export function createMcpTools(mcpManager: McpManager): Tool[] { function: { name: prefixedName, description: mcpTool.description ?? '', - parameters: mcpTool.inputSchema as Record, + parameters: sanitizeToolSchema(mcpTool.inputSchema as Record), }, } diff --git a/src/server/providers/adapters/transport-client.test.ts b/src/server/providers/adapters/transport-client.test.ts index a4c58a44..ca338d8f 100644 --- a/src/server/providers/adapters/transport-client.test.ts +++ b/src/server/providers/adapters/transport-client.test.ts @@ -368,6 +368,67 @@ describe('createTransportLLMClient', () => { expect(client.getReasoningEffort?.()).toBe('deep') }) + it('sanitizes tool schemas before passing to complete() and stream()', async () => { + const provider: Provider = { + id: 'copilot', + name: 'GitHub Copilot', + url: 'https://api.githubcopilot.com', + backend: 'openai', + models: [{ id: 'gpt-4o', contextWindow: 128000, source: 'backend' }], + isActive: true, + createdAt: new Date().toISOString(), + } + + let capturedTools: unknown[] = [] + const mockComplete = vi.fn(async (request: { tools?: unknown[] }) => { + capturedTools = request.tools ?? [] + return { + id: 'r1', + content: '', + toolCalls: [], + finishReason: 'stop' as const, + usage: { promptTokens: 0, completionTokens: 0, totalTokens: 0 }, + } + }) + const client = createTransportLLMClient(provider, 'gpt-4o', { ...transport, complete: mockComplete }) + + await client.complete({ + messages: [{ role: 'user', content: 'test' }], + tools: [ + { + type: 'function', + function: { + name: 'test_tool', + description: 'A test tool', + parameters: { + type: 'object', + additionalProperties: false, + properties: { + opt: { type: 'string', default: null }, + }, + }, + }, + }, + ], + }) + + expect(capturedTools).toEqual([ + { + type: 'function', + function: { + name: 'test_tool', + description: 'A test tool', + parameters: { + type: 'object', + properties: { + opt: { type: 'string' }, + }, + }, + }, + }, + ]) + }) + it('passes an in-list client effort through unchanged', () => { const provider: Provider = { id: 'openai', diff --git a/src/server/providers/adapters/transport-client.ts b/src/server/providers/adapters/transport-client.ts index 22feb442..42501e30 100644 --- a/src/server/providers/adapters/transport-client.ts +++ b/src/server/providers/adapters/transport-client.ts @@ -4,6 +4,7 @@ import { getModelProfile } from '../../llm/profiles.js' import type { LLMClientWithModel } from '../../llm/client.js' import type { ProviderTransportAdapter } from '../../../provider/index.js' import { resolveAttachmentsInMessages } from '../../llm/client-pure.js' +import { sanitizeToolSchema } from '../../llm/schema-sanitizer.js' import { resolveEffortForModel, resolveModeModelId } from '../../../shared/reasoning-effort.js' export function createTransportLLMClient( @@ -80,6 +81,17 @@ export function createTransportLLMClient( request: { ...request, messages: await resolveAttachmentsInMessages(request.messages, supportsVision), + ...(request.tools + ? { + tools: request.tools.map((t) => ({ + ...t, + function: { + ...t.function, + parameters: sanitizeToolSchema(t.function.parameters), + }, + })), + } + : {}), ...(resolvedEffort && !send.suppressEffort ? { reasoningEffort: resolvedEffort } : {}), }, effort: resolvedEffort, From 79499c60703aa034e47ab9a0aa7f3acadc08b3fb Mon Sep 17 00:00:00 2001 From: JamesDAdams Date: Wed, 2 Sep 2026 15:53:37 +0200 Subject: [PATCH 3/4] fix(mcp): alias properties parameter name to avoid schema collisions with LLM providers --- src/server/llm/schema-sanitizer.test.ts | 2 +- src/server/llm/schema-sanitizer.ts | 16 +++++++++++++--- src/server/mcp/manager.ts | 3 ++- src/server/mcp/tool-adapter.ts | 6 +++++- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/server/llm/schema-sanitizer.test.ts b/src/server/llm/schema-sanitizer.test.ts index 15a8e53f..f9cb91c7 100644 --- a/src/server/llm/schema-sanitizer.test.ts +++ b/src/server/llm/schema-sanitizer.test.ts @@ -152,7 +152,7 @@ describe('sanitizeToolSchema', () => { description: '(Optional) Fields to expand', type: 'string', }, - properties: { + props: { description: '(Optional) A comma-separated list of issue properties to return', type: 'string', }, diff --git a/src/server/llm/schema-sanitizer.ts b/src/server/llm/schema-sanitizer.ts index d6e81a17..45cade0b 100644 --- a/src/server/llm/schema-sanitizer.ts +++ b/src/server/llm/schema-sanitizer.ts @@ -47,6 +47,11 @@ function cleanSchemaNode(node: Record): Record continue } + if (key === 'required' && Array.isArray(val)) { + result['required'] = val.map((item) => (item === 'properties' ? 'props' : item)) + continue + } + if (key === 'const') { result['enum'] = [val] continue @@ -55,13 +60,18 @@ function cleanSchemaNode(node: Record): Record if (key === 'properties' && val && typeof val === 'object' && !Array.isArray(val)) { const sanitizedProps: Record = {} for (const [propKey, propVal] of Object.entries(val as Record)) { + // A property named 'properties' inside a properties map creates ambiguous/conflicting + // Protobuf schemas for Gemini / Vertex AI (e.g. parameters.properties.properties). + // Safely rename it to 'props'. + const targetKey = propKey === 'properties' ? 'props' : propKey + if (typeof propVal === 'string') { - sanitizedProps[propKey] = + sanitizedProps[targetKey] = propVal === 'object' ? { type: 'object', properties: {} } : { type: propVal } } else if (propVal && typeof propVal === 'object' && !Array.isArray(propVal)) { - sanitizedProps[propKey] = cleanSchemaNode(propVal as Record) + sanitizedProps[targetKey] = cleanSchemaNode(propVal as Record) } else { - sanitizedProps[propKey] = propVal + sanitizedProps[targetKey] = propVal } } result['properties'] = sanitizedProps diff --git a/src/server/mcp/manager.ts b/src/server/mcp/manager.ts index 67a88704..c56117a1 100644 --- a/src/server/mcp/manager.ts +++ b/src/server/mcp/manager.ts @@ -8,6 +8,7 @@ import type { LLMToolDefinition } from '../llm/types.js' import { logger } from '../utils/logger.js' import { McpOAuthProvider } from './oauth-provider.js' import { readMcpOAuthEntry } from './oauth-store.js' +import { sanitizeToolSchema } from '../llm/schema-sanitizer.js' /** * The SDK merges requestInit headers after the ones it derives from the auth provider, so a static @@ -243,7 +244,7 @@ export class McpManager { function: { name: `${entry.state.name}_${tool.name}`, description: tool.description ?? '', - parameters: tool.inputSchema as Record, + parameters: sanitizeToolSchema(tool.inputSchema as Record), }, }) } diff --git a/src/server/mcp/tool-adapter.ts b/src/server/mcp/tool-adapter.ts index 61b76c0c..1b374a79 100644 --- a/src/server/mcp/tool-adapter.ts +++ b/src/server/mcp/tool-adapter.ts @@ -28,7 +28,11 @@ export function createMcpTools(mcpManager: McpManager): Tool[] { mcpServer: server.name, execute: async (args: Record, _context: ToolContext): Promise => { const start = Date.now() - const result = await mcpManager.callTool(server.name, mcpTool.name, args) + const normalizedArgs = { ...args } + if ('props' in normalizedArgs && !('properties' in normalizedArgs)) { + normalizedArgs['properties'] = normalizedArgs['props'] + } + const result = await mcpManager.callTool(server.name, mcpTool.name, normalizedArgs) return { success: result.success, ...(result.output ? { output: result.output } : {}), From cd496a27dc8bffdede526b02642f5e8c3b6dcf8d Mon Sep 17 00:00:00 2001 From: conrad Date: Wed, 2 Sep 2026 21:33:11 +0300 Subject: [PATCH 4/4] review: PR #309 fixes (drop stray props on MCP exec remap, cover with test, format sanitizer) --- src/server/llm/schema-sanitizer.ts | 3 +-- src/server/mcp/manager.test.ts | 35 ++++++++++++++++++++++++++++++ src/server/mcp/tool-adapter.ts | 1 + 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/server/llm/schema-sanitizer.ts b/src/server/llm/schema-sanitizer.ts index 45cade0b..e457f3d7 100644 --- a/src/server/llm/schema-sanitizer.ts +++ b/src/server/llm/schema-sanitizer.ts @@ -66,8 +66,7 @@ function cleanSchemaNode(node: Record): Record const targetKey = propKey === 'properties' ? 'props' : propKey if (typeof propVal === 'string') { - sanitizedProps[targetKey] = - propVal === 'object' ? { type: 'object', properties: {} } : { type: propVal } + sanitizedProps[targetKey] = propVal === 'object' ? { type: 'object', properties: {} } : { type: propVal } } else if (propVal && typeof propVal === 'object' && !Array.isArray(propVal)) { sanitizedProps[targetKey] = cleanSchemaNode(propVal as Record) } else { diff --git a/src/server/mcp/manager.test.ts b/src/server/mcp/manager.test.ts index 86130f16..1e05db7f 100644 --- a/src/server/mcp/manager.test.ts +++ b/src/server/mcp/manager.test.ts @@ -519,6 +519,41 @@ describe('createMcpTools', () => { expect(result.success).toBe(true) expect(result.output).toBe('Sunny, 72°F') }) + + it('remaps a renamed props argument back to properties on execution', async () => { + mockClientInstance.listTools.mockResolvedValueOnce({ + tools: [ + { + name: 'config_tool', + description: 'Config tool', + inputSchema: { + type: 'object', + properties: { + properties: { type: 'object', properties: { a: { type: 'string' } } }, + }, + }, + }, + ], + }) + const manager = new McpManager() + await manager.addServer('test', { transport: 'stdio', command: 'node' }) + + const tools = createMcpTools(manager) + // The sanitizer renames the top-level `properties` param to `props` in the + // LLM-facing schema, so the model answers with `props`. + expect(tools[0]!.definition.function.parameters).toEqual({ + type: 'object', + properties: { props: { type: 'object', properties: { a: { type: 'string' } } } }, + }) + + await tools[0]!.execute({ props: { a: 'x' } }, {} as any) + + // The MCP server must receive the original param name, with no stray `props` key. + const lastCall = mockClientInstance.callTool.mock.calls.at(-1)! + const payload = lastCall[lastCall.length - 1] as { arguments?: Record } + expect(payload.arguments).toEqual({ properties: { a: 'x' } }) + expect(payload.arguments).not.toHaveProperty('props') + }) }) describe('estimateToolTokens', () => { diff --git a/src/server/mcp/tool-adapter.ts b/src/server/mcp/tool-adapter.ts index 1b374a79..3ca32bed 100644 --- a/src/server/mcp/tool-adapter.ts +++ b/src/server/mcp/tool-adapter.ts @@ -31,6 +31,7 @@ export function createMcpTools(mcpManager: McpManager): Tool[] { const normalizedArgs = { ...args } if ('props' in normalizedArgs && !('properties' in normalizedArgs)) { normalizedArgs['properties'] = normalizedArgs['props'] + delete normalizedArgs['props'] } const result = await mcpManager.callTool(server.name, mcpTool.name, normalizedArgs) return {