Skip to content

Commit 64c2862

Browse files
committed
Check and add tests for notification capabilities too
1 parent dc45f5d commit 64c2862

File tree

5 files changed

+214
-12
lines changed

5 files changed

+214
-12
lines changed

src/client/index.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,108 @@ test("should respect server capabilities", async () => {
223223
);
224224
});
225225

226+
test("should respect client notification capabilities", async () => {
227+
const server = new Server(
228+
{
229+
name: "test server",
230+
version: "1.0",
231+
},
232+
{
233+
capabilities: {},
234+
},
235+
);
236+
237+
const client = new Client(
238+
{
239+
name: "test client",
240+
version: "1.0",
241+
},
242+
{
243+
capabilities: {
244+
roots: {
245+
listChanged: true,
246+
},
247+
},
248+
},
249+
);
250+
251+
const [clientTransport, serverTransport] =
252+
InMemoryTransport.createLinkedPair();
253+
254+
await Promise.all([
255+
client.connect(clientTransport),
256+
server.connect(serverTransport),
257+
]);
258+
259+
// This should work because the client has the roots.listChanged capability
260+
await expect(client.sendRootsListChanged()).resolves.not.toThrow();
261+
262+
// Create a new client without the roots.listChanged capability
263+
const clientWithoutCapability = new Client(
264+
{
265+
name: "test client without capability",
266+
version: "1.0",
267+
},
268+
{
269+
capabilities: {},
270+
enforceStrictCapabilities: true,
271+
},
272+
);
273+
274+
await clientWithoutCapability.connect(clientTransport);
275+
276+
// This should throw because the client doesn't have the roots.listChanged capability
277+
await expect(clientWithoutCapability.sendRootsListChanged()).rejects.toThrow(
278+
/^Client does not support/,
279+
);
280+
});
281+
282+
test("should respect server notification capabilities", async () => {
283+
const server = new Server(
284+
{
285+
name: "test server",
286+
version: "1.0",
287+
},
288+
{
289+
capabilities: {
290+
logging: {},
291+
resources: {
292+
listChanged: true,
293+
},
294+
},
295+
},
296+
);
297+
298+
const client = new Client(
299+
{
300+
name: "test client",
301+
version: "1.0",
302+
},
303+
{
304+
capabilities: {},
305+
},
306+
);
307+
308+
const [clientTransport, serverTransport] =
309+
InMemoryTransport.createLinkedPair();
310+
311+
await Promise.all([
312+
client.connect(clientTransport),
313+
server.connect(serverTransport),
314+
]);
315+
316+
// These should work because the server has the corresponding capabilities
317+
await expect(
318+
server.sendLoggingMessage({ level: "info", data: "Test" }),
319+
).resolves.not.toThrow();
320+
await expect(server.sendResourceListChanged()).resolves.not.toThrow();
321+
322+
// This should throw because the server doesn't have the tools capability
323+
await expect(server.sendToolListChanged()).rejects.toThrow(
324+
"Server does not support notifying of tool list changes",
325+
);
326+
});
327+
226328
/*
227329
Test that custom request/notification/result schemas can be used with the Client class.
228330
*/

src/client/index.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,28 @@ export class Client<
226226
}
227227
}
228228

229+
protected assertNotificationCapability(
230+
method: NotificationT["method"],
231+
): void {
232+
switch (method as ClientNotification["method"]) {
233+
case "notifications/roots/list_changed":
234+
if (!this._capabilities.roots?.listChanged) {
235+
throw new Error(
236+
"Client does not support roots list changed notifications",
237+
);
238+
}
239+
break;
240+
241+
case "notifications/initialized":
242+
// No specific capability required for initialized
243+
break;
244+
245+
case "notifications/progress":
246+
// Progress notifications are always allowed
247+
break;
248+
}
249+
}
250+
229251
async ping() {
230252
return this.request({ method: "ping" }, EmptyResultSchema);
231253
}

src/server/index.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,39 @@ test("should respect client capabilities", async () => {
260260
await expect(server.listRoots()).rejects.toThrow(/^Client does not support/);
261261
});
262262

263+
test("should respect server notification capabilities", async () => {
264+
const server = new Server(
265+
{
266+
name: "test server",
267+
version: "1.0",
268+
},
269+
{
270+
capabilities: {
271+
logging: {},
272+
},
273+
enforceStrictCapabilities: true,
274+
},
275+
);
276+
277+
const [clientTransport, serverTransport] =
278+
InMemoryTransport.createLinkedPair();
279+
280+
await server.connect(serverTransport);
281+
282+
// This should work because logging is supported by the server
283+
await expect(
284+
server.sendLoggingMessage({
285+
level: "info",
286+
data: "Test log message",
287+
}),
288+
).resolves.not.toThrow();
289+
290+
// This should throw because resource notificaitons are not supported by the server
291+
await expect(
292+
server.sendResourceUpdated({ uri: "test://resource" }),
293+
).rejects.toThrow(/^Server does not support/);
294+
});
295+
263296
/*
264297
Test that custom request/notification/result schemas can be used with the Server class.
265298
*/

src/server/index.ts

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export type ServerOptions = ProtocolOptions & {
3232
/**
3333
* Capabilities to advertise as being supported by this server.
3434
*/
35-
capabilities: ClientCapabilities;
35+
capabilities: ServerCapabilities;
3636
};
3737

3838
/**
@@ -120,6 +120,49 @@ export class Server<
120120
}
121121
}
122122

123+
protected assertNotificationCapability(
124+
method: (ServerNotification | NotificationT)["method"],
125+
): void {
126+
switch (method as ServerNotification["method"]) {
127+
case "notifications/message":
128+
if (!this._capabilities.logging) {
129+
throw new Error(
130+
`Server does not support logging (required for ${method})`,
131+
);
132+
}
133+
break;
134+
135+
case "notifications/resources/updated":
136+
case "notifications/resources/list_changed":
137+
if (!this._capabilities.resources) {
138+
throw new Error(
139+
`Server does not support notifying about resources (required for ${method})`,
140+
);
141+
}
142+
break;
143+
144+
case "notifications/tools/list_changed":
145+
if (!this._capabilities.tools) {
146+
throw new Error(
147+
`Server does not support notifying of tool list changes (required for ${method})`,
148+
);
149+
}
150+
break;
151+
152+
case "notifications/prompts/list_changed":
153+
if (!this._capabilities.prompts) {
154+
throw new Error(
155+
`Server does not support notifying of prompt list changes (required for ${method})`,
156+
);
157+
}
158+
break;
159+
160+
case "notifications/progress":
161+
// Progress notifications are always allowed
162+
break;
163+
}
164+
}
165+
123166
private async _oninitialize(
124167
request: InitializeRequest,
125168
): Promise<InitializeResult> {
@@ -155,17 +198,6 @@ export class Server<
155198
return this._capabilities;
156199
}
157200

158-
private assertCapability(
159-
capability: keyof ClientCapabilities,
160-
method: string,
161-
) {
162-
if (!this._clientCapabilities?.[capability]) {
163-
throw new Error(
164-
`Client does not support ${capability} (required for ${method})`,
165-
);
166-
}
167-
}
168-
169201
async ping() {
170202
return this.request({ method: "ping" }, EmptyResultSchema);
171203
}

src/shared/protocol.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export type ProtocolOptions = {
2828
/**
2929
* Whether to restrict emitted requests to only those that the remote side has indicated that they can handle, through their advertised capabilities.
3030
*
31+
* Note that this DOES NOT affect checking of _local_ side capabilities, as it is considered a logic error to mis-specify those.
32+
*
3133
* Currently this defaults to false, for backwards compatibility with SDK versions that did not advertise capabilities correctly. In future, this will default to true.
3234
*/
3335
enforceStrictCapabilities?: boolean;
@@ -266,6 +268,15 @@ export abstract class Protocol<
266268
method: SendRequestT["method"],
267269
): void;
268270

271+
/**
272+
* A method to check if a notification is supported by the local side, for the given method to be sent.
273+
*
274+
* This should be implemented by subclasses.
275+
*/
276+
protected abstract assertNotificationCapability(
277+
method: SendNotificationT["method"],
278+
): void;
279+
269280
/**
270281
* Sends a request and wait for a response, with optional progress notifications in the meantime (if supported by the server).
271282
*
@@ -326,6 +337,8 @@ export abstract class Protocol<
326337
throw new Error("Not connected");
327338
}
328339

340+
this.assertNotificationCapability(notification.method);
341+
329342
const jsonrpcNotification: JSONRPCNotification = {
330343
...notification,
331344
jsonrpc: "2.0",

0 commit comments

Comments
 (0)