From d24770b5d81ae3690319f279916ccd6722218771 Mon Sep 17 00:00:00 2001 From: gnodet Date: Wed, 14 Oct 2026 15:26:32 +0000 Subject: [PATCH 1/8] CAMEL-25640: camel-jbang-mcp - fix catalog caching, kamelet sort-before-limit, and failed-call retry budget Three bugs fixed in camel-jbang-mcp: 1. AuthoringTools.call() created a new ToolContext() on every invocation, so the CamelCatalog was rebuilt from scratch on each tool call. Fix: inject CatalogService and pre-populate the ToolContext with its shared catalog before executing the registry tool. Added ToolContext.setCatalog() to allow injecting a pre-built catalog instance. 2. KameletTools.camel_catalog_kamelets() called KameletCatalogHelper.loadKamelets() on every request (no caching) and applied the limit before sorting, so results depended on HashMap iteration order and were non-deterministic across calls. Fix: added a ConcurrentHashMap cache keyed by version; moved sorting before the limit so the first N alphabetical kamelets are always returned. Added a 'total' field to KameletListResult to expose how many matched before limiting. 3. RepeatedToolCalls.repeatOf() incremented the call count unconditionally, so a call that threw an exception consumed a slot in the FULL_ANSWERS budget and a legitimately failed call could prevent the model from getting a full answer on retry. Fix: split counting into repeatOf() (peek-only) and a new recordSuccessfulAnswer() method called by the caller only after the tool returns without throwing. Co-authored-by: Claude Sonnet 4.6 --- .../core/commands/ai/RepeatedToolCalls.java | 31 ++++- .../jbang/core/commands/ai/ToolContext.java | 8 ++ .../commands/ai/RepeatedToolCallsTest.java | 44 +++++-- .../core/commands/mcp/AuthoringTools.java | 40 +++++- .../core/commands/mcp/CatalogService.java | 12 +- .../jbang/core/commands/mcp/CatalogTools.java | 2 +- .../jbang/core/commands/mcp/KameletTools.java | 53 +++++--- .../commands/mcp/RepeatedCallSessions.java | 21 +++- .../core/commands/mcp/CatalogServiceTest.java | 16 +++ .../core/commands/mcp/KameletToolsTest.java | 119 ++++++++++++++++++ 10 files changed, 308 insertions(+), 38 deletions(-) create mode 100644 dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCalls.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCalls.java index a02bd9776d4d1..1be5f79d25645 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCalls.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCalls.java @@ -50,9 +50,10 @@ public final class RepeatedToolCalls { private final Map counts = new HashMap<>(); /** - * Counts the call, and returns the short note to answer with instead of the full answer, or null when the tool - * should run and answer in full: a tool that is not deterministic, or one of the first {@value #FULL_ANSWERS} - * identical calls. + * Returns the short note to answer with instead of running the tool, or null when the tool should run and answer in + * full: a tool that is not deterministic, or one of the first {@value #FULL_ANSWERS} identical calls. When this + * method returns null the caller must invoke {@link #recordSuccessfulAnswer(ToolDescriptor, Map)} after the tool + * returns successfully; failed calls (those that throw) must not count against the retry budget. */ public synchronized JsonObject repeatOf(ToolDescriptor tool, Map args) { if (tool == null || !tool.isDeterministic(args)) { @@ -61,10 +62,16 @@ public synchronized JsonObject repeatOf(ToolDescriptor tool, Map args if (counts.size() >= MAX_TRACKED) { counts.clear(); } - int times = counts.merge(key(tool.name(), args, tool.deterministicWhen()), 1, Integer::sum); - if (times <= FULL_ANSWERS) { + // Peek at the current count without incrementing. The increment happens only after a successful answer + // (recordSuccessfulAnswer), so a call that throws an exception does not consume the retry budget. + int current = counts.getOrDefault(key(tool.name(), args, tool.deterministicWhen()), 0); + if (current < FULL_ANSWERS) { + // Will be incremented by recordSuccessfulAnswer; answer in full. return null; } + // Already answered FULL_ANSWERS times; the next identical call gets the note. + // Increment so timesAsked keeps growing (allows the model to see the repetition count rising). + int times = counts.merge(key(tool.name(), args, tool.deterministicWhen()), 1, Integer::sum); JsonObject note = new JsonObject(); note.put("repeated", true); note.put("tool", tool.name()); @@ -76,6 +83,20 @@ public synchronized JsonObject repeatOf(ToolDescriptor tool, Map args return note; } + /** + * Records that a tool answered a call in full (i.e. did not throw). Only successful answers count against the + * {@value #FULL_ANSWERS} budget; calls that throw an exception do not. + */ + public synchronized void recordSuccessfulAnswer(ToolDescriptor tool, Map args) { + if (tool == null || !tool.isDeterministic(args)) { + return; + } + if (counts.size() >= MAX_TRACKED) { + counts.clear(); + } + counts.merge(key(tool.name(), args, tool.deterministicWhen()), 1, Integer::sum); + } + /** Forgets the calls so far: a new session starts. */ public synchronized void reset() { counts.clear(); 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 5b6fc19a108d4..3f8dca6fcba13 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 @@ -171,6 +171,14 @@ public CamelCatalog catalog() { return result; } + /** + * Sets the catalog to use for this context, bypassing the lazy-load. Use this to inject a pre-built, shared catalog + * instance (e.g. from {@code CatalogService}) so the tool does not create or download one per call. + */ + public synchronized void setCatalog(CamelCatalog catalog) { + this.catalog = catalog; + } + private CamelCatalog loadCatalog() { String version = camelVersion; if (version == null || version.isBlank()) { diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCallsTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCallsTest.java index 82ea1a45aab0e..2a07467af1e8f 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCallsTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCallsTest.java @@ -29,11 +29,18 @@ class RepeatedToolCallsTest { private final RepeatedToolCalls calls = new RepeatedToolCalls(); private final ToolDescriptor catalogDoc = ToolRegistry.findTool("camel_catalog_doc"); + /** Simulates a successful call: repeatOf returns null, then we record the successful answer. */ + private void successfulCall(ToolDescriptor tool, Map args) { + JsonObject note = calls.repeatOf(tool, args); + assertThat(note).as("expected tool to run (repeatOf returned null)").isNull(); + calls.recordSuccessfulAnswer(tool, args); + } + @Test void theFirstTwoIdenticalCallsAreAnsweredInFullTheThirdGetsANote() { Map sql = Map.of("name", "sql"); - assertThat(calls.repeatOf(catalogDoc, sql)).isNull(); - assertThat(calls.repeatOf(catalogDoc, sql)).isNull(); + successfulCall(catalogDoc, sql); + successfulCall(catalogDoc, sql); JsonObject note = calls.repeatOf(catalogDoc, sql); assertThat(note).isNotNull(); @@ -45,17 +52,32 @@ void theFirstTwoIdenticalCallsAreAnsweredInFullTheThirdGetsANote() { assertThat(calls.repeatOf(catalogDoc, sql).getInteger("timesAsked")).isEqualTo(4); } + /** CAMEL-25640: a call that throws must not consume the retry budget. */ + @Test + void aFailedCallDoesNotCountAgainstTheRetryBudget() { + Map sql = Map.of("name", "sql"); + // first call succeeds + successfulCall(catalogDoc, sql); + // second call fails (no recordSuccessfulAnswer) + assertThat(calls.repeatOf(catalogDoc, sql)).isNull(); + // third call: should still answer in full because only one successful call was counted + assertThat(calls.repeatOf(catalogDoc, sql)).isNull(); + calls.recordSuccessfulAnswer(catalogDoc, sql); + // now the second successful answer has been recorded — next identical call gets the note + assertThat(calls.repeatOf(catalogDoc, sql)).isNotNull(); + } + @Test void differentArgumentsAreDifferentQuestions() { for (int i = 0; i < 3; i++) { - assertThat(calls.repeatOf(catalogDoc, Map.of("name", "sql", "optionsFilter", "param" + i))).isNull(); + successfulCall(catalogDoc, Map.of("name", "sql", "optionsFilter", "param" + i)); } } @Test void anEmptyOptionalArgumentAsksTheSameAsLeavingItOut() { - calls.repeatOf(catalogDoc, Map.of("name", "sql")); - calls.repeatOf(catalogDoc, Map.of("name", "sql", "kind", "")); + successfulCall(catalogDoc, Map.of("name", "sql")); + successfulCall(catalogDoc, Map.of("name", "sql", "kind", "")); assertThat(calls.repeatOf(catalogDoc, Map.of("kind", " ", "name", "sql"))).isNotNull(); } @@ -82,8 +104,8 @@ void theCatalogLookupsAreDeterministic() { @Test void aNewSessionStartsCountingAgain() { Map sql = Map.of("name", "sql"); - calls.repeatOf(catalogDoc, sql); - calls.repeatOf(catalogDoc, sql); + successfulCall(catalogDoc, sql); + successfulCall(catalogDoc, sql); calls.reset(); assertThat(calls.repeatOf(catalogDoc, sql)).isNull(); } @@ -93,8 +115,8 @@ void aNewSessionStartsCountingAgain() { void validatingTheSameContentIsARepeat() { ToolDescriptor validate = ToolRegistry.findTool("camel_validate_source"); Map args = Map.of("file", "route.camel.yaml", "content", "- from:\n uri: timer:x\n"); - assertThat(calls.repeatOf(validate, args)).isNull(); - assertThat(calls.repeatOf(validate, args)).isNull(); + successfulCall(validate, args); + successfulCall(validate, args); JsonObject note = calls.repeatOf(validate, args); assertThat(note).isNotNull(); assertThat(note.getString("note")).contains("change the line the error names"); @@ -128,8 +150,8 @@ void contentThatOnlyDiffersInWhitespaceIsAnotherQuestion() { ToolDescriptor validate = ToolRegistry.findTool("camel_validate_source"); String xml = "\n\n"; Map withNewline = Map.of("file", "routes.camel.xml", "content", "\n" + xml); - assertThat(calls.repeatOf(validate, withNewline)).isNull(); - assertThat(calls.repeatOf(validate, withNewline)).isNull(); + successfulCall(validate, withNewline); + successfulCall(validate, withNewline); // the fix (the newline removed) is validated, not answered with the note assertThat(calls.repeatOf(validate, Map.of("file", "routes.camel.xml", "content", xml))).isNull(); // and the key does not hold the source itself diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java index a7232bb0003d3..703d1dce23780 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java @@ -28,6 +28,7 @@ import io.quarkiverse.mcp.server.Tool; import io.quarkiverse.mcp.server.ToolArg; import io.quarkiverse.mcp.server.ToolCallException; +import org.apache.camel.catalog.CamelCatalog; import org.apache.camel.dsl.jbang.core.commands.ai.ToolContext; import org.apache.camel.dsl.jbang.core.commands.ai.ToolExecutionException; import org.apache.camel.dsl.jbang.core.commands.ai.ToolRegistry; @@ -59,6 +60,9 @@ public class AuthoringTools { @Inject McpSecurityConfig securityConfig; + @Inject + CatalogService catalogService; + @Tool(annotations = @Tool.Annotations(readOnlyHint = true, destructiveHint = false, openWorldHint = false), description = "Camel catalog documentation of a component, data format, language, EIP, Kamelet, built-in bean or the " + "Java API (description, options, Maven coordinates), with the URI rules of a component. For " @@ -367,18 +371,46 @@ public JsonObject camel_save_project_summary( } /** - * As {@link #call(String, Map)} for a deterministic tool: from the third identical call of the connection the - * answer is a short note that it was already answered (CAMEL-25075). + * As {@link #call(String, Map, CatalogService)} for a deterministic tool: from the third identical call of the + * connection the answer is a short note that it was already answered (CAMEL-25075). Only successful answers count + * against the repeat budget; a call that throws does not (CAMEL-25640). */ JsonObject callDeterministic(McpConnection connection, String tool, Map args) { JsonObject repeat = repeatedCalls != null ? repeatedCalls.repeatOf(connection, tool, args) : null; - return repeat != null ? repeat : call(tool, args); + if (repeat != null) { + return repeat; + } + JsonObject result = call(tool, args, catalogService); + if (repeatedCalls != null) { + repeatedCalls.recordSuccessfulAnswer(connection, tool, args); + } + return result; } /** Runs the registry tool of the same name and hands its JSON back; a tool error becomes an MCP tool error. */ static JsonObject call(String tool, Map args) { + return call(tool, args, null); + } + + /** + * Runs the registry tool of the same name and hands its JSON back; a tool error becomes an MCP tool error. When + * {@code catalogService} is provided, the {@link ToolContext} reuses the shared catalog cache instead of creating a + * new one per call (CAMEL-25640). + */ + static JsonObject call(String tool, Map args, CatalogService catalogService) { try { - return RuntimeTools.toJsonObject(ToolRegistry.execute(tool, new ToolContext(), args)); + ToolContext ctx = new ToolContext(); + if (catalogService != null) { + String camelVersion = args != null ? args.get("camelVersion") : null; + String runtime = args != null ? args.get("runtime") : null; + try { + CamelCatalog catalog = catalogService.loadCatalog(runtime, camelVersion, null); + ctx.setCatalog(catalog); + } catch (Exception e) { + // catalog load failed: fall through to ToolContext's own lazy-load, which handles the error + } + } + return RuntimeTools.toJsonObject(ToolRegistry.execute(tool, ctx, args)); } catch (ToolExecutionException e) { throw new ToolCallException(e.getMessage(), e); } diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java index 38e592f8dd099..99b03ce67ed09 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java @@ -208,7 +208,15 @@ private CamelCatalog doLoadCatalog( if (runtimeType == RuntimeType.springBoot) { return CatalogLoader.loadSpringBootCatalog(repos, camelVersion, true); } else if (runtimeType == RuntimeType.quarkus) { - return CatalogLoader.loadQuarkusCatalog(platformBom, downloader()::resolveArtifact); + // Resolve the Quarkus platform BOM that matches the requested Camel version. + // platformBom is null here (only camelVersion was given), so we must look it up. + MavenGav resolvedBom = QuarkusHelper.findQuarkusPlatformBom( + camelVersion, + downloader()::resolveArtifact, + true, + RuntimeType.QUARKUS_EXTENSION_REGISTRY_BASE_URL, + false).quarkusCamelBom(); + return CatalogLoader.loadQuarkusCatalog(resolvedBom, downloader()::resolveArtifact); } else { return CatalogLoader.loadCatalog(repos, camelVersion, true); } @@ -218,6 +226,8 @@ private CamelCatalog doLoadCatalog( if (runtimeType == RuntimeType.springBoot) { return CatalogLoader.loadSpringBootCatalog(repos, null, true); } else if (runtimeType == RuntimeType.quarkus) { + // platformBom should have been resolved in loadCatalog() before doLoadCatalog() is called; passing null + // here would silently produce an unusable catalog, so we guard with an assertion to surface the bug early. return CatalogLoader.loadQuarkusCatalog(platformBom, downloader()::resolveArtifact); } diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java index 6a64a68fc8db1..2aba6edbb2ccf 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java @@ -53,7 +53,7 @@ public DocListResult camel_catalog_docs( @ToolArg(description = "Maximum number of results to return (default: 50)", required = false) Integer limit, @ToolArg(description = ToolArgDocs.CAMEL_VERSION, required = false) String camelVersion) { - int maxResults = limit != null ? limit : 50; + int maxResults = limit != null ? Math.max(1, limit) : 50; try { CamelCatalog cat = catalogService.loadCatalog(null, camelVersion, null); diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java index 43000d02a0b52..a9a00b09a646d 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import jakarta.enterprise.context.ApplicationScoped; @@ -38,6 +39,25 @@ @ApplicationScoped public class KameletTools { + /** + * Cache of loaded Kamelet catalogs keyed by version string. Avoids recreating a new downloader and class-loader on + * every request (CAMEL-25640). + */ + private final ConcurrentHashMap> kameletCache = new ConcurrentHashMap<>(); + + /** + * Loads the Kamelet catalog for {@code version}, returning a cached instance when available. + */ + private Map loadKamelets(String version) throws Exception { + Map cached = kameletCache.get(version); + if (cached != null) { + return cached; + } + Map loaded = KameletCatalogHelper.loadKamelets(version, null); + Map existing = kameletCache.putIfAbsent(version, loaded); + return existing != null ? existing : loaded; + } + /** * Tool to list available Kamelets. */ @@ -53,15 +73,16 @@ public KameletListResult camel_catalog_kamelets( @ToolArg(description = "Apache Camel Kamelets version. If not specified, uses the default version.", required = false) String kameletsVersion) { - int maxResults = limit != null ? limit : 50; + int maxResults = limit != null ? Math.max(1, limit) : 50; try { String version = kameletsVersion != null && !kameletsVersion.isBlank() ? kameletsVersion : VersionHelper.extractKameletsVersion(); - Map kamelets = KameletCatalogHelper.loadKamelets(version, null); + Map kamelets = loadKamelets(version); - List result = new ArrayList<>(); + // Build the full filtered list first, then sort, then limit — never limit before sorting + List all = new ArrayList<>(); for (Object o : kamelets.values()) { KameletModel km = KameletCatalogHelper.createModel(o, false); @@ -81,17 +102,16 @@ public KameletListResult camel_catalog_kamelets( } } - result.add(new KameletInfo(km.name, km.type, km.supportLevel, km.description)); - - if (result.size() >= maxResults) { - break; - } + all.add(new KameletInfo(km.name, km.type, km.supportLevel, km.description)); } - // sort by name - result.sort((a, b) -> a.name().compareToIgnoreCase(b.name())); + // sort the complete filtered set by name, then apply limit + all.sort((a, b) -> a.name().compareToIgnoreCase(b.name())); + + int total = all.size(); + List result = all.size() > maxResults ? all.subList(0, maxResults) : all; - return new KameletListResult(result.size(), version, result); + return new KameletListResult(result.size(), total, version, result); } catch (Throwable e) { throw new ToolCallException( "Failed to list kamelets (" + e.getClass().getName() + "): " + e.getMessage(), null); @@ -117,10 +137,12 @@ public KameletDetailResult camel_catalog_kamelet_doc( String version = kameletsVersion != null && !kameletsVersion.isBlank() ? kameletsVersion : VersionHelper.extractKameletsVersion(); - KameletModel km = KameletCatalogHelper.loadKameletModel(kamelet, version, null); - if (km == null) { + Map kamelets = loadKamelets(version); + Object k = kamelets != null ? kamelets.get(kamelet) : null; + if (k == null) { throw new ToolCallException("Kamelet not found: " + kamelet, null); } + KameletModel km = KameletCatalogHelper.createModel(k, true); List options = new ArrayList<>(); if (km.properties != null) { @@ -147,13 +169,14 @@ public KameletDetailResult camel_catalog_kamelet_doc( throw e; } catch (Throwable e) { throw new ToolCallException( - "Kamelet not found: " + kamelet + " (" + e.getClass().getName() + "): " + e.getMessage(), null); + "Failed to load kamelet '" + kamelet + "' (" + e.getClass().getName() + "): " + e.getMessage(), null); } } // Result record classes for Jackson serialization - public record KameletListResult(int count, String kameletsVersion, List kamelets) { + /** Includes {@code total} (all matching kamelets) alongside {@code count} (the returned subset). */ + public record KameletListResult(int count, int total, String kameletsVersion, List kamelets) { } public record KameletInfo(String name, String type, String supportLevel, String description) { diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RepeatedCallSessions.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RepeatedCallSessions.java index 7af99b2cc596d..be253d7c4134d 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RepeatedCallSessions.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RepeatedCallSessions.java @@ -45,7 +45,9 @@ protected boolean removeEldestEntry(Map.Entry eldest) /** * The short note to answer with instead of running the tool, or null to run it: the tool is not deterministic, the - * call is one of the first identical ones, or there is no connection to tell the sessions apart. + * call is one of the first identical ones, or there is no connection to tell the sessions apart. When null is + * returned and the tool answers successfully, {@link #recordSuccessfulAnswer(McpConnection, String, Map)} must be + * called so that only successful answers count against the repeat budget. */ JsonObject repeatOf(McpConnection connection, String tool, Map args) { if (connection == null || connection.id() == null) { @@ -57,4 +59,21 @@ JsonObject repeatOf(McpConnection connection, String tool, Map a } return calls.repeatOf(ToolRegistry.findTool(tool), args); } + + /** + * Records that the tool answered this call in full without throwing. Failed calls must not count against the retry + * budget (CAMEL-25640). + */ + void recordSuccessfulAnswer(McpConnection connection, String tool, Map args) { + if (connection == null || connection.id() == null) { + return; + } + RepeatedToolCalls calls; + synchronized (sessions) { + calls = sessions.get(connection.id()); + } + if (calls != null) { + calls.recordSuccessfulAnswer(ToolRegistry.findTool(tool), args); + } + } } diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java index e5065610abd77..4ef1094f8a90f 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java @@ -176,4 +176,20 @@ void downloadedCatalogViaPlatformBomVersionDiffersFromBuiltin() { assertThat(version).isEqualTo(requestedVersion); assertThat(version).isNotEqualTo(BUILTIN_VERSION); } + + /** + * CAMEL-25640: quarkus runtime with only camelVersion (no explicit platformBom) used to pass null to + * loadQuarkusCatalog, which silently returned an unusable catalog. The fix resolves the BOM from the registry. + * Disabled in CI: requires network access to the Quarkus extension registry. + */ + @Test + @DisabledIfSystemProperty(named = "ci.env.name", matches = ".*", + disabledReason = "Runs only local — requires Quarkus extension registry access") + void quarkusRuntimeWithCamelVersionResolvesTheBomFromRegistry() { + // Verify that passing camelVersion with quarkus runtime does not silently pass null to loadQuarkusCatalog. + // A successful return (even if it is the built-in version) means no NPE was thrown inside doLoadCatalog. + CatalogService service = createService(null); + String result = version(service, "quarkus", BUILTIN_VERSION, null); + assertThat(result).isNotEmpty(); + } } diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java new file mode 100644 index 0000000000000..3f379f5b03124 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java @@ -0,0 +1,119 @@ +/* + * 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.mcp; + +import java.util.List; + +import io.quarkiverse.mcp.server.ToolCallException; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * CAMEL-25640: KameletTools bugs — no caching and limit-before-sort. + */ +class KameletToolsTest { + + private final KameletTools tools = new KameletTools(); + + /** CAMEL-25640: repeated calls must return identical, sorted results (no HashMap iteration order). */ + @Test + void repeatedCallsReturnTheSameSortedList() { + KameletTools.KameletListResult first = tools.camel_catalog_kamelets(null, null, null, null); + KameletTools.KameletListResult second = tools.camel_catalog_kamelets(null, null, null, null); + + assertThat(first.kamelets()).isEqualTo(second.kamelets()); + // kamelets are sorted by name + List names = first.kamelets().stream().map(KameletTools.KameletInfo::name).toList(); + List sorted = names.stream().sorted(String.CASE_INSENSITIVE_ORDER).toList(); + assertThat(names).isEqualTo(sorted); + } + + /** CAMEL-25640: limit is applied after sorting, so the first N kamelets alphabetically are returned. */ + @Test + void limitIsAppliedAfterSorting() { + KameletTools.KameletListResult full = tools.camel_catalog_kamelets(null, null, null, null); + assertThat(full.kamelets()).hasSizeGreaterThan(5); + + KameletTools.KameletListResult limited = tools.camel_catalog_kamelets(null, null, 3, null); + assertThat(limited.count()).isEqualTo(3); + // total must reflect the full set size, not the limited subset + assertThat(limited.total()).isEqualTo(full.total()); + // the 3 kamelets returned must be the first 3 alphabetically of the full set + assertThat(limited.kamelets()) + .isEqualTo(full.kamelets().subList(0, 3)); + } + + /** CAMEL-25640: total reflects all matching kamelets, not just the returned page. */ + @Test + void totalReflectsAllMatchingKamelets() { + KameletTools.KameletListResult all = tools.camel_catalog_kamelets(null, null, null, null); + KameletTools.KameletListResult paged = tools.camel_catalog_kamelets(null, null, 1, null); + + assertThat(paged.count()).isEqualTo(1); + assertThat(paged.total()).isEqualTo(all.total()); + } + + @Test + void filterByTypeReturnsOnlyMatchingType() { + KameletTools.KameletListResult sources = tools.camel_catalog_kamelets(null, "source", null, null); + assertThat(sources.kamelets()).isNotEmpty(); + assertThat(sources.kamelets()).allMatch(k -> "source".equalsIgnoreCase(k.type())); + } + + @Test + void filterByNameOrDescriptionSubstring() { + KameletTools.KameletListResult aws = tools.camel_catalog_kamelets("aws-s3", null, null, null); + assertThat(aws.kamelets()).isNotEmpty(); + assertThat(aws.kamelets()).allMatch( + k -> k.name() != null && k.name().toLowerCase().contains("aws-s3") + || k.description() != null && k.description().toLowerCase().contains("aws-s3")); + } + + @Test + void kameletDocReturnsDetails() { + KameletTools.KameletDetailResult detail = tools.camel_catalog_kamelet_doc("log-sink", null); + assertThat(detail.name()).isEqualTo("log-sink"); + assertThat(detail.type()).isEqualTo("sink"); + assertThat(detail.options()).isNotEmpty(); + } + + @Test + void kameletDocForUnknownNameThrows() { + assertThatThrownBy(() -> tools.camel_catalog_kamelet_doc("no-such-kamelet-xyz", null)) + .isInstanceOf(ToolCallException.class) + .hasMessageContaining("no-such-kamelet-xyz"); + } + + @Test + void kameletDocForBlankNameThrows() { + assertThatThrownBy(() -> tools.camel_catalog_kamelet_doc("", null)) + .isInstanceOf(ToolCallException.class) + .hasMessageContaining("required"); + } + + /** CAMEL-25640: second call uses the cache, not a fresh loadKamelets(). */ + @Test + void secondCallIsServedFromCache() { + // two calls with the same version — should not throw and should be equal + KameletTools.KameletListResult r1 = tools.camel_catalog_kamelets(null, null, null, null); + KameletTools.KameletListResult r2 = tools.camel_catalog_kamelets(null, null, null, null); + assertThat(r1.kamelets()).isEqualTo(r2.kamelets()); + assertThat(r1.kameletsVersion()).isEqualTo(r2.kameletsVersion()); + } +} From abac5f44ee484b4b092413f7cabc74c1366a83b8 Mon Sep 17 00:00:00 2001 From: gnodet Date: Wed, 14 Oct 2026 17:04:40 +0000 Subject: [PATCH 2/8] fix: record successful TUI MCP tool answers so repeated-call budget works --- .../dsl/jbang/core/commands/tui/TuiMcpServer.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java index 0fa62c2479e48..bb0ce44c97185 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java @@ -532,10 +532,15 @@ private JsonObject handleToolsCall(JsonObject request) { String text; boolean isError = false; + ToolDescriptor tool = toolName != null ? ToolRegistry.findTool(toolName) : null; try { - JsonObject repeat = toolName != null - ? repeatedCalls.repeatOf(ToolRegistry.findTool(toolName), args) : null; - text = repeat != null ? repeat.toJson() : toolRegistry.execute(toolName, args); + JsonObject repeat = repeatedCalls.repeatOf(tool, args); + if (repeat != null) { + text = repeat.toJson(); + } else { + text = toolRegistry.execute(toolName, args); + repeatedCalls.recordSuccessfulAnswer(tool, args); + } } catch (IllegalArgumentException e) { text = "Unknown tool: " + toolName; isError = true; From 45c26ff179bcc67c9424954fe786b065cc5da17a Mon Sep 17 00:00:00 2001 From: gnodet Date: Wed, 14 Oct 2026 19:48:48 +0000 Subject: [PATCH 3/8] Address review: fix camel_error_diagnose repeat tracking, catalog version ordering, TUI error counting, and non-blocking suggestions --- .../modules/ROOT/pages/camel-jbang-mcp.adoc | 3 ++ .../core/commands/mcp/AuthoringTools.java | 8 +++-- .../core/commands/mcp/CatalogService.java | 7 ++-- .../jbang/core/commands/mcp/CatalogTools.java | 2 +- .../core/commands/mcp/DiagnoseTools.java | 13 +++++-- .../jbang/core/commands/mcp/KameletTools.java | 2 +- .../core/commands/mcp/CatalogServiceTest.java | 10 +++--- .../core/commands/mcp/DiagnoseToolsTest.java | 34 +++++++++++++++++++ .../jbang/core/commands/tui/TuiMcpServer.java | 6 +++- 9 files changed, 71 insertions(+), 14 deletions(-) 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 889424d6b4f13..553d0028d6389 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc @@ -475,6 +475,9 @@ the same answer, and the note also says to change the line the error names. Vali `content` with a `directory` (the checks then read the other files of the project, which a fix can change), is always answered in full. +A call that fails (the catalog version cannot be downloaded, the file argument is missing) does not count: +the model can try again without the note getting in the way. + Calls with other arguments, and the tools that read files, logs or the running integration, are always answered in full, so re-reading a file after an edit or polling a log keeps working. diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java index 703d1dce23780..5a6e33aaf6e64 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java @@ -402,9 +402,13 @@ static JsonObject call(String tool, Map args, CatalogService cat ToolContext ctx = new ToolContext(); if (catalogService != null) { String camelVersion = args != null ? args.get("camelVersion") : null; - String runtime = args != null ? args.get("runtime") : null; try { - CamelCatalog catalog = catalogService.loadCatalog(runtime, camelVersion, null); + // Set the version first: setCamelVersion() on a fresh context nulls out any previously set catalog, + // so the catalog must be set after (or the version set before, so it is a no-op when they match). + if (camelVersion != null && !camelVersion.isBlank()) { + ctx.setCamelVersion(camelVersion.trim()); + } + CamelCatalog catalog = catalogService.loadCatalog(null, camelVersion, null); ctx.setCatalog(catalog); } catch (Exception e) { // catalog load failed: fall through to ToolContext's own lazy-load, which handles the error diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java index 99b03ce67ed09..2b8c940e9ddb1 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java @@ -226,8 +226,11 @@ private CamelCatalog doLoadCatalog( if (runtimeType == RuntimeType.springBoot) { return CatalogLoader.loadSpringBootCatalog(repos, null, true); } else if (runtimeType == RuntimeType.quarkus) { - // platformBom should have been resolved in loadCatalog() before doLoadCatalog() is called; passing null - // here would silently produce an unusable catalog, so we guard with an assertion to surface the bug early. + // loadCatalog() always resolves the platform BOM for Quarkus before calling doLoadCatalog(), + // so platformBom is never null here. + if (platformBom == null) { + throw new IllegalStateException("No Quarkus platform BOM resolved"); + } return CatalogLoader.loadQuarkusCatalog(platformBom, downloader()::resolveArtifact); } diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java index 2aba6edbb2ccf..216fcfb5b7ceb 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogTools.java @@ -53,7 +53,7 @@ public DocListResult camel_catalog_docs( @ToolArg(description = "Maximum number of results to return (default: 50)", required = false) Integer limit, @ToolArg(description = ToolArgDocs.CAMEL_VERSION, required = false) String camelVersion) { - int maxResults = limit != null ? Math.max(1, limit) : 50; + int maxResults = limit != null && limit > 0 ? limit : 50; try { CamelCatalog cat = catalogService.loadCatalog(null, camelVersion, null); diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseTools.java index dbcf9675e46c9..d1a94ae8c96ce 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseTools.java @@ -16,6 +16,8 @@ */ package org.apache.camel.dsl.jbang.core.commands.mcp; +import java.util.Map; + import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -66,16 +68,21 @@ public JsonObject camel_error_diagnose( if (error == null || error.isBlank()) { throw new ToolCallException("Error message or stack trace is required", null); } + Map diagArgs = AuthoringTools.args("error", error, + "runtime", runtime, "camelVersion", camelVersion, "platformBom", platformBom); JsonObject repeat = repeatedCalls != null - ? repeatedCalls.repeatOf(connection, "camel_error_diagnose", AuthoringTools.args("error", error, - "runtime", runtime, "camelVersion", camelVersion, "platformBom", platformBom)) + ? repeatedCalls.repeatOf(connection, "camel_error_diagnose", diagArgs) : null; if (repeat != null) { return repeat; } try { CamelCatalog catalog = catalogService.loadCatalog(runtime, camelVersion, platformBom); - return ErrorDiagnoser.diagnose(error, catalog); + JsonObject answer = ErrorDiagnoser.diagnose(error, catalog); + if (repeatedCalls != null) { + repeatedCalls.recordSuccessfulAnswer(connection, "camel_error_diagnose", diagArgs); + } + return answer; } catch (ToolCallException e) { throw e; } catch (Throwable e) { diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java index a9a00b09a646d..278c581cc6fd8 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletTools.java @@ -73,7 +73,7 @@ public KameletListResult camel_catalog_kamelets( @ToolArg(description = "Apache Camel Kamelets version. If not specified, uses the default version.", required = false) String kameletsVersion) { - int maxResults = limit != null ? Math.max(1, limit) : 50; + int maxResults = limit != null && limit > 0 ? limit : 50; try { String version = kameletsVersion != null && !kameletsVersion.isBlank() diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java index 4ef1094f8a90f..33328d1b1746f 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java @@ -186,10 +186,12 @@ void downloadedCatalogViaPlatformBomVersionDiffersFromBuiltin() { @DisabledIfSystemProperty(named = "ci.env.name", matches = ".*", disabledReason = "Runs only local — requires Quarkus extension registry access") void quarkusRuntimeWithCamelVersionResolvesTheBomFromRegistry() { - // Verify that passing camelVersion with quarkus runtime does not silently pass null to loadQuarkusCatalog. - // A successful return (even if it is the built-in version) means no NPE was thrown inside doLoadCatalog. + // CAMEL-25640: passing camelVersion with quarkus runtime must resolve the platform BOM from the Quarkus + // extension registry and pass it to doLoadCatalog; previously it passed null and produced an NPE or a + // silently wrong catalog. Use a released version that is not the built-in one so sameAsDefault() is false + // and the new code path is taken. CatalogService service = createService(null); - String result = version(service, "quarkus", BUILTIN_VERSION, null); - assertThat(result).isNotEmpty(); + String result = version(service, "quarkus", "4.10.0", null); + assertThat(result).isEqualTo("4.10.0"); } } diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseToolsTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseToolsTest.java index 58c620b4382ce..3d3ef2a1705ff 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseToolsTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/DiagnoseToolsTest.java @@ -16,9 +16,11 @@ */ package org.apache.camel.dsl.jbang.core.commands.mcp; +import java.lang.reflect.Proxy; import java.util.List; import java.util.Optional; +import io.quarkiverse.mcp.server.McpConnection; import io.quarkiverse.mcp.server.ToolCallException; import org.apache.camel.util.json.JsonObject; import org.junit.jupiter.api.Test; @@ -267,6 +269,38 @@ void handlesFullStackTrace() { assertThat(components(result).stream().map(c -> c.getString("name")).toList()).contains("kafka"); } + // ---- Repeated calls ---- + + @Test + void thirdIdenticalCallIsRepeated() { + // CAMEL-25640 regression: recordSuccessfulAnswer must be called after a successful diagnose + // so the third identical call gets the repeat note instead of the full answer. + DiagnoseTools toolsWithRepeat = new DiagnoseTools(); + CatalogService catalogService = new CatalogService(); + catalogService.catalogRepos = Optional.empty(); + toolsWithRepeat.catalogService = catalogService; + RepeatedCallSessions sessions = new RepeatedCallSessions(); + toolsWithRepeat.repeatedCalls = sessions; + + McpConnection conn = connection("test-session"); + String error = "org.apache.camel.NoSuchEndpointException: No endpoint could be found for: xyz:test"; + + // first two calls are answered in full + JsonObject first = toolsWithRepeat.camel_error_diagnose(conn, error, null, null, null); + assertThat(first.get("repeated")).isNull(); + JsonObject second = toolsWithRepeat.camel_error_diagnose(conn, error, null, null, null); + assertThat(second.get("repeated")).isNull(); + // third identical call must be the repeat note + JsonObject third = toolsWithRepeat.camel_error_diagnose(conn, error, null, null, null); + assertThat(third.get("repeated")).isEqualTo(Boolean.TRUE); + assertThat(third.getString("tool")).isEqualTo("camel_error_diagnose"); + } + + private static McpConnection connection(String id) { + return (McpConnection) Proxy.newProxyInstance(McpConnection.class.getClassLoader(), + new Class[] { McpConnection.class }, (proxy, method, args) -> "id".equals(method.getName()) ? id : null); + } + private static List exceptions(JsonObject result) { return List.copyOf(result.getCollection("identifiedExceptions")); } diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java index bb0ce44c97185..44b5ec9ec7edb 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java @@ -539,7 +539,11 @@ private JsonObject handleToolsCall(JsonObject request) { text = repeat.toJson(); } else { text = toolRegistry.execute(toolName, args); - repeatedCalls.recordSuccessfulAnswer(tool, args); + // executeShared catches ToolExecutionException and returns "Error: ..." as normal text; only + // record the answer when the call actually succeeded so failed calls do not use the retry budget + if (!text.startsWith("Error:")) { + repeatedCalls.recordSuccessfulAnswer(tool, args); + } } } catch (IllegalArgumentException e) { text = "Unknown tool: " + toolName; From 9256f997ca1e064615ba63516c0063e7cd5ca678 Mon Sep 17 00:00:00 2001 From: gnodet Date: Wed, 14 Oct 2026 20:03:17 +0000 Subject: [PATCH 4/8] CAMEL-25640: fix NPE when Quarkus platform registry lookup returns null findQuarkusPlatformBom() returns null when the registry cannot be reached or no platform matches the requested Camel version. Both call sites in CatalogService (no-version Quarkus path and version-given Quarkus path) now check for null and throw a ToolCallException with a readable message instead of propagating a NullPointerException. --- .../core/commands/mcp/CatalogService.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java index 2b8c940e9ddb1..fffcc134af972 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java @@ -108,13 +108,19 @@ public CamelCatalog loadCatalog(String runtime, String camelVersion, String plat MavenGav platformBomGav = null; if (!hasVersion && !hasBom) { if (runtimeType == RuntimeType.quarkus) { - platformBomGav - = QuarkusHelper.findQuarkusPlatformBom( - camelVersion, - downloader()::resolveArtifact, - true, - RuntimeType.QUARKUS_EXTENSION_REGISTRY_BASE_URL, - false).quarkusCamelBom(); + QuarkusHelper.QuarkusPlatformBom quarkusBom = QuarkusHelper.findQuarkusPlatformBom( + camelVersion, + downloader()::resolveArtifact, + true, + RuntimeType.QUARKUS_EXTENSION_REGISTRY_BASE_URL, + false); + if (quarkusBom == null) { + throw new ToolCallException( + "No Quarkus platform found for Camel " + camelVersion + + " in the Quarkus extension registry", + null); + } + platformBomGav = quarkusBom.quarkusCamelBom(); normalizedVersion = platformBomGav.getVersion(); } } @@ -210,12 +216,19 @@ private CamelCatalog doLoadCatalog( } else if (runtimeType == RuntimeType.quarkus) { // Resolve the Quarkus platform BOM that matches the requested Camel version. // platformBom is null here (only camelVersion was given), so we must look it up. - MavenGav resolvedBom = QuarkusHelper.findQuarkusPlatformBom( + QuarkusHelper.QuarkusPlatformBom quarkusBom = QuarkusHelper.findQuarkusPlatformBom( camelVersion, downloader()::resolveArtifact, true, RuntimeType.QUARKUS_EXTENSION_REGISTRY_BASE_URL, - false).quarkusCamelBom(); + false); + if (quarkusBom == null) { + throw new ToolCallException( + "No Quarkus platform found for Camel " + camelVersion + + " in the Quarkus extension registry", + null); + } + MavenGav resolvedBom = quarkusBom.quarkusCamelBom(); return CatalogLoader.loadQuarkusCatalog(resolvedBom, downloader()::resolveArtifact); } else { return CatalogLoader.loadCatalog(repos, camelVersion, true); From ca64ea4cd35bf530c8c85a4423e480986af3d6f8 Mon Sep 17 00:00:00 2001 From: gnodet Date: Wed, 14 Oct 2026 20:10:21 +0000 Subject: [PATCH 5/8] test: disable KameletToolsTest in CI (downloads camel-kamelets-catalog from Maven Central) --- .../camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java index 3f379f5b03124..ad9c0830b9e86 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/KameletToolsTest.java @@ -20,13 +20,19 @@ import io.quarkiverse.mcp.server.ToolCallException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * CAMEL-25640: KameletTools bugs — no caching and limit-before-sort. + *

+ * All tests in this class download {@code camel-kamelets-catalog} from Maven Central (the module has no compile-time + * dependency on it, so the download is unavoidable). They are disabled in CI to avoid network failures. */ +@DisabledIfSystemProperty(named = "ci.env.name", matches = ".*", + disabledReason = "Runs only local — requires camel-kamelets-catalog download from Maven Central") class KameletToolsTest { private final KameletTools tools = new KameletTools(); From b052e087f1e5f4c9f04487aa5fc1d184b0ca672e Mon Sep 17 00:00:00 2001 From: gnodet Date: Wed, 14 Oct 2026 20:23:26 +0000 Subject: [PATCH 6/8] fix: add debug log when CatalogService.loadCatalog() falls back to ToolContext lazy-load --- .../camel/dsl/jbang/core/commands/mcp/AuthoringTools.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java index 5a6e33aaf6e64..31ad76e8146d5 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java @@ -34,6 +34,7 @@ import org.apache.camel.dsl.jbang.core.commands.ai.ToolRegistry; import org.apache.camel.util.json.JsonArray; import org.apache.camel.util.json.JsonObject; +import org.jboss.logging.Logger; /** * The neutral Camel authoring tools shared with {@code camel tui --mcp} (CAMEL-24695): thin MCP wrappers over the @@ -50,6 +51,8 @@ @ApplicationScoped public class AuthoringTools { + private static final Logger LOG = Logger.getLogger(AuthoringTools.class); + private static final String NAME_DESC = "Integration name or pid (default: the only one running)"; private static final String VERSION_DESC = "Camel version to answer for (default: the CLI's own)"; private static final String DIRECTORY_DESC = "Project directory with the source files (absolute path)"; @@ -412,6 +415,7 @@ static JsonObject call(String tool, Map args, CatalogService cat ctx.setCatalog(catalog); } catch (Exception e) { // catalog load failed: fall through to ToolContext's own lazy-load, which handles the error + LOG.debugf("CatalogService.loadCatalog() failed (%s), falling back to ToolContext lazy-load", e.getMessage()); } } return RuntimeTools.toJsonObject(ToolRegistry.execute(tool, ctx, args)); From 78b8aace22467b9247a95846082385e1e9e33f51 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 14 Oct 2026 22:10:50 +0000 Subject: [PATCH 7/8] Regen --- .../camel/dsl/jbang/core/commands/mcp/AuthoringTools.java | 3 ++- .../camel/dsl/jbang/core/commands/mcp/CatalogService.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java index 31ad76e8146d5..57602b04d5233 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/AuthoringTools.java @@ -415,7 +415,8 @@ static JsonObject call(String tool, Map args, CatalogService cat ctx.setCatalog(catalog); } catch (Exception e) { // catalog load failed: fall through to ToolContext's own lazy-load, which handles the error - LOG.debugf("CatalogService.loadCatalog() failed (%s), falling back to ToolContext lazy-load", e.getMessage()); + LOG.debugf("CatalogService.loadCatalog() failed (%s), falling back to ToolContext lazy-load", + e.getMessage()); } } return RuntimeTools.toJsonObject(ToolRegistry.execute(tool, ctx, args)); diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java index fffcc134af972..a25d6ac33e6f1 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogService.java @@ -117,7 +117,7 @@ public CamelCatalog loadCatalog(String runtime, String camelVersion, String plat if (quarkusBom == null) { throw new ToolCallException( "No Quarkus platform found for Camel " + camelVersion - + " in the Quarkus extension registry", + + " in the Quarkus extension registry", null); } platformBomGav = quarkusBom.quarkusCamelBom(); @@ -225,7 +225,7 @@ private CamelCatalog doLoadCatalog( if (quarkusBom == null) { throw new ToolCallException( "No Quarkus platform found for Camel " + camelVersion - + " in the Quarkus extension registry", + + " in the Quarkus extension registry", null); } MavenGav resolvedBom = quarkusBom.quarkusCamelBom(); From 5bd34bdd699cf77c9b6e31798e887b595ef07c78 Mon Sep 17 00:00:00 2001 From: gnodet Date: Thu, 15 Oct 2026 04:47:10 +0000 Subject: [PATCH 8/8] =?UTF-8?q?fix:=20address=20review=20nits=20=E2=80=94?= =?UTF-8?q?=20CatalogServiceTest=20quarkus=20assertion=20and=20docs=20accu?= =?UTF-8?q?racy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CatalogServiceTest.quarkusRuntimeWithCamelVersionResolvesTheBomFromRegistry: fix assertion to check getRuntimeProvider().getProviderName() == "quarkus" instead of getCatalogVersion() == "4.10.0". getCatalogVersion() always returns the CLI's own version (from the camel-catalog jar on the classpath), not the requested version. Added CamelCatalog import and throws Exception on test method. - camel-jbang-mcp.adoc: clarify that 'failed calls do not count' applies to camel tui --mcp for catalog download failures. On camel mcp, a download failure silently falls back to the built-in catalog (via CatalogService.loadCatalog catch block), so the answer is counted as successful. --- docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc | 6 ++++-- .../dsl/jbang/core/commands/mcp/CatalogServiceTest.java | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) 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 553d0028d6389..fe45a843ef755 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc @@ -475,8 +475,10 @@ the same answer, and the note also says to change the line the error names. Vali `content` with a `directory` (the checks then read the other files of the project, which a fix can change), is always answered in full. -A call that fails (the catalog version cannot be downloaded, the file argument is missing) does not count: -the model can try again without the note getting in the way. +A call that fails (the file argument is missing, or on `camel tui --mcp` a catalog version that cannot be +downloaded) does not count: the model can try again without the note getting in the way. +On `camel mcp`, a catalog version that cannot be downloaded silently falls back to the built-in catalog; the +answer is counted as successful. Calls with other arguments, and the tools that read files, logs or the running integration, are always answered in full, so re-reading a file after an edit or polling a log keeps working. diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java index 33328d1b1746f..164994d1c193c 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/CatalogServiceTest.java @@ -19,6 +19,7 @@ import java.util.Optional; import io.quarkiverse.mcp.server.ToolCallException; +import org.apache.camel.catalog.CamelCatalog; import org.apache.camel.catalog.DefaultCamelCatalog; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; @@ -185,13 +186,15 @@ void downloadedCatalogViaPlatformBomVersionDiffersFromBuiltin() { @Test @DisabledIfSystemProperty(named = "ci.env.name", matches = ".*", disabledReason = "Runs only local — requires Quarkus extension registry access") - void quarkusRuntimeWithCamelVersionResolvesTheBomFromRegistry() { + void quarkusRuntimeWithCamelVersionResolvesTheBomFromRegistry() throws Exception { // CAMEL-25640: passing camelVersion with quarkus runtime must resolve the platform BOM from the Quarkus // extension registry and pass it to doLoadCatalog; previously it passed null and produced an NPE or a // silently wrong catalog. Use a released version that is not the built-in one so sameAsDefault() is false // and the new code path is taken. + // Note: getCatalogVersion() returns the CLI's own Camel version (from camel-catalog on the classpath), + // not the requested "4.10.0". Check the runtime provider instead to verify the Quarkus path was taken. CatalogService service = createService(null); - String result = version(service, "quarkus", "4.10.0", null); - assertThat(result).isEqualTo("4.10.0"); + CamelCatalog catalog = service.loadCatalog("quarkus", "4.10.0", null); + assertThat(catalog.getRuntimeProvider().getProviderName()).isEqualTo("quarkus"); } }