Skip to content

Commit 873f9d0

Browse files
committed
Handle notifications correctly
1 parent 94bfa36 commit 873f9d0

4 files changed

Lines changed: 93 additions & 6 deletions

File tree

mcp/mcp-schemas/model/main.smithy

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ structure JsonRpcRequest {
99
@required
1010
method: String
1111

12-
@required
1312
id: Document
1413

1514
params: Document
@@ -23,7 +22,6 @@ structure JsonRpcResponse {
2322

2423
error: JsonRpcErrorResponse
2524

26-
@required
2725
id: Document
2826
}
2927

mcp/mcp-server/src/main/java/software/amazon/smithy/java/mcp/server/McpServer.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,19 @@ private void handleRequest(JsonRpcRequest req) {
182182

183183
private void validate(JsonRpcRequest req) {
184184
Document id = req.getId();
185-
if (!(id.isType(ShapeType.INTEGER) || id.isType(ShapeType.STRING))) {
186-
throw ValidationException.builder()
187-
.message("Request id is of invalid type " + id.type().name())
188-
.build();
185+
boolean isRequest = !req.getMethod().startsWith("notifications/");
186+
if (isRequest) {
187+
if (id == null) {
188+
throw ValidationException.builder()
189+
.withoutStackTrace()
190+
.message("Requests are expected to have ids")
191+
.build();
192+
} else if (!(id.isType(ShapeType.INTEGER) || id.isType(ShapeType.STRING))) {
193+
throw ValidationException.builder()
194+
.withoutStackTrace()
195+
.message("Request id is of invalid type " + id.type().name())
196+
.build();
197+
}
189198
}
190199
}
191200

mcp/mcp-server/src/test/java/software/amazon/smithy/java/mcp/server/McpServerTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,27 @@ void testInvalidIds() {
204204
assertTrue(response.getError().getMessage().contains("Request id is of invalid type"));
205205
}
206206

207+
@Test
208+
void testRequestsRequireIds() {
209+
server = McpServer.builder()
210+
.input(input)
211+
.output(output)
212+
.addService(ProxyService.builder()
213+
.service(ShapeId.from("smithy.test#TestService"))
214+
.proxyEndpoint("http://localhost")
215+
.model(MODEL)
216+
.build())
217+
.build();
218+
219+
server.start();
220+
221+
// Test regular request without ID (should fail with specific message)
222+
write("tools/list", Document.of(Map.of()), null);
223+
var response = read();
224+
assertNotNull(response.getError());
225+
assertTrue(response.getError().getMessage().contains("Requests are expected to have ids"));
226+
}
227+
207228
@Test
208229
void testInputAdaptation() {
209230
AtomicReference<StructDocument> capturedInput = new AtomicReference<>();
@@ -309,6 +330,34 @@ public void readBeforeSerialization(InputHook<?, ?> hook) {
309330
server.shutdown().join();
310331
}
311332

333+
@Test
334+
void testNotificationsDoNotRequireRequestId() {
335+
server = McpServer.builder()
336+
.input(input)
337+
.output(output)
338+
.addService(ProxyService.builder()
339+
.service(ShapeId.from("smithy.test#TestService"))
340+
.proxyEndpoint("http://localhost")
341+
.model(MODEL)
342+
.build())
343+
.build();
344+
345+
server.start();
346+
347+
// Test notifications/initialized without ID (should not produce any error response)
348+
writeNotification("notifications/initialized", Document.of(Map.of()));
349+
output.assertNoOutput();
350+
351+
// Test another notification to ensure it's consistently handled
352+
writeNotification("notifications/progress", Document.of(Map.of("progressToken", Document.of("test"))));
353+
output.assertNoOutput();
354+
355+
// Send a regular request to verify the server is still functioning
356+
write("tools/list", Document.of(Map.of()));
357+
var response = read();
358+
assertNotNull(response.getResult());
359+
}
360+
312361
private void validateNestedStructure(Map<String, Document> properties) {
313362
var nestedStr = properties.get("nestedStr").asStringMap();
314363
assertEquals("string", nestedStr.get("type").asString());
@@ -348,6 +397,16 @@ private JsonRpcResponse read() {
348397
return CODEC.deserializeShape(line, JsonRpcResponse.builder());
349398
}
350399

400+
private void writeNotification(String method, Document params) {
401+
var request = JsonRpcRequest.builder()
402+
.method(method)
403+
.params(params)
404+
.jsonrpc("2.0")
405+
.build();
406+
input.write(CODEC.serializeToString(request));
407+
input.write("\n");
408+
}
409+
351410
private static final String MODEL_STR = """
352411
$version: "2"
353412

mcp/mcp-server/src/test/java/software/amazon/smithy/java/mcp/server/TestOutputStream.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,25 @@ String read() {
6363
throw new RuntimeException(e);
6464
}
6565
}
66+
67+
boolean hasOutput() {
68+
return !lines.isEmpty();
69+
}
70+
71+
void assertNoOutput() {
72+
assertNoOutput(50);
73+
}
74+
75+
void assertNoOutput(long waitMillis) {
76+
// Wait briefly to allow any potential response to be written
77+
try {
78+
Thread.sleep(waitMillis);
79+
} catch (InterruptedException e) {
80+
Thread.currentThread().interrupt();
81+
}
82+
// Verify no output was produced
83+
if (hasOutput()) {
84+
throw new AssertionError("Expected no output but got : " + read());
85+
}
86+
}
6687
}

0 commit comments

Comments
 (0)