CAMEL-25640: camel-jbang-mcp - fix catalog caching, kamelet sort-before-limit, and failed-call retry budget

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.
 

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<String, Integer> 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<String, ?> args) {
         if (tool == null || !tool.isDeterministic(args)) {
@@ -61,10 +62,16 @@ public synchronized JsonObject repeatOf(ToolDescriptor tool, Map<String, ?> 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<String, ?> 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<String, ?> 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();

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()) {

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<String, ?> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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 = "<?xml version=\"1.0\"?>\n<routes/>\n";
         Map<String, String> 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

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<String, String> 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<String, String> 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<String, String> 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);
         }

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);
         }
 

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);

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<String, String> 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) {