diff --git a/docs/architecture/mcp-v2-protocol/spec.md b/docs/architecture/mcp-v2-protocol/spec.md index 188019ed2..29b04fc76 100644 --- a/docs/architecture/mcp-v2-protocol/spec.md +++ b/docs/architecture/mcp-v2-protocol/spec.md @@ -154,22 +154,36 @@ instead of executing whichever target currently owns the model-visible name. ### Schemas, Metadata, And Headers -Tool input and output schemas are arbitrary JSON Schema 2020-12 documents. DeepChat preserves the -raw schema and metadata alongside any provider-specific projection. +Modern tool input and output schemas are arbitrary JSON Schema 2020-12 documents. DeepChat +preserves the raw schema and metadata alongside any provider-specific projection. -Validate a declared schema dialect and support JSON Schema 2020-12. Do not network-dereference -external `$ref` values by default; reject an unresolved external reference instead of treating it -as permissive. Apply byte, depth, key-count, and composition-expansion limits before projection, -persistence, or renderer delivery. +Validate a declared schema dialect and support JSON Schema 2020-12 for modern connections and +DeepChat-owned packaged catalogs. Do not network-dereference external `$ref` values. Modern and +packaged definitions reject unresolved external references instead of treating them as permissive. + +User-configured external legacy connections preserve schema documents as opaque bounded JSON and +must not acquire modern-only dialect, remote-reference, or composition-shape rejection after the +legacy SDK accepts the response. This compatibility does not permit network dereferencing or relax +the structural and size boundaries on an individual schema or metadata value. + +Apply byte, depth, key-count, node-count, and composition-expansion limits before projection, +persistence, or renderer delivery. Collection envelopes also have a total byte limit, but member +key and node counts do not accumulate into one fixed catalog-wide quota; validate those limits on +each independently consumed member. The model-provider projection may simplify a schema only at the final provider adapter boundary. -The original MCP definition must still be available when calling the tool so the SDK can: +For modern connections and DeepChat-owned catalogs, the original MCP definition must still be +available when calling the tool so the SDK can: - validate output; - mirror fields annotated by `x-mcp-header`; - emit standard `Mcp-Method` and `Mcp-Name` headers; - preserve tool `_meta`, including MCP Apps metadata. +For user-configured external legacy connections, retain the original schema in DeepChat's tool +record but do not opt the call into v2-only output-schema compilation that the v1 client did not +perform. This exception does not relax DeepChat's bounded result validation. + Structured content, result `_meta`, and the original content array remain available to extension handlers and durable assistant blocks. The text projection shown to the model remains bounded and provider-compatible. @@ -397,7 +411,9 @@ upstream gate opens. - No DeepChat-owned MCP core module imports the v1 SDK. - Existing legacy stdio, Streamable HTTP, SSE, and in-memory fixtures retain their current - observable behavior. + observable behavior, including schemas previously accepted by the legacy SDK subject to + per-value host bounds. +- External legacy tool calls do not acquire v2-only output-schema compilation before dispatch. - Existing servers receive immutable local IDs without losing configuration; renames preserve the ID, while re-pointing invalidates the prior binding. - Modern stdio and HTTP fixtures connect without initialize/session assumptions. @@ -406,8 +422,9 @@ upstream gate opens. - A failed stdio probe leaves no sibling process. - JSON Schema 2020-12 features and tool/result metadata survive discovery, provider projection, call execution, and persistence. -- Declared dialects are validated, unresolved external references fail closed, and schema - composition remains within explicit limits. +- Modern and packaged schemas validate declared dialects, fail closed on unresolved external + references, and keep schema composition within explicit limits. External legacy references stay + opaque and are never network-dereferenced. - `ttlMs`, `cacheScope`, discovery changes, and subscriptions update the rendered catalog without a manual protocol cache. - Multi-round input requests complete or cancel without issuing a duplicate tool call. diff --git a/docs/issues/mcp-large-tool-catalog-compatibility/spec.md b/docs/issues/mcp-large-tool-catalog-compatibility/spec.md new file mode 100644 index 000000000..5ff885a8e --- /dev/null +++ b/docs/issues/mcp-large-tool-catalog-compatibility/spec.md @@ -0,0 +1,164 @@ +# Large MCP Tool Catalog Compatibility + +Status: Implemented + +GitHub issue: [#2085](https://github.com/ThinkInAIXYZ/deepchat/issues/2085) + +## Issue + +A connected external stdio server can expose a large but valid tool catalog and still appear in +DeepChat with zero tools. The report in #2085 uses 151 tools and an approximately 267 KB +`tools/list` response. + +The transport and SDK limits are not the immediate bottleneck. DeepChat validates the complete +control response with one global JSON key and node budget before validating each tool. A catalog +whose individual tools are all within their limits can therefore exceed the aggregate 10,000-key +budget and fail discovery while the transport remains connected. + +A local synthetic catalog matching the reported tool count and approximate byte size reproduces +this failure. Both the v1 SDK and the v2 SDK legacy result schema accept that fixture; DeepChat's +aggregate bounded-JSON check rejects it. The reporter's original payload is not available, so the +fixture proves the defective boundary and matching symptom, not that every field in the reported +payload is identical. + +The v2 migration also introduced stricter host-side JSON Schema semantics for every protocol era. +For example, an external `$ref` accepted by both SDK legacy schemas is rejected by DeepChat after +the SDK response has already passed validation. That is a separate compatibility gap: modern +schema policy must not silently redefine the accepted legacy wire contract. + +## Related Precedents + +- [`7b50b1cc`](https://github.com/ThinkInAIXYZ/deepchat/commit/7b50b1cc50867258d351b13f7b5948f866c4788f) + stopped treating JSON object prototypes and insertion order as protocol differences. It supports + the same boundary principle, but it does not address catalog limits or legacy schema semantics. +- [`5814a04c`](https://github.com/ThinkInAIXYZ/deepchat/commit/5814a04c8937a4fc45975dcf0ca2b4646fad8fec) + is the closer failure-isolation precedent: an optional invalid output schema no longer removes an + otherwise usable tool. This issue still requires collection-aware limits rather than another + schema exception. + +## Impact + +- A server can show `connected` while all of its tools are absent from the model-visible catalog. +- Catalog complexity grows with tool count, so otherwise ordinary schemas can fail only when + combined into one response. +- User-configured legacy servers can lose tools solely because DeepChat applies modern-only schema + semantics after legacy SDK validation. +- Raising the shared JSON limits would weaken unrelated trust boundaries such as tool arguments, + route input, and individual tool results. + +## Root Cause + +`McpClient.listTools()` and `McpClient.listToolsPage()` call the generic control-result validator on +the complete `tools/list` object. The validator counts every nested key and node against constants +designed for one bounded JSON value. Only after that aggregate check does DeepChat call +`validateAndCloneMcpTool()` for each member. + +The MCP App host applies another aggregate bounded-JSON check when returning the already validated +tool list. Fixing only `McpClient` would therefore leave a second catalog-wide key and node quota in +the App path. + +Separately, `validateAndCloneMcpTool()` always applies DeepChat's modern dialect, reference, and +composition rules. It does not distinguish a negotiated external legacy connection from a modern +connection or a DeepChat-owned packaged catalog. + +The v2 client also compiles a supplied `outputSchema` before every `callTool()` request, including +legacy-era calls. The v1 client did not enforce that schema. Passing an opaque legacy definition +back to v2 unchanged can therefore reject the call before it reaches the server. + +## Required Behavior + +### Collection And Member Budgets + +- Keep the existing total serialized-size limit on every `tools/list` response. +- Keep JSON-only values, finite numbers, cycle rejection, and depth limits on the collection. +- Do not accumulate the fixed 10,000-key and 100,000-node limits across all tools in a catalog. +- Apply existing schema and metadata byte, key, node, depth, and composition limits independently + to every tool through `validateAndCloneMcpTool()`. +- Apply the same collection policy to the MCP App tool-list boundary so a validated catalog is not + rejected by a second aggregate complexity count. +- Preserve the current default aggregate limits for non-collection callers. Do not increase the + global constants. + +The smallest implementation is to let the existing bounded-JSON traversal select a collection +policy at the two tool-list boundaries. The default policy remains unchanged. Collection policy +still clones and validates the complete value and enforces its byte and depth bounds, while key and +node complexity is enforced on each tool member rather than accumulated across the catalog. + +### Legacy Schema Compatibility + +For a user-configured external connection whose negotiated protocol era is `legacy`: + +- require input and output schemas to remain bounded JSON objects; +- preserve schema documents and references as opaque protocol data; +- never network-dereference `$ref` or `$dynamicRef` values; +- do not newly reject a tool solely for an older or unknown declared dialect, a remote reference, + or a schema-composition shape that the legacy SDK accepted. +- retain the original output schema in DeepChat's tool definition, but do not opt the legacy call + into the v2-only output-schema compiler. + +Modern external connections keep the strict host-side dialect, reference, and composition checks. +DeepChat-owned built-in servers and packaged plugin catalogs also keep strict validation regardless +of their transport implementation because those definitions cross a separate local catalog trust +boundary. + +The v2 SDK legacy response schema remains the parser. No parallel v1 client or permanent dual-SDK +adapter is part of this fix. If a captured server response later proves that the v2 SDK itself +rejects a legacy response that v1 accepted, that incompatibility needs a concrete fixture and a +separate SDK-boundary decision. + +### Failure Semantics + +Transport connection state and tool discovery remain distinct. A rejected tool list must continue +to surface as a discovery failure with the server name and violated boundary; this change does not +relabel an established transport as disconnected or add a new renderer diagnostics contract. + +## Implementation Plan + +1. Extend the bounded-JSON traversal options without changing default callers, adding the + collection behavior needed by tool-list responses. +2. Use collection validation in `McpClient.listTools()`, `McpClient.listToolsPage()`, and the MCP App + tool-list response boundary; retain their existing total byte caps. +3. Select tool-schema semantic validation from the connection owner and negotiated protocol era: + strict for modern and DeepChat-owned catalogs, compatibility-preserving for external legacy + servers. +4. Keep per-tool schema and metadata cloning and structural limits in both modes, while suppressing + v2-only output-schema compilation at the external legacy call boundary. +5. Add focused regressions at the client, ToolManager, and App-host boundaries. + +## Non-Goals + +- Increasing stdio buffering or the 32 MB control-result cap. +- Increasing shared key or node limits globally. +- Restoring the monolithic v1 SDK or maintaining two MCP client implementations. +- Dereferencing or fetching remote schemas. +- Quarantining arbitrary malformed members while accepting the rest of a modern catalog. +- Redefining the MCP connection-state UI. + +## Task Checklist + +- [x] Add collection-aware bounded JSON validation with unchanged secure defaults. +- [x] Remove catalog-wide key and node accumulation from both MCP tool-list boundaries. +- [x] Preserve strict per-tool schema and metadata limits. +- [x] Gate modern schema semantics by connection ownership and negotiated protocol era. +- [x] Preserve legacy output schemas without enabling v2-only call-time compilation. +- [x] Add a 151-tool regression fixture above 10,000 aggregate keys and near 267 KB. +- [x] Cover direct `listTools`, paginated `listToolsPage`, ToolManager registration, and MCP App + listing. +- [x] Cover modern, external legacy, and packaged-catalog schema policies. +- [x] Run formatting, i18n validation, lint, typecheck, and the focused MCP test suites. + +## Validation + +- A valid 151-tool response above 10,000 aggregate keys returns and registers all 151 tools in both + modern and legacy client modes. +- The same catalog passes through the MCP App tool-list boundary without a second aggregate-key + failure. +- A response above the existing total byte cap still fails before renderer or provider delivery. +- A single oversized schema, metadata object, or non-collection JSON value still fails at the + current limits. +- An external legacy fixture containing a remote `$ref` or unsupported declared dialect remains + opaque and usable without network access. +- Calling that legacy tool preserves the raw output schema in DeepChat while preventing the v2 + client from compiling it before dispatch. +- The equivalent modern fixture fails closed, and packaged catalog drift validation remains strict. +- Existing tool discovery, catalog verification, and MCP App tests remain green. diff --git a/src/main/mcp/apps/appHost.ts b/src/main/mcp/apps/appHost.ts index 3a057ff71..88ca7bac4 100644 --- a/src/main/mcp/apps/appHost.ts +++ b/src/main/mcp/apps/appHost.ts @@ -417,7 +417,9 @@ export class McpAppHost implements McpAppHostPort { ...(result.nextCursor ? { nextCursor: result.nextCursor } : {}), ...(result._meta ? { _meta: result._meta } : {}) } - assertBoundedMcpJson(output, 'MCP App tool list', MAX_APP_ACTION_BYTES) + assertBoundedMcpJson(output, 'MCP App tool list', MAX_APP_ACTION_BYTES, { + independentArrayItemsAtPath: '#/tools' + }) return output } diff --git a/src/main/mcp/mcpClient.ts b/src/main/mcp/mcpClient.ts index 103b890e1..fe0bddfe4 100644 --- a/src/main/mcp/mcpClient.ts +++ b/src/main/mcp/mcpClient.ts @@ -1460,9 +1460,14 @@ export class McpClient { name: toolName, arguments: args } + // The v2 client compiles outputSchema in legacy mode, while the v1 client did not. + const toolDefinition = + options?.toolDefinition?.outputSchema && this.getToolSchemaPolicy() === 'legacy' + ? { ...options.toolDefinition, outputSchema: undefined } + : options?.toolDefinition const result = (await this.client.callTool(request, { signal: options?.signal, - toolDefinition: options?.toolDefinition as SdkTool | undefined + toolDefinition: toolDefinition as SdkTool | undefined })) as unknown as ToolCallResult options?.signal?.throwIfAborted() assertBoundedMcpJson( @@ -1489,6 +1494,16 @@ export class McpClient { return capabilities !== undefined && capabilities[capability] === undefined } + private getToolSchemaPolicy(): 'strict' | 'legacy' { + const isPluginOwned = + Boolean(this.serverConfig.ownerPluginId) || this.serverConfig.source === 'plugin' + return this.client?.getProtocolEra() === 'legacy' && + this.serverConfig.type !== 'inmemory' && + !isPluginOwned + ? 'legacy' + : 'strict' + } + async listTools(options?: { signal?: AbortSignal }): Promise { options?.signal?.throwIfAborted() @@ -1507,10 +1522,13 @@ export class McpClient { ? await this.client.listTools(undefined, { signal: options.signal }) : await this.client.listTools() options?.signal?.throwIfAborted() - this.assertControlResult(response, 'tool list') + this.assertControlResult(response, 'tool list', { + independentArrayItemsAtPath: '#/tools' + }) if (Array.isArray(response.tools)) { + const schemaPolicy = this.getToolSchemaPolicy() return (response.tools as unknown as Tool[]).map((tool) => - validateAndCloneMcpTool(tool, this.serverName) + validateAndCloneMcpTool(tool, this.serverName, schemaPolicy) ) } throw new Error('Invalid tool response format') @@ -1546,11 +1564,14 @@ export class McpClient { { tools: [] }, signal ) - this.assertControlResult(result, 'tool list page') + this.assertControlResult(result, 'tool list page', { + independentArrayItemsAtPath: '#/tools' + }) + const schemaPolicy = this.getToolSchemaPolicy() return { ...result, tools: (result.tools as unknown as Tool[]).map((tool) => - validateAndCloneMcpTool(tool, this.serverName) + validateAndCloneMcpTool(tool, this.serverName, schemaPolicy) ) } as unknown as ListToolsResult } @@ -1787,11 +1808,16 @@ export class McpClient { return 'transport-error' } - private assertControlResult(value: unknown, label: string): void { + private assertControlResult( + value: unknown, + label: string, + options: { independentArrayItemsAtPath?: string } = {} + ): void { assertBoundedMcpJson( value, `MCP ${label} from ${this.serverName}`, - MCP_CONTROL_RESULT_MAX_BYTES + MCP_CONTROL_RESULT_MAX_BYTES, + options ) } diff --git a/src/main/mcp/schemaValidation.ts b/src/main/mcp/schemaValidation.ts index 982f89832..3e0b3b831 100644 --- a/src/main/mcp/schemaValidation.ts +++ b/src/main/mcp/schemaValidation.ts @@ -18,12 +18,12 @@ const SUPPORTED_JSON_SCHEMA_DIALECTS = new Set([ interface CloneLimits { maxBytes: number + independentArrayItemsAtPath?: string } interface CloneState { keys: number nodes: number - seen: WeakSet } export type JsonValueDifference = { @@ -113,17 +113,22 @@ export function findJsonValueDifference( function cloneBoundedJson(value: unknown, label: string, limits: CloneLimits): unknown { const state: CloneState = { keys: 0, - nodes: 0, - seen: new WeakSet() + nodes: 0 } - - const visit = (current: unknown, depth: number, path: string): unknown => { + const seen = new WeakSet() + + const visit = ( + current: unknown, + depth: number, + path: string, + complexity: CloneState + ): unknown => { if (depth > MAX_JSON_DEPTH) { throw new Error(`${label} exceeds the maximum JSON depth`) } - state.nodes += 1 - if (state.nodes > MAX_JSON_NODES) { + complexity.nodes += 1 + if (complexity.nodes > MAX_JSON_NODES) { throw new Error(`${label} exceeds the maximum JSON node count`) } @@ -139,14 +144,22 @@ function cloneBoundedJson(value: unknown, label: string, limits: CloneLimits): u } if (Array.isArray(current)) { - if (state.seen.has(current)) { + if (seen.has(current)) { throw new Error(`${label} contains a circular reference at ${path}`) } - state.seen.add(current) + seen.add(current) + const independentItems = path === limits.independentArrayItemsAtPath const cloned = current.map((entry, index) => - entry === undefined ? null : visit(entry, depth + 1, `${path}/${index}`) + entry === undefined + ? null + : visit( + entry, + depth + 1, + `${path}/${index}`, + independentItems ? { keys: 0, nodes: 0 } : complexity + ) ) - state.seen.delete(current) + seen.delete(current) return cloned } @@ -154,27 +167,27 @@ function cloneBoundedJson(value: unknown, label: string, limits: CloneLimits): u throw new Error(`${label} contains a non-JSON value at ${path}`) } - if (state.seen.has(current)) { + if (seen.has(current)) { throw new Error(`${label} contains a circular reference at ${path}`) } - state.seen.add(current) + seen.add(current) const entries = Object.entries(current).filter(([, entry]) => entry !== undefined) - state.keys += entries.length - if (state.keys > MAX_JSON_KEYS) { + complexity.keys += entries.length + if (complexity.keys > MAX_JSON_KEYS) { throw new Error(`${label} exceeds the maximum JSON key count`) } const cloned: Record = Object.create(null) for (const [key, entry] of entries) { - cloned[key] = visit(entry, depth + 1, `${path}/${key}`) + cloned[key] = visit(entry, depth + 1, `${path}/${key}`, complexity) } - state.seen.delete(current) + seen.delete(current) return cloned } - const cloned = visit(value, 0, '#') + const cloned = visit(value, 0, '#', state) const serialized = JSON.stringify(cloned) if (Buffer.byteLength(serialized, 'utf8') > limits.maxBytes) { throw new Error(`${label} exceeds the maximum serialized size`) @@ -269,13 +282,23 @@ function validateSchemaTree(schema: Record, label: string): voi visit(schema, '#') } -export function assertBoundedMcpJson(value: unknown, label: string, maxBytes: number): void { +export function assertBoundedMcpJson( + value: unknown, + label: string, + maxBytes: number, + options: { independentArrayItemsAtPath?: string } = {} +): void { cloneBoundedJson(value, label, { - maxBytes + maxBytes, + ...options }) } -export function validateAndCloneJsonSchema(value: unknown, label: string): Record { +function cloneJsonSchema( + value: unknown, + label: string, + validateSemantics: boolean +): Record { if (!value || typeof value !== 'object' || Array.isArray(value)) { throw new Error(`${label} must be a JSON object`) } @@ -283,16 +306,22 @@ export function validateAndCloneJsonSchema(value: unknown, label: string): Recor const cloned = cloneBoundedJson(value, label, { maxBytes: MAX_SCHEMA_BYTES }) as Record - const dialect = cloned.$schema - if (dialect !== undefined) { - if (typeof dialect !== 'string' || !SUPPORTED_JSON_SCHEMA_DIALECTS.has(dialect)) { - throw new Error(`${label} uses an unsupported JSON Schema dialect`) + if (validateSemantics) { + const dialect = cloned.$schema + if (dialect !== undefined) { + if (typeof dialect !== 'string' || !SUPPORTED_JSON_SCHEMA_DIALECTS.has(dialect)) { + throw new Error(`${label} uses an unsupported JSON Schema dialect`) + } } + validateSchemaTree(cloned, label) } - validateSchemaTree(cloned, label) return cloned } +export function validateAndCloneJsonSchema(value: unknown, label: string): Record { + return cloneJsonSchema(value, label, true) +} + function cloneMetadata( value: Record | undefined, label: string @@ -305,12 +334,21 @@ function cloneMetadata( }) as Record } -export function validateAndCloneMcpTool(tool: Tool, serverName: string): Tool { +export function validateAndCloneMcpTool( + tool: Tool, + serverName: string, + schemaPolicy: 'strict' | 'legacy' = 'strict' +): Tool { const label = `MCP tool ${serverName}/${tool.name}` + const validateSchemaSemantics = schemaPolicy === 'strict' let outputSchema: Record | undefined if (tool.outputSchema !== undefined) { try { - outputSchema = validateAndCloneJsonSchema(tool.outputSchema, `${label} outputSchema`) + outputSchema = cloneJsonSchema( + tool.outputSchema, + `${label} outputSchema`, + validateSchemaSemantics + ) } catch (error) { const reason = error instanceof Error ? error.message : String(error) console.warn(`Ignoring invalid ${label} outputSchema: ${reason}`) @@ -326,7 +364,7 @@ export function validateAndCloneMcpTool(tool: Tool, serverName: string): Tool { maxBytes: MAX_METADATA_BYTES }) as Tool['icons']) : undefined, - inputSchema: validateAndCloneJsonSchema(tool.inputSchema, `${label} inputSchema`), + inputSchema: cloneJsonSchema(tool.inputSchema, `${label} inputSchema`, validateSchemaSemantics), outputSchema, annotations: cloneMetadata(tool.annotations, `${label} annotations`), _meta: cloneMetadata(tool._meta, `${label} metadata`), diff --git a/test/main/mcp/mcpAppHost.test.ts b/test/main/mcp/mcpAppHost.test.ts index d12cef1ad..3ef13a2db 100644 --- a/test/main/mcp/mcpAppHost.test.ts +++ b/test/main/mcp/mcpAppHost.test.ts @@ -38,6 +38,7 @@ const createHarness = () => { const client = { isServerRunning: vi.fn(() => true), listTools: vi.fn().mockResolvedValue([tool]), + listToolsPage: vi.fn().mockResolvedValue({ tools: [tool] }), readResourceContents: vi.fn().mockResolvedValue([ { uri: descriptor.resourceUri, @@ -189,6 +190,36 @@ describe('MCP App host', () => { }) }) + it('returns a large App-visible tool catalog without a catalog-wide key budget', async () => { + const { client, host } = createHarness() + const tools = Array.from({ length: 151 }, (_, toolIndex) => ({ + name: `app_tool_${toolIndex}`, + inputSchema: { + type: 'object', + properties: Object.fromEntries( + Array.from({ length: 22 }, (_, propertyIndex) => [ + `field_${propertyIndex}`, + { + type: 'string', + description: `Input field ${propertyIndex} owned by tool ${toolIndex}` + } + ]) + ) + }, + _meta: { + ui: { + visibility: ['app'] + } + } + })) + client.listToolsPage.mockResolvedValueOnce({ tools }) + + const result = await host.listTools('instance-id', undefined, context) + + expect(result.tools).toHaveLength(151) + expect(result.tools.at(-1)?.name).toBe('app_tool_150') + }) + it('does not create an App instance if its binding changes while resources load', async () => { const { client, host, registry, setConfig } = createHarness() let resolveResource!: (value: Awaited>) => void diff --git a/test/main/mcp/mcpClient.test.ts b/test/main/mcp/mcpClient.test.ts index db8020550..0342c2e1f 100644 --- a/test/main/mcp/mcpClient.test.ts +++ b/test/main/mcp/mcpClient.test.ts @@ -90,6 +90,38 @@ function createMcpClient( ) } +const createLargeToolCatalog = () => + Array.from({ length: 151 }, (_, toolIndex) => ({ + name: `tool_${toolIndex}`, + description: `Tool ${toolIndex}`, + inputSchema: { + type: 'object', + properties: Object.fromEntries( + Array.from({ length: 22 }, (_, propertyIndex) => [ + `field_${propertyIndex}`, + { + type: 'string', + description: `Input field ${propertyIndex} owned by tool ${toolIndex}` + } + ]) + ) + } + })) + +const createSdkToolClient = (tools: unknown[], era: 'modern' | 'legacy') => ({ + connect: vi.fn().mockResolvedValue(undefined), + callTool: vi.fn().mockResolvedValue({ content: [] }), + listTools: vi.fn().mockResolvedValue({ tools }), + listPrompts: vi.fn(), + getPrompt: vi.fn(), + listResources: vi.fn(), + readResource: vi.fn(), + setNotificationHandler: vi.fn(), + setRequestHandler: vi.fn(), + getProtocolEra: vi.fn(() => era), + getServerCapabilities: vi.fn(() => ({ tools: {} })) +}) + vi.mock('@/agent/shared/process/processTree', () => ({ terminateProcessTreeByPid: terminateProcessTreeMock })) @@ -799,6 +831,118 @@ describe('McpClient Runtime Command Processing Tests', () => { }) }) + describe('Tool discovery validation', () => { + it.each(['modern', 'legacy'] as const)( + 'accepts a large valid %s tool catalog without a catalog-wide key budget', + async (era) => { + const tools = createLargeToolCatalog() + const responseBytes = Buffer.byteLength(JSON.stringify({ tools }), 'utf8') + expect(responseBytes).toBeGreaterThan(260_000) + expect(responseBytes).toBeLessThan(275_000) + + const sdkClient = createSdkToolClient(tools, era) + vi.mocked(Client).mockImplementationOnce(() => sdkClient as any) + const client = createMcpClient('large-catalog', { + type: 'stdio', + command: 'large-catalog', + args: [], + forceLegacyWire: era === 'legacy' + }) + + await expect(client.listTools()).resolves.toHaveLength(151) + await expect(client.listToolsPage()).resolves.toMatchObject({ + tools: expect.arrayContaining([ + expect.objectContaining({ name: 'tool_0' }), + expect.objectContaining({ name: 'tool_150' }) + ]) + }) + } + ) + + it('preserves modern-incompatible schemas for user-owned external legacy servers', async () => { + const inputSchema = { + $schema: 'https://example.com/legacy-dialect', + type: 'object', + properties: { + remote: { $ref: 'https://example.com/input.json' } + } + } + const sdkClient = createSdkToolClient( + [ + { + name: 'legacy_tool', + inputSchema, + outputSchema: { $ref: 'https://example.com/output.json' } + } + ], + 'legacy' + ) + vi.mocked(Client).mockImplementationOnce(() => sdkClient as any) + const client = createMcpClient('legacy-server', { + type: 'stdio', + command: 'legacy-server', + args: [], + forceLegacyWire: true + }) + + const tools = await client.listTools() + + expect(tools[0].inputSchema).toEqual(inputSchema) + expect(tools[0].outputSchema).toEqual({ + $ref: 'https://example.com/output.json' + }) + + await client.callTool('legacy_tool', {}, { toolDefinition: tools[0] }) + + expect(sdkClient.callTool).toHaveBeenCalledWith( + { name: 'legacy_tool', arguments: {} }, + { + signal: undefined, + toolDefinition: expect.objectContaining({ + name: 'legacy_tool', + outputSchema: undefined + }) + } + ) + }) + + it.each([ + ['modern external', 'modern', {}], + [ + 'plugin-owned legacy', + 'legacy', + { forceLegacyWire: true, source: 'plugin', ownerPluginId: 'com.deepchat.fixture' } + ] + ] as const)('keeps strict schema semantics for %s servers', async (_label, era, config) => { + const sdkClient = createSdkToolClient( + [ + { + name: 'strict_tool', + inputSchema: { + type: 'object', + properties: { + remote: { $ref: 'https://example.com/input.json' } + } + } + } + ], + era + ) + vi.mocked(Client).mockImplementationOnce(() => sdkClient as any) + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) + const client = createMcpClient('strict-server', { + type: 'stdio', + command: 'strict-server', + args: [], + ...config + }) + + await expect(client.listTools()).rejects.toThrow('remote $ref') + + consoleErrorSpy.mockRestore() + }) + }) + describe('Path Expansion', () => { it('should expand tilde (~) in paths', () => { const client = createMcpClient('test', { type: 'stdio' }) diff --git a/test/main/mcp/schemaValidation.test.ts b/test/main/mcp/schemaValidation.test.ts index 89b957a14..f5c2f0efe 100644 --- a/test/main/mcp/schemaValidation.test.ts +++ b/test/main/mcp/schemaValidation.test.ts @@ -131,6 +131,74 @@ describe('MCP schema validation', () => { ).toThrow('maximum schema composition size') }) + it('counts configured collection members independently without weakening member limits', () => { + const member = Object.fromEntries( + Array.from({ length: 6_000 }, (_, index) => [`key_${index}`, index]) + ) + const payload = { tools: [member, member] } + + expect(() => assertBoundedMcpJson(payload, 'tool list', 1024 * 1024)).toThrow( + 'maximum JSON key count' + ) + expect(() => + assertBoundedMcpJson(payload, 'tool list', 1024 * 1024, { + independentArrayItemsAtPath: '#/tools' + }) + ).not.toThrow() + expect(() => + assertBoundedMcpJson(payload, 'tool list', 1024, { + independentArrayItemsAtPath: '#/tools' + }) + ).toThrow('maximum serialized size') + + const oversizedMember = Object.fromEntries( + Array.from({ length: 10_001 }, (_, index) => [`key_${index}`, index]) + ) + expect(() => + assertBoundedMcpJson({ tools: [oversizedMember] }, 'tool list', 1024 * 1024, { + independentArrayItemsAtPath: '#/tools' + }) + ).toThrow('maximum JSON key count') + + const cyclicMember: Record = {} + cyclicMember.self = cyclicMember + expect(() => + assertBoundedMcpJson({ tools: [cyclicMember] }, 'tool list', 1024 * 1024, { + independentArrayItemsAtPath: '#/tools' + }) + ).toThrow('circular reference') + }) + + it('preserves bounded legacy schemas without applying modern semantics', () => { + const inputSchema = { + $schema: 'https://example.com/legacy-dialect', + type: 'object', + properties: { + remote: { $ref: 'https://example.com/schema.json' } + } + } + + const cloned = validateAndCloneMcpTool( + { + name: 'legacy_inspect', + inputSchema, + outputSchema: { + type: 'object', + $ref: 'https://example.com/output.json' + } + }, + 'legacy-server', + 'legacy' + ) + + expect(cloned.inputSchema).toEqual(inputSchema) + expect(cloned.inputSchema).not.toBe(inputSchema) + expect(cloned.outputSchema).toEqual({ + type: 'object', + $ref: 'https://example.com/output.json' + }) + }) + it('retains standard tool metadata while cloning untrusted values', () => { const tool = { name: 'inspect', diff --git a/test/main/mcp/toolManager.test.ts b/test/main/mcp/toolManager.test.ts index 1e5ca34e3..5e3124405 100644 --- a/test/main/mcp/toolManager.test.ts +++ b/test/main/mcp/toolManager.test.ts @@ -226,6 +226,36 @@ describe('ToolManager', () => { ).toBe('Regular launch app description') }) + it('registers every tool from a large valid MCP catalog', async () => { + const serverName = 'large-catalog' + const tools = Array.from({ length: 151 }, (_, toolIndex) => ({ + name: `tool_${toolIndex}`, + description: `Tool ${toolIndex}`, + inputSchema: { + type: 'object', + properties: Object.fromEntries( + Array.from({ length: 22 }, (_, propertyIndex) => [ + `field_${propertyIndex}`, + { + type: 'string', + description: `Input field ${propertyIndex} owned by tool ${toolIndex}` + } + ]) + ) + } + })) + const client = createClient(serverName, tools) + const manager = createToolManager( + createProviderSettings(serverName), + createServerManager([client]) + ) + + const definitions = await manager.getAllToolDefinitions() + + expect(definitions).toHaveLength(151) + expect(definitions.at(-1)?.function.name).toBe('tool_150') + }) + it('keeps MCP tools sequential even when the server declares readOnlyHint', async () => { const client = createClient('untrusted-server', [ {