diff --git a/examples/servers/typescript/invalid-tool-names.ts b/examples/servers/typescript/invalid-tool-names.ts new file mode 100644 index 00000000..a71925d9 --- /dev/null +++ b/examples/servers/typescript/invalid-tool-names.ts @@ -0,0 +1,68 @@ +#!/usr/bin/env node + +/** + * Negative fixture for the tools-name-format check: advertises tool names that + * violate the 2025-11-25 Tool Names rules (spec #tool-names) by bypassing the + * SDK's registerTool validation, so the check can be shown to emit WARNING. + */ + +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import { ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js'; +import express from 'express'; + +function createServer() { + const server = new Server( + { name: 'invalid-tool-names', version: '1.0.0' }, + { capabilities: { tools: {} } } + ); + + server.setRequestHandler(ListToolsRequestSchema, async () => ({ + tools: [ + { + name: 'bad tool name', + description: 'Deliberately invalid tool name for conformance testing', + inputSchema: { type: 'object' } + }, + { + name: 'valid_tool', + description: 'A conformant tool name', + inputSchema: { type: 'object' } + } + ] + })); + + return server; +} + +const app = express(); +app.use(express.json()); + +app.post('/mcp', async (req, res) => { + try { + const server = createServer(); + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: undefined + }); + await server.connect(transport); + await transport.handleRequest(req, res, req.body); + } catch (error) { + if (!res.headersSent) { + res.status(500).json({ + jsonrpc: '2.0', + error: { + code: -32603, + message: `Internal error: ${error instanceof Error ? error.message : String(error)}` + }, + id: null + }); + } + } +}); + +const PORT = parseInt(process.env.PORT || '3009', 10); +app.listen(PORT, '127.0.0.1', () => { + console.log( + `Invalid tool names negative test server running on http://localhost:${PORT}/mcp` + ); +}); diff --git a/src/scenarios/server/negative.test.ts b/src/scenarios/server/negative.test.ts index 0d248272..f70aa90d 100644 --- a/src/scenarios/server/negative.test.ts +++ b/src/scenarios/server/negative.test.ts @@ -8,6 +8,7 @@ import { JsonSchema2020_12Scenario, sep2106KeywordCheckStatus } from './json-schema-2020-12'; +import { ToolsListScenario } from './tools'; import { DRAFT_PROTOCOL_VERSION, LATEST_SPEC_VERSION } from '../../types'; import { takeWireViolations } from '../../validation/wire-schema'; @@ -216,6 +217,41 @@ describe('Server scenario negative tests', () => { }, 10000); }); + describe('tools-name-format', () => { + let serverProcess: ChildProcess | null = null; + const PORT = 3009; + + beforeAll(async () => { + serverProcess = await startServer( + path.join( + process.cwd(), + 'examples/servers/typescript/invalid-tool-names.ts' + ), + PORT + ); + }, 35000); + + afterAll(async () => { + await stopServer(serverProcess); + }); + + it('emits WARNING for tools-name-format against a server advertising invalid tool names', async () => { + const scenario = new ToolsListScenario(); + const checks = await scenario.run( + testContext(`http://localhost:${PORT}/mcp`, '2025-11-25') + ); + + const formatCheck = checks.find((c) => c.id === 'tools-name-format'); + expect(formatCheck?.status).toBe('WARNING'); + expect(formatCheck?.errorMessage).toContain('bad tool name'); + expect(formatCheck?.details).toMatchObject({ + results: expect.objectContaining({ + 'bad tool name': expect.stringMatching(/invalid:/) + }) + }); + }, 10000); + }); + describe('sep2106KeywordCheckStatus (soft version gate)', () => { it('passes preserved keywords at any target version', () => { expect(sep2106KeywordCheckStatus(true, DRAFT_PROTOCOL_VERSION)).toBe( diff --git a/src/scenarios/server/tools.test.ts b/src/scenarios/server/tools.test.ts index d54daa4c..b1fe0f77 100644 --- a/src/scenarios/server/tools.test.ts +++ b/src/scenarios/server/tools.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from 'vitest'; -import { buildToolsNameFormatCheck, validateToolNameFormat } from './tools.js'; +import { + buildToolsNameFormatCheck, + toolNameFormatCheckApplies, + ToolsListScenario, + validateToolNameFormat +} from './tools.js'; +import type { Connection, RunContext } from '../../connection'; describe('validateToolNameFormat', () => { it('accepts a typical snake_case name', () => { @@ -7,24 +13,30 @@ describe('validateToolNameFormat', () => { }); it('accepts all allowed character classes', () => { - expect(validateToolNameFormat('Aa0_.-/')).toBeNull(); + expect(validateToolNameFormat('Aa0_.-')).toBeNull(); }); it('accepts a single-character name (lower length boundary)', () => { expect(validateToolNameFormat('a')).toBeNull(); }); - it('accepts a 64-character name (upper length boundary)', () => { - expect(validateToolNameFormat('a'.repeat(64))).toBeNull(); + it('accepts a 128-character name (upper length boundary)', () => { + expect(validateToolNameFormat('a'.repeat(128))).toBeNull(); }); it('rejects an empty name', () => { expect(validateToolNameFormat('')).toMatch(/length 0 is outside/); }); - it('rejects a 65-character name', () => { - expect(validateToolNameFormat('a'.repeat(65))).toMatch( - /length 65 is outside/ + it('rejects a 129-character name', () => { + expect(validateToolNameFormat('a'.repeat(129))).toMatch( + /length 129 is outside/ + ); + }); + + it('rejects forward slash (allowed in SEP-986 markdown but not spec prose)', () => { + expect(validateToolNameFormat('namespace/tool')).toMatch( + /contains characters outside/ ); }); @@ -57,7 +69,7 @@ describe('buildToolsNameFormatCheck', () => { it('returns SUCCESS when all tool names are valid', () => { const check = buildToolsNameFormatCheck([ { name: 'test_simple_text' }, - { name: 'namespace/tool-v1.2' } + { name: 'admin.tools.list' } ]); expect(check.status).toBe('SUCCESS'); expect(check.errorMessage).toBeUndefined(); @@ -65,33 +77,33 @@ describe('buildToolsNameFormatCheck', () => { toolCount: 2, results: { test_simple_text: 'valid', - 'namespace/tool-v1.2': 'valid' + 'admin.tools.list': 'valid' } }); }); - it('returns FAILURE with per-tool details when some names are invalid', () => { + it('returns WARNING with per-tool details when some names are invalid', () => { const check = buildToolsNameFormatCheck([ { name: 'good_tool' }, { name: 'bad name with spaces' }, - { name: 'a'.repeat(65) } + { name: 'a'.repeat(129) } ]); - expect(check.status).toBe('FAILURE'); + expect(check.status).toBe('WARNING'); expect(check.errorMessage).toContain( - '2 tool name(s) violate SEP-986 format' + '2 tool name(s) violate spec Tool Names SHOULD rules' ); const results = (check.details as { results: Record }) .results; expect(results['good_tool']).toBe('valid'); expect(results['bad name with spaces']).toMatch(/^invalid: /); - expect(results['a'.repeat(65)]).toMatch(/^invalid: length 65/); + expect(results['a'.repeat(129)]).toMatch(/^invalid: length 129/); }); it('flags a tool whose name is not a string', () => { const check = buildToolsNameFormatCheck([{ name: 123 as unknown }]); - expect(check.status).toBe('FAILURE'); + expect(check.status).toBe('WARNING'); const results = (check.details as { results: Record }) .results; expect(results['']).toBe( @@ -99,9 +111,85 @@ describe('buildToolsNameFormatCheck', () => { ); }); - it('includes both MCP-Tools-List and SEP-986 spec references', () => { + it('lists core spec Tool Names first, then SEP history for context', () => { const check = buildToolsNameFormatCheck([{ name: 'ok' }]); const ids = check.specReferences?.map((r) => r.id); - expect(ids).toEqual(['MCP-Tools-List', 'SEP-986']); + expect(ids).toEqual([ + 'MCP-Tool-Names', + 'MCP-Tool-Names-Draft', + 'SEP-986-History', + 'SEP-986-Spec-Integration' + ]); + expect(check.specReferences?.[0]?.url).toContain( + '2025-11-25/server/tools#tool-names' + ); + expect(check.specReferences?.[1]?.url).toContain( + 'draft/server/tools#tool-names' + ); + expect(check.specReferences?.[2]?.url).toContain('issues/986'); + expect(check.specReferences?.[3]?.url).toContain('pull/1603'); + }); +}); + +describe('toolNameFormatCheckApplies', () => { + it('is false before 2025-11-25 and true from that version onward', () => { + expect(toolNameFormatCheckApplies('2025-03-26')).toBe(false); + expect(toolNameFormatCheckApplies('2025-06-18')).toBe(false); + expect(toolNameFormatCheckApplies('2025-11-25')).toBe(true); + expect(toolNameFormatCheckApplies('2026-07-28')).toBe(true); + }); +}); + +describe('ToolsListScenario version gate', () => { + // Scenario-level gate: invalid names must not emit the check before 2025-11-25. + function mockContext( + specVersion: RunContext['specVersion'], + tools: Array<{ name: string; description: string; inputSchema: object }> + ): RunContext { + const connection: Connection = { + notifications: [], + request: (async () => ({ tools })) as Connection['request'], + discover: async () => ({}), + close: async () => {} + }; + + return { + serverUrl: 'http://example.test/mcp', + specVersion, + connect: async () => connection + }; + } + + it('does not emit tools-name-format on 2025-06-18 even when names violate 2025-11-25 rules', async () => { + const scenario = new ToolsListScenario(); + const checks = await scenario.run( + mockContext('2025-06-18', [ + { + name: 'bad name', + description: 'invalid under 2025-11-25 rules', + inputSchema: { type: 'object' } + } + ]) + ); + + expect(checks.some((c) => c.id === 'tools-name-format')).toBe(false); + expect(checks.find((c) => c.id === 'tools-list')?.status).toBe('SUCCESS'); + }); + + it('emits tools-name-format on 2025-11-25 when names violate spec prose', async () => { + const scenario = new ToolsListScenario(); + const checks = await scenario.run( + mockContext('2025-11-25', [ + { + name: 'bad name', + description: 'invalid under 2025-11-25 rules', + inputSchema: { type: 'object' } + } + ]) + ); + + expect(checks.find((c) => c.id === 'tools-name-format')?.status).toBe( + 'WARNING' + ); }); }); diff --git a/src/scenarios/server/tools.ts b/src/scenarios/server/tools.ts index f18df3cf..943681b4 100644 --- a/src/scenarios/server/tools.ts +++ b/src/scenarios/server/tools.ts @@ -5,7 +5,9 @@ import { ClientScenario, ConformanceCheck, - DRAFT_PROTOCOL_VERSION + DRAFT_PROTOCOL_VERSION, + specVersionAtLeast, + type SpecVersion } from '../../types'; import type { RunContext } from '../../connection'; import type { @@ -21,26 +23,45 @@ import { ElicitRequestSchema } from '@modelcontextprotocol/sdk/types.js'; -const TOOL_NAME_PATTERN = /^[A-Za-z0-9_./-]+$/; -const TOOL_NAME_MAX_LENGTH = 64; +/** + * Tool name rules per the 2025-11-25 spec prose (#tool-names): 1–128 chars of + * [A-Za-z0-9_.-]. (The SEP-986 markdown still shows the older 64-char / `/` + * rules; the published spec is authoritative.) + */ +const TOOL_NAME_PATTERN = /^[A-Za-z0-9_.-]+$/; +const TOOL_NAME_MAX_LENGTH = 128; const TOOLS_NAME_FORMAT_SPEC_REFS = [ { - id: 'MCP-Tools-List', - url: 'https://modelcontextprotocol.io/specification/2025-11-25/server/tools#listing-tools' + id: 'MCP-Tool-Names', + url: 'https://modelcontextprotocol.io/specification/2025-11-25/server/tools#tool-names' + }, + { + id: 'MCP-Tool-Names-Draft', + url: 'https://modelcontextprotocol.io/specification/draft/server/tools#tool-names' }, + // Background only; the dated spec above is the rule source. { - id: 'SEP-986', - url: 'https://modelcontextprotocol.io/specification/2025-11-25/server/tools#tool-names' + id: 'SEP-986-History', + url: 'https://github.com/modelcontextprotocol/modelcontextprotocol/issues/986' + }, + { + id: 'SEP-986-Spec-Integration', + url: 'https://github.com/modelcontextprotocol/modelcontextprotocol/pull/1603' } ]; +/** The Tool Names rules first appear in the 2025-11-25 revision. */ +export function toolNameFormatCheckApplies(specVersion: SpecVersion): boolean { + return specVersionAtLeast(specVersion, '2025-11-25'); +} + export function validateToolNameFormat(name: string): string | null { if (name.length < 1 || name.length > TOOL_NAME_MAX_LENGTH) { return `length ${name.length} is outside 1-${TOOL_NAME_MAX_LENGTH}`; } if (!TOOL_NAME_PATTERN.test(name)) { - return 'contains characters outside [A-Za-z0-9_./-]'; + return 'contains characters outside [A-Za-z0-9_.-]'; } return null; } @@ -52,7 +73,8 @@ export function buildToolsNameFormatCheck( const baseCheck = { id: 'tools-name-format', name: 'ToolsNameFormat', - description: 'Tool names are 1-64 characters and match ^[A-Za-z0-9_./-]+$', + description: + 'Tool names SHOULD be 1-128 characters and match ^[A-Za-z0-9_.-]+$', specReferences: TOOLS_NAME_FORMAT_SPEC_REFS, timestamp }; @@ -86,10 +108,10 @@ export function buildToolsNameFormatCheck( return { ...baseCheck, - status: violations.length === 0 ? 'SUCCESS' : 'FAILURE', + status: violations.length === 0 ? 'SUCCESS' : 'WARNING', errorMessage: violations.length > 0 - ? `${violations.length} tool name(s) violate SEP-986 format: ${violations.join('; ')}` + ? `${violations.length} tool name(s) violate spec Tool Names SHOULD rules: ${violations.join('; ')}` : undefined, details: { toolCount: tools.length, @@ -110,9 +132,10 @@ export class ToolsListScenario implements ClientScenario { **Requirements**: - Return array of all available tools - Each tool MUST have: - - \`name\` (string, 1-64 chars, matching \`^[A-Za-z0-9_./-]+$\`) + - \`name\` (string) - \`description\` (string) - - \`inputSchema\` (valid JSON Schema object)`; + - \`inputSchema\` (valid JSON Schema object) +- From 2025-11-25 onward, advertised \`name\` values SHOULD follow the spec Tool Names rules (1–128 chars, \`[A-Za-z0-9_.-]\` only) — see \`tools-name-format\` check`; async run(ctx: RunContext): Promise { const checks: ConformanceCheck[] = []; @@ -159,9 +182,9 @@ export class ToolsListScenario implements ClientScenario { } }); - // Validate tool name format per SEP-986: - // names MUST be 1-64 chars matching ^[A-Za-z0-9_./-]+$ - checks.push(buildToolsNameFormatCheck(result.tools)); + if (toolNameFormatCheckApplies(ctx.specVersion)) { + checks.push(buildToolsNameFormatCheck(result.tools)); + } await conn.close(); } catch (error) {