gnodet-bot commented on code in PR #29454:
URL: https://github.com/apache/camel/pull/29454#discussion_r4512942381
##########
components/camel-ai/camel-mcp-server-api/src/main/java/org/apache/camel/component/mcp/server/McpServerBridge.java:
##########
@@ -405,6 +433,191 @@ private McpResourceReadResult read(AiResourceSpec spec) {
}
}
+ private void publishPrompt(AiPromptSpec spec) {
+ lock.lock();
+ try {
+ AiPromptSpec existing = publishedPrompts.get(spec.getName());
+ if (existing == spec) {
+ return;
+ }
+ if (existing != null) {
+ LOG.error("Refusing to expose MCP prompt '{}': the name
collides with an already exposed prompt.",
+ spec.getName());
+ return;
+ }
+ if (!engine.supportsPrompts()) {
+ if (!promptsUnsupportedWarned) {
+ promptsUnsupportedWarned = true;
+ LOG.warn("Engine {} does not serve MCP prompts: ai-prompt
routes matching the configured tags "
+ + "are not exposed",
+ engine.getClass().getSimpleName());
+ }
+ return;
+ }
+ // Publish to the engine before recording the prompt. If the
engine refuses
+ // the name, keep the route running but do not track it as
published.
+ try {
+ engine.promptAdded(createPrompt(spec));
+ publishedPrompts.put(spec.getName(), spec);
+ } catch (McpServerRefusedException e) {
+ LOG.error("Refusing to expose MCP prompt '{}': {}",
spec.getName(), e.getMessage(), e);
+ }
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ private void unpublishPrompt(AiPromptSpec spec) {
+ lock.lock();
+ try {
+ if (publishedPrompts.get(spec.getName()) != spec) {
+ return;
+ }
+ boolean stillSelected =
promptRegistry.getPrompts().entrySet().stream()
+ .anyMatch(entry -> matchesTag(entry.getKey()) &&
entry.getValue().contains(spec));
+ if (!stillSelected) {
+ publishedPrompts.remove(spec.getName());
+ engine.promptRemoved(spec.getName());
+ }
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ private McpServerPrompt createPrompt(AiPromptSpec spec) {
+ McpPromptHandler handler = arguments -> executePrompt(spec, arguments);
+ return new McpServerPrompt() {
+ @Override
+ public String name() {
+ return spec.getName();
+ }
+
+ @Override
+ public String description() {
+ return spec.getDescription();
+ }
+
+ @Override
+ public String title() {
+ return spec.getTitle();
+ }
+
+ @Override
+ public List<McpPromptArgument> arguments() {
+ return spec.getArguments().stream()
+ .map(argument -> new McpPromptArgument(
+ argument.name(), argument.description(),
argument.required()))
+ .toList();
+ }
+
+ @Override
+ public McpPromptHandler handler() {
+ return handler;
+ }
+ };
+ }
+
+ private McpPromptResult executePrompt(AiPromptSpec spec, Map<String,
String> arguments) {
+ Exchange exchange = spec.getConsumer().getEndpoint().createExchange();
+
+ // 0 = queued/not started, 1 = running, 2 = exchange ownership
completed.
+ // The worker releases the exchange after route execution. The caller
only
+ // releases it when cancellation wins before the worker starts.
+ AtomicInteger executionState = new
java.util.concurrent.atomic.AtomicInteger();
Review Comment:
⚠️ **Nit (from @davsclaus's second review — still open):** `AtomicInteger`
is already imported (line 30 of the new imports). The FQN `new
java.util.concurrent.atomic.AtomicInteger()` should use the simple name.
```suggestion
AtomicInteger executionState = new AtomicInteger();
```
##########
components/camel-ai/camel-mcp-server/src/test/java/org/apache/camel/component/mcp/server/stdio/StdioMcpServerEngineTest.java:
##########
@@ -80,6 +86,28 @@ void toolAddedBeforeStartIsQueuedWithoutNpe() {
assertThatCode(() ->
engine.toolRemoved("queued_tool")).doesNotThrowAnyException();
}
+ @Test
+ void promptAddedBeforeStartIsFlushedWhenServerStarts() throws Exception {
+ StdioMcpServerEngine engine = new StdioMcpServerEngine();
+ engine.initialize(new
org.apache.camel.component.mcp.server.McpServerInfo(
+ "test", "1.0", "/mcp", 0, 0, null, null, null, null, null));
+ engine.setTransportStreams(new ByteArrayInputStream(new byte[0]), new
ByteArrayOutputStream());
+ engine.promptAdded(samplePrompt("queued_prompt", "Queued before
start"));
+
+ assertThatCode(engine::start).doesNotThrowAnyException();
Review Comment:
💡 **Low (optional, from @davsclaus's second review):**
`promptAddedBeforeStartIsFlushedWhenServerStarts` only asserts
`doesNotThrowAnyException()` — it doesn't verify the prompt was actually
registered in the started server. Similarly,
`promptRemovedBeforeStartRemovesItFromPendingQueue` only checks no exception,
not that the pending queue is empty. The existing tool-queuing test has the
same weakness, but adding a prompt-specific assertion (e.g. that the started
engine serves the queued prompt, or that `pendingPrompts` is empty after
`promptRemoved`) would make these tests meaningful rather than just smoke
checks.
##########
components/camel-ai/camel-mcp-server/src/test/java/org/apache/camel/component/mcp/server/stdio/StdioMcpServerEngineTest.java:
##########
@@ -80,6 +86,28 @@ void toolAddedBeforeStartIsQueuedWithoutNpe() {
assertThatCode(() ->
engine.toolRemoved("queued_tool")).doesNotThrowAnyException();
}
+ @Test
+ void promptAddedBeforeStartIsFlushedWhenServerStarts() throws Exception {
+ StdioMcpServerEngine engine = new StdioMcpServerEngine();
+ engine.initialize(new
org.apache.camel.component.mcp.server.McpServerInfo(
Review Comment:
💡 **Nit (optional, from @davsclaus's second review):** `McpServerInfo` is
used via FQN in both new test methods. Adding `import
org.apache.camel.component.mcp.server.McpServerInfo;` alongside the other new
imports would clean this up — the file already follows the import pattern for
all other new types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
