Skip to content

Commit 29cb080

Browse files
kentcdoddsfelixweinberger
kentcdodds
and
felixweinberger
authored
Implementation of SEP-986: Specify Format for Tool Names (#900)
Co-authored-by: Felix Weinberger <[email protected]> Co-authored-by: Felix Weinberger <[email protected]>
1 parent 9f06b6b commit 29cb080

File tree

4 files changed

+303
-0
lines changed

4 files changed

+303
-0
lines changed

‎src/server/mcp.test.ts‎

Lines changed: 54 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,56 @@ 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(
1712+
'valid-tool-name',
1713+
{
1714+
description: 'A valid tool name'
1715+
},
1716+
async () => ({ content: [{ type: 'text' as const, text: 'Success' }] })
1717+
);
1718+
1719+
// Test tool name with warnings (starts with dash)
1720+
testServer.registerTool(
1721+
'-warning-tool',
1722+
{
1723+
description: 'A tool name that generates warnings'
1724+
},
1725+
async () => ({ content: [{ type: 'text' as const, text: 'Success' }] })
1726+
);
1727+
1728+
// Test invalid tool name (contains spaces)
1729+
testServer.registerTool(
1730+
'invalid tool name',
1731+
{
1732+
description: 'An invalid tool name'
1733+
},
1734+
async () => ({ content: [{ type: 'text' as const, text: 'Success' }] })
1735+
);
1736+
1737+
// Verify that warnings were issued (both for warnings and validation failures)
1738+
expect(warnSpy).toHaveBeenCalled();
1739+
1740+
// Verify specific warning content
1741+
const warningCalls = warnSpy.mock.calls.map(call => call.join(' '));
1742+
expect(warningCalls.some(call => call.includes('Tool name starts or ends with a dash'))).toBe(true);
1743+
expect(warningCalls.some(call => call.includes('Tool name contains spaces'))).toBe(true);
1744+
expect(warningCalls.some(call => call.includes('Tool name contains invalid characters'))).toBe(true);
1745+
1746+
// Clean up spies
1747+
warnSpy.mockRestore();
1748+
});
16951749
});
16961750

16971751
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: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
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.each`
17+
description | toolName
18+
${'simple alphanumeric names'} | ${'getUser'}
19+
${'names with underscores'} | ${'get_user_profile'}
20+
${'names with dashes'} | ${'user-profile-update'}
21+
${'names with dots'} | ${'admin.tools.list'}
22+
${'mixed character names'} | ${'DATA_EXPORT_v2.1'}
23+
${'single character names'} | ${'a'}
24+
${'128 character names'} | ${'a'.repeat(128)}
25+
`('should accept $description', ({ toolName }) => {
26+
const result = validateToolName(toolName);
27+
expect(result.isValid).toBe(true);
28+
expect(result.warnings).toHaveLength(0);
29+
});
30+
});
31+
32+
describe('invalid tool names', () => {
33+
test.each`
34+
description | toolName | expectedWarning
35+
${'empty names'} | ${''} | ${'Tool name cannot be empty'}
36+
${'names longer than 128 characters'} | ${'a'.repeat(129)} | ${'Tool name exceeds maximum length of 128 characters (current: 129)'}
37+
${'names with spaces'} | ${'get user profile'} | ${'Tool name contains invalid characters: " "'}
38+
${'names with commas'} | ${'get,user,profile'} | ${'Tool name contains invalid characters: ","'}
39+
${'names with forward slashes'} | ${'user/profile/update'} | ${'Tool name contains invalid characters: "/"'}
40+
${'names with other special chars'} | ${'[email protected]'} | ${'Tool name contains invalid characters: "@"'}
41+
${'names with multiple invalid chars'} | ${'user name@domain,com'} | ${'Tool name contains invalid characters: " ", "@", ","'}
42+
${'names with unicode characters'} | ${'user-ñame'} | ${'Tool name contains invalid characters: "ñ"'}
43+
`('should reject $description', ({ toolName, expectedWarning }) => {
44+
const result = validateToolName(toolName);
45+
expect(result.isValid).toBe(false);
46+
expect(result.warnings).toContain(expectedWarning);
47+
});
48+
});
49+
50+
describe('warnings for potentially problematic patterns', () => {
51+
test.each`
52+
description | toolName | expectedWarning | shouldBeValid
53+
${'names with spaces'} | ${'get user profile'} | ${'Tool name contains spaces, which may cause parsing issues'} | ${false}
54+
${'names with commas'} | ${'get,user,profile'} | ${'Tool name contains commas, which may cause parsing issues'} | ${false}
55+
${'names starting with dash'} | ${'-get-user'} | ${'Tool name starts or ends with a dash, which may cause parsing issues in some contexts'} | ${true}
56+
${'names ending with dash'} | ${'get-user-'} | ${'Tool name starts or ends with a dash, which may cause parsing issues in some contexts'} | ${true}
57+
${'names starting with dot'} | ${'.get.user'} | ${'Tool name starts or ends with a dot, which may cause parsing issues in some contexts'} | ${true}
58+
${'names ending with dot'} | ${'get.user.'} | ${'Tool name starts or ends with a dot, which may cause parsing issues in some contexts'} | ${true}
59+
${'names with leading and trailing dots'} | ${'.get.user.'} | ${'Tool name starts or ends with a dot, which may cause parsing issues in some contexts'} | ${true}
60+
`('should warn about $description', ({ toolName, expectedWarning, shouldBeValid }) => {
61+
const result = validateToolName(toolName);
62+
expect(result.isValid).toBe(shouldBeValid);
63+
expect(result.warnings).toContain(expectedWarning);
64+
});
65+
});
66+
});
67+
68+
describe('issueToolNameWarning', () => {
69+
test('should output warnings to console.warn', () => {
70+
const warnings = ['Warning 1', 'Warning 2'];
71+
issueToolNameWarning('test-tool', warnings);
72+
73+
expect(warnSpy).toHaveBeenCalledTimes(6); // Header + 2 warnings + 3 guidance lines
74+
const calls = warnSpy.mock.calls.map(call => call.join(' '));
75+
expect(calls[0]).toContain('Tool name validation warning for "test-tool"');
76+
expect(calls[1]).toContain('- Warning 1');
77+
expect(calls[2]).toContain('- Warning 2');
78+
expect(calls[3]).toContain('Tool registration will proceed, but this may cause compatibility issues.');
79+
expect(calls[4]).toContain('Consider updating the tool name');
80+
expect(calls[5]).toContain('See SEP: Specify Format for Tool Names');
81+
});
82+
83+
test('should handle empty warnings array', () => {
84+
issueToolNameWarning('test-tool', []);
85+
expect(warnSpy).toHaveBeenCalledTimes(0);
86+
});
87+
});
88+
89+
describe('validateAndWarnToolName', () => {
90+
test.each`
91+
description | toolName | expectedResult | shouldWarn
92+
${'valid names with warnings'} | ${'-get-user-'} | ${true} | ${true}
93+
${'completely valid names'} | ${'get-user-profile'} | ${true} | ${false}
94+
${'invalid names with spaces'} | ${'get user profile'} | ${false} | ${true}
95+
${'empty names'} | ${''} | ${false} | ${true}
96+
${'names exceeding length limit'} | ${'a'.repeat(129)} | ${false} | ${true}
97+
`('should handle $description', ({ toolName, expectedResult, shouldWarn }) => {
98+
const result = validateAndWarnToolName(toolName);
99+
expect(result).toBe(expectedResult);
100+
101+
if (shouldWarn) {
102+
expect(warnSpy).toHaveBeenCalled();
103+
} else {
104+
expect(warnSpy).not.toHaveBeenCalled();
105+
}
106+
});
107+
108+
test('should include space warning for invalid names with spaces', () => {
109+
validateAndWarnToolName('get user profile');
110+
const warningCalls = warnSpy.mock.calls.map(call => call.join(' '));
111+
expect(warningCalls.some(call => call.includes('Tool name contains spaces'))).toBe(true);
112+
});
113+
});
114+
115+
describe('edge cases and robustness', () => {
116+
test.each`
117+
description | toolName | shouldBeValid | expectedWarning
118+
${'names with only dots'} | ${'...'} | ${true} | ${'Tool name starts or ends with a dot, which may cause parsing issues in some contexts'}
119+
${'names with only dashes'} | ${'---'} | ${true} | ${'Tool name starts or ends with a dash, which may cause parsing issues in some contexts'}
120+
${'names with only forward slashes'} | ${'///'} | ${false} | ${'Tool name contains invalid characters: "/"'}
121+
${'names with mixed valid/invalid chars'} | ${'user@name123'} | ${false} | ${'Tool name contains invalid characters: "@"'}
122+
`('should handle $description', ({ toolName, shouldBeValid, expectedWarning }) => {
123+
const result = validateToolName(toolName);
124+
expect(result.isValid).toBe(shouldBeValid);
125+
expect(result.warnings).toContain(expectedWarning);
126+
});
127+
});

‎src/shared/toolNameValidation.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/**
2+
* Tool name validation utilities according to SEP: Specify Format for Tool Names
3+
*
4+
* Tool names SHOULD be between 1 and 128 characters in length (inclusive).
5+
* Tool names are case-sensitive.
6+
* Allowed characters: uppercase and lowercase ASCII letters (A-Z, a-z), digits
7+
* (0-9), underscore (_), dash (-), and dot (.).
8+
* Tool names SHOULD NOT contain spaces, commas, or other special characters.
9+
*/
10+
11+
/**
12+
* Regular expression for valid tool names according to SEP-986 specification
13+
*/
14+
const TOOL_NAME_REGEX = /^[A-Za-z0-9._-]{1,128}$/;
15+
16+
/**
17+
* Validates a tool name according to the SEP specification
18+
* @param name - The tool name to validate
19+
* @returns An object containing validation result and any warnings
20+
*/
21+
export function validateToolName(name: string): {
22+
isValid: boolean;
23+
warnings: string[];
24+
} {
25+
const warnings: string[] = [];
26+
27+
// Check length
28+
if (name.length === 0) {
29+
return {
30+
isValid: false,
31+
warnings: ['Tool name cannot be empty']
32+
};
33+
}
34+
35+
if (name.length > 128) {
36+
return {
37+
isValid: false,
38+
warnings: [`Tool name exceeds maximum length of 128 characters (current: ${name.length})`]
39+
};
40+
}
41+
42+
// Check for specific problematic patterns (these are warnings, not validation failures)
43+
if (name.includes(' ')) {
44+
warnings.push('Tool name contains spaces, which may cause parsing issues');
45+
}
46+
47+
if (name.includes(',')) {
48+
warnings.push('Tool name contains commas, which may cause parsing issues');
49+
}
50+
51+
// Check for potentially confusing patterns (leading/trailing dashes, dots, slashes)
52+
if (name.startsWith('-') || name.endsWith('-')) {
53+
warnings.push('Tool name starts or ends with a dash, which may cause parsing issues in some contexts');
54+
}
55+
56+
if (name.startsWith('.') || name.endsWith('.')) {
57+
warnings.push('Tool name starts or ends with a dot, which may cause parsing issues in some contexts');
58+
}
59+
60+
// Check for invalid characters
61+
if (!TOOL_NAME_REGEX.test(name)) {
62+
const invalidChars = name
63+
.split('')
64+
.filter(char => !/[A-Za-z0-9._-]/.test(char))
65+
.filter((char, index, arr) => arr.indexOf(char) === index); // Remove duplicates
66+
67+
warnings.push(
68+
`Tool name contains invalid characters: ${invalidChars.map(c => `"${c}"`).join(', ')}`,
69+
'Allowed characters are: A-Z, a-z, 0-9, underscore (_), dash (-), and dot (.)'
70+
);
71+
72+
return {
73+
isValid: false,
74+
warnings
75+
};
76+
}
77+
78+
return {
79+
isValid: true,
80+
warnings
81+
};
82+
}
83+
84+
/**
85+
* Issues warnings for non-conforming tool names
86+
* @param name - The tool name that triggered the warnings
87+
* @param warnings - Array of warning messages
88+
*/
89+
export function issueToolNameWarning(name: string, warnings: string[]): void {
90+
if (warnings.length > 0) {
91+
console.warn(`Tool name validation warning for "${name}":`);
92+
for (const warning of warnings) {
93+
console.warn(` - ${warning}`);
94+
}
95+
console.warn('Tool registration will proceed, but this may cause compatibility issues.');
96+
console.warn('Consider updating the tool name to conform to the MCP tool naming standard.');
97+
console.warn(
98+
'See SEP: Specify Format for Tool Names (https://github.com/modelcontextprotocol/modelcontextprotocol/issues/986) for more details.'
99+
);
100+
}
101+
}
102+
103+
/**
104+
* Validates a tool name and issues warnings for non-conforming names
105+
* @param name - The tool name to validate
106+
* @returns true if the name is valid, false otherwise
107+
*/
108+
export function validateAndWarnToolName(name: string): boolean {
109+
const result = validateToolName(name);
110+
111+
// Always issue warnings for any validation issues (both invalid names and warnings)
112+
issueToolNameWarning(name, result.warnings);
113+
114+
return result.isValid;
115+
}

0 commit comments

Comments
 (0)