Skip to content

fix: improve MCP server plugin path handling and backward compatibility for argsPosition #6293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aias00 wants to merge 6 commits into
base: master
Choose a base branch
from
+257 −77
Open

fix: improve MCP server plugin path handling and backward compatibility for argsPosition #6293

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dcbc652
fix: improve MCP server plugin path handling and backward compatibili…
Aias00 Feb 9, 2026
31c9f53
fix: enhance MCP server plugin path validation and improve handling o…
Aias00 Feb 10, 2026
7544378
Update shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apach…
Aias00 Feb 10, 2026
823830c
Update shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apach…
Aias00 Feb 10, 2026
a92191f
fix: streamline MCP server plugin path handling by removing redundant…
Aias00 Feb 10, 2026
057eca9
Merge branch 'master' into fix/fix_mcp_server_plugin
Aias00 Feb 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ...ommon/src/main/java/org/apache/shenyu/client/mcp/generator/McpRequestConfigGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ public static JsonObject generateRequestConfig(final JsonObject openApiJson, fin
requestTemplate.addProperty(RequestTemplateConstants.METHOD_KEY, methodType);

// argsPosition
JsonObject argsPosition = new JsonObject();
JsonObject methodTypeJson = method.getAsJsonObject(methodType);
JsonArray parameters = methodTypeJson.getAsJsonArray(OpenApiConstants.OPEN_API_PATH_OPERATION_METHOD_PARAMETERS_KEY);
if (Objects.nonNull(parameters)) {
JsonObject argsPosition = new JsonObject();

for (JsonElement parameter : parameters) {
JsonObject paramObj = parameter.getAsJsonObject();

Expand All @@ -77,8 +76,11 @@ public static JsonObject generateRequestConfig(final JsonObject openApiJson, fin
argsPosition.addProperty(name, inValue);
}
}
requestTemplate.add(RequestTemplateConstants.ARGS_POSITION_KEY, argsPosition);
}
// Keep root-level argsPosition as canonical format used by gateway parser.
root.add(RequestTemplateConstants.ARGS_POSITION_KEY, argsPosition.deepCopy());
// Keep requestTemplate-level argsPosition for backward compatibility.
requestTemplate.add(RequestTemplateConstants.ARGS_POSITION_KEY, argsPosition);

// argsToJsonBody
requestTemplate.addProperty(RequestTemplateConstants.BODY_JSON_KEY, shenyuMcpRequestConfig.getBodyToJson());
Expand Down
5 changes: 4 additions & 1 deletion ...ient-mcp-register/src/main/java/org/apache/shenyu/client/mcp/McpServiceEventListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ private static String concatPaths(final String path1, final String path2) {
@Override
protected String buildApiSuperPath(final Class<?> clazz, final ShenyuMcpTool beanShenyuClient) {
Server[] servers = beanShenyuClient.definition().servers();
if (servers.length == 0) {
return "";
}
if (servers.length != 1) {
log.warn("The shenyuMcp service supports only a single server entry. Please ensure that only one server is configured");
}
Expand Down Expand Up @@ -363,7 +366,7 @@ private McpToolsRegisterDTO buildMcpToolsRegisterDTO(final Object bean, final Cl
validateClientConfig(shenyuMcpTool, url);
JsonObject openApiJson = McpOpenApiGenerator.generateOpenApiJson(classShenyuClient, shenyuMcpTool, url);
McpToolsRegisterDTO mcpToolsRegisterDTO = McpToolsRegisterDTOGenerator.generateRegisterDTO(shenyuMcpTool, openApiJson, url, namespaceId);
MetaDataRegisterDTO metaDataRegisterDTO = buildMetaDataDTO(bean, classShenyuClient, superPath, clazz, method, namespaceId);
MetaDataRegisterDTO metaDataRegisterDTO = buildMetaDataDTO(bean, classShenyuClient, url, clazz, method, namespaceId);
metaDataRegisterDTO.setEnabled(shenyuMcpTool.getEnable());
mcpToolsRegisterDTO.setMetaDataRegisterDTO(metaDataRegisterDTO);
return mcpToolsRegisterDTO;
Expand Down
64 changes: 39 additions & 25 deletions ...src/main/java/org/apache/shenyu/plugin/mcp/server/handler/McpServerPluginDataHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,17 @@ public void handlerSelector(final SelectorData selectorData) {
return;
}

// Get the URI from selector data
String uri = selectorData.getConditionList().stream()
.filter(condition -> Constants.URI.equals(condition.getParamType()))
.map(ConditionData::getParamValue)
.findFirst()
.orElse(null);

String path = StringUtils.removeEnd(uri, SLASH);
path = StringUtils.removeEnd(path, STAR);
String uri = extractSelectorUri(selectorData);
String path = normalizeSelectorPath(uri);
ShenyuMcpServer shenyuMcpServer = GsonUtils.getInstance().fromJson(StringUtils.isBlank(selectorData.getHandle()) ? DEFAULT_MESSAGE_ENDPOINT : selectorData.getHandle(), ShenyuMcpServer.class);
shenyuMcpServer.setPath(path);
CACHED_SERVER.get().cachedHandle(
selectorData.getId(),
shenyuMcpServer);
String messageEndpoint = shenyuMcpServer.getMessageEndpoint();
// Get or create McpServer for this URI
if (StringUtils.isNotBlank(uri) && !shenyuMcpServerManager.hasMcpServer(uri)) {
shenyuMcpServerManager.getOrCreateMcpServerTransport(uri, messageEndpoint);
if (StringUtils.isNotBlank(path) && !shenyuMcpServerManager.hasMcpServer(path)) {
shenyuMcpServerManager.getOrCreateMcpServerTransport(path, messageEndpoint);
}
if (StringUtils.isNotBlank(path)) {
shenyuMcpServerManager.getOrCreateStreamableHttpTransport(path + STREAMABLE_HTTP_PATH);
Expand All @@ -108,31 +101,27 @@ public void handlerSelector(final SelectorData selectorData) {

@Override
public void removeSelector(final SelectorData selectorData) {
if (Objects.isNull(selectorData) || Objects.isNull(selectorData.getId())) {
return;
}
UpstreamCacheManager.getInstance().removeByKey(selectorData.getId());
MetaDataCache.getInstance().clean();
CACHED_TOOL.get().removeHandle(CacheKeyUtils.INST.getKey(selectorData.getId(), Constants.DEFAULT_RULE));

// Remove the McpServer for this URI
// First try to get URI from handle, then from condition list
String uri = selectorData.getHandle();
if (StringUtils.isBlank(uri)) {
// Try to get URI from condition list
uri = selectorData.getConditionList().stream()
.filter(condition -> Constants.URI.equals(condition.getParamType()))
.map(ConditionData::getParamValue)
.findFirst()
.orElse(null);
}
String path = normalizeSelectorPath(extractSelectorUri(selectorData));

CACHED_SERVER.get().removeHandle(selectorData.getId());

if (StringUtils.isNotBlank(uri) && shenyuMcpServerManager.hasMcpServer(uri)) {
shenyuMcpServerManager.removeMcpServer(uri);
if (StringUtils.isNotBlank(path) && shenyuMcpServerManager.hasMcpServer(path)) {
shenyuMcpServerManager.removeMcpServer(path);
}
}

@Override
public void handlerRule(final RuleData ruleData) {
if (Objects.isNull(ruleData)) {
return;
}
Optional.ofNullable(ruleData.getHandle()).ifPresent(s -> {
ShenyuMcpServerTool mcpServerTool = GsonUtils.getInstance().fromJson(s, ShenyuMcpServerTool.class);
CACHED_TOOL.get().cachedHandle(CacheKeyUtils.INST.getKey(ruleData), mcpServerTool);
Comment on lines 127 to 133
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handlerRule now guards against ruleData == null, but it can still call addTool(server.getPath(), ...) later with a null/blank server.getPath() (e.g., selector had no URI condition or cached server path is empty). That can throw inside ShenyuMcpServerManager (ConcurrentHashMap null key). Consider also gating tool registration on server != null && StringUtils.isNotBlank(server.getPath()) (consistent with the removeRule check).

Copilot uses AI. Check for mistakes.
Expand All @@ -158,10 +147,15 @@ public void handlerRule(final RuleData ruleData) {

@Override
public void removeRule(final RuleData ruleData) {
if (Objects.isNull(ruleData)) {
return;
}
Optional.ofNullable(ruleData.getHandle()).ifPresent(s -> {
CACHED_TOOL.get().removeHandle(CacheKeyUtils.INST.getKey(ruleData));
ShenyuMcpServer server = CACHED_SERVER.get().obtainHandle(ruleData.getSelectorId());
shenyuMcpServerManager.removeTool(server.getPath(), ruleData.getName());
if (Objects.nonNull(server) && StringUtils.isNotBlank(server.getPath())) {
shenyuMcpServerManager.removeTool(server.getPath(), ruleData.getName());
}
});
MetaDataCache.getInstance().clean();
}
Expand All @@ -171,4 +165,24 @@ public String pluginNamed() {
return PluginEnum.MCP_SERVER.getName();
}

private String extractSelectorUri(final SelectorData selectorData) {
if (Objects.isNull(selectorData) || CollectionUtils.isEmpty(selectorData.getConditionList())) {
return null;
}
return selectorData.getConditionList().stream()
.filter(condition -> Constants.URI.equals(condition.getParamType()))
.map(ConditionData::getParamValue)
.findFirst()
.orElse(null);
}

private String normalizeSelectorPath(final String selectorUri) {
if (StringUtils.isBlank(selectorUri)) {
return selectorUri;
}
String path = StringUtils.removeEnd(selectorUri, STAR);
path = StringUtils.removeEnd(path, SLASH);
return StringUtils.defaultIfBlank(path, SLASH);
}

}
132 changes: 94 additions & 38 deletions ...ver/src/main/java/org/apache/shenyu/plugin/mcp/server/manager/ShenyuMcpServerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.util.Set;
import java.util.HashSet;
import java.util.Collections;
import java.net.URI;

/**
* Enhanced Manager for MCP servers supporting shared server instances across multiple transport protocols.
Expand Down Expand Up @@ -151,7 +152,7 @@ private <T> T getOrCreateTransport(final String normalizedPath, final String pro
* @return normalized path
*/
private String processPath(final String uri) {
return normalizeServerPath(extractBasePath(uri));
return normalizeServerPath(uri);
}

/**
Expand Down Expand Up @@ -223,7 +224,7 @@ private McpAsyncServer getOrCreateSharedServer(final String normalizedPath) {
* Creates SSE transport provider.
*/
private ShenyuSseServerTransportProvider createSseTransport(final String normalizedPath, final String messageEndPoint) {
String messageEndpoint = normalizedPath + messageEndPoint;
String messageEndpoint = joinPath(normalizedPath, messageEndPoint);
ShenyuSseServerTransportProvider transportProvider = ShenyuSseServerTransportProvider.builder()
.objectMapper(objectMapper)
.sseEndpoint(normalizedPath)
Expand Down Expand Up @@ -263,39 +264,17 @@ private ShenyuStreamableHttpServerTransportProvider createStreamableHttpTranspor
*/
private void registerRoutes(final String primaryPath, final String secondaryPath,
final HandlerFunction<?> primaryHandler, final HandlerFunction<?> secondaryHandler) {
routeMap.put(primaryPath, primaryHandler);
routeMap.put(primaryPath + "/**", primaryHandler);
String normalizedPrimaryPath = normalizeRoutePath(primaryPath);
routeMap.put(normalizedPrimaryPath, primaryHandler);
routeMap.put(normalizedPrimaryPath + "/**", primaryHandler);

if (Objects.nonNull(secondaryPath) && Objects.nonNull(secondaryHandler)) {
routeMap.put(secondaryPath, secondaryHandler);
routeMap.put(secondaryPath + "/**", secondaryHandler);
String normalizedSecondaryPath = normalizeRoutePath(secondaryPath);
routeMap.put(normalizedSecondaryPath, secondaryHandler);
routeMap.put(normalizedSecondaryPath + "/**", secondaryHandler);
}
}

/**
* Extract the base path from a URI by removing the /message suffix and any sub-paths.
*
* @param uri The URI to extract base path from
* @return The base path
*/
private String extractBasePath(final String uri) {
String basePath = uri;

// Remove /message suffix if present
if (basePath.endsWith("/message")) {
basePath = basePath.substring(0, basePath.length() - "/message".length());
}

// For sub-paths, extract the main MCP server path
String[] pathSegments = basePath.split("/");
if (pathSegments.length >= 2) {
// Keep only the first two segments (empty + server-name)
basePath = "/" + pathSegments[1];
}

return basePath;
}

/**
* Check if a McpServer exists for the given URI.
*
Expand Down Expand Up @@ -368,7 +347,7 @@ public void removeMcpServer(final String uri) {
*/
public synchronized void addTool(final String serverPath, final String name, final String description,
final String requestTemplate, final String inputSchema) {
String normalizedPath = normalizeServerPath(extractBasePath(serverPath));
String normalizedPath = processPath(serverPath);

// Remove existing tool first
try {
Expand Down Expand Up @@ -420,7 +399,7 @@ public synchronized void addTool(final String serverPath, final String name, fin
* @param name the tool name
*/
public void removeTool(final String serverPath, final String name) {
String normalizedPath = normalizeServerPath(serverPath);
String normalizedPath = processPath(serverPath);
LOG.debug("Removing tool from shared server - name: {}, path: {}", name, normalizedPath);

McpAsyncServer sharedServer = sharedServerMap.get(normalizedPath);
Expand Down Expand Up @@ -463,7 +442,7 @@ private boolean isToolNotFoundError(final Throwable error) {
* @return Set of supported protocols
*/
public Set<String> getSupportedProtocols(final String serverPath) {
String normalizedPath = normalizeServerPath(serverPath);
String normalizedPath = processPath(serverPath);
CompositeTransportProvider compositeTransport = compositeTransportMap.get(normalizedPath);
return Objects.nonNull(compositeTransport) ? compositeTransport.getSupportedProtocols() : new HashSet<>();
}
Expand All @@ -479,17 +458,94 @@ private String normalizeServerPath(final String path) {
return null;
}

String normalizedPath = path;
String normalizedPath = path.trim();
if (normalizedPath.isEmpty()) {
return "/";
}

try {
URI uri = URI.create(normalizedPath);
if (Objects.nonNull(uri.getScheme())) {
normalizedPath = uri.getRawPath();
}
} catch (IllegalArgumentException ignored) {
// Keep original input when it's not a full URI.
}

if (Objects.isNull(normalizedPath) || normalizedPath.isEmpty()) {
normalizedPath = "/";
}
if (!normalizedPath.startsWith("/")) {
normalizedPath = "/" + normalizedPath;
}
int queryStart = normalizedPath.indexOf('?');
if (queryStart >= 0) {
normalizedPath = normalizedPath.substring(0, queryStart);
}
int fragmentStart = normalizedPath.indexOf('#');
if (fragmentStart >= 0) {
normalizedPath = normalizedPath.substring(0, fragmentStart);
}

// Remove /streamablehttp suffix
if (normalizedPath.endsWith("/streamablehttp")) {
normalizedPath = normalizedPath.substring(0, normalizedPath.length() - "/streamablehttp".length());
LOG.debug("Normalized Streamable HTTP path from '{}' to '{}' for shared server", path, normalizedPath);
normalizedPath = normalizedPath.replaceAll("/{2,}", "/");
if (normalizedPath.endsWith("/**")) {
normalizedPath = normalizedPath.substring(0, normalizedPath.length() - "/**".length());
}
normalizedPath = removeSuffix(normalizedPath, "/message");
normalizedPath = removeSuffix(normalizedPath, "/sse");
normalizedPath = removeSuffix(normalizedPath, "/streamablehttp");

if (normalizedPath.length() > 1 && normalizedPath.endsWith("/")) {
normalizedPath = normalizedPath.substring(0, normalizedPath.length() - 1);
}
if (normalizedPath.isEmpty()) {
return "/";
}
return normalizedPath;
}

private String normalizeRoutePath(final String path) {
String routePath = Objects.isNull(path) ? "/" : path;
routePath = routePath.trim();
if (routePath.isEmpty()) {
return "/";
}
if (!routePath.startsWith("/")) {
routePath = "/" + routePath;
}
routePath = routePath.replaceAll("/{2,}", "/");
if (routePath.length() > 1 && routePath.endsWith("/")) {
routePath = routePath.substring(0, routePath.length() - 1);
}
return routePath;
}

private String joinPath(final String basePath, final String subPath) {
String safeBase = normalizeRoutePath(basePath);
if (Objects.isNull(subPath) || subPath.trim().isEmpty()) {
return safeBase;
}
String safeSub = subPath.trim();
if (safeBase.endsWith("/") && safeSub.startsWith("/")) {
return safeBase + safeSub.substring(1);
}
if (!safeBase.endsWith("/") && !safeSub.startsWith("/")) {
return safeBase + "/" + safeSub;
}
return safeBase + safeSub;
}

private String removeSuffix(final String value, final String suffix) {
if (Objects.isNull(value) || Objects.isNull(suffix) || suffix.isEmpty()) {
return value;
}
if (value.endsWith(suffix)) {
String result = value.substring(0, value.length() - suffix.length());
return result.isEmpty() ? "/" : result;
}
return value;
}

/**
* Composite transport provider that delegates to multiple transport implementations.
* Enhanced with protocol-aware session management and improved error handling.
Expand Down
12 changes: 11 additions & 1 deletion ...server/src/main/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import com.google.gson.JsonObject;
import org.apache.shenyu.common.utils.GsonUtils;

import java.util.Objects;

/**
* Helper class for parsing and handling requestConfig.
*/
Expand Down Expand Up @@ -51,7 +53,15 @@ public JsonObject getRequestTemplate() {
* @return the argument position json object
*/
public JsonObject getArgsPosition() {
return configJson.has("argsPosition") ? configJson.getAsJsonObject("argsPosition") : new JsonObject();
if (configJson.has("argsPosition")) {
return configJson.getAsJsonObject("argsPosition");
}
// Backward compatibility for configs generated with nested argsPosition.
JsonObject requestTemplate = getRequestTemplate();
if (Objects.nonNull(requestTemplate) && requestTemplate.has("argsPosition")) {
return requestTemplate.getAsJsonObject("argsPosition");
}
return new JsonObject();
}

/**
Expand Down
Loading
Loading