From 601198c9f428a02238dfc6766b8451e797152e50 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 02:39:52 +0000 Subject: [PATCH 1/4] test(mcp): pin what an AIToolDefinition puts on the MCP wire (RED) --- .../src/mcp-tool-bridge-input-schema.test.ts | 275 ++++++++++++++++++ 1 file changed, 275 insertions(+) create mode 100644 packages/mcp/src/mcp-tool-bridge-input-schema.test.ts diff --git a/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts b/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts new file mode 100644 index 0000000000..8242e0b98f --- /dev/null +++ b/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts @@ -0,0 +1,275 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `bridgeTools` surface must forward what an `AIToolDefinition` declares. + * + * THE DEFECT. `registerToolFromDefinition` passed `description` and three + * annotation hints to `McpServer.registerTool` and never read + * `tool.parameters`. Two prose sites said otherwise — `bridgeTools`' own + * docblock ("Each registered tool becomes an MCP tool with the same name, + * description, and JSON Schema parameters") and the comment on the call + * ("pass the JSON Schema as annotations metadata") — so a reader checking + * whether `parameters` was handled found a sentence saying yes. + * + * WHY THESE CASES DRIVE A REAL `StdioServerTransport`. The pins that were + * green through the whole defect asserted the bridge's *log line* + * ("Bridged N tools from ToolRegistry"), which stays true of a bridge that + * forwards nothing. What a client receives is only visible on the wire, so + * every case below speaks newline-delimited JSON-RPC down a real transport + * attached to the real long-lived server and reads `tools/list` / + * `tools/call` results, exactly as a desktop MCP host does. The transport is + * fed `PassThrough` pipes instead of `process.stdin`/`stdout` (its + * constructor takes both), the same technique the #8034 stdio pins use. + * + * WHAT THE CONTROLS ARE FOR. `name + description reach the client` and + * `a tool that declares no parameters` were GREEN before the fix and stay + * green after, so a red from the two schema/argument pins is a statement + * about the bridge rather than about the harness. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { PassThrough } from 'node:stream'; +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { AIToolDefinition, ToolCallPart } from '@objectstack/spec/contracts'; + +import { MCPServerRuntime } from './mcp-server-runtime.js'; +import type { ToolRegistry, ToolExecutionResult } from './types.js'; + +// --------------------------------------------------------------------------- +// A real stdio client: newline-delimited JSON-RPC over the transport's pipes +// --------------------------------------------------------------------------- + +interface JsonRpcFrame { + jsonrpc: string; + id?: number; + result?: any; + error?: { code: number; message: string }; +} + +interface StdioSession { + rpc(method: string, params?: unknown): Promise; + notify(method: string, params?: unknown): void; + close(): Promise; +} + +async function openStdio(server: McpServer): Promise { + const serverStdin = new PassThrough(); + const serverStdout = new PassThrough(); + const transport = new StdioServerTransport(serverStdin, serverStdout); + await server.connect(transport); + + let nextId = 1; + let buffered = ''; + const waiting = new Map void>(); + + serverStdout.on('data', (chunk: Buffer | string) => { + buffered += String(chunk); + let newline = buffered.indexOf('\n'); + while (newline >= 0) { + const line = buffered.slice(0, newline).trim(); + buffered = buffered.slice(newline + 1); + newline = buffered.indexOf('\n'); + if (!line) continue; + let frame: JsonRpcFrame; + try { + frame = JSON.parse(line) as JsonRpcFrame; + } catch { + continue; + } + const resolve = typeof frame.id === 'number' ? waiting.get(frame.id) : undefined; + if (resolve && typeof frame.id === 'number') { + waiting.delete(frame.id); + resolve(frame); + } + } + }); + + return { + rpc(method, params) { + const id = nextId++; + return new Promise((resolve, reject) => { + const giveUp = setTimeout( + () => reject(new Error(`stdio: no answer to "${method}" (id ${id}) within 5s`)), + 5_000, + ); + waiting.set(id, (frame) => { + clearTimeout(giveUp); + resolve(frame); + }); + serverStdin.write( + `${JSON.stringify({ jsonrpc: '2.0', id, method, ...(params ? { params } : {}) })}\n`, + ); + }); + }, + notify(method, params) { + serverStdin.write(`${JSON.stringify({ jsonrpc: '2.0', method, ...(params ? { params } : {}) })}\n`); + }, + async close() { + await transport.close().catch(() => {}); + }, + }; +} + +async function handshake(session: StdioSession): Promise { + await session.rpc('initialize', { + protocolVersion: '2025-06-18', + capabilities: {}, + clientInfo: { name: 'tool-bridge-pin', version: '0.0.0' }, + }); + session.notify('notifications/initialized'); +} + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +/** A tool whose `parameters` declares a real argument shape. */ +const QUERY_RECORDS: AIToolDefinition = { + name: 'query_records', + description: 'Query records of an object', + parameters: { + type: 'object', + properties: { + objectName: { type: 'string', description: 'The object/table name' }, + limit: { type: 'number', description: 'Max rows' }, + }, + required: ['objectName'], + }, +}; + +/** Control: a tool that genuinely declares no parameters. */ +const LIST_OBJECTS: AIToolDefinition = { + name: 'list_objects', + description: 'List all objects', + parameters: { type: 'object', properties: {} }, +}; + +interface RecordingRegistry extends ToolRegistry { + calls: ToolCallPart[]; +} + +function makeRegistry(tools: AIToolDefinition[]): RecordingRegistry { + const calls: ToolCallPart[] = []; + return { + calls, + getAll: () => tools, + async execute(toolCall: ToolCallPart): Promise { + calls.push(toolCall); + return { + type: 'tool-result', + toolCallId: toolCall.toolCallId, + toolName: toolCall.toolName, + output: { type: 'text', value: `executed ${toolCall.toolName}` }, + } as ToolExecutionResult; + }, + }; +} + +async function bridged(tools: AIToolDefinition[]) { + const registry = makeRegistry(tools); + const runtime = new MCPServerRuntime({ name: 'bridge-pin', version: '0.0.0-test' }); + runtime.bridgeTools(registry); + const session = await openStdio(runtime.server); + await handshake(session); + return { registry, session }; +} + +// --------------------------------------------------------------------------- + +describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { + let session: StdioSession | undefined; + + beforeEach(() => { + session = undefined; + }); + + it('CONTROL: name and description reach the client', async () => { + const s = await bridged([QUERY_RECORDS, LIST_OBJECTS]); + session = s.session; + + const listed = (await s.session.rpc('tools/list')).result?.tools ?? []; + const byName = Object.fromEntries(listed.map((t: any) => [t.name, t])); + + expect(Object.keys(byName).sort()).toEqual(['list_objects', 'query_records']); + expect(byName.query_records.description).toBe('Query records of an object'); + expect(byName.list_objects.description).toBe('List all objects'); + + await s.session.close(); + }); + + it('forwards `parameters` as the tool inputSchema', async () => { + const s = await bridged([QUERY_RECORDS]); + session = s.session; + + const listed = (await s.session.rpc('tools/list')).result?.tools ?? []; + const tool = listed.find((t: any) => t.name === 'query_records'); + + expect(tool?.inputSchema?.type).toBe('object'); + expect(tool?.inputSchema?.properties?.objectName).toMatchObject({ + type: 'string', + description: 'The object/table name', + }); + expect(tool?.inputSchema?.properties?.limit).toMatchObject({ type: 'number' }); + expect(tool?.inputSchema?.required).toEqual(['objectName']); + + await s.session.close(); + }); + + it('delivers the client arguments to the ToolRegistry', async () => { + const s = await bridged([QUERY_RECORDS]); + session = s.session; + + const called = await s.session.rpc('tools/call', { + name: 'query_records', + arguments: { objectName: 'task', limit: 5 }, + }); + + expect(called.result?.isError).toBeFalsy(); + expect(s.registry.calls).toHaveLength(1); + expect(s.registry.calls[0].toolName).toBe('query_records'); + expect(s.registry.calls[0].input).toEqual({ objectName: 'task', limit: 5 }); + + await s.session.close(); + }); + + it('CONTROL: a tool that declares no parameters still registers and still executes', async () => { + const s = await bridged([LIST_OBJECTS]); + session = s.session; + + const listed = (await s.session.rpc('tools/list')).result?.tools ?? []; + const tool = listed.find((t: any) => t.name === 'list_objects'); + expect(tool?.inputSchema?.type).toBe('object'); + expect(tool?.inputSchema?.properties).toEqual({}); + + const called = await s.session.rpc('tools/call', { name: 'list_objects', arguments: {} }); + expect(called.result?.isError).toBeFalsy(); + expect(s.registry.calls).toHaveLength(1); + expect(s.registry.calls[0].input).toEqual({}); + + await s.session.close(); + }); + + /** + * The measured, inseparable consequence of declaring an inputSchema in + * @modelcontextprotocol/sdk 1.30.0 — recorded here so the FROM → TO on this + * published surface is auditable, NOT as a validation step this bridge adds. + * `McpServer.validateToolInput()` runs whenever `tool.inputSchema` is set; + * the SDK offers no advertise-without-validate mode. + */ + it('CONSEQUENCE: the SDK rejects arguments that do not match the declared schema', async () => { + const s = await bridged([QUERY_RECORDS]); + session = s.session; + + const called = await s.session.rpc('tools/call', { + name: 'query_records', + arguments: { limit: 5 }, + }); + + expect(called.result?.isError).toBe(true); + expect(String(called.result?.content?.[0]?.text)).toContain('objectName'); + expect(s.registry.calls).toHaveLength(0); + + await s.session.close(); + }); +}); From 06193360b2ecaf668efc740bd654caccde9dfd4b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 02:42:12 +0000 Subject: [PATCH 2/4] fix(mcp): forward AIToolDefinition.parameters as the bridged tool's inputSchema --- packages/mcp/src/mcp-server-runtime.ts | 86 ++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 13 deletions(-) diff --git a/packages/mcp/src/mcp-server-runtime.ts b/packages/mcp/src/mcp-server-runtime.ts index e9cb5269d7..0c6b4d2ca7 100644 --- a/packages/mcp/src/mcp-server-runtime.ts +++ b/packages/mcp/src/mcp-server-runtime.ts @@ -73,6 +73,63 @@ const DESTRUCTIVE_TOOLS = new Set([ 'delete_field', ]); +// ── AIToolDefinition.parameters → MCP inputSchema ──────────────────────────── + +/** + * Convert an {@link AIToolDefinition}'s JSON Schema `parameters` into the Zod + * schema `McpServer.registerTool` requires for `inputSchema`. + * + * ⚠️ The conversion is not a stylistic choice, it is the only door. Measured + * against `@modelcontextprotocol/sdk` 1.30.0: `registerTool`'s `inputSchema` + * is typed `ZodRawShapeCompat | AnySchema`, and a raw JSON Schema object + * reaches the SDK's `getZodSchemaObject()`, which throws `inputSchema must be + * a Zod schema or raw shape, received an unrecognized object`. `zod@4`'s own + * `fromJSONSchema` opens that door with no new dependency (this package + * already depends on `zod`), and the SDK converts the result straight back to + * JSON Schema for `tools/list` — so what a client receives is the shape the + * definition declared. + * + * Skipping `inputSchema` is NOT the cheaper half of the same behaviour. The + * SDK synthesises `{ type: 'object', properties: {} }` for a tool registered + * without one — a positive claim that the tool takes no arguments — and + * `executeToolHandler()` then invokes the handler as `handler(extra)`, where + * `RequestHandlerExtra` carries no `arguments` member at all. A schema-less + * bridged tool therefore both mis-advertises itself AND executes with `{}` + * whatever the client sent. + * + * A `parameters` that does not describe an object — absent, `{}`, or untyped, + * all of which `fromJSONSchema` turns into `z.any()` — becomes a LOOSE EMPTY + * OBJECT. MCP requires `Tool.inputSchema.type` to be `"object"`, and a loose + * empty object is the honest report of "this definition declares no + * arguments": it advertises exactly what the SDK would have synthesised, it + * constrains nothing, and it keeps the arguments flowing to the handler. + * + * A `parameters` that cannot be converted at all is logged and gets the same + * loose empty object. Deliberately not a throw: this runs inside + * {@link MCPServerRuntime.bridgeTools}, so one unconvertible definition would + * otherwise take the server's ENTIRE tool surface down. + */ +function toolInputSchema(tool: AIToolDefinition, logger?: Logger): z.ZodType> { + const declaresNothing = () => z.looseObject({}) as unknown as z.ZodType>; + + let converted: unknown; + try { + converted = z.fromJSONSchema(tool.parameters as never); + } catch (err) { + logger?.warn(`[MCP] Tool "${tool.name}" has unconvertible JSON Schema parameters; bridged with no declared arguments`, { + error: err instanceof Error ? err.message : String(err), + }); + return declaresNothing(); + } + + if (converted instanceof z.ZodObject) { + return converted as unknown as z.ZodType>; + } + + logger?.debug(`[MCP] Tool "${tool.name}" declares no object parameters; bridged with no declared arguments`); + return declaresNothing(); +} + // ── Metadata outage vs. metadata miss (#6055, ADR-0110 D3) ─────────────────── /** @@ -753,9 +810,11 @@ export class MCPServerRuntime { /** * Bridge all tools from the ToolRegistry to MCP tools. * - * Each registered tool becomes an MCP tool with the same name, description, - * and JSON Schema parameters. The handler delegates to the ToolRegistry's - * execute path. + * Each registered tool becomes an MCP tool with the same name, description + * and declared arguments: `AIToolDefinition.parameters` is JSON Schema, and + * {@link toolInputSchema} converts it into the Zod schema the SDK requires + * for `inputSchema`. The handler delegates to the ToolRegistry's execute + * path. */ bridgeTools(toolRegistry: ToolRegistry): void { const tools = toolRegistry.getAll(); @@ -808,18 +867,24 @@ export class MCPServerRuntime { /** * Register a single tool on the MCP server from an AIToolDefinition. + * + * The definition's JSON Schema `parameters` is forwarded as the tool's + * `inputSchema` (see {@link toolInputSchema} for why it must be converted + * first). Declaring it is what makes the SDK hand the call's arguments to + * this handler at all: `McpServer.executeToolHandler()` branches on + * `tool.inputSchema` and invokes a schema-less tool as `handler(extra)` — + * with no `arguments` anywhere on that `extra` (`RequestHandlerExtra` has no + * such member), which is why a bridged tool used to execute with `{}` no + * matter what the client sent. */ private registerToolFromDefinition(tool: AIToolDefinition, toolRegistry: ToolRegistry): void { const logger = this.config.logger; - // Convert JSON Schema parameters to Zod-compatible format for MCP SDK - // The MCP SDK registerTool with inputSchema expects a Zod raw shape or AnySchema. - // Since our tools use JSON Schema, we use the low-level .tool() with a raw callback - // and pass the JSON Schema as annotations metadata. this.mcpServer.registerTool( tool.name, { description: tool.description, + inputSchema: toolInputSchema(tool, logger), annotations: { // Mark tools with write side-effects for destructive operations destructiveHint: this.isDestructiveTool(tool.name), @@ -827,12 +892,7 @@ export class MCPServerRuntime { openWorldHint: false, }, }, - async (extra) => { - // The MCP SDK passes tool arguments via the extra.arguments property - // when registerTool is called without an inputSchema. - const rawExtra = extra as Record; - const args = (rawExtra.arguments ?? {}) as Record; - + async (args) => { try { const result = await toolRegistry.execute({ type: 'tool-call', From 8bd109a88276e33844b172ff7a8a998182a652a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 02:43:25 +0000 Subject: [PATCH 3/4] chore(changeset): mcp tool bridge forwards the declared input schema --- .../mcp-bridge-forwards-tool-input-schema.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 .changeset/mcp-bridge-forwards-tool-input-schema.md diff --git a/.changeset/mcp-bridge-forwards-tool-input-schema.md b/.changeset/mcp-bridge-forwards-tool-input-schema.md new file mode 100644 index 0000000000..4d62b69fd2 --- /dev/null +++ b/.changeset/mcp-bridge-forwards-tool-input-schema.md @@ -0,0 +1,48 @@ +--- +'@objectstack/mcp': minor +--- + +Tools bridged from an AI service's `ToolRegistry` now reach MCP clients with the +input schema their definition declares, and the arguments a client sends now +reach the tool. + +`MCPServerRuntime.registerToolFromDefinition` passed a name, a description and +three annotation hints to `McpServer.registerTool` and never read +`tool.parameters`. Measured over a real `StdioServerTransport` at `74049254`, a +bridged `query_records` declaring +`{ objectName: string (required), limit: number }` was served to `tools/list` as +`inputSchema: { "type": "object", "properties": {} }` — the SDK's synthesised +empty schema, i.e. a positive claim that the tool takes no arguments — and a +`tools/call` carrying `{ objectName: 'task', limit: 5 }` reached +`toolRegistry.execute` as `input: {}`. + +The second half was invisible for the same reason as the first. The handler read +`extra.arguments`, a member `RequestHandlerExtra` does not have in any version of +the SDK this package has depended on, so it was always `undefined`; and +`McpServer.executeToolHandler()` branches on `tool.inputSchema`, invoking a +schema-less tool as `handler(extra)`. Declaring the schema is what makes the SDK +hand the call's arguments to the handler at all, so both halves are one fix. + +`AIToolDefinition.parameters` is JSON Schema and `registerTool` accepts only Zod +— a raw JSON Schema object reaches the SDK's `getZodSchemaObject()` and throws +`inputSchema must be a Zod schema or raw shape, received an unrecognized object` +— so the new `toolInputSchema()` converts it with `zod@4`'s own +`fromJSONSchema`, adding no dependency. The SDK converts the result straight back +to JSON Schema for `tools/list`; properties, types, descriptions, `required`, +enums, nested objects and `anyOf`/`oneOf` survive the round trip. + +Two consequences worth stating rather than discovering. Declaring an +`inputSchema` is also what turns on `McpServer.validateToolInput()`, which this +SDK offers no way to decline: a call whose arguments do not match the declared +schema is now answered with an `isError` result naming the offending field +instead of being executed with `{}`. And a definition whose `parameters` does not +describe an object — absent, `{}`, or untyped — is bridged with a loose empty +object, which advertises exactly what the SDK synthesised before and constrains +nothing, so a tool that genuinely declares no arguments behaves as it did. + +The docblocks were the reason this survived a reading: `bridgeTools` claimed each +tool became "an MCP tool with the same name, description, and JSON Schema +parameters", and the comment on the call claimed the schema was passed "as +annotations metadata" — through an `annotations` object that carries only +`destructiveHint` / `readOnlyHint` / `openWorldHint`, and is typed to accept +nothing else. Both now describe what the code does. From a876ebe6fb3f9d0fee59e96cf55955b569470ee1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 03:13:19 +0000 Subject: [PATCH 4/4] test(mcp): close the transport in afterEach so the pins add no test-typecheck debt --- .../src/mcp-tool-bridge-input-schema.test.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts b/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts index 8242e0b98f..f5c34c844e 100644 --- a/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts +++ b/packages/mcp/src/mcp-tool-bridge-input-schema.test.ts @@ -27,7 +27,7 @@ * about the bridge rather than about the harness. */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, afterEach } from 'vitest'; import { PassThrough } from 'node:stream'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; @@ -178,15 +178,18 @@ async function bridged(tools: AIToolDefinition[]) { // --------------------------------------------------------------------------- describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { - let session: StdioSession | undefined; + let openSession: StdioSession | undefined; - beforeEach(() => { - session = undefined; + // Teardown lives here rather than at the end of each case so a failing + // assertion still closes the transport it opened. + afterEach(async () => { + await openSession?.close(); + openSession = undefined; }); it('CONTROL: name and description reach the client', async () => { const s = await bridged([QUERY_RECORDS, LIST_OBJECTS]); - session = s.session; + openSession = s.session; const listed = (await s.session.rpc('tools/list')).result?.tools ?? []; const byName = Object.fromEntries(listed.map((t: any) => [t.name, t])); @@ -195,12 +198,11 @@ describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { expect(byName.query_records.description).toBe('Query records of an object'); expect(byName.list_objects.description).toBe('List all objects'); - await s.session.close(); }); it('forwards `parameters` as the tool inputSchema', async () => { const s = await bridged([QUERY_RECORDS]); - session = s.session; + openSession = s.session; const listed = (await s.session.rpc('tools/list')).result?.tools ?? []; const tool = listed.find((t: any) => t.name === 'query_records'); @@ -213,12 +215,11 @@ describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { expect(tool?.inputSchema?.properties?.limit).toMatchObject({ type: 'number' }); expect(tool?.inputSchema?.required).toEqual(['objectName']); - await s.session.close(); }); it('delivers the client arguments to the ToolRegistry', async () => { const s = await bridged([QUERY_RECORDS]); - session = s.session; + openSession = s.session; const called = await s.session.rpc('tools/call', { name: 'query_records', @@ -230,12 +231,11 @@ describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { expect(s.registry.calls[0].toolName).toBe('query_records'); expect(s.registry.calls[0].input).toEqual({ objectName: 'task', limit: 5 }); - await s.session.close(); }); it('CONTROL: a tool that declares no parameters still registers and still executes', async () => { const s = await bridged([LIST_OBJECTS]); - session = s.session; + openSession = s.session; const listed = (await s.session.rpc('tools/list')).result?.tools ?? []; const tool = listed.find((t: any) => t.name === 'list_objects'); @@ -247,7 +247,6 @@ describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { expect(s.registry.calls).toHaveLength(1); expect(s.registry.calls[0].input).toEqual({}); - await s.session.close(); }); /** @@ -259,7 +258,7 @@ describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { */ it('CONSEQUENCE: the SDK rejects arguments that do not match the declared schema', async () => { const s = await bridged([QUERY_RECORDS]); - session = s.session; + openSession = s.session; const called = await s.session.rpc('tools/call', { name: 'query_records', @@ -270,6 +269,5 @@ describe('bridgeTools — what an AIToolDefinition puts on the wire', () => { expect(String(called.result?.content?.[0]?.text)).toContain('objectName'); expect(s.registry.calls).toHaveLength(0); - await s.session.close(); }); });