diff --git a/.changeset/rename-then-remove.md b/.changeset/rename-then-remove.md new file mode 100644 index 0000000000..02d11ff581 --- /dev/null +++ b/.changeset/rename-then-remove.md @@ -0,0 +1,13 @@ +--- +'@modelcontextprotocol/server': patch +--- + +Fix `remove()` being a silent no-op after a resource, resource template, or prompt +has been renamed via `update()`. The closures for these three registration types +captured the original registry key and never reassigned it after a rename, so +`remove()` deleted the stale key instead of the one the entry now lives under — +leaving the entry listed and callable, with a `list_changed` notification firing +regardless. `RegisteredTool` already tracked its current key correctly; resources, +resource templates, and prompts now do the same. + +Fixes #2723 diff --git a/packages/server/src/server/mcp.ts b/packages/server/src/server/mcp.ts index d2e40181e4..b80c1d5f7f 100644 --- a/packages/server/src/server/mcp.ts +++ b/packages/server/src/server/mcp.ts @@ -675,7 +675,10 @@ export class McpServer { update: updates => { if (updates.uri !== undefined && updates.uri !== uri) { delete this._registeredResources[uri]; - if (updates.uri) this._registeredResources[updates.uri] = registeredResource; + if (updates.uri) { + this._registeredResources[updates.uri] = registeredResource; + uri = updates.uri; // track the current registry key + } } if (updates.name !== undefined) registeredResource.name = updates.name; if (updates.title !== undefined) registeredResource.title = updates.title; @@ -708,7 +711,10 @@ export class McpServer { update: updates => { if (updates.name !== undefined && updates.name !== name) { delete this._registeredResourceTemplates[name]; - if (updates.name) this._registeredResourceTemplates[updates.name] = registeredResourceTemplate; + if (updates.name) { + this._registeredResourceTemplates[updates.name] = registeredResourceTemplate; + name = updates.name; // track the current registry key + } } if (updates.title !== undefined) registeredResourceTemplate.title = updates.title; if (updates.template !== undefined) registeredResourceTemplate.resourceTemplate = updates.template; @@ -757,7 +763,12 @@ export class McpServer { update: updates => { if (updates.name !== undefined && updates.name !== name) { delete this._registeredPrompts[name]; - if (updates.name) this._registeredPrompts[updates.name] = registeredPrompt; + if (updates.name) { + this._registeredPrompts[updates.name] = registeredPrompt; + // Tracks the current registry key; also feeds createPromptHandler() below, so a + // rename followed by a schema change regenerates against the new name. + name = updates.name; + } } if (updates.title !== undefined) registeredPrompt.title = updates.title; if (updates.description !== undefined) registeredPrompt.description = updates.description; diff --git a/test/integration/test/issues/test2723.remove-after-rename.test.ts b/test/integration/test/issues/test2723.remove-after-rename.test.ts new file mode 100644 index 0000000000..ffc027f4ff --- /dev/null +++ b/test/integration/test/issues/test2723.remove-after-rename.test.ts @@ -0,0 +1,72 @@ +/** + * Regression test for https://github.com/modelcontextprotocol/typescript-sdk/issues/2723 + * + * The `update` closure for prompts, resources and resource templates captured the + * registration key and never reassigned it, so after a rename `remove()` deleted the + * vacated key and left the live entry registered and callable. `RegisteredTool` + * already reassigned its key; these three did not. + */ +import { Client } from '@modelcontextprotocol/client'; +import { InMemoryTransport } from '@modelcontextprotocol/core-internal'; +import { McpServer, ResourceTemplate } from '@modelcontextprotocol/server'; + +describe('Issue #2723: remove() after rename is a no-op', () => { + test('removes a prompt after it has been renamed', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const prompt = server.registerPrompt('original', { description: 'x' }, async () => ({ messages: [] })); + prompt.update({ name: 'renamed' }); + prompt.remove(); + + const client = new Client({ name: 'client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); + + const result = await client.listPrompts(); + expect(result.prompts).toHaveLength(0); + }); + + test('removes a resource after it has been renamed', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const resource = server.registerResource('test', 'test://original', {}, async () => ({ contents: [] })); + resource.update({ uri: 'test://renamed' }); + resource.remove(); + + const client = new Client({ name: 'client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); + + const result = await client.listResources(); + expect(result.resources).toHaveLength(0); + }); + + test('removes a resource template after it has been renamed', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const template = server.registerResource('original', new ResourceTemplate('test://{id}', { list: undefined }), {}, async () => ({ + contents: [] + })); + template.update({ name: 'renamed' }); + template.remove(); + + const client = new Client({ name: 'client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); + + const result = await client.listResourceTemplates(); + expect(result.resourceTemplates).toHaveLength(0); + }); + + test('should remove a prompt that was renamed and renamed back', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const prompt = server.registerPrompt('original', { description: 'x' }, async () => ({ messages: [] })); + prompt.update({ name: 'renamed' }); + prompt.update({ name: 'original' }); + prompt.remove(); + + const client = new Client({ name: 'client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); + + const result = await client.listPrompts(); + expect(result.prompts).toHaveLength(0); + }); +});