Skip to content

Commit db46ee4

Browse files
authored
CAMEL-26396: camel-jbang - AI tools: the MCP server never takes itself for an integration (#28678)
A local camel_eval_expression starts a scratch CamelContext inside the MCP server. With camel-cli-connector on the classpath each scratch context started a CLI connector, which wrote ~/.camel/<server pid>-status.json, and the connector's stop deleted the file without waiting for a status write in flight, which could recreate it. The tools then took the server for an integration: camel_runtime_processes listed it, an evaluation without a name went to the server and waited 10 s for an answer, a call without a name was refused as ambiguous next to a real integration, and camel_control kill of its pid ended as an internal error (the JDK refuses to destroy the current process). - The scratch context of ExpressionEvaluator and the Camel Main that camel version list (camel_version_list) starts to download run without the CLI connector (new ScratchContextHelper). - RuntimeHelper.discoverProcesses / findProcess never return this process, also not by its pid; ProcessFinder is unchanged for the CLI commands. - A tool call that names this process's pid is refused with a clear error (ToolContext.selectProcess, select_process, MCP RuntimeService), and IntegrationLauncher.control refuses it for every action. - FileCliConnectorTransport.doStop waits (at most 5 s) for the poll in flight before it deletes the files, so a stopped CamelContext cannot leave its status file behind; not when stopped from the poll thread itself.
1 parent ba27c6a commit db46ee4

15 files changed

Lines changed: 583 additions & 13 deletions

File tree

‎docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,8 @@ help validate and scaffold that workflow, following the rules of the `rest-opena
850850
NOTE: Runtime tools require a running Camel application started via `camel run`. They communicate with the
851851
application through the file-based IPC protocol in `~/.camel/`. All tools accept an optional `nameOrPid`
852852
parameter; when omitted, the server auto-discovers the running Camel process (this works when exactly one
853-
process is running).
853+
process is running). The MCP server itself is never one of them: it is not listed, not selected when no name is
854+
given, and a call that names its pid is refused.
854855

855856
==== Process Discovery
856857

‎dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/FileCliConnectorTransport.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,16 @@ public class FileCliConnectorTransport extends ServiceSupport implements CliConn
6262
private CliSnapshotProducer snapshots;
6363
private Runnable shutdown;
6464
private ScheduledExecutorService executor;
65+
private volatile Thread pollThread;
6566
private ScheduledFuture<?> scheduledFuture;
6667
private int delay = 1000;
6768
private long counter;
6869
private final Map<File, PendingAction> pending = new HashMap<>();
6970

7071
// an action file this young that cannot be read may still be written: the Camel CLI before 4.24 writes it in place
7172
private static final long UNREADABLE_GRACE_MS = 2000;
73+
// how long a stop waits for the poll in flight before it removes the files
74+
private static final long STOP_POLL_TIMEOUT_MS = 5000;
7275

7376
private record PendingAction(File output, boolean replyAlways, CompletableFuture<CliActionOutcome> outcome) {
7477
}
@@ -108,6 +111,7 @@ protected void doStart() throws Exception {
108111
String threadName = ThreadHelper.resolveThreadName(null, "LocalCliConnector");
109112
Thread thread = new Thread(r, threadName);
110113
thread.setDaemon(true);
114+
pollThread = thread;
111115
return thread;
112116
});
113117

@@ -436,6 +440,7 @@ protected void doStop() throws Exception {
436440
if (scheduledFuture != null) {
437441
scheduledFuture.cancel(false);
438442
}
443+
awaitPoll();
439444
synchronized (this) {
440445
pending.forEach((request, action) -> {
441446
action.outcome().cancel(true);
@@ -480,6 +485,29 @@ protected void doStop() throws Exception {
480485
}
481486
}
482487

488+
/**
489+
* Waits a little for the poll in flight: a status written after the files are removed would stay behind, and the
490+
* Camel CLI would list this process as a running integration after its Camel has stopped (CAMEL-26396). Not when
491+
* the poll itself stops the transport, which would wait for itself.
492+
*/
493+
private void awaitPoll() {
494+
if (executor == null) {
495+
return;
496+
}
497+
// the poll is cancelled, so the thread ends once the poll in flight is done
498+
executor.shutdown();
499+
if (Thread.currentThread() == pollThread) {
500+
return;
501+
}
502+
try {
503+
if (!executor.awaitTermination(STOP_POLL_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
504+
LOG.debug("The poll of the Camel CLI connector has not finished within {} millis", STOP_POLL_TIMEOUT_MS);
505+
}
506+
} catch (InterruptedException e) {
507+
Thread.currentThread().interrupt();
508+
}
509+
}
510+
483511
private static String getPid() {
484512
return String.valueOf(ProcessHandle.current().pid());
485513
}
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.camel.cli.connector;
18+
19+
import java.io.File;
20+
import java.nio.file.Path;
21+
import java.util.concurrent.CountDownLatch;
22+
import java.util.concurrent.TimeUnit;
23+
24+
import org.apache.camel.impl.DefaultCamelContext;
25+
import org.apache.camel.util.json.JsonObject;
26+
import org.junit.jupiter.api.AfterEach;
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.io.TempDir;
30+
import org.junit.jupiter.api.parallel.Isolated;
31+
32+
import static org.assertj.core.api.Assertions.assertThat;
33+
import static org.awaitility.Awaitility.await;
34+
35+
/**
36+
* CAMEL-26396: a stop waits for the status write in flight, so a stopped integration leaves no status file behind. The
37+
* Camel CLI lists a live process with a status file as a running integration: the MCP server, whose scratch contexts
38+
* ran with the connector, listed itself and took itself for the integration to evaluate in.
39+
* <p>
40+
* Isolated because it changes {@code user.home}, and every connector in this JVM writes files named after the same pid.
41+
*/
42+
@Isolated
43+
class FileCliConnectorTransportStopTest {
44+
45+
private static final String PID = String.valueOf(ProcessHandle.current().pid());
46+
47+
@TempDir
48+
Path home;
49+
50+
private String oldHome;
51+
private DefaultCamelContext context;
52+
53+
@BeforeEach
54+
void useHome() {
55+
oldHome = System.getProperty("user.home");
56+
System.setProperty("user.home", home.toString());
57+
context = new DefaultCamelContext();
58+
}
59+
60+
@AfterEach
61+
void restoreHome() {
62+
context.stop();
63+
System.setProperty("user.home", oldHome);
64+
}
65+
66+
@Test
67+
void aStatusWriteInFlightDoesNotOutliveTheStop() throws Exception {
68+
SlowSnapshots snapshots = new SlowSnapshots();
69+
FileCliConnectorTransport transport = new FileCliConnectorTransport();
70+
transport.configure(context, (action, output) -> false, snapshots, () -> {
71+
});
72+
transport.start();
73+
File status = home.resolve(".camel").resolve(PID + "-status.json").toFile();
74+
// the first poll collects the status, which takes until the test lets it go on
75+
assertThat(snapshots.collecting.await(10, TimeUnit.SECONDS)).isTrue();
76+
77+
Thread stopper = new Thread(transport::stop, "test-stop");
78+
stopper.start();
79+
// the stop is done, as it was at once before the fix, or it waits for the poll
80+
await().atMost(10, TimeUnit.SECONDS).until(() -> switch (stopper.getState()) {
81+
case TERMINATED, WAITING, TIMED_WAITING -> true;
82+
default -> false;
83+
});
84+
snapshots.collected.countDown();
85+
86+
stopper.join(10_000);
87+
assertThat(stopper.isAlive()).isFalse();
88+
// the poll has written the status, before the stop removed the files or after
89+
snapshots.poll.join(10_000);
90+
assertThat(snapshots.poll.isAlive()).isFalse();
91+
assertThat(status).doesNotExist();
92+
}
93+
94+
/**
95+
* Snapshots whose status takes until the test counts down {@link #collected}.
96+
*/
97+
private static final class SlowSnapshots implements CliSnapshotProducer {
98+
99+
final CountDownLatch collecting = new CountDownLatch(1);
100+
final CountDownLatch collected = new CountDownLatch(1);
101+
volatile Thread poll;
102+
103+
@Override
104+
public JsonObject status() throws Exception {
105+
poll = Thread.currentThread();
106+
collecting.countDown();
107+
collected.await(10, TimeUnit.SECONDS);
108+
JsonObject context = new JsonObject();
109+
context.put("name", "demo");
110+
context.put("phase", 4);
111+
JsonObject root = new JsonObject();
112+
root.put("context", context);
113+
return root;
114+
}
115+
116+
@Override
117+
public JsonObject trace() {
118+
return null;
119+
}
120+
121+
@Override
122+
public JsonObject debug() {
123+
return null;
124+
}
125+
126+
@Override
127+
public JsonObject messageHistory() {
128+
return null;
129+
}
130+
131+
@Override
132+
public JsonObject errors() {
133+
return null;
134+
}
135+
136+
@Override
137+
public JsonObject receive() {
138+
return null;
139+
}
140+
141+
@Override
142+
public JsonObject activity() {
143+
return null;
144+
}
145+
}
146+
}

‎dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ExpressionEvaluator.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.camel.Expression;
2525
import org.apache.camel.Predicate;
2626
import org.apache.camel.catalog.LanguageValidationResult;
27+
import org.apache.camel.dsl.jbang.core.common.ScratchContextHelper;
2728
import org.apache.camel.impl.DefaultCamelContext;
2829
import org.apache.camel.main.download.DependencyDownloaderClassLoader;
2930
import org.apache.camel.main.download.MavenDependencyDownloader;
@@ -193,7 +194,8 @@ private static void evaluateLocally(
193194
*/
194195
private static boolean evaluateWith(
195196
ClassLoader loader, String lang, String expression, String body, boolean predicate, JsonObject result) {
196-
try (DefaultCamelContext context = new DefaultCamelContext(false)) {
197+
// without the CLI connector, which would make this process, such as the MCP server, a running integration
198+
try (DefaultCamelContext context = ScratchContextHelper.withoutCliConnector(new DefaultCamelContext(false))) {
197199
if (loader != null) {
198200
// before the start: the language resolver reads the class loader as the context comes up
199201
context.setApplicationContextClassLoader(loader);

‎dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,10 @@ private static String readTail(Path output) {
570570
public static String control(ToolContext ctx, String action) {
571571
ctx.requireProcess();
572572
long pid = ctx.pid();
573+
if (pid == ProcessHandle.current().pid()) {
574+
// the tools run in this process: it cannot kill itself (the JDK refuses), nor answer an action it waits for
575+
throw ToolContext.thisProcess(pid);
576+
}
573577
return switch (action) {
574578
case "stop" -> ctx.stopApplication();
575579
case "kill" -> {

‎dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolContext.java‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,12 +222,13 @@ private CamelCatalog loadCatalog() {
222222
* Selects the process a tool call names ({@code name} is an integration name or pid), or keeps the current
223223
* selection when the call names none.
224224
*
225-
* @throws ToolExecutionException when the name matches no running process
225+
* @throws ToolExecutionException when the name matches no running process, or is the pid of this process
226226
*/
227227
public void selectProcess(String nameOrPid) {
228228
if (nameOrPid == null || nameOrPid.isBlank()) {
229229
return;
230230
}
231+
refuseThisProcess(nameOrPid);
231232
RuntimeHelper.ProcessInfo p = RuntimeHelper.findProcess(nameOrPid);
232233
if (p == null) {
233234
throw new ToolExecutionException(
@@ -237,6 +238,27 @@ public void selectProcess(String nameOrPid) {
237238
selectProcess(p.pid());
238239
}
239240

241+
/**
242+
* Refuses the pid of this process, which runs the tools: a status file of its own comes from a CamelContext it ran
243+
* for itself, never from an integration, and it would wait for itself to answer an action (CAMEL-26396).
244+
*
245+
* @throws ToolExecutionException when the name or pid is the pid of this process
246+
*/
247+
static void refuseThisProcess(String nameOrPid) {
248+
if (RuntimeHelper.isThisProcess(nameOrPid)) {
249+
throw thisProcess(ProcessHandle.current().pid());
250+
}
251+
}
252+
253+
/**
254+
* The error for a tool call on the pid of this process (see {@link #refuseThisProcess(String)}).
255+
*/
256+
static ToolExecutionException thisProcess(long pid) {
257+
return new ToolExecutionException(
258+
"pid " + pid + " is the process that runs these tools (the MCP server, camel ask or the TUI), not a"
259+
+ " Camel integration; list_processes shows the integrations");
260+
}
261+
240262
public String readStatus(String section) {
241263
requireProcess();
242264
return RuntimeHelper.readStatusSection(pid, section);

‎dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ private static void registerProcessTools() {
134134
if (name == null || name.isBlank()) {
135135
throw new ToolExecutionException("name or PID is required");
136136
}
137+
ToolContext.refuseThisProcess(name);
137138
RuntimeHelper.ProcessInfo found = ctx.findProcess(name);
138139
if (found == null) {
139140
List<RuntimeHelper.ProcessInfo> processes = ctx.discoverProcesses();

‎dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/version/VersionList.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,12 @@
6060
import org.apache.camel.dsl.jbang.core.common.RuntimeCompletionCandidates;
6161
import org.apache.camel.dsl.jbang.core.common.RuntimeType;
6262
import org.apache.camel.dsl.jbang.core.common.RuntimeTypeConverter;
63+
import org.apache.camel.dsl.jbang.core.common.ScratchContextHelper;
6364
import org.apache.camel.dsl.jbang.core.common.VersionHelper;
6465
import org.apache.camel.dsl.jbang.core.model.VersionListDTO;
6566
import org.apache.camel.main.KameletMain;
6667
import org.apache.camel.main.download.MavenDependencyDownloader;
68+
import org.apache.camel.spi.CliConnectorFactory;
6769
import org.apache.camel.tooling.maven.RepositoryResolver;
6870
import org.apache.camel.tooling.model.ReleaseModel;
6971
import org.apache.camel.util.StringHelper;
@@ -400,6 +402,9 @@ Stream<CamelAndRuntimeVersions> downloadReleases() {
400402
mavenResolver.fresh());
401403
}
402404
KameletMain main = new KameletMain(CAMEL_INSTANCE_TYPE);
405+
// only to download: without the CLI connector, which would list this process (such as the MCP server, whose
406+
// camel_version_list runs this) as a running integration
407+
main.bind(CliConnectorFactory.FACTORY, ScratchContextHelper.disabledCliConnectorFactory());
403408
try {
404409
main.setFresh(mavenResolver.fresh());
405410
main.setDownload(mavenResolver.download());

‎dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/RuntimeHelper.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,13 @@ private RuntimeHelper() {
6161
/**
6262
* Discovers all running Camel processes by scanning {@code ~/.camel/} for {@code {pid}-status.json} files and
6363
* verifying each PID is still alive.
64+
* <p>
65+
* This process is never one of them: it runs the tools (camel ask, the TUI, the MCP server), and a status file of
66+
* its own comes from a CamelContext it ran for itself, not from an integration (CAMEL-26396).
6467
*/
6568
public static List<ProcessInfo> discoverProcesses() {
6669
List<ProcessInfo> result = new ArrayList<>();
67-
for (ProcessFinder.CamelProcess p : ProcessFinder.findAll()) {
70+
for (ProcessFinder.CamelProcess p : others(ProcessFinder.findAll())) {
6871
result.add(new ProcessInfo(p.pid(), p.name(), p.contextName()));
6972
}
7073
return result;
@@ -73,20 +76,37 @@ public static List<ProcessInfo> discoverProcesses() {
7376
/**
7477
* Finds a single Camel process matching the given name or PID. Returns {@code null} if no match is found or if
7578
* multiple processes match. When {@code nameOrPid} is {@code null} and exactly one process is running, returns that
76-
* process.
79+
* process. This process never matches, also not by its pid (see {@link #discoverProcesses()}).
7780
*
7881
* @param nameOrPid a process name (with optional wildcard), a numeric PID string, or {@code null} for auto-detect
7982
*/
8083
public static ProcessInfo findProcess(String nameOrPid) {
81-
List<ProcessFinder.CamelProcess> found = nameOrPid != null && !nameOrPid.isBlank()
82-
? ProcessFinder.find(nameOrPid) : ProcessFinder.findAll();
84+
List<ProcessFinder.CamelProcess> found = others(nameOrPid != null && !nameOrPid.isBlank()
85+
? ProcessFinder.find(nameOrPid) : ProcessFinder.findAll());
8386
if (found.size() == 1) {
8487
ProcessFinder.CamelProcess p = found.get(0);
8588
return new ProcessInfo(p.pid(), p.name(), p.contextName());
8689
}
8790
return null;
8891
}
8992

93+
/**
94+
* Whether the name or pid is the pid of this process, which {@link #findProcess(String)} never finds.
95+
*
96+
* @since 4.24
97+
*/
98+
public static boolean isThisProcess(String nameOrPid) {
99+
if (nameOrPid == null || !nameOrPid.matches("\\d{1,18}")) {
100+
return false;
101+
}
102+
return Long.parseLong(nameOrPid) == ProcessHandle.current().pid();
103+
}
104+
105+
private static List<ProcessFinder.CamelProcess> others(List<ProcessFinder.CamelProcess> processes) {
106+
long self = ProcessHandle.current().pid();
107+
return processes.stream().filter(p -> p.pid() != self).toList();
108+
}
109+
90110
/**
91111
* Reads the full status JSON for the given process.
92112
*

0 commit comments

Comments
 (0)