Repository navigation
CAMEL-26396: camel-jbang - AI tools: the MCP server never takes itself for an integration - #28678
Conversation
…f for an integration 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.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
LGTM. Well-engineered fix for CAMEL-26396.
The two-layer approach is correct: suppressing the CLI connector in scratch contexts prevents the file from being written in the first place, and the awaitPoll() drain in doStop closes the race for the (now-suppressed) connector path as defense in depth. Process discovery and tool routing are all guarded independently — listing, auto-selection, explicit PID selection, and control actions each refuse this process.
Pre-fix test validation: FileCliConnectorTransportStopTest would fail without awaitPoll() (stop returns immediately, the in-flight status write recreates the deleted file); ThisProcessIsNoIntegrationTest would fail without the others() filter. Tests are correctly structured to validate the fix.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
oscerd
left a comment
oscerd
left a comment
There was a problem hiding this comment.
Read the self-exclusion logic across the connector, the runtime helpers and the launcher. It's a correct and well-layered fix for a real foot-gun: the tools (camel ask, the TUI, the MCP server) run inside a Camel/jbang process that can leave a <pid>-status.json, and without this the AI tools would "discover" their own runner as an integration and try to eval/trace/kill it.
The fix works at two levels, which is the right shape:
- Don't advertise itself.
ScratchContextHelperbinds a turned-off CLI connector factory in the registry of the Camel Main the CLI runs for itself, so the tool's own scratch context doesn't publish a status file that looks like an integration — fixing the cause rather than just filtering the symptom. - Exclude this pid defensively everywhere it matters.
RuntimeHelper.isThisProcesscompares againstProcessHandle.current().pid()(bounded\d{1,18}parse),others(...)filters the current pid out of discovery,RuntimeService.findSingleProcessturns a request that names this pid into a clear message ("pid X is this MCP server ... not a Camel integration; camel_runtime_processes shows the integrations"), andIntegrationLauncherrefuses an action targeting the current pid up front. That last one matters most for correctness — an action against self would either be refused by the JDK (can't kill yourself) or deadlock, since the tool would be waiting on a response from the very process that's blocked running the tool.
Identifying "self" by the actual OS pid is the robust choice (name matching alone would be fooled by two Camel processes), and the error messages point the model at the right tool instead of just failing. The tests cover the scenarios directly — a self-pid status file not being treated as an integration (theServerIsNeverAProcessOfItsOwn, ThisProcessIsNoIntegration...) and the launcher guard. It's MCP-surface code but this doesn't change what the tools can do — it only stops them acting on their own process — so no trust-boundary impact.
CI was nearly green (one check still running) when I looked — worth confirming it finishes green before merge.
No blockers.
Claude Code on behalf of oscerd
|
🧪 CI tested the following changed modules:
🔁 1 test passed only after a retry on JDK 21 (1 retried attempt) Recovered flaky tests on JDK 21 (1)
🔬 Scalpel shadow comparison — Scalpel: 7 of 709 tested, 8 compile-only — current: 7 all testedMaveniverse Scalpel detected 7 affected modules (current approach: 7). Skip-tests mode would test 7 modules (4 direct + 5 downstream), skip tests for 8 (generated code, meta-modules) Modules Scalpel would test (7)
Modules with tests skipped (8)
💡 Manual integration tests recommended:
All tested modules (17 modules, 1m 54s total)Total reactor time: 1m 54s
Top 20 slowest modules:
|
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Looks good. The callers of discoverProcesses/findProcess are all AI/MCP code, so camel ps is unaffected. Suggestion: FileCliConnectorTransport.doStop waits up to 5 s for the poll in flight before cancelling pending; cancelling pending first would avoid a 5 s stop if a poll waits on those entries. This conflicts with #28683 in ExpressionEvaluator.evaluateWith; whichever merges second needs a rebase.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Claude Code on behalf of Croway
CAMEL-26396
Problem
A local
camel_eval_expression(no integration selected) starts a scratchDefaultCamelContextinside the MCPserver. camel-cli-connector is on the server's classpath, so every scratch context started a CLI connector, which
wrote
~/.camel/<server pid>-status.jsonwhile it ran.FileCliConnectorTransport.doStopcancelled its status taskwith
cancel(false)and deleted the files right away, so a status write already in flight recreated the file afterthe delete: the status of a context that had already shut down (
phase 4,state Starting, namedcamel-14and thelike) stayed behind.
From then on the tool layer took the server for an integration, because
RuntimeHelpermatches any live pid with astatus file, the current one included:
camel_runtime_processeslisted the server itself;camel_eval_expressionwithout a name selected "the only running integration", the server, and waited 10 s for ananswer nobody gives ("No answer from the integration with pid 6 to the action eval within 10 seconds"); this is
sticky, as no scratch context runs any more that could delete the file;
camel_control killof the server's pid ended as JSON-RPC-32603(IllegalStateException: destroy of current process not allowedfromProcessHandle.destroyForcibly).Reproduced in a Linux container (
--cpus=1and--cpus=0.5, 3 of 3 runs): after 11-26 local evaluations the statusfile stays behind. On macOS, while a slow local evaluation runs, the server lists itself as
camel-3, and a concurrentevaluation without a name is answered by the scratch context's connector and labelled as evaluated in the running
integration.
Fix
ScratchContextHelper(new, camel-jbang-corecommon)withoutCliConnector(context)binds a disabledCliConnectorFactoryin the context's registry (picked up when the context builds) and sets it as the context plugin (for a context built already);disabledCliConnectorFactory()for a Camel Main.ExpressionEvaluator.evaluateWithVersionList.downloadReleasesKameletMainit starts only to download (also from the MCP toolcamel_version_list, inside the server) runs without the CLI connector.RuntimeHelper.discoverProcesses/findProcessisThisProcess(nameOrPid).ProcessFinderis unchanged: CLI commands can still match the current process by pid.ToolContext.selectProcess(String),select_processToolExecutionException: "pid N is the process that runs these tools (the MCP server, camel ask or the TUI), not a Camel integration; list_processes shows the integrations".IntegrationLauncher.controlToolExecutionException, so no action on it ends as-32603(andrestartcannot start a copy of the server).RuntimeService.findSingleProcesscamel_runtime_*tools: "pid N is this MCP server (the process that runs the tools), not a Camel integration".FileCliConnectorTransport.doStop(camel-cli-connector)Audited and left unchanged, as these contexts are never initialised (the CLI connector starts in
AbstractCamelContext.doInit):RouteDslConverter.read,TransformJavaRoutes.dump,SourceHelper(classpathresolver) and
Init(GitHub / Gist resolvers). Checked with a throwaway test: converting YAML to XML and Java, Javato YAML, and dumping the structure of a Java route create nothing under
user.home.The TUI already skips its own pid in its process list (
TuiHelper.findPids) and gives the shared tools only theintegration the user selected. camel ask only targets processes it discovers. Neither needs to select its own JVM.
The one case is an integration that camel ask's
cli_execruns in its own JVM (camel run --max-seconds=N): the callwaits for that run (and interrupts it after 60 s), so no other tool call inspects it meanwhile; a run that ignores
the interrupt and goes on can no longer be selected by its pid.
Testing
New tests, each failing on origin/main (with a no-op
ScratchContextHelperfor the class that is new):FileCliConnectorTransportStopTest(camel-cli-connector): the status provider blocks the first poll, the transport stops meanwhile, then the poll goes on; the status file must not exist afterwards<pid>-status.jsonafter the stop deleted itThisProcessIsNoIntegrationTest(camel-jbang-core), with a status file of the test JVM's own pid in a temp camel dirdiscoverProcesses()returns the JVM itself;selectProcess("<own pid>")selects it;camel_eval_expressionwithout a name ends with "No answer from the integration with pid N to the action eval within 10 seconds"; a local evaluation creates~/.camel(the scratch context's connector);control(kill)throwsIllegalStateException: destroy of current process not allowedScratchContextHelperTest: a context built or not, and aKameletMain, start without aCliConnectorand write nothing underuser.homeRuntimeServiceTest.theServerIsNeverAProcessOfItsOwn(camel-jbang-mcp)IntegrationLauncherGuardTestfaked the running integration with the test JVM's own pid; it now starts a small childJVM (
java Wait.java, blocking on stdin) and writes the status file for that pid.Full module suites, in one reactor (
mvnd -P apache-snapshots -T 2 test -pl dsl/camel-cli-connector,dsl/camel-jbang/camel-jbang-core,dsl/camel-jbang/camel-jbang-mcp,dsl/camel-jbang/camel-jbang-plugin-tui):MonitorStartupTest.browserOnlyServesUntilTheServerStopsflaked once with "Connection refused" and passed on the rerun)End to end in a Linux container (
camel-mcp-repro:21,--cpus=1): a stdio MCP client runs up to 200 localcamel_eval_expressioncalls and checks for~/.camel/<server pid>-status.jsonafter each one.evals run: 17, stale status file: Trueevals run: 200, stale status file: Falseevals run: 200, stale status file: FalseOn this branch the server log shows 200 scratch contexts starting and no "Camel CLI connector enabled" line.