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..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,6 +475,11 @@ 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 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-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..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 @@ -28,11 +28,13 @@ 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; 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 @@ -49,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)"; @@ -59,6 +63,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 +374,52 @@ 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; + try { + // 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 + LOG.debugf("CatalogService.loadCatalog() failed (%s), falling back to ToolContext lazy-load", + e.getMessage()); + } + } + 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..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 @@ -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(); } } @@ -208,7 +214,22 @@ 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. + 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); + } + MavenGav resolvedBom = quarkusBom.quarkusCamelBom(); + return CatalogLoader.loadQuarkusCatalog(resolvedBom, downloader()::resolveArtifact); } else { return CatalogLoader.loadCatalog(repos, camelVersion, true); } @@ -218,6 +239,11 @@ private CamelCatalog doLoadCatalog( if (runtimeType == RuntimeType.springBoot) { return CatalogLoader.loadSpringBootCatalog(repos, null, true); } else if (runtimeType == RuntimeType.quarkus) { + // 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 6a64a68fc8db1..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 ? 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 43000d02a0b52..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 @@ -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 && limit > 0 ? 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..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; @@ -176,4 +177,24 @@ 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() 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); + CamelCatalog catalog = service.loadCatalog("quarkus", "4.10.0", null); + assertThat(catalog.getRuntimeProvider().getProviderName()).isEqualTo("quarkus"); + } } 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-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..ad9c0830b9e86 --- /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,125 @@ +/* + * 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 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(); + + /** 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()); + } +} 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..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 @@ -532,10 +532,19 @@ 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); + // 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; isError = true;