-
Notifications
You must be signed in to change notification settings - Fork 13
feat: GitHub MCP server via HTTP instead of STDIO (fixes #1733) #1797
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,11 @@ | |
| */ | ||
| package dev.enola.ai.mcp; | ||
|
|
||
| import static dev.enola.ai.mcp.McpServerConnectionsConfig.ServerConnection.Type.*; | ||
| import static dev.enola.ai.mcp.McpServerConnectionsConfig.ServerConnection.Type.http; | ||
| import static dev.enola.ai.mcp.McpServerConnectionsConfig.ServerConnection.Type.stdio; | ||
|
|
||
| import com.google.common.annotations.VisibleForTesting; | ||
| import com.google.common.base.Strings; | ||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.ImmutableMap; | ||
| import com.google.common.collect.MapMaker; | ||
|
|
@@ -36,6 +37,7 @@ | |
| import io.modelcontextprotocol.client.McpClient; | ||
| import io.modelcontextprotocol.client.McpSyncClient; | ||
| import io.modelcontextprotocol.client.transport.HttpClientSseClientTransport; | ||
| import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport; | ||
| import io.modelcontextprotocol.client.transport.ServerParameters; | ||
| import io.modelcontextprotocol.client.transport.StdioClientTransport; | ||
| import io.modelcontextprotocol.spec.McpClientTransport; | ||
|
|
@@ -46,6 +48,7 @@ | |
|
|
||
| import java.io.IOException; | ||
| import java.net.URI; | ||
| import java.net.http.HttpRequest; | ||
| import java.nio.file.Paths; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -140,15 +143,17 @@ McpServerConnectionsConfig.ServerConnection replaceSecretPlaceholders( | |
| private McpClientTransport createTransport( | ||
| McpServerConnectionsConfig.ServerConnection connectionConfig) { | ||
| var origin = connectionConfig.origin.toString(); | ||
| if (!Strings.isNullOrEmpty(connectionConfig.url) && !sse.equals(connectionConfig.type)) | ||
| connectionConfig.type = http; | ||
| switch (connectionConfig.type) { | ||
| case stdio -> { | ||
| var params = createStdIoServerParameters(connectionConfig); | ||
| var transport = new StdioClientTransport(params); | ||
| transport.setStdErrorHandler(new McpServerStdErrLogConsumer(origin)); | ||
| return transport; | ||
| } | ||
| case http -> { | ||
| return createHttpClientSseClientTransport(connectionConfig); | ||
| case http, sse -> { | ||
| return createHttpTransport(connectionConfig); | ||
| } | ||
| default -> | ||
| throw new IllegalArgumentException( | ||
|
|
@@ -169,9 +174,8 @@ private McpSyncClient createSyncClient(McpServerConnectionsConfig.ServerConnecti | |
| McpClient.sync(transport) | ||
| .clientInfo(implementation) | ||
| .capabilities(capabilities.build()) | ||
| // TODO Make this configurable - but how & from where? | ||
| // .initializationTimeout(Duration.ofSeconds(7)) | ||
| // .requestTimeout(Duration.ofSeconds(7)) | ||
| .initializationTimeout(config.timeout) | ||
| .requestTimeout(config.timeout) | ||
| .loggingConsumer(new McpServerLogConsumer(origin)) | ||
| .build(); | ||
|
|
||
|
|
@@ -196,21 +200,27 @@ private McpSyncClient createSyncClient(McpServerConnectionsConfig.ServerConnecti | |
| return client; | ||
| } | ||
|
|
||
| // TODO Inline (again) into above? | ||
|
|
||
| private ServerParameters createStdIoServerParameters(ServerConnection connectionConfig) { | ||
| if (connectionConfig.type != stdio) throw new IllegalArgumentException(); | ||
| return ServerParameters.builder(connectionConfig.command) | ||
| .args(connectionConfig.args) | ||
| .env(connectionConfig.env) | ||
| .build(); | ||
| } | ||
|
|
||
| private HttpClientSseClientTransport createHttpClientSseClientTransport( | ||
| ServerConnection connectionConfig) { | ||
| if (connectionConfig.type != http) throw new IllegalArgumentException(); | ||
| // TODO Set headers, timeout etc. | ||
| return HttpClientSseClientTransport.builder(connectionConfig.url).build(); | ||
| private McpClientTransport createHttpTransport(ServerConnection connectionConfig) { | ||
| var requestBuilder = HttpRequest.newBuilder(); | ||
| connectionConfig.headers.forEach(requestBuilder::header); | ||
| if (connectionConfig.type == http) { | ||
| var transportBuilder = HttpClientStreamableHttpTransport.builder(connectionConfig.url); | ||
| transportBuilder.requestBuilder(requestBuilder); | ||
| transportBuilder.connectTimeout(connectionConfig.timeout); | ||
| return transportBuilder.build(); | ||
| } else { // SSE | ||
| var transportBuilder = HttpClientSseClientTransport.builder(connectionConfig.url); | ||
| transportBuilder.requestBuilder(requestBuilder); | ||
| transportBuilder.connectTimeout(connectionConfig.timeout); | ||
| return transportBuilder.build(); | ||
| } | ||
| } | ||
|
|
||
| private Map<String, String> replaceSecretPlaceholders(Map<String, String> map) | ||
|
|
@@ -237,19 +247,31 @@ private List<String> replaceSecretPlaceholders(List<String> list) throws IOExcep | |
| } | ||
|
|
||
| private String replaceSecretPlaceholders(String value) throws IOException { | ||
| final String prefix = "${secret:"; | ||
| final String suffix = "}"; | ||
| if (!value.startsWith(prefix) || !value.endsWith(suffix)) return value; | ||
|
|
||
| var nameStartIndex = prefix.length(); | ||
| var nameEndIndex = value.length() - suffix.length(); | ||
| if (nameStartIndex >= nameEndIndex) | ||
| throw new IOException("Invalid secret placeholder: " + value); | ||
| var secretName = value.substring(nameStartIndex, nameEndIndex); | ||
|
|
||
| return secretManager | ||
| .getOptional(secretName) | ||
| .map(secret -> secret.map(String::new)) | ||
| .orElseThrow(() -> new IOException("Secret not found: " + value)); | ||
| final var prefix = "${secret:"; | ||
| final var suffix = '}'; | ||
| if (value.contains("${") && !value.contains(prefix)) | ||
| throw new IOException("Invalid secret placeholder; must be ${secret:XYZ}: " + value); | ||
| if (!value.contains(prefix)) return value; | ||
|
|
||
| int placeholderStart = value.indexOf(prefix); | ||
| if (placeholderStart == -1) { | ||
| return value; | ||
| } | ||
|
|
||
| int nameEnd = value.indexOf(suffix, placeholderStart); | ||
| if (nameEnd == -1) { | ||
| throw new IOException("Invalid secret placeholder, missing " + suffix + ": " + value); | ||
| } | ||
|
|
||
| int nameStart = placeholderStart + prefix.length(); | ||
| var secretName = value.substring(nameStart, nameEnd); | ||
|
|
||
| var secretValue = | ||
| secretManager | ||
| .getOptional(secretName) | ||
| .map(secretOpt -> secretOpt.map(String::new)) | ||
| .orElseThrow(() -> new IOException("Secret not found: " + secretName)); | ||
|
|
||
| return value.substring(0, placeholderStart) + secretValue + value.substring(nameEnd + 1); | ||
| } | ||
|
Comment on lines
249
to
276
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation of private String replaceSecretPlaceholders(String value) throws IOException {
final var prefix = "${secret:";
final var suffix = '}';
if (value.contains("${") && !value.contains(prefix)) {
throw new IOException("Invalid secret placeholder; must be ${secret:XYZ}: " + value);
}
if (!value.contains(prefix)) {
return value;
}
final var result = new StringBuilder(value.length());
int lastIndex = 0;
int placeholderStart;
while ((placeholderStart = value.indexOf(prefix, lastIndex)) != -1) {
result.append(value, lastIndex, placeholderStart);
int nameEnd = value.indexOf(suffix, placeholderStart);
if (nameEnd == -1) {
throw new IOException("Invalid secret placeholder, missing " + suffix + ": " + value);
}
int nameStart = placeholderStart + prefix.length();
var secretName = value.substring(nameStart, nameEnd);
var secretValue =
secretManager
.getOptional(secretName)
.map(secretOpt -> secretOpt.map(String::new))
.orElseThrow(() -> new IOException("Secret not found: " + secretName));
result.append(secretValue);
lastIndex = nameEnd + 1;
}
result.append(value.substring(lastIndex));
return result.toString();
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct, of course, but this shouldn't be required; I'll keep it as-is, for now - just for simplicity. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| $schema: https://enola.dev/ai/agent | ||
| model: google://?model=gemini-2.5-flash | ||
| tools: | ||
| - github |
Uh oh!
There was an error while loading. Please reload this page.