Skip to content

Commit 010294f

Browse files
committed
fix(server): throw on resource subscribe capability mismatches
Align with other local capability checks: sendResourceUpdated throws SdkError(CapabilityNotSupported) via assertNotificationCapability when resources.subscribe is missing, and setRequestHandler gates resources/subscribe|unsubscribe the same way. Covers both halves of #2545 (notification send + handler registration).
1 parent aa98e65 commit 010294f

2 files changed

Lines changed: 50 additions & 31 deletions

File tree

packages/server/src/server/server.ts

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,6 @@ export class Server extends Protocol<ServerContext> {
289289
private _requestStateVerify?: (state: string, ctx: ServerContext) => unknown | Promise<unknown>;
290290
private _inputRequiredServing: { maxRounds: number; roundTimeoutMs: number; legacyShim: boolean };
291291
private _legacyShim?: LegacyInputRequiredShim;
292-
/** Emit at most one warn when sendResourceUpdated is used without resources.subscribe. */
293-
private _warnedResourceUpdatedWithoutSubscribe = false;
294-
295292
/** Lazily-built legacy shim; the loop lives in legacyInputRequiredShim.ts behind a narrow host contract. */
296293
private _legacyInputRequiredShim(): LegacyInputRequiredShim {
297294
return (this._legacyShim ??= new LegacyInputRequiredShim({
@@ -795,7 +792,19 @@ export class Server extends Protocol<ServerContext> {
795792
break;
796793
}
797794

798-
case 'notifications/resources/updated':
795+
case 'notifications/resources/updated': {
796+
// Resource updates only reach clients that opted in via
797+
// resources/subscribe, which requires the advertised
798+
// resources.subscribe capability (#2545).
799+
if (!this._capabilities.resources?.subscribe) {
800+
throw new SdkError(
801+
SdkErrorCode.CapabilityNotSupported,
802+
`Server does not support resource subscriptions (required for ${method})`
803+
);
804+
}
805+
break;
806+
}
807+
799808
case 'notifications/resources/list_changed': {
800809
if (!this._capabilities.resources) {
801810
throw new SdkError(
@@ -881,6 +890,20 @@ export class Server extends Protocol<ServerContext> {
881890
break;
882891
}
883892

893+
case 'resources/subscribe':
894+
case 'resources/unsubscribe': {
895+
// Handler registration must match the advertised bit; otherwise
896+
// clients that subscribe (or the server's own handlers) see a
897+
// silent dead-end when the capability was never declared (#2545).
898+
if (!this._capabilities.resources?.subscribe) {
899+
throw new SdkError(
900+
SdkErrorCode.CapabilityNotSupported,
901+
`Server does not support resource subscriptions (required for ${method})`
902+
);
903+
}
904+
break;
905+
}
906+
884907
case 'tools/call':
885908
case 'tools/list': {
886909
if (!this._capabilities.tools) {
@@ -1300,19 +1323,7 @@ export class Server extends Protocol<ServerContext> {
13001323
}
13011324

13021325
async sendResourceUpdated(params: ResourceUpdatedNotification['params']) {
1303-
// Resource update notifications only reach clients that subscribed via
1304-
// resources/subscribe. That method is gated on the advertised
1305-
// `resources.subscribe` capability — without it, clients cannot opt in
1306-
// and the notification is effectively a no-op for them. Warn once so
1307-
// the missing capability is easy to spot during development (#2545).
1308-
if (!this._capabilities.resources?.subscribe && !this._warnedResourceUpdatedWithoutSubscribe) {
1309-
this._warnedResourceUpdatedWithoutSubscribe = true;
1310-
console.warn(
1311-
'[mcp-sdk] sendResourceUpdated() called without advertising capabilities.resources.subscribe. ' +
1312-
'Clients cannot subscribe to resource updates unless the server sets ' +
1313-
'{ resources: { subscribe: true } } in its capabilities.'
1314-
);
1315-
}
1326+
// assertNotificationCapability requires resources.subscribe (#2545).
13161327
return this.notification({
13171328
method: 'notifications/resources/updated',
13181329
params

packages/server/test/server/server.test.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -243,34 +243,42 @@ describe('Server', () => {
243243
});
244244
});
245245

246-
describe('sendResourceUpdated capability warning', () => {
246+
describe('resource subscription capabilities (#2545)', () => {
247247
async function connectServer(server: Server): Promise<void> {
248248
const [, serverTransport] = InMemoryTransport.createLinkedPair();
249249
await server.connect(serverTransport);
250250
}
251251

252-
it('warns once when resources.subscribe is not advertised', async () => {
253-
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
252+
it('sendResourceUpdated throws without resources.subscribe', async () => {
254253
const server = new Server({ name: 'test', version: '1.0.0' }, { capabilities: { resources: { listChanged: true } } });
255254
await connectServer(server);
256255

257-
await server.sendResourceUpdated({ uri: 'test://resource' });
258-
await server.sendResourceUpdated({ uri: 'test://resource-2' });
259-
260-
expect(warnSpy).toHaveBeenCalledTimes(1);
261-
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('resources.subscribe'));
262-
warnSpy.mockRestore();
256+
await expect(server.sendResourceUpdated({ uri: 'test://resource' })).rejects.toThrow(/resource subscriptions/);
263257
});
264258

265-
it('does not warn when resources.subscribe is advertised', async () => {
266-
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
259+
it('sendResourceUpdated succeeds when resources.subscribe is advertised', async () => {
267260
const server = new Server({ name: 'test', version: '1.0.0' }, { capabilities: { resources: { subscribe: true } } });
268261
await connectServer(server);
269262

270-
await server.sendResourceUpdated({ uri: 'test://resource' });
263+
await expect(server.sendResourceUpdated({ uri: 'test://resource' })).resolves.toBeUndefined();
264+
});
265+
266+
it('setRequestHandler rejects resources/subscribe without the capability', () => {
267+
const server = new Server({ name: 'test', version: '1.0.0' }, { capabilities: { resources: { listChanged: true } } });
268+
269+
expect(() => server.setRequestHandler('resources/subscribe', async () => ({}))).toThrow(/resource subscriptions/);
270+
});
271+
272+
it('setRequestHandler rejects resources/unsubscribe without the capability', () => {
273+
const server = new Server({ name: 'test', version: '1.0.0' }, { capabilities: { resources: {} } });
274+
275+
expect(() => server.setRequestHandler('resources/unsubscribe', async () => ({}))).toThrow(/resource subscriptions/);
276+
});
277+
278+
it('setRequestHandler allows resources/subscribe when the capability is advertised', () => {
279+
const server = new Server({ name: 'test', version: '1.0.0' }, { capabilities: { resources: { subscribe: true } } });
271280

272-
expect(warnSpy).not.toHaveBeenCalled();
273-
warnSpy.mockRestore();
281+
expect(() => server.setRequestHandler('resources/subscribe', async () => ({}))).not.toThrow();
274282
});
275283
});
276284
});

0 commit comments

Comments
 (0)