From 34f91cb30badbcfc35a0d10178410cfaecc38ebf Mon Sep 17 00:00:00 2001 From: Krishnendu Date: Mon, 7 Sep 2026 00:03:35 +0530 Subject: [PATCH] fix(server): track the current registry key when a handle is renamed The update closure for prompts, resources and resource templates captured the registration key and never reassigned it, so remove() deleted the key the entry had already vacated and left the live entry listed and callable. RegisteredTool already tracked its current key; these three now do too. Fixes #2723 --- .changeset/rename-then-remove.md | 13 ++++ packages/server/src/server/mcp.ts | 17 ++++- .../test2723.remove-after-rename.test.ts | 72 +++++++++++++++++++ 3 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 .changeset/rename-then-remove.md create mode 100644 test/integration/test/issues/test2723.remove-after-rename.test.ts 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); + }); +});