feat: Add support for native remote MCP servers #943
Conversation
| this.client = HttpClient.newBuilder() | ||
| .connectTimeout(Duration.ofSeconds(30)) | ||
| .build(); |
There was a problem hiding this comment.
Use the smithy-java ClientTransport, not the Java HttpClient directly.
| throw new IllegalArgumentException("JsonRpcRequest cannot be null"); | ||
| } | ||
| String body = JSON_CODEC.serializeToString(request); | ||
| LOG.debug("Sending HTTP request to {}", endpoint); |
There was a problem hiding this comment.
This should be trace at the minimum.
| .connectTimeout(Duration.ofSeconds(30)) | ||
| .build(); | ||
| this.endpoint = URI.create(builder.url); | ||
| this.name = builder.name != null ? builder.name : "HTTP-" + endpoint.getHost(); |
There was a problem hiding this comment.
MCP server names have restrictions on what characters they can have. I don't think we can put the whole URI in here.
|
|
||
| public Builder headers(Map<String, String> headers) { | ||
| this.headers = headers; | ||
| return this; |
There was a problem hiding this comment.
This is assuming a static set of headers can be provided at initialization time which is wrong. Headers specially Auth Headers change over time and with requestbased on things like Session expiry, credentials refresh etc.
| boolean supportsOutputSchema = supportsOutputSchema(protocolVersion); | ||
| List<ToolInfo> filteredTools = tools.values() | ||
| .stream() | ||
| .filter(tool -> toolFilter.allowTool(tool.serverId(), tool.toolInfo().getName())) | ||
| .map(tool -> extractToolInfo(tool, supportsOutputSchema)) | ||
| .toList(); | ||
|
|
||
| return createSuccessResponse(req.getId(), | ||
| ListToolsResult.builder().tools(filteredTools).build()); |
| // Initialize proxies lazily after we have a real initialize request | ||
| if (!proxiesInitialized) { | ||
| initializeProxies(response -> { | ||
| // Proxies are initialized, no additional response needed | ||
| }); | ||
| proxiesInitialized = true; | ||
| } | ||
|
|
| public void startProxies() { | ||
| for (McpServerProxy proxy : proxies.values()) { | ||
| proxy.start(); | ||
| proxy.initialize(responseWriter, initializeRequest.get()); |
There was a problem hiding this comment.
initializeRequest.get() is giving me an NPE because we don't have any JSON-RPC request when the proxy starts. We need to either manually pass a JSON-RPC request to initialize the call or perform the initialization. For example:
2 participants
Issue #, if available:
Description of changes:
This commit adds support for native remote MCP servers with the following key changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.