feat: GitHub MCP server via HTTP instead of STDIO (fixes #1733) #1797

vorburger · 2025-09-22T19:07:36Z

./test.bash passes locally
Appropriate changes to documentation are included in the PR

vorburger · 2025-09-22T19:08:41Z

Currently still fails with Caused by: io.modelcontextprotocol.spec.McpTransportException: Invalid SSE response. Status code: 404 Line: 404 page not found, most likely because headers are not passed through here...

vorburger · 2025-09-22T20:35:34Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for GitHub MCP servers over HTTP, moving away from STDIO. The changes include updating documentation, adding new transport types (http, sse), making timeouts configurable, and implementing logic to handle HTTP-based MCP connections. The implementation for replacing secret placeholders in configuration values has also been improved to support placeholders anywhere in a string. My review focuses on improving the robustness of the new secret replacement logic, enhancing code clarity through better naming, and correcting a minor inaccuracy in the documentation.

gemini-code-assist · 2025-09-22T20:36:54Z

java/dev/enola/ai/mcp/McpLoader.java

    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);
    }


The current implementation of replaceSecretPlaceholders only replaces the first secret placeholder found in the input string. If a string contains multiple placeholders (e.g., "key1=${secret:A}&key2=${secret:B}"), subsequent placeholders will not be replaced. The implementation should be updated to iteratively replace all occurrences of secret placeholders.

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(); }

You are correct, of course, but this shouldn't be required; I'll keep it as-is, for now - just for simplicity.

docs/concepts/mcp.md

java/dev/enola/ai/mcp/McpLoader.java

vorburger · 2025-09-22T21:27:42Z

https://github.com/enola-dev/enola/actions/runs/17927982154/job/50978765188?pr=1797 failed due to:

2025-09-22 20:58:28 INFO ch.vorburger.exec.ManagedProcess startPreparation Starting Program [/usr/bin/env, bash, -c, cd ../.././.. && ./enola fetch --ipfs-gateway=https://dweb.link/ipfs/ ipfs://QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu] (in working directory /home/runner/work/enola/enola/docs/use/fetch)
2025-09-22 20:58:28 INFO ch.vorburger.exec.ManagedProcess waitForExitMaxMs Thread is now going to wait max. 7000ms for process to terminate itself: Program [/usr/bin/env, bash, -c, cd ../.././.. && ./enola fetch --ipfs-gateway=https://dweb.link/ipfs/ ipfs://QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu] (in working directory /home/runner/work/enola/enola/docs/use/fetch)
2025-09-22 20:58:30 SEVERE ch.vorburger.exec.SLF4jLogOutputStream processLine env: Internal Problem occurred, add -vvv flags for technical details: IOException: 500 https://dweb.link/ipfs/QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu : 
2025-09-22 20:58:31 SEVERE ch.vorburger.exec.ManagedProcess lambda$new$0 Program [/usr/bin/env, bash, -c, cd ../.././.. && ./enola fetch --ipfs-gateway=https://dweb.link/ipfs/ ipfs://QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu] (in working directory /home/runner/work/enola/enola/docs/use/fetch) failed unexpectedly
org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:355)
	at org.apache.commons.exec.DefaultExecutor.lambda$execute$0(DefaultExecutor.java:269)
	at java.base/java.lang.Thread.run(Thread.java:1583)
2025-09-22 20:58:31 SEVERE ch.vorburger.exec.ManagedProcess waitForExitMaxMsWithoutLog Program [/usr/bin/env, bash, -c, cd ../.././.. && ./enola fetch --ipfs-gateway=https://dweb.link/ipfs/ ipfs://QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu] (in working directory /home/runner/work/enola/enola/docs/use/fetch) failed
java.util.concurrent.ExecutionException: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:396)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2096)
	at ch.vorburger.exec.ManagedProcess.waitForExitMaxMsWithoutLog(ManagedProcess.java:478)
	at ch.vorburger.exec.ManagedProcess.waitForExitMaxMs(ManagedProcess.java:470)
	at dev.enola.common.exec.vorburger.VorburgerExecRunner.exec(VorburgerExecRunner.java:57)
	at dev.enola.common.exec.vorburger.Runner.bash(Runner.java:52)
	at dev.enola.common.markdown.exec.ExecMD.exec(ExecMD.java:146)
	at dev.enola.common.markdown.exec.ExecMD.process(ExecMD.java:97)
	at dev.enola.common.markdown.exec.ExecMD.process(ExecMD.java:42)
	at dev.enola.cli.ExecMdCommand.call(ExecMdCommand.java:66)
	at dev.enola.cli.ExecMdCommand.call(ExecMdCommand.java:35)
	at picocli.CommandLine.executeUserObject(CommandLine.java:2031)
	at picocli.CommandLine.access$1500(CommandLine.java:148)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2469)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2461)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2423)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2277)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2425)
	at dev.enola.cli.common.LoggingMixin.executionStrategy(LoggingMixin.java:77)
	at picocli.CommandLine.execute(CommandLine.java:2174)
	at dev.enola.cli.common.CLI.execute(CLI.java:93)
	at dev.enola.cli.EnolaApplication.main(EnolaApplication.java:64)
Caused by: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:355)
	at org.apache.commons.exec.DefaultExecutor.lambda$execute$0(DefaultExecutor.java:269)
	at java.base/java.lang.Thread.run(Thread.java:1583)
dev.enola.common.markdown.exec.MarkdownProcessingException: exec failed (use ```bash $? marker if that's expected): cd ../.././.. && ./enola fetch --ipfs-gateway=https://dweb.link/ipfs/ ipfs://QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu
	at dev.enola.common.markdown.exec.ExecMD.exec(ExecMD.java:148)
	at dev.enola.common.markdown.exec.ExecMD.process(ExecMD.java:97)
	at dev.enola.common.markdown.exec.ExecMD.process(ExecMD.java:42)
	at dev.enola.cli.ExecMdCommand.call(ExecMdCommand.java:66)
	at dev.enola.cli.ExecMdCommand.call(ExecMdCommand.java:35)
	at picocli.CommandLine.executeUserObject(CommandLine.java:2031)
	at picocli.CommandLine.access$1500(CommandLine.java:148)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2469)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2461)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2423)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2277)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2425)
	at dev.enola.cli.common.LoggingMixin.executionStrategy(LoggingMixin.java:77)
	at picocli.CommandLine.execute(CommandLine.java:2174)
	at dev.enola.cli.common.CLI.execute(CLI.java:93)
	at dev.enola.cli.EnolaApplication.main(EnolaApplication.java:64)
Caused by: ch.vorburger.exec.ManagedProcessException: Program [/usr/bin/env, bash, -c, cd ../.././.. && ./enola fetch --ipfs-gateway=https://dweb.link/ipfs/ ipfs://QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu] (in working directory /home/runner/work/enola/enola/docs/use/fetch) failed with Exception: , last 100 lines of console:
Internal Problem occurred, add -vvv flags for technical details: IOException: 500 https://dweb.link/ipfs/QmXV7pL1CB7A8Tzk7jP2XE9kRyk8HZd145KDptdxzmNLfu : 
	at ch.vorburger.exec.ManagedProcess.waitForExitMaxMsWithoutLog(ManagedProcess.java:493)
	at ch.vorburger.exec.ManagedProcess.waitForExitMaxMs(ManagedProcess.java:470)
	at dev.enola.common.exec.vorburger.VorburgerExecRunner.exec(VorburgerExecRunner.java:57)
	at dev.enola.common.exec.vorburger.Runner.bash(Runner.java:52)
	at dev.enola.common.markdown.exec.ExecMD.exec(ExecMD.java:146)
	... 15 more
Caused by: java.util.concurrent.ExecutionException: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:396)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2096)
	at ch.vorburger.exec.ManagedProcess.waitForExitMaxMsWithoutLog(ManagedProcess.java:478)
	... 19 more
Caused by: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:355)
	at org.apache.commons.exec.DefaultExecutor.lambda$execute$0(DefaultExecutor.java:269)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Error: Process completed with exit code 1.

This seems to be a tempoary failure on https://dweb.link ...

... which really shouldn't block this build! So #1798 is probably a good idea.

vorburger force-pushed the mcp-github branch 2 times, most recently from 2c0e275 to d9da416 Compare September 22, 2025 20:33

vorburger marked this pull request as ready for review September 22, 2025 20:35

gemini-code-assist bot reviewed Sep 22, 2025

View reviewed changes

feat: GitHub MCP server via HTTP instead of STDIO (fixes enola-dev#1733)

3bf97a6

vorburger force-pushed the mcp-github branch from d9da416 to 3bf97a6 Compare September 22, 2025 20:43

vorburger enabled auto-merge (rebase) September 22, 2025 20:45

vorburger mentioned this pull request Sep 22, 2025

feat: ExecMD $% for ExpectedExitCode.IGNORE #1798

Merged

vorburger merged commit 0706d57 into enola-dev:main Sep 22, 2025
17 of 18 checks passed

vorburger deleted the mcp-github branch September 22, 2025 21:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: GitHub MCP server via HTTP instead of STDIO (fixes #1733) #1797

feat: GitHub MCP server via HTTP instead of STDIO (fixes #1733) #1797

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Sep 22, 2025

Uh oh!

vorburger Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

feat: GitHub MCP server via HTTP instead of STDIO (fixes #1733) #1797

feat: GitHub MCP server via HTTP instead of STDIO (fixes #1733) #1797

Uh oh!

Conversation

vorburger commented Sep 22, 2025

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

vorburger Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vorburger commented Sep 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant