From a907a43f00e3faa78f55632669923cb550651a17 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Tue, 13 Oct 2026 18:05:24 +0200 Subject: [PATCH] Fix NPE in addRoot/removeRoot when roots listChanged is unset - Follow-up to #1178 Signed-off-by: Daniel Garnier-Moiroux --- .../client/McpAsyncClient.java | 4 +-- .../client/McpAsyncClientTest.java | 26 +++++++++++++++++++ .../client/McpSyncClientTest.java | 20 ++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java b/mcp-core/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java index 502b23f6b..cd071b690 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java @@ -511,7 +511,7 @@ public Mono addRoot(Root root) { logger.debug("Added root: {}", root); - if (this.clientCapabilities.roots().listChanged()) { + if (Boolean.TRUE.equals(this.clientCapabilities.roots().listChanged())) { if (this.isInitialized()) { return this.rootsListChangedNotification(); } @@ -539,7 +539,7 @@ public Mono removeRoot(String rootUri) { if (removed != null) { logger.debug("Removed Root: {}", rootUri); - if (this.clientCapabilities.roots().listChanged()) { + if (Boolean.TRUE.equals(this.clientCapabilities.roots().listChanged())) { if (this.isInitialized()) { return this.rootsListChangedNotification(); } diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/client/McpAsyncClientTest.java b/mcp-core/src/test/java/io/modelcontextprotocol/client/McpAsyncClientTest.java index dea7d42e9..e4403f5b1 100644 --- a/mcp-core/src/test/java/io/modelcontextprotocol/client/McpAsyncClientTest.java +++ b/mcp-core/src/test/java/io/modelcontextprotocol/client/McpAsyncClientTest.java @@ -12,11 +12,15 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** @@ -179,4 +183,26 @@ void noElicitationFromCapabilities() { } + @Nested + class Roots { + + @Test + void addAndRemoveRootWithUnsetListChanged() { + McpClientTransport transport = mock(McpClientTransport.class); + when(transport.protocolVersions()).thenReturn(List.of("2024-11-05")); + var client = McpClient.async(transport) + .capabilities(McpSchema.ClientCapabilities.builder().roots(null).build()) + .jsonSchemaValidator(mock(JsonSchemaValidator.class)) + .build(); + + assertThat(client.getClientCapabilities().roots().listChanged()).isNull(); + + StepVerifier.create(client.addRoot(new McpSchema.Root("file:///added", "added"))).verifyComplete(); + StepVerifier.create(client.removeRoot("file:///added")).verifyComplete(); + + verify(transport, never()).sendMessage(any()); + } + + } + } diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/client/McpSyncClientTest.java b/mcp-core/src/test/java/io/modelcontextprotocol/client/McpSyncClientTest.java index 9790dea6a..0d611b4c4 100644 --- a/mcp-core/src/test/java/io/modelcontextprotocol/client/McpSyncClientTest.java +++ b/mcp-core/src/test/java/io/modelcontextprotocol/client/McpSyncClientTest.java @@ -13,6 +13,7 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -91,4 +92,23 @@ void noElicitationFromCapabilities() { } + @Nested + class Roots { + + @Test + void addAndRemoveRootWithUnsetListChanged() { + McpClientTransport transport = mock(McpClientTransport.class); + when(transport.protocolVersions()).thenReturn(List.of("2024-11-05")); + var client = McpClient.sync(transport) + .capabilities(McpSchema.ClientCapabilities.builder().roots(null).build()) + .jsonSchemaValidator(mock(JsonSchemaValidator.class)) + .build(); + + assertThatCode(() -> client.addRoot(new McpSchema.Root("file:///added", "added"))) + .doesNotThrowAnyException(); + assertThatCode(() -> client.removeRoot("file:///added")).doesNotThrowAnyException(); + } + + } + }