diff --git a/examples/servers/typescript/invalid-tool-names.ts b/examples/servers/typescript/invalid-tool-names.ts new file mode 100644 index 00000000..20d4cd1a --- /dev/null +++ b/examples/servers/typescript/invalid-tool-names.ts @@ -0,0 +1,91 @@ +#!/usr/bin/env node + +/** + * Negative test server for spec Tool Names SHOULD rules (2025-11-25+). + * + * AGENTS.md negative-fixture pattern: bypass SDK registerTool validation via + * setRequestHandler(ListToolsRequestSchema) so tools/list advertises a name + * that violates core spec prose at #tool-names. Proves tools-name-format emits + * WARNING (SHOULD-level per AGENTS.md), not FAILURE. everything-server unchanged. + * + * ## Why this file is not named after SEP-986 + * + * Tool name format rules trace to [SEP-986](https://github.com/modelcontextprotocol/modelcontextprotocol/issues/986) + * (opened 2025-07-16), but the **authoritative rules live in dated spec prose**, + * not the SEP markdown artifact. That split is a process wart worth preserving: + * + * - **SEP-986 markdown** (`seps/986-specify-format-for-tool-names.md`) still + * documents stale rules: 1–64 chars, `[A-Za-z0-9_./-]` including `/`. The + * finalized SEP file was **never updated** after spec integration. + * - **Integrated spec** ([PR #1603](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/1603), + * Oct 2025) landed different prose in draft, then **2025-11-25** `#tool-names`: + * 1–128 chars, `[A-Za-z0-9_.-]` only (no `/`). + * - **Conformance #240** incorrectly encoded the stale SEP markdown (64 + `/`, + * FAILURE severity). This suite follows the integrated spec diff per AGENTS.md; + * see `tools.ts` block comment and `specReferences` (core URLs first, SEP links + * context-only). There is intentionally **no `sep-986.yaml`** traceability row. + * + * Naming the fixture after the SEP would imply traceability we deliberately + * declined — and would obscure that the SEP artifact and published spec diverged. + */ + +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 583c5ebd..511bdad5 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'; function startServer(scriptPath: string, port: number): Promise { @@ -211,6 +212,42 @@ describe('Server scenario negative tests', () => { }, 10000); }); + describe('tools-name-format', () => { + // AGENTS.md: negative vitest pins tools-name-format → WARNING, not failures.length. + 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..2804f788 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,86 @@ 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', () => { + // Locks version gate: no tools-name-format on 2025-03-26 / 2025-06-18 (AGENTS.md). + 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 88464859..aebe5142 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,61 @@ import { ElicitRequestSchema } from '@modelcontextprotocol/sdk/types.js'; -const TOOL_NAME_PATTERN = /^[A-Za-z0-9_./-]+$/; -const TOOL_NAME_MAX_LENGTH = 64; +/** + * Tool name format validation tracks dated MCP spec prose (2025-11-25+ + * `#tool-names`), not the SEP-986 markdown file. There is no sep-986.yaml + * traceability row — the requirement lives in the core specification. + * + * specReferences order: core spec URLs first (authoritative), then SEP/history + * links for context only (SEP-986 markdown still documents stale 64 + `/` rules). + * + * Divergence to preserve when updating this check: + * - SEP-986 markdown (modelcontextprotocol#986): 1–64 chars, `[A-Za-z0-9_./-]` + * including `/` — never updated after spec integration. + * - Published spec (PR modelcontextprotocol#1603): 1–128 chars, + * `[A-Za-z0-9_.-]` only (no `/`). + * + * Conformance #240 incorrectly encoded the stale SEP rules; this check follows + * the integrated spec diff per AGENTS.md. + */ +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: 'SEP-986', - 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' + }, + // Context only — not the rule source. SEP markdown was never updated to match PR #1603. + { + 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' } ]; +/** + * Tool Names SHOULD rules apply only from 2025-11-25 spec prose onward. + * tools-list stays introducedIn 2025-06-18 for structural MUST checks; gating + * here avoids false signals on versions that never had Tool Names prose. + */ +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 +89,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 +124,11 @@ export function buildToolsNameFormatCheck( return { ...baseCheck, - status: violations.length === 0 ? 'SUCCESS' : 'FAILURE', + // AGENTS.md: SHOULD in spec prose → WARNING (Tier-1 CI still treats as 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 +149,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 +199,12 @@ 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)); + // Gate per AGENTS.md version applicability: Tool Names prose is 2025-11-25+ only. + // everything-server already passes on applicable versions (positive path in + // all-scenarios.test.ts); failure proof is invalid-tool-names.ts. + if (toolNameFormatCheckApplies(ctx.specVersion)) { + checks.push(buildToolsNameFormatCheck(result.tools)); + } await conn.close(); } catch (error) {