diff --git a/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc b/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc index f6d9ecf98fa0c..2bbecb27085ad 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc @@ -812,7 +812,8 @@ help validate and scaffold that workflow, following the rules of the `rest-opena NOTE: Runtime tools require a running Camel application started via `camel run`. They communicate with the application through the file-based IPC protocol in `~/.camel/`. All tools accept an optional `nameOrPid` parameter; when omitted, the server auto-discovers the running Camel process (this works when exactly one -process is running). +process is running). The MCP server itself is never one of them: it is not listed, not selected when no name is +given, and a call that names its pid is refused. ==== Process Discovery diff --git a/dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/FileCliConnectorTransport.java b/dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/FileCliConnectorTransport.java index 4195b13a47239..278ec3d115570 100644 --- a/dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/FileCliConnectorTransport.java +++ b/dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/FileCliConnectorTransport.java @@ -62,6 +62,7 @@ public class FileCliConnectorTransport extends ServiceSupport implements CliConn private CliSnapshotProducer snapshots; private Runnable shutdown; private ScheduledExecutorService executor; + private volatile Thread pollThread; private ScheduledFuture scheduledFuture; private int delay = 1000; private long counter; @@ -69,6 +70,8 @@ public class FileCliConnectorTransport extends ServiceSupport implements CliConn // an action file this young that cannot be read may still be written: the Camel CLI before 4.24 writes it in place private static final long UNREADABLE_GRACE_MS = 2000; + // how long a stop waits for the poll in flight before it removes the files + private static final long STOP_POLL_TIMEOUT_MS = 5000; private record PendingAction(File output, boolean replyAlways, CompletableFuture outcome) { } @@ -108,6 +111,7 @@ protected void doStart() throws Exception { String threadName = ThreadHelper.resolveThreadName(null, "LocalCliConnector"); Thread thread = new Thread(r, threadName); thread.setDaemon(true); + pollThread = thread; return thread; }); @@ -436,6 +440,7 @@ protected void doStop() throws Exception { if (scheduledFuture != null) { scheduledFuture.cancel(false); } + awaitPoll(); synchronized (this) { pending.forEach((request, action) -> { action.outcome().cancel(true); @@ -480,6 +485,29 @@ protected void doStop() throws Exception { } } + /** + * Waits a little for the poll in flight: a status written after the files are removed would stay behind, and the + * Camel CLI would list this process as a running integration after its Camel has stopped (CAMEL-26396). Not when + * the poll itself stops the transport, which would wait for itself. + */ + private void awaitPoll() { + if (executor == null) { + return; + } + // the poll is cancelled, so the thread ends once the poll in flight is done + executor.shutdown(); + if (Thread.currentThread() == pollThread) { + return; + } + try { + if (!executor.awaitTermination(STOP_POLL_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + LOG.debug("The poll of the Camel CLI connector has not finished within {} millis", STOP_POLL_TIMEOUT_MS); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + private static String getPid() { return String.valueOf(ProcessHandle.current().pid()); } diff --git a/dsl/camel-cli-connector/src/test/java/org/apache/camel/cli/connector/FileCliConnectorTransportStopTest.java b/dsl/camel-cli-connector/src/test/java/org/apache/camel/cli/connector/FileCliConnectorTransportStopTest.java new file mode 100644 index 0000000000000..459bdad2c5aa4 --- /dev/null +++ b/dsl/camel-cli-connector/src/test/java/org/apache/camel/cli/connector/FileCliConnectorTransportStopTest.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.cli.connector; + +import java.io.File; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.camel.util.json.JsonObject; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.parallel.Isolated; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * CAMEL-26396: a stop waits for the status write in flight, so a stopped integration leaves no status file behind. The + * Camel CLI lists a live process with a status file as a running integration: the MCP server, whose scratch contexts + * ran with the connector, listed itself and took itself for the integration to evaluate in. + *

+ * Isolated because it changes {@code user.home}, and every connector in this JVM writes files named after the same pid. + */ +@Isolated +class FileCliConnectorTransportStopTest { + + private static final String PID = String.valueOf(ProcessHandle.current().pid()); + + @TempDir + Path home; + + private String oldHome; + private DefaultCamelContext context; + + @BeforeEach + void useHome() { + oldHome = System.getProperty("user.home"); + System.setProperty("user.home", home.toString()); + context = new DefaultCamelContext(); + } + + @AfterEach + void restoreHome() { + context.stop(); + System.setProperty("user.home", oldHome); + } + + @Test + void aStatusWriteInFlightDoesNotOutliveTheStop() throws Exception { + SlowSnapshots snapshots = new SlowSnapshots(); + FileCliConnectorTransport transport = new FileCliConnectorTransport(); + transport.configure(context, (action, output) -> false, snapshots, () -> { + }); + transport.start(); + File status = home.resolve(".camel").resolve(PID + "-status.json").toFile(); + // the first poll collects the status, which takes until the test lets it go on + assertThat(snapshots.collecting.await(10, TimeUnit.SECONDS)).isTrue(); + + Thread stopper = new Thread(transport::stop, "test-stop"); + stopper.start(); + // the stop is done, as it was at once before the fix, or it waits for the poll + await().atMost(10, TimeUnit.SECONDS).until(() -> switch (stopper.getState()) { + case TERMINATED, WAITING, TIMED_WAITING -> true; + default -> false; + }); + snapshots.collected.countDown(); + + stopper.join(10_000); + assertThat(stopper.isAlive()).isFalse(); + // the poll has written the status, before the stop removed the files or after + snapshots.poll.join(10_000); + assertThat(snapshots.poll.isAlive()).isFalse(); + assertThat(status).doesNotExist(); + } + + /** + * Snapshots whose status takes until the test counts down {@link #collected}. + */ + private static final class SlowSnapshots implements CliSnapshotProducer { + + final CountDownLatch collecting = new CountDownLatch(1); + final CountDownLatch collected = new CountDownLatch(1); + volatile Thread poll; + + @Override + public JsonObject status() throws Exception { + poll = Thread.currentThread(); + collecting.countDown(); + collected.await(10, TimeUnit.SECONDS); + JsonObject context = new JsonObject(); + context.put("name", "demo"); + context.put("phase", 4); + JsonObject root = new JsonObject(); + root.put("context", context); + return root; + } + + @Override + public JsonObject trace() { + return null; + } + + @Override + public JsonObject debug() { + return null; + } + + @Override + public JsonObject messageHistory() { + return null; + } + + @Override + public JsonObject errors() { + return null; + } + + @Override + public JsonObject receive() { + return null; + } + + @Override + public JsonObject activity() { + return null; + } + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ExpressionEvaluator.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ExpressionEvaluator.java index 3f3d44227959e..1bb3c5dfc3ea7 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ExpressionEvaluator.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ExpressionEvaluator.java @@ -24,6 +24,7 @@ import org.apache.camel.Expression; import org.apache.camel.Predicate; import org.apache.camel.catalog.LanguageValidationResult; +import org.apache.camel.dsl.jbang.core.common.ScratchContextHelper; import org.apache.camel.impl.DefaultCamelContext; import org.apache.camel.main.download.DependencyDownloaderClassLoader; import org.apache.camel.main.download.MavenDependencyDownloader; @@ -193,7 +194,8 @@ private static void evaluateLocally( */ private static boolean evaluateWith( ClassLoader loader, String lang, String expression, String body, boolean predicate, JsonObject result) { - try (DefaultCamelContext context = new DefaultCamelContext(false)) { + // without the CLI connector, which would make this process, such as the MCP server, a running integration + try (DefaultCamelContext context = ScratchContextHelper.withoutCliConnector(new DefaultCamelContext(false))) { if (loader != null) { // before the start: the language resolver reads the class loader as the context comes up context.setApplicationContextClassLoader(loader); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java index 49972c6348994..ed6c16f4549e0 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java @@ -570,6 +570,10 @@ private static String readTail(Path output) { public static String control(ToolContext ctx, String action) { ctx.requireProcess(); long pid = ctx.pid(); + if (pid == ProcessHandle.current().pid()) { + // the tools run in this process: it cannot kill itself (the JDK refuses), nor answer an action it waits for + throw ToolContext.thisProcess(pid); + } return switch (action) { case "stop" -> ctx.stopApplication(); case "kill" -> { diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolContext.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolContext.java index 6f3f31f52294a..d057db674dfbc 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolContext.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolContext.java @@ -222,12 +222,13 @@ private CamelCatalog loadCatalog() { * Selects the process a tool call names ({@code name} is an integration name or pid), or keeps the current * selection when the call names none. * - * @throws ToolExecutionException when the name matches no running process + * @throws ToolExecutionException when the name matches no running process, or is the pid of this process */ public void selectProcess(String nameOrPid) { if (nameOrPid == null || nameOrPid.isBlank()) { return; } + refuseThisProcess(nameOrPid); RuntimeHelper.ProcessInfo p = RuntimeHelper.findProcess(nameOrPid); if (p == null) { throw new ToolExecutionException( @@ -237,6 +238,27 @@ public void selectProcess(String nameOrPid) { selectProcess(p.pid()); } + /** + * Refuses the pid of this process, which runs the tools: a status file of its own comes from a CamelContext it ran + * for itself, never from an integration, and it would wait for itself to answer an action (CAMEL-26396). + * + * @throws ToolExecutionException when the name or pid is the pid of this process + */ + static void refuseThisProcess(String nameOrPid) { + if (RuntimeHelper.isThisProcess(nameOrPid)) { + throw thisProcess(ProcessHandle.current().pid()); + } + } + + /** + * The error for a tool call on the pid of this process (see {@link #refuseThisProcess(String)}). + */ + static ToolExecutionException thisProcess(long pid) { + return new ToolExecutionException( + "pid " + pid + " is the process that runs these tools (the MCP server, camel ask or the TUI), not a" + + " Camel integration; list_processes shows the integrations"); + } + public String readStatus(String section) { requireProcess(); return RuntimeHelper.readStatusSection(pid, section); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java index 44f8b209153f1..ea135305d2e7c 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java @@ -134,6 +134,7 @@ private static void registerProcessTools() { if (name == null || name.isBlank()) { throw new ToolExecutionException("name or PID is required"); } + ToolContext.refuseThisProcess(name); RuntimeHelper.ProcessInfo found = ctx.findProcess(name); if (found == null) { List processes = ctx.discoverProcesses(); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/version/VersionList.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/version/VersionList.java index a0e813d1c67f9..b1ee64bf3f3d1 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/version/VersionList.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/version/VersionList.java @@ -60,10 +60,12 @@ import org.apache.camel.dsl.jbang.core.common.RuntimeCompletionCandidates; import org.apache.camel.dsl.jbang.core.common.RuntimeType; import org.apache.camel.dsl.jbang.core.common.RuntimeTypeConverter; +import org.apache.camel.dsl.jbang.core.common.ScratchContextHelper; import org.apache.camel.dsl.jbang.core.common.VersionHelper; import org.apache.camel.dsl.jbang.core.model.VersionListDTO; import org.apache.camel.main.KameletMain; import org.apache.camel.main.download.MavenDependencyDownloader; +import org.apache.camel.spi.CliConnectorFactory; import org.apache.camel.tooling.maven.RepositoryResolver; import org.apache.camel.tooling.model.ReleaseModel; import org.apache.camel.util.StringHelper; @@ -400,6 +402,9 @@ Stream downloadReleases() { mavenResolver.fresh()); } KameletMain main = new KameletMain(CAMEL_INSTANCE_TYPE); + // only to download: without the CLI connector, which would list this process (such as the MCP server, whose + // camel_version_list runs this) as a running integration + main.bind(CliConnectorFactory.FACTORY, ScratchContextHelper.disabledCliConnectorFactory()); try { main.setFresh(mavenResolver.fresh()); main.setDownload(mavenResolver.download()); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/RuntimeHelper.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/RuntimeHelper.java index e224a882f5117..e7a9d35dd7e0a 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/RuntimeHelper.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/RuntimeHelper.java @@ -61,10 +61,13 @@ private RuntimeHelper() { /** * Discovers all running Camel processes by scanning {@code ~/.camel/} for {@code {pid}-status.json} files and * verifying each PID is still alive. + *

+ * This process is never one of them: it runs the tools (camel ask, the TUI, the MCP server), and a status file of + * its own comes from a CamelContext it ran for itself, not from an integration (CAMEL-26396). */ public static List discoverProcesses() { List result = new ArrayList<>(); - for (ProcessFinder.CamelProcess p : ProcessFinder.findAll()) { + for (ProcessFinder.CamelProcess p : others(ProcessFinder.findAll())) { result.add(new ProcessInfo(p.pid(), p.name(), p.contextName())); } return result; @@ -73,13 +76,13 @@ public static List discoverProcesses() { /** * Finds a single Camel process matching the given name or PID. Returns {@code null} if no match is found or if * multiple processes match. When {@code nameOrPid} is {@code null} and exactly one process is running, returns that - * process. + * process. This process never matches, also not by its pid (see {@link #discoverProcesses()}). * * @param nameOrPid a process name (with optional wildcard), a numeric PID string, or {@code null} for auto-detect */ public static ProcessInfo findProcess(String nameOrPid) { - List found = nameOrPid != null && !nameOrPid.isBlank() - ? ProcessFinder.find(nameOrPid) : ProcessFinder.findAll(); + List found = others(nameOrPid != null && !nameOrPid.isBlank() + ? ProcessFinder.find(nameOrPid) : ProcessFinder.findAll()); if (found.size() == 1) { ProcessFinder.CamelProcess p = found.get(0); return new ProcessInfo(p.pid(), p.name(), p.contextName()); @@ -87,6 +90,23 @@ public static ProcessInfo findProcess(String nameOrPid) { return null; } + /** + * Whether the name or pid is the pid of this process, which {@link #findProcess(String)} never finds. + * + * @since 4.24 + */ + public static boolean isThisProcess(String nameOrPid) { + if (nameOrPid == null || !nameOrPid.matches("\\d{1,18}")) { + return false; + } + return Long.parseLong(nameOrPid) == ProcessHandle.current().pid(); + } + + private static List others(List processes) { + long self = ProcessHandle.current().pid(); + return processes.stream().filter(p -> p.pid() != self).toList(); + } + /** * Reads the full status JSON for the given process. * diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ScratchContextHelper.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ScratchContextHelper.java new file mode 100644 index 0000000000000..4d0ca0831dc23 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ScratchContextHelper.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.jbang.core.common; + +import org.apache.camel.CamelContext; +import org.apache.camel.cli.connector.DefaultCliConnectorFactory; +import org.apache.camel.spi.CliConnectorFactory; + +/** + * The CamelContexts the Camel CLI runs for itself, such as the scratch context that evaluates an expression for the AI + * tools: they are not integrations, so they run without the CLI connector. With it, such a context writes the status + * file of this process ({@code ~/.camel/{pid}-status.json}) while it runs, and the MCP server, the TUI or camel ask, + * which run for a long time, showed up as a running integration (CAMEL-26396). + * + * @since 4.24 + */ +public final class ScratchContextHelper { + + private ScratchContextHelper() { + } + + /** + * The context, set up to start without the CLI connector, whether it is built already or not. + */ + public static T withoutCliConnector(T context) { + CliConnectorFactory disabled = disabledCliConnectorFactory(); + // a context that is not built yet takes the factory from its registry as it builds + context.getRegistry().bind(CliConnectorFactory.FACTORY, CliConnectorFactory.class, disabled); + // a built one has its factory already + context.getCamelContextExtension().addContextPlugin(CliConnectorFactory.class, disabled); + return context; + } + + /** + * A CLI connector factory that is turned off, to bind in the registry of a Camel Main the CLI runs for itself. + */ + public static CliConnectorFactory disabledCliConnectorFactory() { + CliConnectorFactory factory = new DefaultCliConnectorFactory(); + factory.setEnabled(false); + return factory; + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncherGuardTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncherGuardTest.java index ae60490788877..bd7225aabdbef 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncherGuardTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncherGuardTest.java @@ -16,6 +16,7 @@ */ package org.apache.camel.dsl.jbang.core.commands.ai; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -32,7 +33,8 @@ /** * CAMEL-25364: camel_run of a directory an integration already runs from returns that one instead of starting a second - * copy. The running integration is this test's own JVM, made visible by a status file that names the directory. + * copy. The running integration is a JVM the test starts, made visible by a status file that names the directory: not + * this JVM, which runs the tools and is never taken for an integration (CAMEL-26396). */ @Isolated class IntegrationLauncherGuardTest { @@ -44,6 +46,7 @@ class IntegrationLauncherGuardTest { Path project; private String originalHome; + private Process integration; @BeforeEach void runningFromTheProject() throws Exception { @@ -60,23 +63,33 @@ void runningFromTheProject() throws Exception { JsonObject status = new JsonObject(); status.put("runtime", runtime); status.put("context", context); - Files.writeString(camelDir.resolve(ProcessHandle.current().pid() + "-status.json"), status.toJson()); + integration = startJvm(); + Files.writeString(camelDir.resolve(integration.pid() + "-status.json"), status.toJson()); } @AfterEach - void restoreHome() { + void restoreHome() throws Exception { + integration.destroyForcibly().waitFor(); CommandLineHelper.useHomeDir(originalHome); } + /** A JVM that waits until it is destroyed. */ + private Process startJvm() throws IOException { + Path main = home.resolve("Wait.java"); + Files.writeString(main, "class Wait { public static void main(String[] args) throws Exception { System.in.read(); } }"); + String java = Path.of(System.getProperty("java.home"), "bin", "java").toString(); + return new ProcessBuilder(java, main.toString()).start(); + } + @Test void theRunningIntegrationIsReturned() { JsonObject result = IntegrationLauncher.run(project, List.of(), null, true, List.of()); assertThat(result.getString("status")).isEqualTo("running"); - assertThat(result.getLong("pid")).isEqualTo(ProcessHandle.current().pid()); + assertThat(result.getLong("pid")).isEqualTo(integration.pid()); assertThat(result.getString("name")).isEqualTo("demo"); assertThat(result.getString("log")).isNotBlank(); assertThat(result.containsKey("command")).isFalse(); - // this JVM was not started with --source-dir, so it is not said to reload an added file + // the JVM was not started with --source-dir, so it is not said to reload an added file assertThat(result.getString("message")).contains("a second copy was not started") .contains("started with its own files").doesNotContain("is reloaded"); } @@ -86,6 +99,6 @@ void theRunningIntegrationIsFoundThroughASymlink() throws Exception { Path link = Files.createSymbolicLink(home.resolve("project-link"), project); JsonObject result = IntegrationLauncher.run(link, List.of(), null, true, List.of()); assertThat(result.getString("status")).isEqualTo("running"); - assertThat(result.getLong("pid")).isEqualTo(ProcessHandle.current().pid()); + assertThat(result.getLong("pid")).isEqualTo(integration.pid()); } } diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ThisProcessIsNoIntegrationTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ThisProcessIsNoIntegrationTest.java new file mode 100644 index 0000000000000..8435952dbabb0 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ThisProcessIsNoIntegrationTest.java @@ -0,0 +1,137 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.jbang.core.commands.ai; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import org.apache.camel.dsl.jbang.core.common.CommandLineHelper; +import org.apache.camel.dsl.jbang.core.common.RuntimeHelper; +import org.apache.camel.util.json.JsonObject; +import org.apache.camel.util.json.Jsoner; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * CAMEL-26396: the process that runs the tools (the MCP server, camel ask, the TUI) is never taken for an integration. + * The scratch context of a local evaluation ran with the CLI connector, which wrote the status file of this process and + * sometimes left it behind: the MCP server then listed itself, evaluated the next expression in itself (waiting 10 s + * for its own answer), and camel_control kill of its pid failed with an internal error. + */ +class ThisProcessIsNoIntegrationTest { + + private static final long PID = ProcessHandle.current().pid(); + private static final String THIS_PROCESS = "pid " + PID + " is the process that runs these tools"; + + @TempDir + Path home; + + private Path previousHome; + + @BeforeEach + void aStatusFileOfThisProcess() throws Exception { + previousHome = CommandLineHelper.getHomeDir(); + CommandLineHelper.useHomeDir(home.toString()); + Path camelDir = CommandLineHelper.getCamelDir(); + Files.createDirectories(camelDir); + // as the scratch context left it: named after the context, which had not stopped yet + JsonObject context = new JsonObject(); + context.put("name", "camel-14"); + context.put("phase", 4); + context.put("state", "Starting"); + JsonObject status = new JsonObject(); + status.put("context", context); + Files.writeString(camelDir.resolve(PID + "-status.json"), status.toJson()); + } + + @AfterEach + void restoreHome() { + CommandLineHelper.useHomeDir(previousHome.toString()); + } + + @Test + void thisProcessIsNotListed() { + assertThat(RuntimeHelper.discoverProcesses()).isEmpty(); + assertThat(RuntimeHelper.findProcess(null)).isNull(); + assertThat(RuntimeHelper.findProcess(String.valueOf(PID))).isNull(); + assertThat(ToolRegistry.execute("list_processes", new ToolContext(), Map.of())) + .asString().startsWith("No running Camel processes found"); + + ToolContext ctx = new ToolContext(); + assertThat(ctx.selectSingleProcessIfNone()).isFalse(); + assertThat(ctx.hasProcess()).isFalse(); + } + + @Test + void aCallThatNamesThisProcessIsRefused() { + ToolContext ctx = new ToolContext(); + assertThatThrownBy(() -> ctx.selectProcess(String.valueOf(PID))) + .isInstanceOf(ToolExecutionException.class).hasMessageStartingWith(THIS_PROCESS); + assertThat(ctx.hasProcess()).isFalse(); + + assertThatThrownBy(() -> ToolRegistry.execute("select_process", ctx, Map.of("name", String.valueOf(PID)))) + .isInstanceOf(ToolExecutionException.class).hasMessageStartingWith(THIS_PROCESS); + assertThatThrownBy(() -> ToolRegistry.execute("camel_eval_expression", ctx, + Map.of("expression", "${body}", "name", String.valueOf(PID)))) + .isInstanceOf(ToolExecutionException.class).hasMessageStartingWith(THIS_PROCESS); + assertThatThrownBy(() -> ToolRegistry.execute("camel_control", ctx, + Map.of("action", "kill", "name", String.valueOf(PID)))) + .isInstanceOf(ToolExecutionException.class).hasMessageStartingWith(THIS_PROCESS); + } + + @Test + void anExpressionWithoutANameIsEvaluatedLocally() { + JsonObject result = Jsoner.deserialize(String.valueOf(ToolRegistry.execute("camel_eval_expression", + new ToolContext(), Map.of("expression", "Hello ${body}", "body", "Camel"))), (JsonObject) null); + assertThat(result.getString("evaluatedIn")).isEqualTo("a local scratch context (no integration selected)"); + assertThat(result.getString("status")).isEqualTo("ok"); + assertThat(result.getString("result")).isEqualTo("Hello Camel"); + } + + @Test + void theScratchContextRunsWithoutTheCliConnector(@TempDir Path userHome) { + String previousUserHome = System.getProperty("user.home"); + // where the CLI connector writes the files of this process while a context runs + System.setProperty("user.home", userHome.toString()); + try { + JsonObject result = ExpressionEvaluator.evaluate(new ToolContext(), "simple", "Hello ${body}", "Camel"); + assertThat(result.getString("status")).isEqualTo("ok"); + } finally { + System.setProperty("user.home", previousUserHome); + } + assertThat(userHome.resolve(".camel")).doesNotExist(); + } + + @Test + void noActionControlsThisProcess() { + ToolContext ctx = new ToolContext(); + // selected anyway, as a caller that has a pid of its own can + ctx.selectProcess(PID); + // kill first: the JDK refuses to kill this process, and restart would start a copy of it + for (String action : List.of("kill", "stop", "restart", "reload", "stop-routes", "start-routes", "reset-stats")) { + assertThatThrownBy(() -> IntegrationLauncher.control(ctx, action)).as(action) + .isInstanceOf(ToolExecutionException.class).hasMessageStartingWith(THIS_PROCESS); + } + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/ScratchContextHelperTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/ScratchContextHelperTest.java new file mode 100644 index 0000000000000..1d5e19485d64a --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/ScratchContextHelperTest.java @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.jbang.core.common; + +import java.nio.file.Path; + +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.camel.main.KameletMain; +import org.apache.camel.spi.CliConnector; +import org.apache.camel.spi.CliConnectorFactory; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-26396: the CamelContexts the CLI runs for itself start without the CLI connector, which would write the status + * file of this process in {@code ~/.camel} (under {@code user.home}, a directory of the test here). + */ +class ScratchContextHelperTest { + + @TempDir + Path userHome; + + private String previousUserHome; + + @BeforeEach + void useHome() { + previousUserHome = System.getProperty("user.home"); + System.setProperty("user.home", userHome.toString()); + } + + @AfterEach + void restoreHome() { + System.setProperty("user.home", previousUserHome); + } + + @Test + void theConnectorIsOnTheClasspath() throws Exception { + // what the helper turns off: a context the CLI runs is managed by the CLI connector by default + try (DefaultCamelContext context = new DefaultCamelContext()) { + CliConnectorFactory factory = context.getCamelContextExtension().getContextPlugin(CliConnectorFactory.class); + assertThat(factory).isNotNull(); + assertThat(factory.isEnabled()).isTrue(); + } + } + + @Test + void aContextThatIsNotBuiltYetStartsWithoutTheConnector() throws Exception { + try (DefaultCamelContext context = ScratchContextHelper.withoutCliConnector(new DefaultCamelContext(false))) { + context.start(); + assertThat(context.hasService(CliConnector.class)).isNull(); + assertThat(context.getCamelContextExtension().getContextPlugin(CliConnectorFactory.class).isEnabled()).isFalse(); + } + assertThat(userHome.resolve(".camel")).doesNotExist(); + } + + @Test + void aBuiltContextStartsWithoutTheConnector() throws Exception { + try (DefaultCamelContext context = ScratchContextHelper.withoutCliConnector(new DefaultCamelContext())) { + context.start(); + assertThat(context.hasService(CliConnector.class)).isNull(); + } + assertThat(userHome.resolve(".camel")).doesNotExist(); + } + + @Test + void aCamelMainStartsWithoutTheConnector() throws Exception { + // as camel version list downloads with Camel Main + KameletMain main = new KameletMain("camel.jbang"); + main.bind(CliConnectorFactory.FACTORY, ScratchContextHelper.disabledCliConnectorFactory()); + main.setDownload(false); + try { + main.start(); + assertThat(main.getCamelContext().hasService(CliConnector.class)).isNull(); + } finally { + main.stop(); + } + assertThat(userHome.resolve(".camel")).doesNotExist(); + } +} diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeService.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeService.java index aeafaeca701cd..3adb1c2c2ad35 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeService.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeService.java @@ -61,6 +61,13 @@ public List discoverProcesses() { } public ProcessInfo findSingleProcess(String nameOrPid) { + if (RuntimeHelper.isThisProcess(nameOrPid)) { + // never an integration, see RuntimeHelper.discoverProcesses() + throw new ToolCallException( + "pid " + nameOrPid + " is this MCP server (the process that runs the tools), not a Camel integration;" + + " camel_runtime_processes shows the integrations", + null); + } List processes = RuntimeHelper.discoverProcesses(); if (processes.isEmpty()) { diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeServiceTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeServiceTest.java index 3581934ede234..36b0a5e66b229 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeServiceTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeServiceTest.java @@ -16,10 +16,14 @@ */ package org.apache.camel.dsl.jbang.core.commands.mcp; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import io.quarkiverse.mcp.server.ToolCallException; +import org.apache.camel.dsl.jbang.core.common.CommandLineHelper; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -54,6 +58,33 @@ void findSingleProcessThrowsForInvalidPid() { .hasMessageContaining("Camel process"); } + /** + * CAMEL-26396: a status file of the server's own pid, as a scratch context left it, does not make the server an + * integration. + */ + @Test + void theServerIsNeverAProcessOfItsOwn(@TempDir Path home) throws Exception { + Path previous = CommandLineHelper.getHomeDir(); + CommandLineHelper.useHomeDir(home.toString()); + try { + long pid = ProcessHandle.current().pid(); + Files.createDirectories(CommandLineHelper.getCamelDir()); + Files.writeString(CommandLineHelper.getCamelDir().resolve(pid + "-status.json"), + "{\"context\":{\"name\":\"camel-14\",\"phase\":4,\"state\":\"Starting\"}}"); + RuntimeService service = new RuntimeService(); + + assertThat(service.discoverProcesses()).isEmpty(); + assertThatThrownBy(() -> service.findSingleProcess(null)) + .isInstanceOf(ToolCallException.class) + .hasMessage("No running Camel processes found"); + assertThatThrownBy(() -> service.findSingleProcess(String.valueOf(pid))) + .isInstanceOf(ToolCallException.class) + .hasMessageStartingWith("pid " + pid + " is this MCP server"); + } finally { + CommandLineHelper.useHomeDir(previous.toString()); + } + } + @Test void readStatusReturnsNullForNonExistentPid() { RuntimeService service = new RuntimeService();