CAMEL-24310: camel-mcp-server - bridge, McpServerEngine SPI and Vert.x engine - #25306
Open
Croway wants to merge 8 commits into
Open
CAMEL-24310: camel-mcp-server - bridge, McpServerEngine SPI and Vert.x engine#25306Croway wants to merge 8 commits into
Croway wants to merge 8 commits into
Conversation
…istration changes Add AiToolRegistryListener with toolRegistered/toolDeregistered callbacks fired on ai-tool consumer lifecycle events (route start/resume registers, stop/suspend deregisters). Callbacks fire outside the registry lock, only on actual state changes, and a failing listener cannot break registration. Prerequisite for MCP tools/list_changed notifications (CAMEL-24308). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…x engine Expose ai-tool routes (CAMEL-23382) as MCP tools over streamable HTTP: - camel-mcp-server-api: runtime-agnostic bridge + McpServerEngine SPI. The bridge selects tools by tag (untagged default pool never exposed), refuses flat-namespace name collisions, executes via AiToolExecutor with a bounded per-call timeout, sanitizes execution errors, and reacts to AiToolRegistry listener events (CAMEL-24309). Enforcer rule bans MCP SDK/Reactor/Vert.x/platform-http from compile/runtime scope. Ships the engine conformance kit as a test-jar (CAMEL-24313). - camel-mcp-server: engine for Camel Main/JBang on the official MCP Java SDK with a custom Vert.x streamable HTTP transport registered on the platform HTTP router (the SDK ships only servlet/stdio server transports): POST json/SSE, GET SSE channel with Last-Event-ID replay, Mcp-Session-Id sessions, DELETE termination, tools/ list_changed on route lifecycle (CAMEL-24312). The api/engine module naming differs from the sub-task sketch (engine-as-runtime-dep would be a Maven dependency cycle): plain Camel users add camel-mcp-server; native-engine runtimes depend on camel-mcp-server-api, following the camel-langchain4j-agent-api precedent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gnodet
reviewed
Aug 3, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
This review was generated by Claude Code, an AI assistant, on behalf of @gnodet.
This is a well-designed PR that adds MCP server support to Camel with a clean architecture split between a runtime-agnostic bridge (SPI + tool selection/execution/sanitization) and a Vert.x engine implementation. The security considerations are excellent -- explicit tag-based opt-in, default pool exclusion, error sanitization, and bounded execution all align perfectly with the Camel security model where external MCP clients are untrusted senders.
The conformance test kit shipped as a test-jar is a smart approach to prevent behavioral drift across engine implementations (Vert.x, Quarkus, Spring Boot).
Architecture & API Design
The SPI design mirrors the established PlatformHttpEngine pattern, which is the right call for consistency. The separation of concerns is clean: the bridge owns tool selection, execution, timeout, and error sanitization; the engine owns protocol serving. The consumesServingConfiguration() method is a thoughtful touch for native engines.
The listener-before-snapshot pattern in McpServerBridge.doStart() correctly avoids a race window between subscribing to changes and reading the initial tool set.
Findings
-
Wrong artifact hint in error message —
resolveEngine()passes"camel-mcp-server-engine-vertx"as the artifact hint toResolverHelper.resolveMandatoryService, but the actual Maven artifact a user needs to add iscamel-mcp-server. When no engine is on the classpath, the error message will suggest adding a non-existent artifact, which is confusing.- Suggestion: Change
"camel-mcp-server-engine-vertx"to"camel-mcp-server"to match the actual Maven artifact name.
- Suggestion: Change
-
Missing
@since 4.22Javadoc tags — The public SPI types incamel-mcp-server-apilack@sincetags. While the strict CLAUDE.md requirement applies tocore/camel-api, these types form a public contract that Quarkus and Spring Boot engine implementations will depend on. Affected types:McpServerEngine,McpServerBridge,McpServerConfiguration,McpServerConstants,McpServerInfo,McpServerTool,McpToolCallHandler,McpToolCallResult. -
No session TTL/cleanup — The
sessionsmap inVertxMcpStreamableServerTransportProvidergrows with eachhandleInitializecall but sessions are only removed on explicit DELETE orcloseGracefully(). A client that disconnects without sending DELETE will leak a session. This could be addressed in a follow-up rather than blocking this PR. -
notifyClients()blocks without timeout —session.sendNotification(method, params).block()is called without a timeout. If a session's transport is stalled, this could block the notification loop indefinitely, preventing notifications to other healthy sessions. Same applies tocloseGracefully(). Consider using.block(Duration.ofSeconds(5)). -
Module listing order (nit) — In
camel-ai/pom.xml,camel-mcp-serveris listed beforecamel-mcp-server-api, but convention is to list dependencies before dependents. -
MESSAGE_EVENT_TYPEvisibility (nit) — Declaredpublic static finalbut only used internally. Could be package-private.
Test Coverage
Test coverage is solid: 8 bridge-level tests with a mock engine, 1 resolution test, and 6 conformance tests driven by the real MCP SDK client. Tests correctly use AssertJ, Awaitility, and package-private visibility.
Note on stacking
This PR is stacked on #25301 (CAMEL-24309, registry listener SPI), which is still open. The AiToolRegistryListener changes in the first commit belong to that PR.
- MainHttpServerMcpConformanceIT: the engine conformance kit against the Camel main HTTP server (camel-platform-http-main), the real Camel Main/JBang serving path. No Docker required. - McpServerOpenAIAgentIT: end-to-end agentic loop from CAMEL-24308 - the application exposes its own ai-tool routes over MCP and an LLM (camel-openai + Ollama test-infra) discovers and calls them with automatic tool execution. CI-gated like the other AI component ITs. - test-execution.md run-book following the camel-openai convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
- Fix engine-resolution error message to hint the real artifact (camel-mcp-server, not the pre-rename engine module name) - Add @SInCE 4.22 to the public SPI types in camel-mcp-server-api - Drop the session from the provider map on a failed SSE write, like the SDK servlet reference transport (session TTL/keep-alive eviction stays a follow-up) - Bound notifyClients/closeGracefully per-session blocks with a 5s timeout so one stalled session cannot starve the others - Reduce MESSAGE_EVENT_TYPE to private Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Croway
marked this pull request as ready for review
August 3, 2026 10:10
davsclaus
reviewed
Aug 3, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Rules & Conventions Review
No blocking issues found. This is a well-structured, thoroughly tested PR that follows project conventions and existing architectural patterns. The security considerations (tag-based opt-in, error sanitization, timeout handling) are solid.
Confirmed — No Issues
- MojoHelper registration —
camel-mcp-serveris added;camel-mcp-server-apiis not. Matches thecamel-langchain4j-agent/camel-langchain4j-agent-apiprecedent. - Parent POM / allcomponents / camel-ai parent — both modules correctly added.
- Test conventions — AssertJ assertions, no
Thread.sleep, Awaitility where appropriate, package-private test classes/methods. All per project standards. - Listener notifications outside lock —
AiToolRegistryfires events after releasing theReentrantLock, avoiding deadlocks. Event firing correctly checksset.add()/set.remove()return values. - Engine resolution — mirrors
PlatformHttpEnginepattern: registry bean wins, FactoryFinder fallback, clear failure message. - Security — default pool never exposed, flat-namespace collision refused with ERROR log, raw route exceptions never reach MCP clients, exchange not returned to pool on timeout.
- Documentation —
mcp-server.adocwith architecture, usage, options, protocol, security section. Symlink + nav entry present.
Questions / Open Points
- Stacked PR — #25301 is still OPEN. Confirming this PR should not be merged until #25301 merges and this is rebased.
test-execution.md— test run-book in the module root is useful but unconventional. Most modules don't have this. Minor style point.failsafe.rerunFailingTestsCount=3incamel-mcp-server/pom.xml— is this because the MCP transport tests are inherently timing-sensitive, or a defensive measure? Worth a comment if intentional.
This review covers rules and conventions only. It does not replace specialized review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Claus Ibsen (@davsclaus)
Use same-version xrefs (ROOT: module / relative) instead of the components:: prefix, which resolves to the latest released docs where the ai-tool page does not exist yet (docs validation failure). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
🤖 AI-assisted review — Claude Code on behalf of @gnodet Thanks for addressing the review observations! Confirmed the fixes in b27df53 and b5a8b00:
All prior findings addressed. No further observations from my side. |
davsclaus
reviewed
Aug 3, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Review: CAMEL-24310 — camel-mcp-server bridge, McpServerEngine SPI and Vert.x engine
Well-structured PR with a clean SPI design mirroring PlatformHttpEngine, solid security model (default pool never exposed, error sanitization, flat namespace collision protection, bounded execution), and comprehensive test coverage (bridge unit tests, conformance kit as test-jar, integration tests including end-to-end agentic loop).
Confirmed findings
- Prerequisite PR #25301 is still OPEN — this PR is stacked on it. Cannot merge until #25301 merges and this is rebased. Acknowledged in the PR description.
- Module ordering in
camel-ai/pom.xml—camel-mcp-serverlisted beforecamel-mcp-server-apidespite depending on it. Minor (Maven reactor handles it), suggestion below. - CI checks are pending — waiting for green.
Observations (non-blocking)
- Exchange leak on timeout (
McpServerBridge.execute()): intentional — the exchange is not released when the route is still running. Well-documented in the log message and PR description. Worth noting for routes that frequently timeout. notifyClientswall-clock: sessions iterated synchronously with.block()per session (5s timeout each). Worst-caseN × 5s. Follows the SDK servlet transport pattern, so consistent.test-execution.md: useful developer guidance; consider whether it could live as a comment in the IT class instead of a standalone file.
Positive observations
- Follows project conventions: AssertJ, no
Thread.sleep(), Awaitility, package-private test classes,@since 4.22on public API,xref:for internal doc links. - MojoHelper registration: only
camel-mcp-serverregistered (not-api), consistent withcamel-langchain4j-agent/camel-langchain4j-agent-apiprecedent. - Thread safety: listener notifications outside the registry lock, proper event-loop context handling in Vert.x transport.
mcp-server.adocdocumentation is thorough (architecture, usage, options, protocol, security).
This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.
This review was generated by an AI agent (Claude Code on behalf of @davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.
Comment on lines
+54
to
+55
| <module>camel-mcp-server</module> | ||
| <module>camel-mcp-server-api</module> |
Contributor
There was a problem hiding this comment.
Nit: camel-mcp-server depends on camel-mcp-server-api, so listing the API module first would be more natural and match the dependency direction:
Suggested change
| <module>camel-mcp-server</module> | |
| <module>camel-mcp-server-api</module> | |
| <module>camel-mcp-server-api</module> | |
| <module>camel-mcp-server</module> |
- Notify the engine while holding the bridge lock so publish/unpublish for the same tool cannot interleave and orphan a tool in the engine - Log the exchange id when a timed-out call keeps its pooled exchange - Reject POST requests without Content-Type application/json with 415 (MCP streamable HTTP spec conformance) - Bound session initialization with a 30s timeout Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Croway
force-pushed
the
CAMEL-24310-camel-mcp-server
branch
from
August 3, 2026 11:50
b5a8b00 to
7045ffa
Compare
orpiske
approved these changes
Aug 3, 2026
This was referenced Aug 3, 2026
davsclaus
reviewed
Aug 3, 2026
| ---- | ||
|
|
||
| NOTE: Configuration through `camel.server.mcp-*` properties (no code at all, | ||
| like Jolokia or Prometheus) is tracked by CAMEL-24311 and arrives together |
|
|
||
| Define tools as regular `ai-tool` routes and give them tags: | ||
|
|
||
| [source,yaml] |
Contributor
There was a problem hiding this comment.
add [tabs] and have this example in java, xml and yaml
| Start the MCP server by adding the `McpServerBridge` service to the | ||
| CamelContext, selecting the tags to expose: | ||
|
|
||
| [source,java] |
Contributor
There was a problem hiding this comment.
show the application properties configuration first. the hand coded is for advanced users
|
|
||
| == Protocol | ||
|
|
||
| The Vert.x engine implements the MCP streamable HTTP transport: |
Contributor
There was a problem hiding this comment.
i guess this is for main / quarkus / but spring boot will use the embedded http server from SB instead of vertx
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 577 tested, 25 compile-only — current: 76 all testedMaveniverse Scalpel detected 602 affected modules (current approach: 76).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CAMEL-24310: camel-mcp-server — bridge, McpServerEngine SPI and Vert.x engine
Sub-tasks of CAMEL-24308: implements CAMEL-24310 and CAMEL-24312, plus the conformance-kit half of CAMEL-24313.
What this adds
Two new modules under
components/camel-aiexposingai-toolroutes (CAMEL-23382) as MCP tools over streamable HTTP. No route is needed for the server itself.camel-mcp-server-api— runtime-agnostic bridge + SPIMcpServerEngineSPI (initialize/toolAdded/toolRemoved), resolution mirroringPlatformHttpEngine: registry bean → FactoryFinder (mcp-server-engine) → fail with a clear message.McpServerBridge(CamelContext service): selects registry tools by tag — never the untagged default pool (external MCP clients are untrusted senders); refuses flat-namespace name collisions with an ERROR log; subscribes to theAiToolRegistrylistener before snapshotting (no race window); executes viaAiToolExecutoron a managed pool with a bounded per-call timeout; mapsAiToolResultto pre-sanitized results — raw route exception messages never reach the engine or the client. A timed-out call does not return its pooled exchange (the route may still be using it).camel-mcp-server— Vert.x engine for Camel Main / JBangmcp-core2.0.0, already used by camel-openai as client) with a customMcpStreamableServerTransportProvider— the SDK ships only servlet and stdio server transports. Ported from the SDK'sHttpServletStreamableServerTransportProviderto the Vert.x model: request handling offloaded to the worker pool (unordered), all response writes on the connection's event-loop context, long-lived GET SSE channel without occupying a worker thread.application/jsonortext/event-stream, GET SSE withLast-Event-IDreplay,Mcp-Session-Idsessions, DELETE termination; session semantics fully delegated to the SDK'sMcpStreamableServerSession.tools/list_changedemitted automatically onaddTool/removeTool.VertxPlatformHttpRouter), so the endpoint serves on the main server port and sits behind its auth/CORS handlers.Module naming vs the sub-task sketch
CAMEL-24310 sketched the engine as a runtime dependency of the bridge module — that is a Maven dependency cycle (the engine compiles against the SPI). Following the
camel-langchain4j-agent-apiprecedent: plain Camel users addcamel-mcp-server(the parent issue's UX promise holds), native-engine runtimes depend oncamel-mcp-server-api. Noted on the JIRA.Out of scope (next PR)
camel.server.mcp-*properties + camel-main autowiring (CAMEL-24311) and the final docs polish (CAMEL-24314). An initialmcp-server.adoccomponent page (usage, options, protocol, security section) is included here.Testing
McpServerBridgeTest(8 tests, mock engine): tag selection, default-pool exclusion, collision refusal, multi-tag remove-once, lifecycle add/remove, success/argument-error/sanitized-error/timeout mapping.McpServerBridgeResolutionTest: clear startup failure when no engine is on the classpath.VertxMcpServerConformanceTest(6 tests): the conformance kit against the real engine over a Vert.x platform HTTP server, driven by the official MCP SDK client (McpClient.sync+HttpClientStreamableHttpTransport) — initialize, tools/list filtering, tools/call success, missing required argument, sanitized execution error, timeout, tools list reflecting route stop/start.Integration tests (failsafe, following the camel-openai/langchain4j-agent conventions):
MainHttpServerMcpConformanceIT— the conformance kit against the Camel main HTTP server (camel-platform-http-main), the real Camel Main/JBang serving path; no Docker required, runs in CI.McpServerOpenAIAgentIT— the end-to-end agentic loop from CAMEL-24308: the application exposes its ownai-toolroutes over MCP and an LLM (camel-openai + Ollama test-infra) discovers and calls them with automatic tool execution; CI-gated (ci.env.name) like the other AI ITs, verified locally against Ollama. Atest-execution.mdrun-book documents the LLM backend options.All 15 unit tests + 13 IT scenarios green;
formatter:format+impsort:sortapplied; generated catalog/spi files committed.This PR was written by Claude Code on behalf of Federico Mariani (@Croway).
🤖 Generated with Claude Code