From 183e9776c6a155d5560d09ca30e3d78244853ca5 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Mon, 31 Aug 2026 17:20:16 +0800 Subject: [PATCH 1/2] Fix contributed tool compatibility --- .../run/halo/mcpserver/McpToolRegistry.java | 35 +++++++++++++++-- .../run/halo/mcpserver/HaloMcpServerTest.java | 3 +- .../halo/mcpserver/McpToolRegistryTest.java | 38 +++++++++++++++++++ 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpToolRegistry.java b/src/main/java/run/halo/mcpserver/McpToolRegistry.java index f35629d..dd508dd 100644 --- a/src/main/java/run/halo/mcpserver/McpToolRegistry.java +++ b/src/main/java/run/halo/mcpserver/McpToolRegistry.java @@ -3,6 +3,10 @@ import io.modelcontextprotocol.json.schema.JsonSchemaValidator; import io.modelcontextprotocol.json.schema.jackson3.DefaultJsonSchemaValidator; import io.modelcontextprotocol.spec.McpSchema; +import java.io.IOException; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -189,7 +193,7 @@ private String validateProviderTools(List definitions) { } private Mono verifiedProviderOwner(McpToolProvider provider, String pluginName) { - final java.net.URI providerLocation; + final URI providerLocation; try { var providerClass = ClassUtils.getUserClass(provider); var codeSource = providerClass.getProtectionDomain().getCodeSource(); @@ -205,13 +209,32 @@ private Mono verifiedProviderOwner(McpToolProvider provider, String plug return extensionClient.fetch(Plugin.class, pluginName) .filter(plugin -> plugin.getStatus() != null && plugin.getStatus().getLoadLocation() != null - && providerLocation.equals(plugin.getStatus().getLoadLocation().normalize())) + && ownsProvider(plugin.getStatus().getLoadLocation(), providerLocation)) .map(ignored -> pluginName) .switchIfEmpty(Mono.error(new McpToolException( "INVALID_TOOL_NAME", "Contributed tool namespace does not match its provider plugin"))); } + static boolean ownsProvider(URI pluginLocation, URI providerLocation) { + var normalizedPlugin = pluginLocation.normalize(); + var normalizedProvider = providerLocation.normalize(); + if (normalizedPlugin.equals(normalizedProvider)) { + return true; + } + if (!"file".equalsIgnoreCase(normalizedPlugin.getScheme()) + || !"file".equalsIgnoreCase(normalizedProvider.getScheme())) { + return false; + } + try { + var pluginPath = Path.of(normalizedPlugin).toRealPath(); + var providerPath = Path.of(normalizedProvider).toRealPath(); + return Files.isDirectory(pluginPath) && providerPath.startsWith(pluginPath); + } catch (IOException | IllegalArgumentException error) { + return false; + } + } + private void validateSchema(String toolName, String kind, Map schema) { var validation = schemaValidator.validateSchema(schema); if (!validation.valid()) { @@ -242,8 +265,12 @@ private McpSchema.CallToolResult result(McpToolResult result) { var builder = McpSchema.CallToolResult.builder() .structuredContent(result.structuredContent()) .isError(result.error()); - if (result.textContent() != null) { - builder.addTextContent(result.textContent()); + var textContent = result.textContent(); + if (textContent == null && result.structuredContent() != null) { + textContent = JsonMapper.shared().writeValueAsString(result.structuredContent()); + } + if (textContent != null) { + builder.addTextContent(textContent); } return builder.build(); } diff --git a/src/test/java/run/halo/mcpserver/HaloMcpServerTest.java b/src/test/java/run/halo/mcpserver/HaloMcpServerTest.java index c0343d7..a7cbfd5 100644 --- a/src/test/java/run/halo/mcpserver/HaloMcpServerTest.java +++ b/src/test/java/run/halo/mcpserver/HaloMcpServerTest.java @@ -282,7 +282,8 @@ void listsAndCallsAContributedToolDirectly() { .exchange() .expectStatus().isOk() .expectBody() - .jsonPath("$.result.structuredContent.message").isEqualTo("Hello Halo"); + .jsonPath("$.result.structuredContent.message").isEqualTo("Hello Halo") + .jsonPath("$.result.content[0].text").isEqualTo("{\"message\":\"Hello Halo\"}"); var page = recentCallHistory.list(new McpRecentCallQuery(1, 20, null, null, null)); org.assertj.core.api.Assertions.assertThat(page.total()).isEqualTo(1); diff --git a/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java b/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java index ba7c543..f94f3db 100644 --- a/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java +++ b/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java @@ -3,6 +3,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; @@ -10,6 +12,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import reactor.core.publisher.Flux; @@ -232,6 +235,41 @@ void usesTheOwningPluginAsTheRequiredNamespace() { .verifyComplete(); } + @Test + void acceptsAProviderLoadedFromItsDevelopmentPluginDirectory() throws Exception { + McpToolProvider developmentProvider = () -> Flux.just(tool("demo/hello")); + var providerLocation = developmentProvider.getClass() + .getProtectionDomain() + .getCodeSource() + .getLocation() + .toURI() + .normalize(); + var plugin = new Plugin(); + var metadata = new Metadata(); + metadata.setName("demo"); + plugin.setMetadata(metadata); + var status = new Plugin.PluginStatus(); + status.setLoadLocation(Path.of(providerLocation).getParent().toUri()); + plugin.setStatus(status); + when(extensionGetter.getEnabledExtensions(McpToolProvider.class)) + .thenReturn(Flux.just(developmentProvider)); + when(extensionClient.fetch(Plugin.class, "demo")).thenReturn(Mono.just(plugin)); + + assertThat(registry.registeredTools().block()) + .extracting(tool -> tool.definition().name()) + .containsExactly("demo/hello"); + } + + @Test + void rejectsAProviderLoadedFromANeighboringDevelopmentDirectory(@TempDir Path tempDir) + throws Exception { + var pluginDirectory = Files.createDirectory(tempDir.resolve("plugin")); + var neighboringDirectory = Files.createDirectory(tempDir.resolve("plugin-other")); + + assertThat(McpToolRegistry.ownsProvider( + pluginDirectory.toUri(), neighboringDirectory.toUri())).isFalse(); + } + @Test void quarantinesInvalidSchemasAndConflictingTools() { var invalidSchema = McpToolDefinition.builder() From b32dc730ac6a18a0ca17df708a9a3c85fcd0a618 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Tue, 1 Sep 2026 10:09:50 +0800 Subject: [PATCH 2/2] Keep provider ownership checks non-blocking --- .../run/halo/mcpserver/McpToolRegistry.java | 30 +++++++++++-------- .../halo/mcpserver/McpToolRegistryTest.java | 23 ++++++++++---- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/McpToolRegistry.java b/src/main/java/run/halo/mcpserver/McpToolRegistry.java index dd508dd..7ac82ee 100644 --- a/src/main/java/run/halo/mcpserver/McpToolRegistry.java +++ b/src/main/java/run/halo/mcpserver/McpToolRegistry.java @@ -20,6 +20,7 @@ import org.springframework.util.ClassUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import run.halo.app.core.extension.Plugin; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.plugin.extensionpoint.ExtensionGetter; @@ -208,31 +209,36 @@ private Mono verifiedProviderOwner(McpToolProvider provider, String plug } return extensionClient.fetch(Plugin.class, pluginName) .filter(plugin -> plugin.getStatus() != null - && plugin.getStatus().getLoadLocation() != null - && ownsProvider(plugin.getStatus().getLoadLocation(), providerLocation)) + && plugin.getStatus().getLoadLocation() != null) + .flatMap(plugin -> ownsProvider( + plugin.getStatus().getLoadLocation(), providerLocation)) + .filter(Boolean::booleanValue) .map(ignored -> pluginName) .switchIfEmpty(Mono.error(new McpToolException( "INVALID_TOOL_NAME", "Contributed tool namespace does not match its provider plugin"))); } - static boolean ownsProvider(URI pluginLocation, URI providerLocation) { + static Mono ownsProvider(URI pluginLocation, URI providerLocation) { var normalizedPlugin = pluginLocation.normalize(); var normalizedProvider = providerLocation.normalize(); if (normalizedPlugin.equals(normalizedProvider)) { - return true; + return Mono.just(true); } if (!"file".equalsIgnoreCase(normalizedPlugin.getScheme()) || !"file".equalsIgnoreCase(normalizedProvider.getScheme())) { - return false; - } - try { - var pluginPath = Path.of(normalizedPlugin).toRealPath(); - var providerPath = Path.of(normalizedProvider).toRealPath(); - return Files.isDirectory(pluginPath) && providerPath.startsWith(pluginPath); - } catch (IOException | IllegalArgumentException error) { - return false; + return Mono.just(false); } + return Mono.fromCallable(() -> { + try { + var pluginPath = Path.of(normalizedPlugin).toRealPath(); + var providerPath = Path.of(normalizedProvider).toRealPath(); + return Files.isDirectory(pluginPath) && providerPath.startsWith(pluginPath); + } catch (IOException | IllegalArgumentException error) { + return false; + } + }) + .subscribeOn(Schedulers.boundedElastic()); } private void validateSchema(String toolName, String kind, Map schema) { diff --git a/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java b/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java index f94f3db..3a2aa27 100644 --- a/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java +++ b/src/test/java/run/halo/mcpserver/McpToolRegistryTest.java @@ -17,6 +17,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import reactor.test.StepVerifier; import run.halo.app.core.extension.Plugin; import run.halo.app.extension.Metadata; @@ -59,7 +60,8 @@ void executesAValidatedContributedTool() { .annotations(McpToolAnnotations.readOnly("Hello")) .permission(invocation -> Mono.just(true)) .handler(invocation -> Mono.just(McpToolResult.success( - Map.of("message", "Hello " + invocation.arguments().get("name"))))) + Map.of("message", "Hello " + invocation.arguments().get("name")), + "Custom greeting"))) .build(); providerTools(provider, "demo", tool); @@ -68,6 +70,10 @@ void executesAValidatedContributedTool() { .assertNext(result -> { assertThat(result).isPresent(); assertThat(result.orElseThrow().structuredContent().toString()).contains("Hello Halo"); + assertThat(result.orElseThrow().content().getFirst()) + .isInstanceOfSatisfying( + io.modelcontextprotocol.spec.McpSchema.TextContent.class, + content -> assertThat(content.text()).isEqualTo("Custom greeting")); }) .verifyComplete(); } @@ -255,9 +261,15 @@ void acceptsAProviderLoadedFromItsDevelopmentPluginDirectory() throws Exception .thenReturn(Flux.just(developmentProvider)); when(extensionClient.fetch(Plugin.class, "demo")).thenReturn(Mono.just(plugin)); - assertThat(registry.registeredTools().block()) - .extracting(tool -> tool.definition().name()) - .containsExactly("demo/hello"); + StepVerifier.create(Mono.defer(registry::registeredTools) + .subscribeOn(Schedulers.parallel())) + .assertNext(tools -> { + assertThat(tools) + .extracting(tool -> tool.definition().name()) + .containsExactly("demo/hello"); + assertThat(Schedulers.isInNonBlockingThread()).isFalse(); + }) + .verifyComplete(); } @Test @@ -267,7 +279,8 @@ void rejectsAProviderLoadedFromANeighboringDevelopmentDirectory(@TempDir Path te var neighboringDirectory = Files.createDirectory(tempDir.resolve("plugin-other")); assertThat(McpToolRegistry.ownsProvider( - pluginDirectory.toUri(), neighboringDirectory.toUri())).isFalse(); + pluginDirectory.toUri(), neighboringDirectory.toUri()).block()) + .isFalse(); } @Test