Skip to content

Commit 5b0d7da

Browse files
kentcdoddsfelixweinberger
kentcdodds
authored and
felixweinberger
committed
feat: implement tool name validation according to SEP specification
- Add comprehensive tool name validation utility with regex patterns - Validate tool names against SEP-0001 specification requirements - Support valid characters: lowercase letters, numbers, hyphens, underscores - Enforce length limits (1-64 characters) and naming conventions - Generate appropriate warnings for non-compliant tool names - Add extensive test coverage with Jest spies for console methods - Integrate validation into McpServer.registerTool() method - Update test suite to cover all validation scenarios - Add documentation and examples for tool name validation This ensures all registered tools comply with the Model Context Protocol specification for tool naming, improving interoperability and consistency.
1 parent ce420f8 commit 5b0d7da

File tree

4 files changed

+402
-0
lines changed

4 files changed

+402
-0
lines changed

‎src/server/mcp.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ describe('ResourceTemplate', () => {
227227
});
228228

229229
describe('tool()', () => {
230+
afterEach(() => {
231+
jest.restoreAllMocks();
232+
});
233+
230234
/***
231235
* Test: Zero-Argument Tool Registration
232236
*/
@@ -1692,6 +1696,44 @@ describe('tool()', () => {
16921696
expect(result.tools[0].name).toBe('test-without-meta');
16931697
expect(result.tools[0]._meta).toBeUndefined();
16941698
});
1699+
1700+
test('should validate tool names according to SEP specification', () => {
1701+
// Create a new server instance for this test
1702+
const testServer = new McpServer({
1703+
name: 'test server',
1704+
version: '1.0',
1705+
});
1706+
1707+
// Spy on console.warn to verify warnings are logged
1708+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
1709+
1710+
// Test valid tool names
1711+
testServer.registerTool('valid-tool-name', {
1712+
description: 'A valid tool name'
1713+
}, async () => ({ content: [{ type: 'text', text: 'Success' }] }));
1714+
1715+
// Test tool name with warnings (starts with dash)
1716+
testServer.registerTool('-warning-tool', {
1717+
description: 'A tool name that generates warnings'
1718+
}, async () => ({ content: [{ type: 'text', text: 'Success' }] }));
1719+
1720+
// Test invalid tool name (contains spaces)
1721+
testServer.registerTool('invalid tool name', {
1722+
description: 'An invalid tool name'
1723+
}, async () => ({ content: [{ type: 'text', text: 'Success' }] }));
1724+
1725+
// Verify that warnings were issued (both for warnings and validation failures)
1726+
expect(warnSpy).toHaveBeenCalled();
1727+
1728+
// Verify specific warning content
1729+
const warningCalls = warnSpy.mock.calls.map(call => call.join(' '));
1730+
expect(warningCalls.some(call => call.includes('Tool name starts or ends with a dash'))).toBe(true);
1731+
expect(warningCalls.some(call => call.includes('Tool name contains spaces'))).toBe(true);
1732+
expect(warningCalls.some(call => call.includes('Tool name contains invalid characters'))).toBe(true);
1733+
1734+
// Clean up spies
1735+
warnSpy.mockRestore();
1736+
});
16951737
});
16961738

16971739
describe('resource()', () => {

‎src/server/mcp.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { Completable, CompletableDef } from './completable.js';
4040
import { UriTemplate, Variables } from '../shared/uriTemplate.js';
4141
import { RequestHandlerExtra } from '../shared/protocol.js';
4242
import { Transport } from '../shared/transport.js';
43+
import { validateAndWarnToolName } from '../shared/toolNameValidation.js';
4344

4445
/**
4546
* High-level MCP server that provides a simpler API for working with resources, tools, and prompts.
@@ -664,6 +665,9 @@ export class McpServer {
664665
_meta: Record<string, unknown> | undefined,
665666
callback: ToolCallback<ZodRawShape | undefined>
666667
): RegisteredTool {
668+
// Validate tool name according to SEP specification
669+
validateAndWarnToolName(name);
670+
667671
const registeredTool: RegisteredTool = {
668672
title,
669673
description,
@@ -678,6 +682,9 @@ export class McpServer {
678682
remove: () => registeredTool.update({ name: null }),
679683
update: updates => {
680684
if (typeof updates.name !== 'undefined' && updates.name !== name) {
685+
if (typeof updates.name === 'string') {
686+
validateAndWarnToolName(updates.name);
687+
}
681688
delete this._registeredTools[name];
682689
if (updates.name) this._registeredTools[updates.name] = registeredTool;
683690
}

‎src/shared/toolNameValidation.test.ts‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
import { validateToolName, validateAndWarnToolName, issueToolNameWarning } from './toolNameValidation.js';
2+
3+
// Spy on console.warn to capture output
4+
let warnSpy: jest.SpyInstance;
5+
6+
beforeEach(() => {
7+
warnSpy = jest.spyOn(console, 'warn').mockImplementation();
8+
});
9+
10+
afterEach(() => {
11+
jest.restoreAllMocks();
12+
});
13+
14+
describe('validateToolName', () => {
15+
describe('valid tool names', () => {
16+
test('should accept simple alphanumeric names', () => {
17+
const result = validateToolName('getUser');
18+
expect(result.isValid).toBe(true);
19+
expect(result.warnings).toHaveLength(0);
20+
});
21+
22+
test('should accept names with underscores', () => {
23+
const result = validateToolName('get_user_profile');
24+
expect(result.isValid).toBe(true);
25+
expect(result.warnings).toHaveLength(0);
26+
});
27+
28+
test('should accept names with dashes', () => {
29+
const result = validateToolName('user-profile-update');
30+
expect(result.isValid).toBe(true);
31+
expect(result.warnings).toHaveLength(0);
32+
});
33+
34+
test('should accept names with dots', () => {
35+
const result = validateToolName('admin.tools.list');
36+
expect(result.isValid).toBe(true);
37+
expect(result.warnings).toHaveLength(0);
38+
});
39+
40+
test('should accept names with forward slashes', () => {
41+
const result = validateToolName('user/profile/update');
42+
expect(result.isValid).toBe(true);
43+
expect(result.warnings).toHaveLength(0);
44+
});
45+
46+
test('should accept mixed character names', () => {
47+
const result = validateToolName('DATA_EXPORT_v2.1');
48+
expect(result.isValid).toBe(true);
49+
expect(result.warnings).toHaveLength(0);
50+
});
51+
52+
test('should accept single character names', () => {
53+
const result = validateToolName('a');
54+
expect(result.isValid).toBe(true);
55+
expect(result.warnings).toHaveLength(0);
56+
});
57+
58+
test('should accept 128 character names', () => {
59+
const name = 'a'.repeat(128);
60+
const result = validateToolName(name);
61+
expect(result.isValid).toBe(true);
62+
expect(result.warnings).toHaveLength(0);
63+
});
64+
});
65+
66+
describe('invalid tool names', () => {
67+
test('should reject empty names', () => {
68+
const result = validateToolName('');
69+
expect(result.isValid).toBe(false);
70+
expect(result.warnings).toContain('Tool name cannot be empty');
71+
});
72+
73+
test('should reject names longer than 128 characters', () => {
74+
const name = 'a'.repeat(129);
75+
const result = validateToolName(name);
76+
expect(result.isValid).toBe(false);
77+
expect(result.warnings).toContain('Tool name exceeds maximum length of 128 characters (current: 129)');
78+
});
79+
80+
test('should reject names with spaces', () => {
81+
const result = validateToolName('get user profile');
82+
expect(result.isValid).toBe(false);
83+
expect(result.warnings).toContain('Tool name contains invalid characters: " "');
84+
});
85+
86+
test('should reject names with commas', () => {
87+
const result = validateToolName('get,user,profile');
88+
expect(result.isValid).toBe(false);
89+
expect(result.warnings).toContain('Tool name contains invalid characters: ","');
90+
});
91+
92+
test('should reject names with other special characters', () => {
93+
const result = validateToolName('user@domain.com');
94+
expect(result.isValid).toBe(false);
95+
expect(result.warnings).toContain('Tool name contains invalid characters: "@"');
96+
});
97+
98+
test('should reject names with multiple invalid characters', () => {
99+
const result = validateToolName('user name@domain,com');
100+
expect(result.isValid).toBe(false);
101+
expect(result.warnings).toContain('Tool name contains invalid characters: " ", "@", ","');
102+
});
103+
104+
test('should reject names with unicode characters', () => {
105+
const result = validateToolName('user-ñame');
106+
expect(result.isValid).toBe(false);
107+
expect(result.warnings).toContain('Tool name contains invalid characters: "ñ"');
108+
});
109+
});
110+
111+
describe('warnings for potentially problematic patterns', () => {
112+
test('should warn about names with spaces', () => {
113+
const result = validateToolName('get user profile');
114+
expect(result.isValid).toBe(false);
115+
expect(result.warnings).toContain('Tool name contains spaces, which may cause parsing issues');
116+
});
117+
118+
test('should warn about names with commas', () => {
119+
const result = validateToolName('get,user,profile');
120+
expect(result.isValid).toBe(false);
121+
expect(result.warnings).toContain('Tool name contains commas, which may cause parsing issues');
122+
});
123+
124+
test('should warn about names starting with dash', () => {
125+
const result = validateToolName('-get-user');
126+
expect(result.isValid).toBe(true);
127+
expect(result.warnings).toContain('Tool name starts or ends with a dash, which may cause parsing issues in some contexts');
128+
});
129+
130+
test('should warn about names ending with dash', () => {
131+
const result = validateToolName('get-user-');
132+
expect(result.isValid).toBe(true);
133+
expect(result.warnings).toContain('Tool name starts or ends with a dash, which may cause parsing issues in some contexts');
134+
});
135+
136+
test('should warn about names starting with dot', () => {
137+
const result = validateToolName('.get.user');
138+
expect(result.isValid).toBe(true);
139+
expect(result.warnings).toContain('Tool name starts or ends with a dot, which may cause parsing issues in some contexts');
140+
});
141+
142+
test('should warn about names ending with dot', () => {
143+
const result = validateToolName('get.user.');
144+
expect(result.isValid).toBe(true);
145+
expect(result.warnings).toContain('Tool name starts or ends with a dot, which may cause parsing issues in some contexts');
146+
});
147+
148+
test('should warn about names with both leading and trailing dots', () => {
149+
const result = validateToolName('.get.user.');
150+
expect(result.isValid).toBe(true);
151+
expect(result.warnings).toContain('Tool name starts or ends with a dot, which may cause parsing issues in some contexts');
152+
});
153+
});
154+
});
155+
156+
describe('issueToolNameWarning', () => {
157+
test('should output warnings to console.warn', () => {
158+
const warnings = ['Warning 1', 'Warning 2'];
159+
issueToolNameWarning('test-tool', warnings);
160+
161+
expect(warnSpy).toHaveBeenCalledTimes(6); // Header + 2 warnings + 3 guidance lines
162+
const calls = warnSpy.mock.calls.map(call => call.join(' '));
163+
expect(calls[0]).toContain('Tool name validation warning for "test-tool"');
164+
expect(calls[1]).toContain('- Warning 1');
165+
expect(calls[2]).toContain('- Warning 2');
166+
expect(calls[3]).toContain('Tool registration will proceed, but this may cause compatibility issues.');
167+
expect(calls[4]).toContain('Consider updating the tool name');
168+
expect(calls[5]).toContain('See SEP: Specify Format for Tool Names');
169+
});
170+
171+
test('should handle empty warnings array', () => {
172+
issueToolNameWarning('test-tool', []);
173+
expect(warnSpy).toHaveBeenCalledTimes(0);
174+
});
175+
});
176+
177+
describe('validateAndWarnToolName', () => {
178+
test('should return true and issue warnings for valid names with warnings', () => {
179+
const result = validateAndWarnToolName('-get-user-');
180+
expect(result).toBe(true);
181+
expect(warnSpy).toHaveBeenCalled();
182+
});
183+
184+
test('should return true and not issue warnings for completely valid names', () => {
185+
const result = validateAndWarnToolName('get-user-profile');
186+
expect(result).toBe(true);
187+
expect(warnSpy).not.toHaveBeenCalled();
188+
});
189+
190+
test('should return false and issue warnings for invalid names', () => {
191+
const result = validateAndWarnToolName('get user profile');
192+
expect(result).toBe(false);
193+
expect(warnSpy).toHaveBeenCalled(); // Now issues warnings instead of errors
194+
const warningCalls = warnSpy.mock.calls.map(call => call.join(' '));
195+
expect(warningCalls.some(call => call.includes('Tool name contains spaces'))).toBe(true);
196+
});
197+
198+
test('should return false for empty names', () => {
199+
const result = validateAndWarnToolName('');
200+
expect(result).toBe(false);
201+
expect(warnSpy).toHaveBeenCalled(); // Now issues warnings instead of errors
202+
});
203+
204+
test('should return false for names exceeding length limit', () => {
205+
const longName = 'a'.repeat(129);
206+
const result = validateAndWarnToolName(longName);
207+
expect(result).toBe(false);
208+
expect(warnSpy).toHaveBeenCalled(); // Now issues warnings instead of errors
209+
});
210+
});
211+
212+
describe('edge cases and robustness', () => {
213+
test('should warn about names with only dots', () => {
214+
const result = validateToolName('...');
215+
expect(result.isValid).toBe(true);
216+
expect(result.warnings).toContain('Tool name starts or ends with a dot, which may cause parsing issues in some contexts');
217+
});
218+
219+
test('should handle names with only dashes', () => {
220+
const result = validateToolName('---');
221+
expect(result.isValid).toBe(true);
222+
expect(result.warnings).toContain('Tool name starts or ends with a dash, which may cause parsing issues in some contexts');
223+
});
224+
225+
test('should warn about names with only forward slashes', () => {
226+
const result = validateToolName('///');
227+
expect(result.isValid).toBe(true);
228+
expect(result.warnings).toContain('Tool name starts or ends with a slash, which may cause parsing issues in some contexts');
229+
});
230+
231+
test('should handle names with mixed valid and invalid characters', () => {
232+
const result = validateToolName('user@name123');
233+
expect(result.isValid).toBe(false);
234+
expect(result.warnings).toContain('Tool name contains invalid characters: "@"');
235+
});
236+
});

0 commit comments

Comments
 (0)