From b623b619de1c5bb73dd2eb85b99922d2207681cd Mon Sep 17 00:00:00 2001 From: debian Date: Fri, 30 Oct 2026 08:52:54 -0300 Subject: [PATCH 1/2] fix(core): resolve and preserve MCP tool inputSchema $defs/$ref definitions --- packages/core/src/mcp/mcp-client.ts | 2 + packages/core/src/mcp/mcp-manager.ts | 111 +++++++++++++++++++-- packages/core/src/prompt.ts | 2 + packages/core/src/tests/mcp-client.test.ts | 92 +++++++++++++++++ 4 files changed, 199 insertions(+), 8 deletions(-) diff --git a/packages/core/src/mcp/mcp-client.ts b/packages/core/src/mcp/mcp-client.ts index 4420c569..1ab06bfd 100644 --- a/packages/core/src/mcp/mcp-client.ts +++ b/packages/core/src/mcp/mcp-client.ts @@ -31,6 +31,8 @@ export type McpToolDefinition = { properties: Record; required?: string[]; additionalProperties?: boolean; + $defs?: Record; + definitions?: Record; }; }; diff --git a/packages/core/src/mcp/mcp-manager.ts b/packages/core/src/mcp/mcp-manager.ts index 6d2edc63..28ba64f5 100644 --- a/packages/core/src/mcp/mcp-manager.ts +++ b/packages/core/src/mcp/mcp-manager.ts @@ -313,6 +313,8 @@ export class McpManager { properties: Record; required?: string[]; additionalProperties?: boolean; + $defs?: Record; + definitions?: Record; }; }; }> { @@ -321,14 +323,7 @@ export class McpManager { function: { name: t.namespacedName, description: this.buildMcpToolDescription(t), - parameters: { - type: "object" as const, - properties: t.definition.inputSchema.properties, - required: t.definition.inputSchema.required, - ...(t.definition.inputSchema.additionalProperties !== undefined - ? { additionalProperties: t.definition.inputSchema.additionalProperties } - : {}), - }, + parameters: normalizeMcpInputSchema(t.definition.inputSchema), }, })); } @@ -522,3 +517,103 @@ function fitApiToolNameWithSuffix(name: string, suffix: string): string { function hashToolName(value: string): string { return createHash("sha256").update(value).digest("hex").slice(0, 8); } + +export function normalizeMcpInputSchema(inputSchema?: McpToolDefinition["inputSchema"]): { + type: "object"; + properties: Record; + required?: string[]; + additionalProperties?: boolean; + $defs?: Record; + definitions?: Record; +} { + if (!inputSchema || typeof inputSchema !== "object") { + return { type: "object", properties: {} }; + } + + const defs: Record = { + ...(isPlainObject(inputSchema.definitions) ? inputSchema.definitions : {}), + ...(isPlainObject(inputSchema.$defs) ? inputSchema.$defs : {}), + }; + + let hasUnresolvedRef = false; + + const deref = (node: unknown, seen: ReadonlySet = new Set()): unknown => { + if (Array.isArray(node)) { + return node.map((item) => deref(item, seen)); + } + if (!isPlainObject(node)) { + return node; + } + + if (typeof node.$ref === "string") { + const ref = node.$ref; + let defKey: string | null = null; + if (ref.startsWith("#/$defs/")) { + defKey = ref.slice("#/$defs/".length); + } else if (ref.startsWith("#/definitions/")) { + defKey = ref.slice("#/definitions/".length); + } + + if (defKey && Object.prototype.hasOwnProperty.call(defs, defKey) && !seen.has(defKey)) { + const nextSeen = new Set(seen); + nextSeen.add(defKey); + const resolvedTarget = deref(defs[defKey], nextSeen); + const rest = { ...node }; + delete rest.$ref; + const resolvedRest = deref(rest, seen); + if (isPlainObject(resolvedTarget) && isPlainObject(resolvedRest)) { + return { ...resolvedTarget, ...resolvedRest }; + } + } + + hasUnresolvedRef = true; + } + + const out: Record = {}; + for (const [key, value] of Object.entries(node)) { + out[key] = deref(value, seen); + } + + if (Array.isArray(out.allOf) && out.allOf.length === 1 && isPlainObject(out.allOf[0])) { + const { allOf, ...rest } = out; + return { ...allOf[0], ...rest }; + } + + return out; + }; + + const resolvedProperties = isPlainObject(inputSchema.properties) + ? (deref(inputSchema.properties) as Record) + : {}; + + const result: { + type: "object"; + properties: Record; + required?: string[]; + additionalProperties?: boolean; + $defs?: Record; + definitions?: Record; + } = { + type: "object", + properties: resolvedProperties, + ...(inputSchema.required !== undefined ? { required: inputSchema.required } : {}), + ...(inputSchema.additionalProperties !== undefined + ? { additionalProperties: inputSchema.additionalProperties } + : {}), + }; + + if (hasUnresolvedRef) { + if (isPlainObject(inputSchema.$defs)) { + result.$defs = inputSchema.$defs; + } + if (isPlainObject(inputSchema.definitions)) { + result.definitions = inputSchema.definitions; + } + } + + return result; +} + +function isPlainObject(value: unknown): value is Record { + return Boolean(value) && typeof value === "object" && !Array.isArray(value); +} diff --git a/packages/core/src/prompt.ts b/packages/core/src/prompt.ts index f7f147d3..7d938c63 100644 --- a/packages/core/src/prompt.ts +++ b/packages/core/src/prompt.ts @@ -394,6 +394,8 @@ export type ToolDefinition = { properties: Record; required?: string[]; additionalProperties?: boolean; + $defs?: Record; + definitions?: Record; }; }; }; diff --git a/packages/core/src/tests/mcp-client.test.ts b/packages/core/src/tests/mcp-client.test.ts index 6a7dc016..d39f9745 100644 --- a/packages/core/src/tests/mcp-client.test.ts +++ b/packages/core/src/tests/mcp-client.test.ts @@ -4,6 +4,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { McpClient, createMcpSpawnSpec } from "../mcp/mcp-client"; +import { normalizeMcpInputSchema } from "../mcp/mcp-manager"; test("createMcpSpawnSpec keeps non-Windows MCP launches shell-free", () => { assert.deepEqual(createMcpSpawnSpec("npx", ["-y", "@playwright/mcp@latest"], "darwin"), { @@ -108,3 +109,94 @@ test("McpClient starts a PATH-resolved cmd MCP server on Windows", { skip: proce rmSync(serverDir, { recursive: true, force: true }); } }); + +test("normalizeMcpInputSchema inlines $defs and definitions $ref pointers", () => { + const schema = normalizeMcpInputSchema({ + type: "object", + properties: { + body: { + $ref: "#/$defs/CreateDelegationRequest", + type: "object", + }, + tier: { + allOf: [{ $ref: "#/definitions/VolumeType" }], + description: "Storage tier override.", + }, + }, + required: ["body"], + $defs: { + CreateDelegationRequest: { + type: "object", + additionalProperties: false, + required: ["resource"], + properties: { + resource: { + type: "string", + description: "ECR resource ARN", + }, + }, + }, + }, + definitions: { + VolumeType: { + type: "string", + description: "Default volume type.", + enum: ["STANDARD", "HIGH_PERFORMANCE"], + }, + }, + }); + + assert.deepEqual(schema, { + type: "object", + properties: { + body: { + type: "object", + additionalProperties: false, + required: ["resource"], + properties: { + resource: { + type: "string", + description: "ECR resource ARN", + }, + }, + }, + tier: { + type: "string", + description: "Storage tier override.", + enum: ["STANDARD", "HIGH_PERFORMANCE"], + }, + }, + required: ["body"], + }); +}); + +test("normalizeMcpInputSchema preserves $defs when cyclic $ref cannot be fully inlined", () => { + const schema = normalizeMcpInputSchema({ + type: "object", + properties: { + node: { + $ref: "#/$defs/TreeNode", + }, + }, + $defs: { + TreeNode: { + type: "object", + properties: { + child: { + $ref: "#/$defs/TreeNode", + }, + }, + }, + }, + }); + + assert.equal(typeof schema.$defs, "object"); + assert.deepEqual(schema.properties.node, { + type: "object", + properties: { + child: { + $ref: "#/$defs/TreeNode", + }, + }, + }); +}); From 70e87400d0fa5dab47f3956a980f673107f4d4ae Mon Sep 17 00:00:00 2001 From: Ji Zhang Date: Wed, 4 Nov 2026 12:55:31 +0800 Subject: [PATCH 2/2] fix(core): preserve MCP schemas and validate converted tuple arguments --- package-lock.json | 66 ++++- packages/core/package.json | 2 + packages/core/src/mcp/mcp-client.ts | 10 +- packages/core/src/mcp/mcp-manager.ts | 132 ++-------- packages/core/src/mcp/mcp-schema.ts | 167 ++++++++++++ packages/core/src/prompt.ts | 2 +- packages/core/src/tests/mcp-client.test.ts | 92 ------- .../src/tests/mcp-schema-dispatch.test.ts | 118 +++++++++ packages/core/src/tests/mcp-schema.test.ts | 247 ++++++++++++++++++ 9 files changed, 616 insertions(+), 220 deletions(-) create mode 100644 packages/core/src/mcp/mcp-schema.ts create mode 100644 packages/core/src/tests/mcp-schema-dispatch.test.ts create mode 100644 packages/core/src/tests/mcp-schema.test.ts diff --git a/package-lock.json b/package-lock.json index 9625a5cd..8131c226 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2351,6 +2351,45 @@ "url": "https://github.com/sponsors/epoberezkin" } }, + "node_modules/ajv-formats": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/ajv-formats/-/ajv-formats-3.0.1.tgz", + "integrity": "sha512-8iUql50EUR+uUcdRQ3HDqa6EVyo3docL8g5WJ3FNcWmu62IbkGUue/pEyLBW8VGKKucTPgqeks4fIU1DA4yowQ==", + "license": "MIT", + "dependencies": { + "ajv": "^8.0.0" + }, + "peerDependencies": { + "ajv": "^8.0.0" + }, + "peerDependenciesMeta": { + "ajv": { + "optional": true + } + } + }, + "node_modules/ajv-formats/node_modules/ajv": { + "version": "8.20.0", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.20.0.tgz", + "integrity": "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA==", + "license": "MIT", + "dependencies": { + "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", + "url": "https://github.com/sponsors/epoberezkin" + } + }, + "node_modules/ajv-formats/node_modules/json-schema-traverse": { + "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/ansi-escapes": { "version": "7.3.0", "license": "MIT", @@ -3483,7 +3522,6 @@ }, "node_modules/fast-deep-equal": { "version": "3.1.3", - "dev": true, "license": "MIT" }, "node_modules/fast-glob": { @@ -3526,7 +3564,6 @@ "version": "3.1.9", "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.9.tgz", "integrity": "sha512-hS/0Wt3Ya043oKXs0sIxrMpKQITXhMLIrVAKKXi4gozGC9v7YRWVnXIZzjNPTO02M9BdxLCWqK1yRXgNziVyJQ==", - "dev": true, "funding": [ { "type": "github", @@ -5719,7 +5756,6 @@ }, "node_modules/require-from-string": { "version": "2.0.2", - "dev": true, "license": "MIT", "engines": { "node": ">=0.10.0" @@ -7017,6 +7053,8 @@ "version": "0.4.5", "license": "MIT", "dependencies": { + "ajv": "^8.20.0", + "ajv-formats": "^3.0.1", "chalk": "^5.6.2", "ejs": "^5.0.2", "gray-matter": "^4.0.3", @@ -7028,6 +7066,22 @@ "zod": "^4.6.7" } }, + "packages/core/node_modules/ajv": { + "version": "8.20.0", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.20.0.tgz", + "integrity": "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA==", + "license": "MIT", + "dependencies": { + "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", + "url": "https://github.com/sponsors/epoberezkin" + } + }, "packages/core/node_modules/chalk": { "version": "5.6.2", "license": "MIT", @@ -7045,6 +7099,12 @@ "node": ">= 4" } }, + "packages/core/node_modules/json-schema-traverse": { + "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" + }, "packages/vscode-ide-companion": { "name": "deepcode-vscode", "version": "0.4.5", diff --git a/packages/core/package.json b/packages/core/package.json index c1a183d7..addc9488 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -56,6 +56,8 @@ "test": "node src/tests/run-tests.mjs" }, "dependencies": { + "ajv": "^8.20.0", + "ajv-formats": "^3.0.1", "chalk": "^5.6.2", "ejs": "^5.0.2", "gray-matter": "^4.0.3", diff --git a/packages/core/src/mcp/mcp-client.ts b/packages/core/src/mcp/mcp-client.ts index 1ab06bfd..957b48e5 100644 --- a/packages/core/src/mcp/mcp-client.ts +++ b/packages/core/src/mcp/mcp-client.ts @@ -2,6 +2,7 @@ import { spawn, type ChildProcess } from "child_process"; import { createInterface, type Interface } from "readline"; import * as path from "path"; import { killProcessTree } from "../common/process-tree"; +import type { McpInputSchema } from "./mcp-schema"; type JsonRpcRequest = { jsonrpc: "2.0"; @@ -26,14 +27,7 @@ type JsonRpcNotification = { export type McpToolDefinition = { name: string; description?: string; - inputSchema: { - type: "object"; - properties: Record; - required?: string[]; - additionalProperties?: boolean; - $defs?: Record; - definitions?: Record; - }; + inputSchema: McpInputSchema; }; type ListToolsResult = { diff --git a/packages/core/src/mcp/mcp-manager.ts b/packages/core/src/mcp/mcp-manager.ts index 28ba64f5..cb3e49a8 100644 --- a/packages/core/src/mcp/mcp-manager.ts +++ b/packages/core/src/mcp/mcp-manager.ts @@ -1,6 +1,8 @@ import { createHash } from "crypto"; import { McpClient, type McpToolDefinition, type McpPromptDefinition, type McpResourceDefinition } from "./mcp-client"; import type { McpServerConfig } from "../settings"; +import type { ToolDefinition } from "../prompt"; +import { prepareMcpInputSchema, type PreparedMcpInputSchema } from "./mcp-schema"; const MCP_STARTUP_TIMEOUT_MS = process.env.DEEPCODE_MCP_TIMEOUT ? parseInt(process.env.DEEPCODE_MCP_TIMEOUT, 10) @@ -59,6 +61,7 @@ function buildMcpNamespacedName( export class McpManager { private clients: McpClient[] = []; private tools: McpToolEntry[] = []; + private toolSchemas = new WeakMap(); private prompts: Array<{ serverName: string; namespacedName: string; @@ -303,27 +306,22 @@ export class McpManager { return result; } - getMcpToolDefinitions(): Array<{ - type: "function"; - function: { - name: string; - description: string; - parameters: { - type: "object"; - properties: Record; - required?: string[]; - additionalProperties?: boolean; - $defs?: Record; - definitions?: Record; - }; - }; - }> { + private getToolSchema(tool: McpToolDefinition): PreparedMcpInputSchema { + let schema = this.toolSchemas.get(tool); + if (!schema) { + schema = prepareMcpInputSchema(tool.inputSchema); + this.toolSchemas.set(tool, schema); + } + return schema; + } + + getMcpToolDefinitions(): ToolDefinition[] { return this.tools.map((t) => ({ type: "function" as const, function: { name: t.namespacedName, description: this.buildMcpToolDescription(t), - parameters: normalizeMcpInputSchema(t.definition.inputSchema), + parameters: this.getToolSchema(t.definition).parameters, }, })); } @@ -343,6 +341,8 @@ export class McpManager { } try { + const validationError = this.getToolSchema(tool.definition).validateArguments?.(args); + if (validationError) return { ok: false, name, error: validationError }; const result = await tool.client.callTool(tool.originalName, args, timeoutMs); const text = result.content .filter((c) => c.type === "text" && c.text) @@ -517,103 +517,3 @@ function fitApiToolNameWithSuffix(name: string, suffix: string): string { function hashToolName(value: string): string { return createHash("sha256").update(value).digest("hex").slice(0, 8); } - -export function normalizeMcpInputSchema(inputSchema?: McpToolDefinition["inputSchema"]): { - type: "object"; - properties: Record; - required?: string[]; - additionalProperties?: boolean; - $defs?: Record; - definitions?: Record; -} { - if (!inputSchema || typeof inputSchema !== "object") { - return { type: "object", properties: {} }; - } - - const defs: Record = { - ...(isPlainObject(inputSchema.definitions) ? inputSchema.definitions : {}), - ...(isPlainObject(inputSchema.$defs) ? inputSchema.$defs : {}), - }; - - let hasUnresolvedRef = false; - - const deref = (node: unknown, seen: ReadonlySet = new Set()): unknown => { - if (Array.isArray(node)) { - return node.map((item) => deref(item, seen)); - } - if (!isPlainObject(node)) { - return node; - } - - if (typeof node.$ref === "string") { - const ref = node.$ref; - let defKey: string | null = null; - if (ref.startsWith("#/$defs/")) { - defKey = ref.slice("#/$defs/".length); - } else if (ref.startsWith("#/definitions/")) { - defKey = ref.slice("#/definitions/".length); - } - - if (defKey && Object.prototype.hasOwnProperty.call(defs, defKey) && !seen.has(defKey)) { - const nextSeen = new Set(seen); - nextSeen.add(defKey); - const resolvedTarget = deref(defs[defKey], nextSeen); - const rest = { ...node }; - delete rest.$ref; - const resolvedRest = deref(rest, seen); - if (isPlainObject(resolvedTarget) && isPlainObject(resolvedRest)) { - return { ...resolvedTarget, ...resolvedRest }; - } - } - - hasUnresolvedRef = true; - } - - const out: Record = {}; - for (const [key, value] of Object.entries(node)) { - out[key] = deref(value, seen); - } - - if (Array.isArray(out.allOf) && out.allOf.length === 1 && isPlainObject(out.allOf[0])) { - const { allOf, ...rest } = out; - return { ...allOf[0], ...rest }; - } - - return out; - }; - - const resolvedProperties = isPlainObject(inputSchema.properties) - ? (deref(inputSchema.properties) as Record) - : {}; - - const result: { - type: "object"; - properties: Record; - required?: string[]; - additionalProperties?: boolean; - $defs?: Record; - definitions?: Record; - } = { - type: "object", - properties: resolvedProperties, - ...(inputSchema.required !== undefined ? { required: inputSchema.required } : {}), - ...(inputSchema.additionalProperties !== undefined - ? { additionalProperties: inputSchema.additionalProperties } - : {}), - }; - - if (hasUnresolvedRef) { - if (isPlainObject(inputSchema.$defs)) { - result.$defs = inputSchema.$defs; - } - if (isPlainObject(inputSchema.definitions)) { - result.definitions = inputSchema.definitions; - } - } - - return result; -} - -function isPlainObject(value: unknown): value is Record { - return Boolean(value) && typeof value === "object" && !Array.isArray(value); -} diff --git a/packages/core/src/mcp/mcp-schema.ts b/packages/core/src/mcp/mcp-schema.ts new file mode 100644 index 00000000..18107360 --- /dev/null +++ b/packages/core/src/mcp/mcp-schema.ts @@ -0,0 +1,167 @@ +import Ajv, { type AnySchema, type ValidateFunction } from "ajv"; +import Ajv2019 from "ajv/dist/2019.js"; +import Ajv2020 from "ajv/dist/2020.js"; +import addFormats from "ajv-formats"; + +export type McpInputSchema = { + type: "object"; + properties?: Record; + required?: string[]; + additionalProperties?: boolean | Record; + $defs?: Record; + definitions?: Record; + [keyword: string]: unknown; +}; + +export type PreparedMcpInputSchema = { + parameters: McpInputSchema & { properties: Record }; + validateArguments?: (args: Record) => string | undefined; +}; + +const SCHEMA_MAPS = new Set(["properties", "patternProperties", "$defs", "definitions", "dependentSchemas"]); +const SCHEMA_ARRAYS = new Set(["allOf", "anyOf", "oneOf", "prefixItems"]); +const SCHEMAS = new Set([ + "items", + "additionalItems", + "additionalProperties", + "unevaluatedItems", + "unevaluatedProperties", + "contains", + "propertyNames", + "not", + "if", + "then", + "else", + "contentSchema", +]); + +/** Keep references and composition intact; only adapt tuples for the model API. */ +export function prepareMcpInputSchema(inputSchema?: McpInputSchema): PreparedMcpInputSchema { + const original: McpInputSchema = structuredClone( + isObject(inputSchema) ? inputSchema : { type: "object", properties: {} } + ); + let hasTuples = false; + let hasPrefixItems = false; + + const locations = new Map(); + const references: Array<{ node: Record; sourceRoot: string; targetRoot: string }> = []; + + const visit = (node: unknown, source = "", target = "", sourceRoot = "", targetRoot = ""): unknown => { + locations.set(source, target); + if (!isObject(node)) return node; + if (typeof node.$id === "string" && !node.$id.startsWith("#")) { + sourceRoot = source; + targetRoot = target; + } + const child = (schema: unknown, from: string[], to = from) => + visit(schema, pointer(source, ...from), pointer(target, ...to), sourceRoot, targetRoot); + const modern = Array.isArray(node.prefixItems); + const tuple = modern ? node.prefixItems : node.items; + const out: Record = Object.fromEntries( + Object.entries(node).map(([key, value]) => { + if (Array.isArray(tuple) && ["items", "prefixItems", "additionalItems"].includes(key)) { + return [key, value]; // Converted below, with reference locations recorded. + } + if ((SCHEMA_MAPS.has(key) || key === "dependencies") && isObject(value)) { + return [ + key, + Object.fromEntries(Object.entries(value).map(([name, schema]) => [name, child(schema, [key, name])])), + ]; + } + if (SCHEMA_ARRAYS.has(key) && Array.isArray(value)) { + return [key, value.map((schema, index) => child(schema, [key, String(index)]))]; + } + return [key, SCHEMAS.has(key) ? child(value, [key]) : value]; + }) + ); + + if (Array.isArray(tuple)) { + hasTuples = true; + hasPrefixItems ||= modern; + const tupleKey = modern ? "prefixItems" : "items"; + const tailKey = modern ? "items" : "additionalItems"; + const tail = node[tailKey]; + const alternatives = tuple.map((schema, index) => + child(schema, [tupleKey, String(index)], ["items", "anyOf", String(index)]) + ); + // An omitted tail is unconstrained. Keep every branch, including an open + // tail, so valid server arguments and references into tuple items survive. + alternatives.push(child(tail ?? {}, [tailKey], ["items", "anyOf", String(tuple.length)])); + out.items = { anyOf: alternatives }; + if (tail === false) { + out.maxItems = typeof out.maxItems === "number" ? Math.min(out.maxItems, tuple.length) : tuple.length; + } + delete out.prefixItems; + delete out.additionalItems; + } + references.push({ node: out, sourceRoot, targetRoot }); + return out; + }; + + const parameters = visit(structuredClone(original)) as PreparedMcpInputSchema["parameters"]; + // Tuple conversion relocates schemas such as /items/0. Rewrite only local + // schema references, never strings in defaults, enums or examples. + for (const { node, sourceRoot, targetRoot } of references) { + for (const keyword of ["$ref", "$dynamicRef", "$recursiveRef"]) { + const ref = node[keyword]; + if (typeof ref !== "string" || (ref !== "#" && !ref.startsWith("#/"))) continue; + let fragment: string; + try { + fragment = decodeURIComponent(ref.slice(1)); + } catch { + continue; + } + const relocated = locations.get(sourceRoot + fragment); + if (relocated !== undefined && relocated !== sourceRoot + fragment) { + node[keyword] = "#" + encodeURI(relocated.slice(targetRoot.length)).replace(/#/g, "%23"); + } + } + } + parameters.properties ??= {}; + if (!hasTuples) return { parameters }; + + // Compile lazily so one unsupported server schema cannot prevent discovery + // of other tools. Compilation failures still block this tool's execution. + let validate: ValidateFunction | undefined; + let compilationError: string | undefined; + return { + parameters, + validateArguments(args) { + if (!validate && !compilationError) { + try { + const dialect = original.$schema; + const Validator = + dialect === "https://json-schema.org/draft/2020-12/schema" || (!dialect && hasPrefixItems) + ? Ajv2020 + : dialect === "https://json-schema.org/draft/2019-09/schema" + ? Ajv2019 + : Ajv; + const ajv = new Validator({ + strict: false, + coerceTypes: false, + useDefaults: false, + removeAdditional: false, + }); + addFormats(ajv); + validate = ajv.compile(original as AnySchema); + if ("$async" in validate && validate.$async) throw new Error("Asynchronous schemas are not supported."); + } catch (error) { + validate = undefined; + compilationError = `Cannot validate MCP tool arguments: ${error instanceof Error ? error.message : String(error)}`; + } + } + if (compilationError) return compilationError; + if (validate!(args)) return undefined; + const error = validate!.errors?.[0]; + return `Invalid MCP tool arguments at ${error?.instancePath || "/"}: ${error?.message ?? "schema validation failed"}.`; + }, + }; +} + +function isObject(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function pointer(base: string, ...parts: string[]): string { + return base + parts.map((part) => "/" + part.replace(/~/g, "~0").replace(/\//g, "~1")).join(""); +} diff --git a/packages/core/src/prompt.ts b/packages/core/src/prompt.ts index 7d938c63..d2552039 100644 --- a/packages/core/src/prompt.ts +++ b/packages/core/src/prompt.ts @@ -393,7 +393,7 @@ export type ToolDefinition = { type: "object"; properties: Record; required?: string[]; - additionalProperties?: boolean; + additionalProperties?: boolean | Record; $defs?: Record; definitions?: Record; }; diff --git a/packages/core/src/tests/mcp-client.test.ts b/packages/core/src/tests/mcp-client.test.ts index d39f9745..6a7dc016 100644 --- a/packages/core/src/tests/mcp-client.test.ts +++ b/packages/core/src/tests/mcp-client.test.ts @@ -4,7 +4,6 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { McpClient, createMcpSpawnSpec } from "../mcp/mcp-client"; -import { normalizeMcpInputSchema } from "../mcp/mcp-manager"; test("createMcpSpawnSpec keeps non-Windows MCP launches shell-free", () => { assert.deepEqual(createMcpSpawnSpec("npx", ["-y", "@playwright/mcp@latest"], "darwin"), { @@ -109,94 +108,3 @@ test("McpClient starts a PATH-resolved cmd MCP server on Windows", { skip: proce rmSync(serverDir, { recursive: true, force: true }); } }); - -test("normalizeMcpInputSchema inlines $defs and definitions $ref pointers", () => { - const schema = normalizeMcpInputSchema({ - type: "object", - properties: { - body: { - $ref: "#/$defs/CreateDelegationRequest", - type: "object", - }, - tier: { - allOf: [{ $ref: "#/definitions/VolumeType" }], - description: "Storage tier override.", - }, - }, - required: ["body"], - $defs: { - CreateDelegationRequest: { - type: "object", - additionalProperties: false, - required: ["resource"], - properties: { - resource: { - type: "string", - description: "ECR resource ARN", - }, - }, - }, - }, - definitions: { - VolumeType: { - type: "string", - description: "Default volume type.", - enum: ["STANDARD", "HIGH_PERFORMANCE"], - }, - }, - }); - - assert.deepEqual(schema, { - type: "object", - properties: { - body: { - type: "object", - additionalProperties: false, - required: ["resource"], - properties: { - resource: { - type: "string", - description: "ECR resource ARN", - }, - }, - }, - tier: { - type: "string", - description: "Storage tier override.", - enum: ["STANDARD", "HIGH_PERFORMANCE"], - }, - }, - required: ["body"], - }); -}); - -test("normalizeMcpInputSchema preserves $defs when cyclic $ref cannot be fully inlined", () => { - const schema = normalizeMcpInputSchema({ - type: "object", - properties: { - node: { - $ref: "#/$defs/TreeNode", - }, - }, - $defs: { - TreeNode: { - type: "object", - properties: { - child: { - $ref: "#/$defs/TreeNode", - }, - }, - }, - }, - }); - - assert.equal(typeof schema.$defs, "object"); - assert.deepEqual(schema.properties.node, { - type: "object", - properties: { - child: { - $ref: "#/$defs/TreeNode", - }, - }, - }); -}); diff --git a/packages/core/src/tests/mcp-schema-dispatch.test.ts b/packages/core/src/tests/mcp-schema-dispatch.test.ts new file mode 100644 index 00000000..239e44bf --- /dev/null +++ b/packages/core/src/tests/mcp-schema-dispatch.test.ts @@ -0,0 +1,118 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import Ajv from "ajv"; +import { McpManager } from "../mcp/mcp-manager"; +import { getTools } from "../prompt"; + +test("MCP tuple tools validate before dispatch, preserve arguments, and refresh validators with tools/list", async (t) => { + const directory = mkdtempSync(path.join(tmpdir(), "deepcode-mcp-schema-")); + const serverPath = path.join(directory, "server.cjs"); + writeFileSync( + serverPath, + ` +const readline = require("node:readline"); +let firstType = "string"; +let calls = 0; +const send = message => process.stdout.write(JSON.stringify(message) + "\\n"); +const schema = () => ({ + type: "object", + required: ["pair"], + additionalProperties: false, + properties: { + pair: { $ref: "#/$defs/Pair" }, + optional: { type: "string", default: "must not insert" } + }, + $defs: { + Pair: { type: "array", items: [{ type: firstType }, { type: "integer" }], additionalItems: false, minItems: 2 } + } +}); +readline.createInterface({ input: process.stdin }).on("line", line => { + const request = JSON.parse(line); + if (!("id" in request)) return; + let result; + if (request.method === "initialize") { + result = { protocolVersion: "2025-03-26", capabilities: { tools: { listChanged: true } } }; + } else if (request.method === "tools/list") { + result = { tools: [ + { name: "tuple.tool", inputSchema: schema() }, + { name: "broken", inputSchema: { ...schema(), allOf: [{ $ref: "#/$defs/Missing" }] } }, + { name: "ordinary", inputSchema: { type: "object", properties: { number: { type: "integer" } } } }, + { name: "refresh", inputSchema: { type: "object", properties: {} } }, + { name: "reject", inputSchema: { type: "object", properties: {} } } + ] }; + } else if (request.method === "tools/call") { + calls++; + result = { content: [{ type: "text", text: JSON.stringify({ calls, ...request.params }) }] }; + if (request.params.name === "refresh") firstType = "boolean"; + if (request.params.name === "reject") result.isError = true; + } else { + result = { prompts: [], resources: [] }; + } + send({ jsonrpc: "2.0", id: request.id, result }); + if (request.method === "tools/call" && request.params.name === "refresh") { + send({ jsonrpc: "2.0", method: "notifications/tools/list_changed" }); + } +}); +`, + "utf8" + ); + const manager = new McpManager(); + t.after(() => { + manager.disconnect(); + rmSync(directory, { recursive: true, force: true }); + }); + await manager.initialize({ probe: { command: process.execPath, args: [serverPath] } }); + assert.equal(manager.getStatus()[0].status, "ready"); + + // Validation must also work before anyone asks for the model's tool definitions. + const name = "mcp__probe__tuple_tool"; + const reversed = { pair: [1, "x"] }; + const rejected = await manager.executeMcpTool(name, reversed); + assert.equal(rejected.ok, false); + assert.match(rejected.error!, /Invalid MCP tool arguments/); + assert.deepEqual(reversed, { pair: [1, "x"] }); + + const wire = JSON.parse(JSON.stringify({ tools: getTools({}, manager.getMcpToolDefinitions()) })); + const parameters = wire.tools.find((tool: any) => tool.function.name === name).function.parameters; + // The outgoing schema is accepted by a JSON Schema validator, resolves its + // references, and represents tuple members as an object-form items schema. + assert.equal(Array.isArray(parameters.$defs.Pair.items), false); + assert.equal(new Ajv({ strict: false }).validate(parameters, reversed), true); + + for (const args of [{ pair: ["x", "1"] }, { pair: ["x", 1], extra: true }, { pair: ["x", 1, 2] }]) { + const copy = structuredClone(args); + assert.equal((await manager.executeMcpTool(name, args)).ok, false); + assert.deepEqual(args, copy); + } + const broken = await manager.executeMcpTool("mcp__probe__broken", { pair: ["x", 1] }); + assert.equal(broken.ok, false); + assert.match(broken.error!, /Cannot validate MCP tool arguments/); + + const args = { pair: ["x", 1] }; + const accepted = await manager.executeMcpTool(name, args); + assert.equal(accepted.ok, true); + assert.deepEqual(JSON.parse(accepted.output!), { calls: 1, name: "tuple.tool", arguments: args }); + assert.deepEqual(args, { pair: ["x", 1] }); + + // Existing tools without tuple conversion still delegate argument validation + // to their server, and server-reported errors remain errors. + const ordinary = await manager.executeMcpTool("mcp__probe__ordinary", { number: "unchanged" }); + assert.equal(ordinary.ok, true); + assert.deepEqual(JSON.parse(ordinary.output!).arguments, { number: "unchanged" }); + assert.equal((await manager.executeMcpTool("mcp__probe__reject", {})).ok, false); + + const refreshed = new Promise((resolve) => manager.setOnToolsListChanged(resolve)); + assert.equal((await manager.executeMcpTool("mcp__probe__refresh", {})).ok, true); + await refreshed; + assert.equal((await manager.executeMcpTool(name, { pair: ["x", 1] })).ok, false); + const afterRefresh = await manager.executeMcpTool(name, { pair: [true, 1] }); + assert.equal(afterRefresh.ok, true); + assert.deepEqual(JSON.parse(afterRefresh.output!), { + calls: 5, + name: "tuple.tool", + arguments: { pair: [true, 1] }, + }); +}); diff --git a/packages/core/src/tests/mcp-schema.test.ts b/packages/core/src/tests/mcp-schema.test.ts new file mode 100644 index 00000000..6dda43bc --- /dev/null +++ b/packages/core/src/tests/mcp-schema.test.ts @@ -0,0 +1,247 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import Ajv from "ajv"; +import { prepareMcpInputSchema, type McpInputSchema } from "../mcp/mcp-schema"; + +test("MCP parameters preserve definitions, reference namespaces and root constraints", () => { + const original: McpInputSchema = { + type: "object", + required: ["body", "legacy", "modern"], + additionalProperties: false, + properties: { + body: { $ref: "#/$defs/CreateDelegationRequest" }, + legacy: { allOf: [{ $ref: "#/definitions/Value" }], description: "Legacy value" }, + modern: { $ref: "#/$defs/Value" }, + escaped: { $ref: "#/$defs/a~1b~0c" }, + }, + $defs: { + CreateDelegationRequest: { + type: "object", + properties: { resource: { type: "string" } }, + required: ["resource"], + additionalProperties: false, + }, + Value: { type: "integer" }, + "a/b~c": { type: "boolean" }, + }, + definitions: { Value: { type: "string" } }, + allOf: [{ minProperties: 3 }], + }; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + assert.deepEqual(parameters, original); + assert.notEqual(parameters, original); + assert.equal(validateArguments, undefined); + const validate = new Ajv({ strict: false }).compile(JSON.parse(JSON.stringify(parameters))); + assert.equal(validate({ body: { resource: "x" }, legacy: "x", modern: 1, escaped: true }), true); + assert.equal(validate({ body: {}, legacy: "x", modern: 1 }), false); + assert.equal(validate({ body: { resource: "x" }, legacy: 1, modern: "x" }), false); +}); + +test("MCP parameters preserve recursive references without expanding them", () => { + const original: McpInputSchema = { + type: "object", + properties: { node: { $ref: "#/$defs/TreeNode" } }, + $defs: { + TreeNode: { type: "object", properties: { child: { $ref: "#/$defs/TreeNode" } } }, + }, + }; + const { parameters } = prepareMcpInputSchema(original); + assert.deepEqual(parameters, original); + assert.equal(new Ajv({ strict: false }).validate(parameters, { node: { child: {} } }), true); +}); + +test("MCP parameters retain allOf constraints and leave enum/default/examples data untouched", () => { + const literal = { allOf: [{ x: 1 }], items: [{ $ref: "#/$defs/Missing" }] }; + const original: McpInputSchema = { + type: "object", + properties: { + body: { + type: "object", + properties: { resource: { type: "string" }, region: { type: "string" } }, + allOf: [{ required: ["resource"] }], + required: ["region"], + }, + payload: { enum: [literal], default: literal, examples: [literal] }, + }, + }; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + assert.deepEqual(parameters, original); + assert.equal(validateArguments, undefined); + const validate = new Ajv({ strict: false }).compile(parameters); + assert.equal(validate({ body: { region: "us" } }), false); + assert.equal(validate({ body: { region: "us", resource: "x" }, payload: literal }), true); +}); + +function tupleSchema(tail: unknown = false): McpInputSchema { + return { + type: "object", + required: ["pair"], + additionalProperties: false, + properties: { + pair: { + type: "array", + items: [{ type: "string" }, { type: "integer" }], + additionalItems: tail, + minItems: 2, + }, + optional: { type: "string", default: "do not insert" }, + }, + }; +} + +test("tuple conversion widens model parameters but enforces original positions before dispatch", () => { + const original = tupleSchema(); + const copy = structuredClone(original); + const { parameters, validateArguments } = prepareMcpInputSchema(original); + const modelValidate = new Ajv({ strict: false }).compile(parameters); + assert.equal(modelValidate({ pair: [1, "x"] }), true); + assert.equal(validateArguments!({ pair: ["x", 1] }), undefined); + for (const args of [{ pair: [1, "x"] }, { pair: ["x", "1"] }, { pair: ["x"] }, { pair: ["x", 1, 2] }]) { + assert.match(validateArguments!(args)!, /Invalid MCP tool arguments/); + } + assert.equal(modelValidate({ pair: ["x", 1, 2] }), false); + assert.deepEqual(original, copy); +}); + +test("validation never coerces values, inserts defaults or removes additional properties", () => { + const { validateArguments } = prepareMcpInputSchema(tupleSchema()); + for (const args of [{ pair: ["x", 1] }, { pair: ["x", "1"] }, { pair: ["x", 1], extra: "keep" }]) { + const copy = structuredClone(args); + const error = validateArguments!(args); + assert.equal(error === undefined, typeof args.pair[1] === "number" && !("extra" in args)); + assert.deepEqual(args, copy); + assert.equal("optional" in args, false); + } + const requiredDefault = tupleSchema(); + requiredDefault.required!.push("optional"); + assert.match(prepareMcpInputSchema(requiredDefault).validateArguments!({ pair: ["x", 1] })!, /required property/); +}); + +test("open and typed tuple tails remain allowed in the model schema and original validation", () => { + for (const tail of [true, { type: "boolean" }]) { + const original = tupleSchema(tail); + const { parameters, validateArguments } = prepareMcpInputSchema(original); + const args = { pair: ["x", 1, true] }; + assert.equal(new Ajv({ strict: false }).validate(parameters, args), true); + assert.equal(validateArguments!(args), undefined); + assert.match(validateArguments!({ pair: [1, "x", true] })!, /Invalid MCP tool arguments/); + if (tail !== true) { + assert.match(validateArguments!({ pair: ["x", 1, "wrong tail"] })!, /must be boolean/); + } + } + const original = tupleSchema(); + delete (original.properties!.pair as Record).additionalItems; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + const args = { pair: ["x", 1, { any: "value" }] }; + assert.equal(new Ajv({ strict: false }).validate(parameters, args), true); + assert.equal(validateArguments!(args), undefined); +}); + +test("tuple conversion retains stricter maxItems and does not require optional positions", () => { + const original = tupleSchema(); + const pair = original.properties!.pair as Record; + delete pair.minItems; + pair.maxItems = 1; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + assert.equal((parameters.properties.pair as Record).maxItems, 1); + assert.equal(validateArguments!({ pair: [] }), undefined); + assert.equal(validateArguments!({ pair: ["x"] }), undefined); + assert.match(validateArguments!({ pair: ["x", 1] })!, /more than 1 items/); +}); + +test("nested, referenced and recursive tuples are converted and checked against the entire original schema", () => { + const original: McpInputSchema = { + $schema: "https://json-schema.org/draft/2019-09/schema", + type: "object", + properties: { node: { $ref: "#/$defs/Node" } }, + $defs: { + Node: { + type: "object", + properties: { + pairs: { type: "array", items: tupleSchema().properties!.pair }, + child: { $ref: "#/$defs/Node" }, + email: { type: "string", format: "email" }, + }, + }, + }, + }; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + const node = parameters.$defs!.Node as any; + assert.equal(Array.isArray(node.properties.pairs.items.items), false); + assert.equal(validateArguments!({ node: { child: { pairs: [["x", 1]], email: "a@example.com" } } }), undefined); + assert.match(validateArguments!({ node: { child: { pairs: [[1, "x"]] } } })!, /Invalid MCP tool arguments/); + assert.match(validateArguments!({ node: { email: "invalid" } })!, /format "email"/); +}); + +test("draft 2020-12 prefixItems use their original positional and tail constraints", () => { + const original: McpInputSchema = { + $schema: "https://json-schema.org/draft/2020-12/schema", + type: "object", + properties: { + pair: { type: "array", prefixItems: [{ type: "string" }, { type: "integer" }], items: false }, + }, + }; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + assert.equal("prefixItems" in (parameters.properties.pair as object), false); + assert.equal(validateArguments!({ pair: ["x", 1] }), undefined); + assert.equal(validateArguments!({ pair: [] }), undefined); + assert.match(validateArguments!({ pair: [1, "x"] })!, /Invalid MCP tool arguments/); + assert.match(validateArguments!({ pair: ["x", 1, 2] })!, /Invalid MCP tool arguments/); +}); + +test("schema maps, dependencies and conditionals are visited without confusing property names with keywords", () => { + const tuple = tupleSchema().properties!.pair; + const original: McpInputSchema = { + type: "object", + properties: { items: { type: "array", items: { type: "string" } }, allOf: { type: "string" } }, + patternProperties: { "^tuple": tuple }, + dependencies: { items: ["allOf"], allOf: { properties: { dependent: tuple } } }, + if: { required: ["flag"] }, + then: { properties: { conditional: tuple } }, + additionalProperties: tuple as Record, + }; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + assert.deepEqual(parameters.properties, original.properties); + assert.deepEqual((parameters.dependencies as any).items, ["allOf"]); + assert.equal(Array.isArray((parameters.additionalProperties as any).items), false); + assert.equal(validateArguments!({ tupleOne: ["x", 1] }), undefined); + assert.match(validateArguments!({ tupleOne: [1, "x"] })!, /Invalid MCP tool arguments/); +}); + +test("unresolvable references and unsupported dialects fail closed without blocking normalization", () => { + for (const extra of [ + { $schema: "https://example.com/unsupported-schema" }, + { allOf: [{ $ref: "https://example.com/unavailable" }] }, + { allOf: [{ $ref: "#/$defs/Missing" }] }, + ]) { + const { parameters, validateArguments } = prepareMcpInputSchema({ ...tupleSchema(), ...extra }); + assert.ok(parameters.properties.pair); + assert.match(validateArguments!({ pair: ["x", 1] })!, /Cannot validate MCP tool arguments/); + assert.match(validateArguments!({ pair: ["x", 1] })!, /Cannot validate MCP tool arguments/); + } +}); + +test("references into converted tuple members and tails keep resolving, including scoped and escaped paths", () => { + const nested = { + $id: "https://example.com/nested", + type: "object", + properties: { + pairs: { type: "array", items: [{ type: "array", items: [{ type: "string" }] }] }, + label: { $ref: "#/properties/pairs/items/0/items/0" }, + }, + }; + const original: McpInputSchema = { + type: "object", + properties: { + "pair /~#": { type: "array", items: [nested], additionalItems: { type: "boolean" } }, + tail: { $ref: "#/properties/pair%20~1~0%23/additionalItems" }, + }, + }; + const { parameters, validateArguments } = prepareMcpInputSchema(original); + const validate = new Ajv({ strict: false }).compile(parameters); + const args = { "pair /~#": [{ pairs: [["x"]], label: "x" }, true], tail: true }; + assert.equal(validate(args), true); + assert.equal(validateArguments!(args), undefined); + assert.equal(validate({ tail: "wrong" }), false); + assert.equal(validate({ "pair /~#": [{ label: 1 }] }), false); +});