From b44a31cc0defe418b2ea9ec2d4cfcf5658d9c8a9 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 20 Apr 2026 20:52:36 +0800 Subject: [PATCH] refactor(mcp): extract buildToolDefs helper for subagent tool registry reuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline operations.map(...) block in src/mcp/server.ts became the only source of truth for agent-facing tool definitions. Extract into a reusable exported helper so the v0.15 subagent tool registry can call it with a filtered OPERATIONS subset instead of duplicating the shape. Byte-for-byte equivalence regression pinned in test/mcp-tool-defs.test.ts — legacy inline mapping kept verbatim inside the test so any future drift between the new helper and the pre-extraction MCP schema fails loudly. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/mcp/server.ts | 24 +++----------- src/mcp/tool-defs.ts | 32 ++++++++++++++++++ test/mcp-tool-defs.test.ts | 67 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 src/mcp/tool-defs.ts create mode 100644 test/mcp-tool-defs.test.ts diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 2072796d8..c9a3260d2 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -6,6 +6,7 @@ import { operations, OperationError } from '../core/operations.ts'; import type { Operation, OperationContext } from '../core/operations.ts'; import { loadConfig } from '../core/config.ts'; import { VERSION } from '../version.ts'; +import { buildToolDefs } from './tool-defs.ts'; /** Validate required params exist and have the expected type */ function validateParams(op: Operation, params: Record): string | null { @@ -32,26 +33,11 @@ export async function startMcpServer(engine: BrainEngine) { { capabilities: { tools: {} } }, ); - // Generate tool definitions from operations + // Generate tool definitions from operations. Extracted to buildToolDefs so + // the subagent tool registry (v0.15+) can call the same mapper against a + // filtered OPERATIONS subset instead of duplicating this shape. server.setRequestHandler(ListToolsRequestSchema, async () => ({ - tools: operations.map(op => ({ - name: op.name, - description: op.description, - inputSchema: { - type: 'object' as const, - properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), - ), - required: Object.entries(op.params) - .filter(([, v]) => v.required) - .map(([k]) => k), - }, - })), + tools: buildToolDefs(operations), })); // Dispatch tool calls to operation handlers diff --git a/src/mcp/tool-defs.ts b/src/mcp/tool-defs.ts new file mode 100644 index 000000000..59ecf80a4 --- /dev/null +++ b/src/mcp/tool-defs.ts @@ -0,0 +1,32 @@ +import type { Operation } from '../core/operations.ts'; + +export interface McpToolDef { + name: string; + description: string; + inputSchema: { + type: 'object'; + properties: Record; + required: string[]; + }; +} + +export function buildToolDefs(ops: Operation[]): McpToolDef[] { + return ops.map(op => ({ + name: op.name, + description: op.description, + inputSchema: { + type: 'object' as const, + properties: Object.fromEntries( + Object.entries(op.params).map(([k, v]) => [k, { + type: v.type === 'array' ? 'array' : v.type, + ...(v.description ? { description: v.description } : {}), + ...(v.enum ? { enum: v.enum } : {}), + ...(v.items ? { items: { type: v.items.type } } : {}), + }]), + ), + required: Object.entries(op.params) + .filter(([, v]) => v.required) + .map(([k]) => k), + }, + })); +} diff --git a/test/mcp-tool-defs.test.ts b/test/mcp-tool-defs.test.ts new file mode 100644 index 000000000..41a6954d6 --- /dev/null +++ b/test/mcp-tool-defs.test.ts @@ -0,0 +1,67 @@ +/** + * Regression test for the MCP tool-def extraction (v0.15.0 Lane 1A). + * + * Before v0.15 the mapping lived inline in src/mcp/server.ts. After the + * extraction, buildToolDefs is the single source of truth; the subagent tool + * registry calls it with a filtered OPERATIONS subset. This test pins the + * extracted output to the pre-extraction shape byte-for-byte so we don't + * silently drift the MCP-facing tool schema. + */ + +import { describe, test, expect } from 'bun:test'; +import { operations } from '../src/core/operations.ts'; +import { buildToolDefs } from '../src/mcp/tool-defs.ts'; + +// Pre-extraction inline shape — lifted verbatim from the original +// src/mcp/server.ts block so any future drift fails this test loudly. +function legacyInlineMap(ops: typeof operations) { + return ops.map(op => ({ + name: op.name, + description: op.description, + inputSchema: { + type: 'object' as const, + properties: Object.fromEntries( + Object.entries(op.params).map(([k, v]) => [k, { + type: v.type === 'array' ? 'array' : v.type, + ...(v.description ? { description: v.description } : {}), + ...(v.enum ? { enum: v.enum } : {}), + ...(v.items ? { items: { type: v.items.type } } : {}), + }]), + ), + required: Object.entries(op.params) + .filter(([, v]) => v.required) + .map(([k]) => k), + }, + })); +} + +describe('buildToolDefs', () => { + test('output equals pre-extraction inline mapping byte-for-byte', () => { + const extracted = buildToolDefs(operations); + const inline = legacyInlineMap(operations); + expect(JSON.stringify(extracted)).toBe(JSON.stringify(inline)); + }); + + test('preserves operation count', () => { + expect(buildToolDefs(operations).length).toBe(operations.length); + }); + + test('accepts an arbitrary Operation subset (for subagent tool registry)', () => { + const subset = operations.slice(0, 3); + const defs = buildToolDefs(subset); + expect(defs.length).toBe(3); + expect(defs.map(d => d.name)).toEqual(subset.map(o => o.name)); + }); + + test('empty input returns empty array', () => { + expect(buildToolDefs([])).toEqual([]); + }); + + test('every def has object inputSchema with properties + required array', () => { + for (const def of buildToolDefs(operations)) { + expect(def.inputSchema.type).toBe('object'); + expect(typeof def.inputSchema.properties).toBe('object'); + expect(Array.isArray(def.inputSchema.required)).toBe(true); + } + }); +});