Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/resource-title-updates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@modelcontextprotocol/server': patch
---

Reflect resource and resource template `update({ title })` changes in list responses. Preserve other metadata and individual resource titles when a template supplies list entries.
10 changes: 8 additions & 2 deletions packages/server/src/server/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,11 @@ export class McpServer {
if (updates.uri) this._registeredResources[updates.uri] = registeredResource;
}
Comment on lines 780 to 781

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: Server authors who move a resource with update({ uri }) and then call remove() or a second update({ uri }) leave the resource registered under the previous URI, so resources/list keeps advertising it and resources/read still serves it. The update callback at mcp.ts:778-781 compares against and deletes the closure-captured original uri, never the current key. The same pattern exists for template names at mcp.ts:819-822. Fix: track the current uri (and template name) in a mutable variable updated on each rename, and delete that key, so remove() and chained renames act on the live registration.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: resource.update({ uri: 'b' }) then resource.remove() (which calls update({ uri: null }) at mcp.ts:776), or a second update({ uri: 'c' }). mcp.ts:778 tests updates.uri !== uri where uri is the registration-time parameter captured at mcp.ts:762, and mcp.ts:779 deletes this._registeredResources[uri], which no longer holds the resource. The entry under 'b' survives, so resources/list at mcp.ts:537 still lists it and resources/read at mcp.ts:586 still serves it; after a second rename the resource is registered twice under 'b' and 'c'. The template path at mcp.ts:819-822 has the identical flaw with the captured name. The base branch behaves the same, which is why it was dismissed, but this PR edits exactly this update callback in both places and adds a changeset promising update() is reflected in list responses, so the review had the function open and the defect sits in the lines directly above the hunk. Remedy: keep a let currentUri / currentName that each rename updates and delete that key.

Verification: pre-existing — the base at b022522 fails by the same route; this PR does not touch the rename branch. Trigger: a server author calls resource.update({ uri: 'b' }) and later resource.remove(). Mechanism at packages/server/src/server/mcp.ts:778-781: uri is the registration-time parameter never reassigned, so remove() is a no-op and the 'b' entry survives.

if (updates.name !== undefined) registeredResource.name = updates.name;
if (updates.title !== undefined) registeredResource.title = updates.title;
if (updates.metadata !== undefined) registeredResource.metadata = updates.metadata;
if (updates.title !== undefined) {
registeredResource.title = updates.title;
registeredResource.metadata = { ...registeredResource.metadata, title: updates.title };
}
Comment on lines 783 to +787

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: Server authors who update a title and later replace metadata will see the title vanish from resources/list while the handle still reports the new title. The sync at packages/server/src/server/mcp.ts:783-787 only runs inside the call that carries title; a following update({ metadata }) at mcp.ts:783 overwrites metadata wholesale and drops the synced title key, and mcp.ts:542 spreads only metadata. The PR makes title the authoritative field but only for one call. Fix: treat registeredResource.title as the source of truth in list handlers (or re-apply it whenever metadata is replaced), in both the resource and template update paths (mcp.ts:824-828).
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Sequence: resource.update({ title: 'New' }) then resource.update({ metadata: { description: 'd' } }). After the first call mcp.ts:786 stores metadata.title = 'New' and the wire shows it. The second call at mcp.ts:783 assigns metadata = updates.metadata, discarding the title key; mcp.ts:784 does not run because updates.title is undefined. resources/list at mcp.ts:542 and the template path at mcp.ts:554/568 spread only metadata, so clients now get no title at all, while registeredResource.title still reads 'New'. The base branch also lost the title in this sequence, but the base never surfaced handle.title on the wire at all; this PR introduces the contract that update({ title }) is reflected in list responses and its changeset says other metadata is preserved, so users will now rely on the handle title persisting. Remedy: have the list handlers emit title from the handle or re-merge the stored title whenever updates.metadata replaces the object, for both resources and templates.

Verification: pre-existing — triggered when a server calls update({ title }) and then, in a separate call, update({ metadata }) without a title key. packages/server/src/server/mcp.ts:783 replaces the metadata object wholesale, and the title mirror at mcp.ts:784-787 only runs when updates.title !== undefined; resources/list at mcp.ts:542 spreads only resource.metadata. The base branch ends in the same state.

if (updates.callback !== undefined) registeredResource.readCallback = updates.callback;
if (updates.scopeChallenge !== undefined) {
registeredResource.scopeChallenge = updates.scopeChallenge === null ? undefined : updates.scopeChallenge;
Expand Down Expand Up @@ -817,9 +820,12 @@ export class McpServer {
delete this._registeredResourceTemplates[name];
if (updates.name) this._registeredResourceTemplates[updates.name] = registeredResourceTemplate;
}
if (updates.title !== undefined) registeredResourceTemplate.title = updates.title;
if (updates.template !== undefined) registeredResourceTemplate.resourceTemplate = updates.template;
if (updates.metadata !== undefined) registeredResourceTemplate.metadata = updates.metadata;
if (updates.title !== undefined) {
registeredResourceTemplate.title = updates.title;
registeredResourceTemplate.metadata = { ...registeredResourceTemplate.metadata, title: updates.title };
}
if (updates.callback !== undefined) registeredResourceTemplate.readCallback = updates.callback;
if (updates.scopeChallenge !== undefined) {
registeredResourceTemplate.scopeChallenge = updates.scopeChallenge === null ? undefined : updates.scopeChallenge;
Expand Down
98 changes: 98 additions & 0 deletions test/integration/test/server/resourceTitleUpdate.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { Client } from '@modelcontextprotocol/client';
import { InMemoryTransport } from '@modelcontextprotocol/core-internal';
import { McpServer, ResourceTemplate } from '@modelcontextprotocol/server';
import { afterEach, beforeEach, describe, expect, test } from 'vitest';

describe('resource title updates', () => {
let server: McpServer;
let client: Client;

beforeEach(() => {
server = new McpServer({ name: 'test-server', version: '1' });
client = new Client({ name: 'test-client', version: '1' });
});

afterEach(async () => {
await client.close();
await server.close();
});

async function connect() {
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
await server.connect(serverTransport);
await client.connect(clientTransport);
}

async function list(kind: 'static' | 'template') {
if (kind === 'static') {
const result = await client.listResources(undefined, { cacheMode: 'bypass' });
return result.resources;
}
const result = await client.listResourceTemplates(undefined, { cacheMode: 'bypass' });
return result.resourceTemplates;
}

for (const kind of ['static', 'template'] as const) {
for (const initialTitle of [undefined, 'Old title']) {
test(`${kind}: advertises an updated title when the initial title is ${initialTitle}`, async () => {
const target = kind === 'static' ? 'docs://one' : new ResourceTemplate('docs://{id}', { list: undefined });
const metadata = { title: initialTitle, description: 'Keep this description', mimeType: 'text/plain' };
const read = async (uri: URL) => ({ contents: [{ uri: uri.href, text: 'document' }] });
const resource =
typeof target === 'string'
? server.registerResource('docs', target, metadata, read)
: server.registerResource('docs', target, metadata, read);
await connect();

resource.update({ title: 'New title' });

const entries = await list(kind);
expect(entries).toEqual([expect.objectContaining({ ...metadata, title: 'New title' })]);
});
}

test(`${kind}: keeps an explicit title update when replacing other metadata`, async () => {
const read = async (uri: URL) => ({ contents: [{ uri: uri.href, text: 'document' }] });
const resource =
kind === 'static'
? server.registerResource('docs', 'docs://one', { title: 'Old title' }, read)
: server.registerResource(
'docs',
new ResourceTemplate('docs://{id}', { list: undefined }),
{ title: 'Old title' },
read
);
await connect();

resource.update({ title: 'New title', metadata: { description: 'New description' } });

const entries = await list(kind);
expect(entries).toEqual([expect.objectContaining({ title: 'New title', description: 'New description' })]);
});
}

test('updates inherited template titles while preserving individual resource titles', async () => {
const resource = server.registerResource(
'docs',
new ResourceTemplate('docs://{id}', {
list: async () => ({
resources: [
{ uri: 'docs://one', name: 'one' },
{ uri: 'docs://two', name: 'two', title: 'Individual title' }
]
})
}),
{ title: 'Old title', mimeType: 'text/plain' },
async uri => ({ contents: [{ uri: uri.href, text: 'document' }] })
);
await connect();

resource.update({ title: 'New title' });

const { resources } = await client.listResources(undefined, { cacheMode: 'bypass' });
expect(resources).toEqual([
{ uri: 'docs://one', name: 'one', title: 'New title', mimeType: 'text/plain' },
{ uri: 'docs://two', name: 'two', title: 'Individual title', mimeType: 'text/plain' }
]);
});
});
Loading