This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24834 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 5520ce9721b6f635269723abd5573866fe2de704 Author: Claus Ibsen <[email protected]> AuthorDate: Sat Oct 3 22:14:06 2026 +0200 CAMEL-24834: SQL in the tool group is not limited to reading (developer tool) The AI panel and the MCP server are developer tools, not meant to reach production databases, so the sql tool group offers the SQL tools as they are: camel_runtime_sql, camel_runtime_datasources and camel_runtime_sql_trace over MCP, tui_execute_sql and tui_update_row in the AI panel. Removes SqlReadOnlyGuard, query_sql / camel_runtime_sql_query, the sqlWrites argument and setting, and the SQL mode in the fingerprint. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m --- .../modules/ROOT/pages/camel-jbang-mcp.adoc | 15 +- .../modules/ROOT/pages/camel-jbang-tui-ai.adoc | 2 +- .../ROOT/pages/camel-jbang-tui-local-models.adoc | 11 +- .../ROOT/pages/camel-jbang-tui-settings.adoc | 11 +- .../jbang/core/commands/ai/SqlReadOnlyGuard.java | 201 --------------------- .../dsl/jbang/core/commands/ai/ToolGroups.java | 38 ++-- .../dsl/jbang/core/commands/ai/ToolRegistry.java | 60 ++---- .../core/commands/ai/SqlReadOnlyGuardTest.java | 101 ----------- .../dsl/jbang/core/commands/ai/ToolGroupsTest.java | 35 ++-- .../jbang/core/commands/ai/ToolRegistryTest.java | 25 +-- .../dsl/jbang/core/commands/mcp/RuntimeTools.java | 37 +--- .../jbang/core/commands/mcp/RuntimeToolsTest.java | 41 +---- .../camel/dsl/jbang/core/commands/tui/AiPanel.java | 66 ++----- .../dsl/jbang/core/commands/tui/TuiSettings.java | 21 --- .../dsl/jbang/core/commands/tui/TuiToolGroups.java | 32 ++-- .../core/commands/tui/AiPanelPromptBudgetTest.java | 44 +++-- .../core/commands/tui/AiPanelToolGroupsTest.java | 68 ++----- 17 files changed, 122 insertions(+), 686 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 c9163983addc..d6d5dba793ef 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-mcp.adoc @@ -793,12 +793,6 @@ process is running). the update count otherwise. Lets the model look at the data a route reads or writes, or try a statement before putting it in a route. -| `camel_runtime_sql_query` -| Run a read-only SQL query (one SELECT, WITH, VALUES, SHOW, EXPLAIN or DESCRIBE statement) against a DataSource - of the running application. A statement that writes (INSERT, UPDATE, DELETE, MERGE, DDL, SELECT ... INTO, - FOR UPDATE, EXPLAIN ANALYZE, a second statement) is refused before anything runs. Marked read-only, so it stays - available at the `read-only` access level where `camel_runtime_sql` is hidden. - | `camel_runtime_tool_groups` | Which runtime tool groups (`sql`, `tracing`, `resilience`) the application needs, from what its status shows. See <<_tool_groups_for_local_models>>. @@ -844,8 +838,7 @@ runtime tool, it can offer the core tools plus the groups the selected applicati | `sql` | a datasource, a `sql`, `sql-stored`, `jdbc`, `spring-jdbc` or `jpa` endpoint, or traced SQL statements -| `camel_runtime_sql_query`, `camel_runtime_datasources`, `camel_runtime_sql_trace`; with `sqlWrites=true` also - `camel_runtime_sql` +| `camel_runtime_sql`, `camel_runtime_datasources`, `camel_runtime_sql_trace` | `tracing` | OpenTelemetry, enabled message tracing, or Micrometer @@ -858,9 +851,9 @@ runtime tool, it can offer the core tools plus the groups the selected applicati The answer names the application and its pid, lists the `core` tools (the shared `camel_*` tools a small model should always get), each loaded group with its tools and one line of guidance for the system prompt (for example -`SQL: datasource(s) orders (HikariCP). Read-only: SELECT only (camel_runtime_sql_query). Table names come from the -SQL trace (camel_runtime_sql_trace); don't guess a schema.`), `sqlReadOnly`, and the `signals` in the status that -loaded each group. Its `fingerprint` stays the same as long as the groups, the datasources and the SQL mode do, +`SQL: datasource(s) orders (HikariCP), used by sql endpoints. Table names come from the SQL trace +(camel_runtime_sql_trace); don't guess a schema.`), and the `signals` in the status that loaded each group. Its +`fingerprint` stays the same as long as the groups and the datasources do, so a client calls the tool when the user selects an application or the application reloads, and rebuilds its tool list and prompt only when the fingerprint changes; the prompt then stays the same and a local model keeps its prompt cache. A status written by an older Camel version may lack some of the keys, which just loads fewer diff --git a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc index f83fbe216eef..4dbfcf0f7314 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc @@ -64,7 +64,7 @@ cycles backward. | Show or switch how file writes by the model (`camel_write_file`) are handled. `confirm` (default) shows the confirm dialog for every write, whatever the model passes; `auto` lets the model skip the dialog with `confirm=false`; `live` replays the edit in the Source editor so you watch it happen (see below). The mode lasts for the session. | `/tools [auto\|core\|full]` (`/t`) -| Show which tool set is sent to the model, or switch it. `auto` (default) sends the core set to local providers and every tool to hosted ones; the choice is saved as `camel.tui.ai.tools`. With the core set it also shows the tool groups loaded for the selected integration and whether SQL is read-only, for example `core (25 of 60 tools), mode auto (local provider); groups: sql, resilience (from the selected integration); SQL read-only`, see xref:camel-jbang-tui-local-models.adoc#_tool_gro [...] +| Show which tool set is sent to the model, or switch it. `auto` (default) sends the core set to local providers and every tool to hosted ones; the choice is saved as `camel.tui.ai.tools`. With the core set it also shows the tool groups loaded for the selected integration, for example `core (26 of 60 tools), mode auto (local provider); groups: sql, resilience (from the selected integration)`, see xref:camel-jbang-tui-local-models.adoc#_tool_groups_from_the_selected_integration[Tool groups]. | `/context` (`/ctx`) | Show what the next request costs: provider and model, tool set, static prefix size, history size and the session total, and with Ollama the context window, the prompt size above which the history is compacted and the last measured prompt. Useful with local models, where prompt size is time. diff --git a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-local-models.adoc b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-local-models.adoc index a9266c3519e1..acf8103abed2 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-local-models.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-local-models.adoc @@ -66,8 +66,8 @@ it writes while it runs: |=== | Group | Loads when the integration has | What the model gets | `sql` | a datasource, a `sql`, `sql-stored`, `jdbc`, `spring-jdbc` or `jpa` endpoint, or traced SQL statements -| `tui_execute_sql` (read-only), and a line naming the datasources and their pool; the table names come from the -*SQL Trace* tab +| `tui_execute_sql` and `tui_update_row`, and a line naming the datasources and their pool; the table names come +from the *SQL Trace* tab | `tracing` | OpenTelemetry, enabled message tracing, or Micrometer | a line pointing at `tui_get_spans`, `tui_get_history` and the *Metrics* tab, which are core tools already | `resilience` | a circuit breaker in a route, or circuit breakers in the Resilience4j or Fault Tolerance status @@ -78,12 +78,9 @@ Each loaded group adds one line at the end of the system prompt. The groups are select another integration or the selected one reloads its routes (after a reload the groups only grow), so the tools and the prompt stay the same from question to question and Ollama keeps reusing the cached prompt. An integration without any group yet is read again on each question, since one that just started may not -have written its status completely. With all three groups the static prefix grows by about 330 tokens. +have written its status completely. With all three groups the static prefix grows by about 520 tokens. -SQL is read-only for a local model: `tui_execute_sql` runs a SELECT (or WITH, SHOW, EXPLAIN, DESCRIBE) -statement and refuses anything that writes, and `tui_update_row` is not offered. Set `camel.tui.ai.sqlWrites` -to `true` to allow writes (see xref:camel-jbang-tui-settings.adoc[Settings]). The full tool set (`/tools full` -and hosted providers) is not affected by the groups and is not limited to reading. `/tools` and `/context` +The full tool set (`/tools full` and hosted providers) is not affected by the groups. `/tools` and `/context` show the loaded groups. == Working with a local Ollama model diff --git a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-settings.adoc b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-settings.adoc index b7c6e530c286..d786640d8fc8 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-settings.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-settings.adoc @@ -153,7 +153,7 @@ Settings are stored under `camel.tui.*` keys (`camel.tui.theme`, `camel.tui.star `camel.tui.selectTab`, `camel.tui.confirmActions`, `camel.tui.defaultFolder`, `camel.tui.panelPosition`, `camel.tui.panelSpace`, `camel.tui.shell.history`, `camel.tui.ai.provider`, `camel.tui.ai.model`, `camel.tui.ai.url`, -`camel.tui.ai.tools`, `camel.tui.ai.overview`, `camel.tui.ai.promptHistory`, `camel.tui.ai.sqlWrites`) in the Camel CLI configuration file. Each key is read from and +`camel.tui.ai.tools`, `camel.tui.ai.overview`, `camel.tui.ai.promptHistory`) in the Camel CLI configuration file. Each key is read from and written back to the file where it currently lives: a key present in the local `./camel-cli.properties` is treated as a project-level override and stays local, while every other key defaults to the global `~/.camel-cli.properties`. This means a project can @@ -161,15 +161,6 @@ deliberately pin a starting tab in its local config without redirecting your per into the project file. See xref:camel-jbang-configuration.adoc[Configuration] for details on the global and local files. -=== SQL writes from the AI panel - -`camel.tui.ai.sqlWrites` has no row in the dialog; set it in `.camel-cli.properties` (or with -`camel config set camel.tui.ai.sqlWrites=true`). With the core tool set, which local models get, the AI panel -only reads the integration's database: `tui_execute_sql` refuses a statement that writes and `tui_update_row` -is not offered. Set the key to `true` to let a local model write as well. The default is `false`; the full tool -set is not limited. See -xref:camel-jbang-tui-local-models.adoc#_tool_groups_from_the_selected_integration[Tool groups from the selected integration]. - === Input history The embedded shell (*F6*) and AI prompt (*F8*) keep a recall list for the command line and prompt diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SqlReadOnlyGuard.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SqlReadOnlyGuard.java deleted file mode 100644 index 916e97634730..000000000000 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SqlReadOnlyGuard.java +++ /dev/null @@ -1,201 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.camel.dsl.jbang.core.commands.ai; - -import java.util.ArrayList; -import java.util.List; -import java.util.Locale; -import java.util.Set; - -/** - * Tells whether a SQL statement only reads (CAMEL-24834), so a small local model can query the integration's database - * without being able to change it. A check of the statement's words, not a SQL parser: comments and quoted literals are - * removed first (a {@code 'DELETE'} or {@code ';'} inside a string does not count), then the statement must be a single - * one starting with SELECT, WITH, VALUES, TABLE, SHOW, EXPLAIN (not EXPLAIN ANALYZE, which runs the statement), - * DESCRIBE or DESC, and must not write anywhere inside: no SELECT ... INTO, no FOR UPDATE / FOR SHARE locks, no INSERT, - * UPDATE, DELETE or MERGE in a CTE. It errs on the side of refusing; the user can enable SQL writes. - */ -public final class SqlReadOnlyGuard { - - static final String ALLOWED = "only a SELECT (or WITH, SHOW, EXPLAIN, DESCRIBE) statement is allowed;" - + " ask the user to enable SQL writes"; - - private static final Set<String> READ_KEYWORDS - = Set.of("SELECT", "WITH", "VALUES", "TABLE", "SHOW", "EXPLAIN", "DESCRIBE", "DESC"); - - /** Words that change data, schema or grants, or run code, wherever they appear outside literals. */ - private static final Set<String> WRITE_KEYWORDS = Set.of( - "INSERT", "UPDATE", "DELETE", "MERGE", "UPSERT", "TRUNCATE", "DROP", "ALTER", "CREATE", - "GRANT", "REVOKE", "CALL", "EXEC", "EXECUTE"); - - /** What follows FOR in a locking read: FOR UPDATE, FOR SHARE, FOR NO KEY UPDATE, FOR KEY SHARE. */ - private static final Set<String> LOCK_MODES = Set.of("UPDATE", "SHARE", "NO", "KEY"); - - private SqlReadOnlyGuard() { - } - - /** - * Checks a statement. - * - * @return null when the statement only reads, otherwise the message to answer with - */ - public static String check(String sql) { - if (sql == null || sql.isBlank()) { - return "read-only: the statement is empty; " + ALLOWED; - } - // MySQL reads quotes and comments differently from standard SQL (a backslash escapes a quote, # starts a - // comment, /*! ... */ is code); where two readings disagree on what is a literal or a comment, a statement - // could hide in the difference, so it must pass both - String answer = check(sql, false); - return answer != null ? answer : check(sql, true); - } - - private static String check(String sql, boolean mysql) { - String code = stripCommentsAndLiterals(sql, mysql); - if (code == null) { - return "read-only: a comment or quoted literal that is not terminated, or a nested or executable comment; " - + ALLOWED; - } - code = code.strip(); - while (code.endsWith(";")) { - code = code.substring(0, code.length() - 1).strip(); - } - if (code.indexOf(';') >= 0) { - return "read-only: one statement at a time; " + ALLOWED; - } - List<String> words = words(code); - if (words.isEmpty()) { - return "read-only: the statement is empty; " + ALLOWED; - } - String first = words.get(0); - if (!READ_KEYWORDS.contains(first)) { - return "read-only: " + first + " is not a read; " + ALLOWED; - } - // SHOW CREATE TABLE and DESCRIBE only read the catalog - boolean catalog = "SHOW".equals(first) || "DESCRIBE".equals(first) || "DESC".equals(first); - for (int i = 0; i < words.size(); i++) { - String w = words.get(i); - String next = i + 1 < words.size() ? words.get(i + 1) : ""; - if (!catalog && WRITE_KEYWORDS.contains(w)) { - return "read-only: the statement contains " + w + "; " + ALLOWED; - } - if ("INTO".equals(w)) { - return "read-only: SELECT ... INTO writes a table; " + ALLOWED; - } - if ("FOR".equals(w) && LOCK_MODES.contains(next) || "LOCK".equals(w) && "IN".equals(next)) { - return "read-only: the statement locks rows; " + ALLOWED; - } - if ("EXPLAIN".equals(first) && ("ANALYZE".equals(w) || "ANALYSE".equals(w))) { - return "read-only: EXPLAIN ANALYZE runs the statement; " + ALLOWED; - } - } - return null; - } - - /** - * The statement with comments and quoted literals or identifiers replaced by a space, read the standard way or the - * MySQL way; null when one is not terminated, or for a comment that a database could read differently (nested, or - * MySQL's executable {@code /*!}). - */ - static String stripCommentsAndLiterals(String sql, boolean mysql) { - StringBuilder sb = new StringBuilder(sql.length()); - int i = 0; - int n = sql.length(); - while (i < n) { - char c = sql.charAt(i); - char next = i + 1 < n ? sql.charAt(i + 1) : 0; - if (c == '-' && next == '-' && (!mysql || i + 2 >= n || Character.isWhitespace(sql.charAt(i + 2))) - || mysql && c == '#') { - // MySQL needs a space after --, otherwise 1--1 is arithmetic - int eol = sql.indexOf('\n', i); - i = eol < 0 ? n : eol; - sb.append(' '); - } else if (c == '/' && next == '*') { - int end = sql.indexOf("*/", i + 2); - if (end < 0 || sql.charAt(i + 2) == '!' || sql.substring(i + 2, end).contains("/*")) { - return null; - } - i = end + 2; - sb.append(' '); - } else if (c == '\'' || c == '"' || c == '`') { - int end = closingQuote(sql, i + 1, c, mysql && c != '`'); - if (end < 0) { - return null; - } - i = end + 1; - sb.append(' '); - } else if (c == '$' && !mysql && (i == 0 || !isIdentifierPart(sql.charAt(i - 1)))) { - // PostgreSQL dollar quoting: $$...$$ or $tag$...$tag$ - int tagEnd = sql.indexOf('$', i + 1); - String tag = tagEnd > 0 ? sql.substring(i, tagEnd + 1) : null; - if (tag != null - && tag.substring(1, tag.length() - 1).chars().allMatch(SqlReadOnlyGuard::isIdentifierPart)) { - int end = sql.indexOf(tag, tagEnd + 1); - if (end < 0) { - return null; - } - i = end + tag.length(); - sb.append(' '); - } else { - sb.append(c); - i++; - } - } else { - sb.append(c); - i++; - } - } - return sb.toString(); - } - - private static boolean isIdentifierPart(int ch) { - return Character.isLetterOrDigit(ch) || ch == '_' || ch == '$'; - } - - /** - * The index of the quote that closes a literal opened before {@code from}, or -1; a doubled quote is an escape, and - * so is a backslash when asked for. - */ - private static int closingQuote(String sql, int from, char quote, boolean backslashEscapes) { - int i = from; - while (i < sql.length()) { - char c = sql.charAt(i); - if (backslashEscapes && c == '\\' && i + 1 < sql.length()) { - i += 2; - } else if (c == quote) { - if (i + 1 < sql.length() && sql.charAt(i + 1) == quote) { - i += 2; - } else { - return i; - } - } else { - i++; - } - } - return -1; - } - - private static List<String> words(String code) { - List<String> words = new ArrayList<>(); - for (String w : code.split("[^A-Za-z0-9_]+")) { - if (!w.isEmpty()) { - words.add(w.toUpperCase(Locale.ROOT)); - } - } - return words; - } -} diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroups.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroups.java index e999a0ad8652..ea1983dd63f4 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroups.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroups.java @@ -29,7 +29,6 @@ import java.util.stream.Collectors; */ public final class ToolGroups { - public static final String SQL_QUERY_TOOL = "camel_runtime_sql_query"; public static final String SQL_TOOL = "camel_runtime_sql"; public static final String DATASOURCES_TOOL = "camel_runtime_datasources"; public static final String SQL_TRACE_TOOL = "camel_runtime_sql_trace"; @@ -56,11 +55,10 @@ public final class ToolGroups { * The groups for an integration. * * @param groups the loaded groups, in {@link ToolGroup} order - * @param sqlReadOnly whether SQL is limited to reading (only meaningful when the SQL group is loaded) - * @param fingerprint stable for the same groups, datasources and SQL mode: a client rebuilds its tool list only - * when it changes + * @param fingerprint stable for the same groups and datasources: a client rebuilds its tool list only when it + * changes */ - public record Selection(List<Group> groups, boolean sqlReadOnly, String fingerprint) { + public record Selection(List<Group> groups, String fingerprint) { public Selection { groups = List.copyOf(groups); @@ -88,18 +86,17 @@ public final class ToolGroups { private ToolGroups() { } - /** The groups an integration needs; with {@code sqlWrites} the SQL group also offers the tool that writes. */ - public static Selection select(AppFeatures features, boolean sqlWrites) { + /** The groups an integration needs. */ + public static Selection select(AppFeatures features) { AppFeatures f = features != null ? features : AppFeatures.none(); List<Group> groups = new ArrayList<>(); for (ToolGroup group : groups(f)) { switch (group) { case SQL -> groups.add(new Group( group, - sqlWrites - ? List.of(SQL_QUERY_TOOL, SQL_TOOL, DATASOURCES_TOOL, SQL_TRACE_TOOL) - : List.of(SQL_QUERY_TOOL, DATASOURCES_TOOL, SQL_TRACE_TOOL), - sqlGuidance(f, sqlWrites))); + List.of(SQL_TOOL, DATASOURCES_TOOL, SQL_TRACE_TOOL), + "SQL: " + describeSql(f) + ". Table names come from the SQL trace (" + SQL_TRACE_TOOL + + "); don't guess a schema.")); case TRACING -> { List<String> tools = new ArrayList<>(); List<String> parts = new ArrayList<>(); @@ -124,15 +121,7 @@ public final class ToolGroups { + " OPEN means the fallback runs.")); } } - return new Selection(groups, !sqlWrites, fingerprint(groups(f), f, sqlWrites)); - } - - private static String sqlGuidance(AppFeatures f, boolean sqlWrites) { - String mode = sqlWrites - ? SQL_QUERY_TOOL + " reads, " + SQL_TOOL + " also writes." - : "Read-only: SELECT only (" + SQL_QUERY_TOOL + ")."; - return "SQL: " + describeSql(f) + ". " + mode + " Table names come from the SQL trace (" + SQL_TRACE_TOOL - + "); don't guess a schema."; + return new Selection(groups, fingerprint(groups(f), f)); } /** The groups the features call for, in {@link ToolGroup} order. */ @@ -151,14 +140,13 @@ public final class ToolGroups { } /** - * Sorted group ids and datasource names, and the SQL mode when the SQL group is loaded: the same integration gives - * the same fingerprint, whatever order its status lists things in. + * Sorted group ids and datasource names: the same integration gives the same fingerprint, whatever order its status + * lists things in. */ - static String fingerprint(List<ToolGroup> groups, AppFeatures features, boolean sqlWrites) { + static String fingerprint(List<ToolGroup> groups, AppFeatures features) { String ids = groups.stream().map(ToolGroup::id).sorted().collect(Collectors.joining(",")); String ds = features.dataSourceNames().stream().sorted().collect(Collectors.joining(",")); - String mode = groups.contains(ToolGroup.SQL) ? (sqlWrites ? "rw" : "ro") : ""; - return ids + "|" + ds + "|" + mode; + return ids + "|" + ds; } /** diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java index efc1829fbfab..ad36c29da355 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistry.java @@ -422,27 +422,23 @@ public final class ToolRegistry { "Name of the DataSource bean (auto-detected if only one exists)", false) .param("maxRows", "string", "Maximum number of rows to return (default: 100)", false) .readOnly(false).destructive(true) - .executor(ToolRegistry::executeSql)); - - // CAMEL-24834: the SQL a small local model gets, which cannot change the database - register(tool("query_sql", - "Run a read-only SQL query (SELECT, WITH, SHOW, EXPLAIN, DESCRIBE) against a DataSource in the running " - + "Camel application. Returns structured JSON with columns, rows, and metadata. " - + "Any statement that writes is refused.") - .param("query", "string", "The SQL query to run", true) - .param("datasource", "string", - "Name of the DataSource bean (auto-detected if only one exists)", false) - .param("maxRows", "string", "Maximum number of rows to return (default: 100)", false) - .readOnly(true).destructive(false) .executor((ctx, args) -> { String sql = args.get("query"); - if (sql != null && !sql.isBlank()) { - String refused = SqlReadOnlyGuard.check(sql); - if (refused != null) { - throw new ToolExecutionException(refused); + if (sql == null || sql.isBlank()) { + throw new ToolExecutionException("'query' parameter is required"); + } + String datasource = args.get("datasource"); + int maxRows = 100; + String maxRowsStr = args.get("maxRows"); + if (maxRowsStr != null && !maxRowsStr.isBlank()) { + try { + maxRows = Integer.parseInt(maxRowsStr); + } catch (NumberFormatException e) { + // use default } } - return executeSql(ctx, args); + JsonObject result = ctx.executeSqlQuery(sql, datasource, maxRows, 30); + return result.toJson(); })); register(tool("get_tool_groups", @@ -451,8 +447,6 @@ public final class ToolRegistry { + "circuit breakers. Returns the tools of each group with one line of guidance, " + "and a fingerprint that changes only when the groups do.") .param("name", "string", "Name or PID of the integration (default: the selected or only one)", false) - .param("sqlWrites", "boolean", "Offer the SQL tool that writes too (default false: read-only SQL)", - false) .executor((ctx, args) -> { String name = args.get("name"); if (name != null && !name.isBlank()) { @@ -460,9 +454,7 @@ public final class ToolRegistry { } else { ctx.selectSingleProcessIfNone(); } - JsonObject status = ctx.readFullStatus(); - boolean sqlWrites = "true".equalsIgnoreCase(args.get("sqlWrites")); - return toolGroups(ctx, status, sqlWrites).toJson(); + return toolGroups(ctx, ctx.readFullStatus()).toJson(); })); // Route control @@ -479,32 +471,13 @@ public final class ToolRegistry { .executor((ctx, args) -> ctx.stopApplication())); } - private static String executeSql(ToolContext ctx, Map<String, String> args) { - String sql = args.get("query"); - if (sql == null || sql.isBlank()) { - throw new ToolExecutionException("'query' parameter is required"); - } - String datasource = args.get("datasource"); - int maxRows = 100; - String maxRowsStr = args.get("maxRows"); - if (maxRowsStr != null && !maxRowsStr.isBlank()) { - try { - maxRows = Integer.parseInt(maxRowsStr); - } catch (NumberFormatException e) { - // use default - } - } - JsonObject result = ctx.executeSqlQuery(sql, datasource, maxRows, 30); - return result.toJson(); - } - /** * The answer of get_tool_groups: the integration, the core tools every client has, the groups its status calls for * and the status keys that called for them. */ - static JsonObject toolGroups(ToolContext ctx, JsonObject status, boolean sqlWrites) { + static JsonObject toolGroups(ToolContext ctx, JsonObject status) { AppFeatures features = AppFeatures.fromStatus(status); - ToolGroups.Selection selection = ToolGroups.select(features, sqlWrites); + ToolGroups.Selection selection = ToolGroups.select(features); JsonObject answer = new JsonObject(); String app = null; for (RuntimeHelper.ProcessInfo p : ctx.discoverProcesses()) { @@ -519,7 +492,6 @@ public final class ToolRegistry { answer.put("app", app); answer.put("pid", ctx.pid()); answer.put("fingerprint", selection.fingerprint()); - answer.put("sqlReadOnly", selection.sqlReadOnly()); JsonArray core = new JsonArray(); authoringTools().stream().filter(ToolDescriptor::isCore).map(ToolDescriptor::name).forEach(core::add); answer.put("core", core); diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SqlReadOnlyGuardTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SqlReadOnlyGuardTest.java deleted file mode 100644 index acc8f9571b60..000000000000 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SqlReadOnlyGuardTest.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.camel.dsl.jbang.core.commands.ai; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - -class SqlReadOnlyGuardTest { - - @ParameterizedTest - @ValueSource(strings = { - "SELECT * FROM orders", - "select id, name from orders where id = 1;", - " SELECT 1 ; ", - "(SELECT 1) UNION (SELECT 2)", - "WITH recent AS (SELECT * FROM orders WHERE ts > now()) SELECT count(*) FROM recent", - "VALUES (1, 'a')", - "TABLE orders", - "SHOW TABLES", - "SHOW CREATE TABLE orders", - "EXPLAIN SELECT * FROM orders", - "DESCRIBE orders", - "DESC orders", - "SELECT * FROM orders ORDER BY id DESC", - "-- the orders\nSELECT * FROM orders", - "/* all of them */ SELECT * FROM orders", - "SELECT 'DELETE FROM orders; DROP TABLE x' AS text FROM orders", - "SELECT 'a;b' FROM orders WHERE note = 'it''s; fine'", - "SELECT \"update\" FROM \"insert\"", - "SELECT replace(name, 'a', 'b') FROM orders", - "SELECT * FROM orders WHERE id = $1", - "SELECT 1 # a MySQL comment\n" - }) - void allowsReads(String sql) { - assertNull(SqlReadOnlyGuard.check(sql), sql); - } - - @ParameterizedTest - @ValueSource(strings = { - "", - "INSERT INTO orders VALUES (1)", - "update orders set name = 'x'", - "DELETE FROM orders", - "DROP TABLE orders", - "MERGE INTO orders USING x ON (1=1) WHEN MATCHED THEN DELETE", - "SELECT 1; DELETE FROM orders", - "SELECT 1; SELECT 2", - "WITH gone AS (DELETE FROM orders RETURNING *) SELECT * FROM gone", - "WITH x AS (INSERT INTO orders VALUES (1) RETURNING id) SELECT id FROM x", - "SELECT * INTO backup FROM orders", - "SELECT * FROM orders FOR UPDATE", - "SELECT * FROM orders FOR SHARE", - "SELECT * FROM orders FOR NO KEY UPDATE", - "SELECT * FROM orders LOCK IN SHARE MODE", - "EXPLAIN ANALYZE DELETE FROM orders", - "EXPLAIN (ANALYZE) SELECT * FROM orders", - "CALL cleanup()", - "SELECT 1 /* unterminated", - "SELECT 'unterminated", - // a comment hides the second statement in one reading only - "SELECT 1 -- \n; DELETE FROM orders", - "SELECT 1 /* /* */ ' */ ; DELETE FROM orders; -- '", - "SELECT 1 /*! ; DELETE FROM orders */", - "SELECT 'a\\' ; DELETE FROM orders; -- '", - "SELECT 1 # '\n; DELETE FROM orders; -- '", - "SELECT 1 --x' \n '; DELETE FROM orders; -- '", - // PostgreSQL dollar quoting is no quoting in MySQL - "SELECT $$; DELETE FROM orders$$" - }) - void refusesWrites(String sql) { - String answer = SqlReadOnlyGuard.check(sql); - assertNotNull(answer, sql); - assertTrue(answer.startsWith("read-only: "), answer); - assertTrue(answer.contains("ask the user to enable SQL writes"), answer); - } - - @ParameterizedTest - @ValueSource(strings = { "SELECT 1; SELECT 2" }) - void saysWhy(String sql) { - assertTrue(SqlReadOnlyGuard.check(sql).contains("one statement at a time")); - assertTrue(SqlReadOnlyGuard.check("DELETE FROM orders").contains("DELETE is not a read")); - } -} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroupsTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroupsTest.java index 2d18477e3ab5..820c1d7aac3f 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroupsTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolGroupsTest.java @@ -36,51 +36,40 @@ class ToolGroupsTest { @Test void nothingLoadsNoGroup() { - ToolGroups.Selection s = ToolGroups.select(AppFeatures.none(), false); + ToolGroups.Selection s = ToolGroups.select(AppFeatures.none()); assertTrue(s.groups().isEmpty()); assertTrue(s.mcpTools().isEmpty()); assertTrue(s.guidance().isEmpty()); - assertEquals("||", s.fingerprint()); + assertEquals("|", s.fingerprint()); } @Test void eachGroupHasItsTools() { - ToolGroups.Selection s = ToolGroups.select(everything(), false); + ToolGroups.Selection s = ToolGroups.select(everything()); assertEquals(List.of(ToolGroup.SQL, ToolGroup.TRACING, ToolGroup.RESILIENCE), s.toolGroups()); - assertEquals(List.of("camel_runtime_sql_query", "camel_runtime_datasources", "camel_runtime_sql_trace"), + assertEquals(List.of("camel_runtime_sql", "camel_runtime_datasources", "camel_runtime_sql_trace"), s.groups().get(0).tools()); assertEquals(List.of("camel_runtime_spans", "camel_runtime_trace", "camel_runtime_metrics"), s.groups().get(1).tools()); assertEquals(List.of("camel_runtime_circuit_breakers"), s.groups().get(2).tools()); - assertTrue(s.sqlReadOnly()); - assertFalse(s.mcpTools().contains("camel_runtime_sql"), "read-only SQL has no tool that writes"); - } - - @Test - void sqlWritesAddTheToolThatWrites() { - ToolGroups.Selection s = ToolGroups.select(everything(), true); - assertFalse(s.sqlReadOnly()); - assertTrue(s.mcpTools().containsAll(List.of("camel_runtime_sql_query", "camel_runtime_sql"))); - assertTrue(s.guidance().get(0).contains("camel_runtime_sql also writes"), s.guidance().get(0)); } @Test void tracingOffersOnlyWhatIsOn() { AppFeatures messageTracing = new AppFeatures( List.of(), List.of(), false, false, List.of(), false, true, false, Map.of()); - ToolGroups.Selection s = ToolGroups.select(messageTracing, false); + ToolGroups.Selection s = ToolGroups.select(messageTracing); assertEquals(List.of("camel_runtime_trace"), s.mcpTools()); assertTrue(s.guidance().get(0).contains("message tracing is on"), s.guidance().get(0)); } @Test void theGuidanceNamesWhatTheIntegrationHas() { - List<String> guidance = ToolGroups.select(everything(), false).guidance(); + List<String> guidance = ToolGroups.select(everything()).guidance(); assertEquals(3, guidance.size()); - assertTrue(guidance.get(0).startsWith("SQL: datasource(s) orders (HikariCP), audit, used by sql endpoints."), + assertEquals("SQL: datasource(s) orders (HikariCP), audit, used by sql endpoints. Table names come from the" + + " SQL trace (camel_runtime_sql_trace); don't guess a schema.", guidance.get(0)); - assertTrue(guidance.get(0).contains("Read-only: SELECT only"), guidance.get(0)); - assertTrue(guidance.get(0).contains("don't guess a schema"), guidance.get(0)); assertTrue(guidance.get(1).contains("OpenTelemetry"), guidance.get(1)); assertTrue(guidance.get(2).startsWith("Circuit breakers in routes pay, ship: camel_runtime_circuit_breakers"), guidance.get(2)); @@ -94,9 +83,9 @@ class ToolGroupsTest { AppFeatures reordered = new AppFeatures( List.of(new AppFeatures.DataSource("audit", null), new AppFeatures.DataSource("orders", "HikariCP")), List.of("sql"), true, true, List.of("ship", "pay"), true, true, true, Map.of("x", "y")); - String fp = ToolGroups.select(everything(), false).fingerprint(); - assertEquals("resilience,sql,tracing|audit,orders|ro", fp); - assertEquals(fp, ToolGroups.select(reordered, false).fingerprint()); - assertNotEquals(fp, ToolGroups.select(everything(), true).fingerprint(), "the SQL mode changes the tools"); + String fp = ToolGroups.select(everything()).fingerprint(); + assertEquals("resilience,sql,tracing|audit,orders", fp); + assertEquals(fp, ToolGroups.select(reordered).fingerprint()); + assertNotEquals(fp, ToolGroups.select(AppFeatures.none()).fingerprint()); } } diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistryTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistryTest.java index 52d4f6a81cec..36100dbb7f73 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistryTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolRegistryTest.java @@ -169,23 +169,6 @@ class ToolRegistryTest { () -> ToolRegistry.execute("execute_sql", ctx, Map.of())); } - @Test - void querySqlRefusesWritesBeforeLookingForAProcess() { - // CAMEL-24834: no process selected, so a statement that got past the guard would fail with "No running Camel - // process" instead - ToolDescriptor tool = ToolRegistry.findTool("query_sql"); - assertNotNull(tool); - assertTrue(tool.isReadOnly()); - assertFalse(tool.isDestructive()); - ToolContext ctx = new ToolContext(); - ToolExecutionException e = assertThrows(ToolExecutionException.class, - () -> ToolRegistry.execute("query_sql", ctx, Map.of("query", "INSERT INTO orders VALUES (1)"))); - assertTrue(e.getMessage().startsWith("read-only: "), e.getMessage()); - e = assertThrows(ToolExecutionException.class, - () -> ToolRegistry.execute("query_sql", ctx, Map.of("query", "SELECT 1"))); - assertFalse(e.getMessage().startsWith("read-only: "), "a read goes on to the process: " + e.getMessage()); - } - @Test void theToolGroupsToolIsInternal() { // the MCP servers export every camel_* tool by name; get_tool_groups is reached through @@ -195,7 +178,6 @@ class ToolRegistryTest { assertTrue(tool.isReadOnly()); assertFalse(tool.isDeterministic()); assertFalse(ToolRegistry.authoringTools().contains(tool)); - assertFalse(ToolRegistry.authoringTools().contains(ToolRegistry.findTool("query_sql"))); } @Test @@ -205,16 +187,15 @@ class ToolRegistryTest { String json = "{'context': {'name': 'shop'}, 'dataSources': {'dataSources': [{'name': 'orders'," + " 'poolType': 'HikariCP'}]}}"; JsonObject status = (JsonObject) Jsoner.deserialize(json.replace('\'', '"')); - JsonObject answer = ToolRegistry.toolGroups(ctx, status, false); + JsonObject answer = ToolRegistry.toolGroups(ctx, status); assertEquals("shop", answer.get("app")); assertEquals(99999L, answer.get("pid")); - assertEquals(Boolean.TRUE, answer.get("sqlReadOnly")); - assertEquals("sql|orders|ro", answer.get("fingerprint")); + assertEquals("sql|orders", answer.get("fingerprint")); JsonArray groups = (JsonArray) answer.get("groups"); assertEquals(1, groups.size()); JsonObject sql = (JsonObject) groups.get(0); assertEquals("sql", sql.get("id")); - assertTrue(((JsonArray) sql.get("tools")).contains("camel_runtime_sql_query")); + assertTrue(((JsonArray) sql.get("tools")).contains("camel_runtime_sql")); assertTrue(sql.get("guidance").toString().contains("orders (HikariCP)")); assertTrue(((JsonArray) answer.get("core")).contains("camel_get_errors")); assertEquals("orders", ((JsonObject) answer.get("signals")).get("dataSources")); diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeTools.java index 595c9053f98e..f8fbe7bdbd48 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeTools.java @@ -26,7 +26,6 @@ import jakarta.inject.Inject; import io.quarkiverse.mcp.server.Tool; import io.quarkiverse.mcp.server.ToolArg; import io.quarkiverse.mcp.server.ToolCallException; -import org.apache.camel.dsl.jbang.core.commands.ai.SqlReadOnlyGuard; 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; @@ -287,32 +286,6 @@ public class RuntimeTools { return delegateToRegistry("execute_sql", nameOrPid, args); } - @Tool(annotations = @Tool.Annotations(readOnlyHint = true, destructiveHint = false, openWorldHint = false), - description = """ - Run a read-only SQL query against a DataSource of the running Camel application: \ - SELECT, WITH, VALUES, SHOW, EXPLAIN or DESCRIBE, one statement. Returns columns, rows and metadata; \ - a statement that writes is refused. Take the table names from camel_runtime_sql_trace.""") - public JsonObject camel_runtime_sql_query( - @ToolArg(description = NAME_OR_PID_DESC, required = false) String nameOrPid, - @ToolArg(description = "The SQL query to run") String query, - @ToolArg(description = "Name of the DataSource bean (auto-detected if only one exists)", - required = false) String datasource, - @ToolArg(description = "Maximum number of rows to return (default 100)", required = false) String maxRows) { - if (query == null || query.isBlank()) { - throw new ToolCallException("query is required", null); - } - // refuse before looking for a process, so a write never depends on what runs - String refused = SqlReadOnlyGuard.check(query); - if (refused != null) { - throw new ToolCallException(refused, null); - } - Map<String, String> args = new HashMap<>(); - args.put("query", query); - putIfNotBlank(args, "datasource", datasource); - putIfNotBlank(args, "maxRows", maxRows); - return delegateToRegistry("query_sql", nameOrPid, args); - } - @Tool(annotations = @Tool.Annotations(readOnlyHint = true, destructiveHint = false, openWorldHint = false), description = """ Which runtime tool groups the Camel application needs, from what it has: sql (datasources, \ @@ -320,14 +293,8 @@ public class RuntimeTools { breakers). Returns the core tools, each group's tools with one line of guidance, and a fingerprint \ that changes only when the groups do. A client for a small model offers the core tools plus these.""") public JsonObject camel_runtime_tool_groups( - @ToolArg(description = NAME_OR_PID_DESC, required = false) String nameOrPid, - @ToolArg(description = "Also offer camel_runtime_sql, which writes (default false: read-only SQL)", - required = false) Boolean sqlWrites) { - Map<String, String> args = new HashMap<>(); - if (sqlWrites != null && sqlWrites) { - args.put("sqlWrites", "true"); - } - return delegateToRegistry("get_tool_groups", nameOrPid, args); + @ToolArg(description = NAME_OR_PID_DESC, required = false) String nameOrPid) { + return delegateToRegistry("get_tool_groups", nameOrPid, Map.of()); } @Tool(annotations = @Tool.Annotations(readOnlyHint = true, destructiveHint = false, openWorldHint = false), diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeToolsTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeToolsTest.java index 7542f3705d38..ebd305f6313f 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeToolsTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/RuntimeToolsTest.java @@ -17,7 +17,6 @@ package org.apache.camel.dsl.jbang.core.commands.mcp; import java.lang.reflect.Method; -import java.util.Arrays; import java.util.List; import io.quarkiverse.mcp.server.Tool; @@ -79,44 +78,18 @@ class RuntimeToolsTest { } @Test - void sqlQueryRequiresQuery() { - RuntimeTools tools = createTools(); - assertThatThrownBy(() -> tools.camel_runtime_sql_query(null, " ", null, null)) - .isInstanceOf(ToolCallException.class) - .hasMessageContaining("query is required"); - } - - @Test - void sqlQueryRefusesWritesBeforeLookingForAProcess() { - // CAMEL-24834: the refusal does not depend on what runs, so it is the same with no process at all - RuntimeTools tools = createTools(); - assertThatThrownBy(() -> tools.camel_runtime_sql_query("no-such-app", "DELETE FROM orders", null, null)) - .isInstanceOf(ToolCallException.class) - .hasMessageStartingWith("read-only: "); - assertThatThrownBy(() -> tools.camel_runtime_sql_query(null, "SELECT 1; DROP TABLE orders", null, null)) - .isInstanceOf(ToolCallException.class) - .hasMessageContaining("one statement at a time"); - } - - @Test - void theReadOnlyToolsAreVisibleAtTheReadOnlyAccessLevel() throws Exception { - // McpAccessFilter decides from the annotations: read-only hints keep the tools for a read-only client - for (String name : List.of("camel_runtime_sql_query", "camel_runtime_tool_groups")) { - Method m = Arrays.stream(RuntimeTools.class.getDeclaredMethods()) - .filter(dm -> dm.getName().equals(name)).findFirst().orElseThrow(); - Tool tool = m.getAnnotation(Tool.class); - assertThat(McpSecurityConfig.AccessLevel.READ_ONLY.permits( - tool.annotations().readOnlyHint(), tool.annotations().destructiveHint())).as(name).isTrue(); - } - Method sql = Arrays.stream(RuntimeTools.class.getDeclaredMethods()) - .filter(dm -> dm.getName().equals("camel_runtime_sql")).findFirst().orElseThrow(); - assertThat(sql.getAnnotation(Tool.class).annotations().readOnlyHint()).isFalse(); + void theToolGroupsToolIsVisibleAtTheReadOnlyAccessLevel() throws Exception { + // McpAccessFilter decides from the annotations: a read-only hint keeps the tool for a read-only client + Method m = RuntimeTools.class.getDeclaredMethod("camel_runtime_tool_groups", String.class); + Tool tool = m.getAnnotation(Tool.class); + assertThat(McpSecurityConfig.AccessLevel.READ_ONLY.permits( + tool.annotations().readOnlyHint(), tool.annotations().destructiveHint())).isTrue(); } @Test void theNewWrappersDelegateToRegistryTools() { // CAMEL-24867: every wrapper names a tool the shared registry has, so a typo cannot hide until runtime - for (String name : List.of("execute_sql", "query_sql", "get_tool_groups", "get_datasources", "get_sql_trace", + for (String name : List.of("execute_sql", "get_tool_groups", "get_datasources", "get_sql_trace", "get_circuit_breakers", "get_metrics", "get_eip_stats", "get_spans", "get_startup_steps", "get_route_analysis", "detect_config_drift")) { assertThat(ToolRegistry.findTool(name)).as(name).isNotNull(); diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java index e8310639d314..250da71c4ccc 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java @@ -77,7 +77,6 @@ import dev.tamboui.widgets.table.TableState; import org.apache.camel.dsl.jbang.core.commands.LlmClient; import org.apache.camel.dsl.jbang.core.commands.ai.AnswerChecks; import org.apache.camel.dsl.jbang.core.commands.ai.AppFeatures; -import org.apache.camel.dsl.jbang.core.commands.ai.SqlReadOnlyGuard; import org.apache.camel.dsl.jbang.core.common.ExampleHelper; import org.apache.camel.dsl.jbang.core.common.Printer; import org.apache.camel.util.json.JsonObject; @@ -323,14 +322,12 @@ class AiPanel { private McpFacade.WriteMode writeMode = McpFacade.WriteMode.CONFIRM; private TuiToolRegistry toolRegistry; // CAMEL-24834: the tool groups (SQL, tracing, resilience) of the selected integration that the core set gets, see - // refreshToolGroups(); camel.tui.ai.sqlWrites decides whether SQL may write + // refreshToolGroups() private AppStatusSource appStatusSource; private String toolGroupsPid; private int toolGroupsReloads = -1; private AppFeatures toolGroupsFeatures = AppFeatures.none(); private volatile TuiToolGroups.Selection toolGroups = TuiToolGroups.Selection.none(); - private volatile boolean sqlWrites; - private Boolean sqlWritesForTesting; private boolean mcpServerActive; private int mcpServerPort; @@ -3364,33 +3361,27 @@ class AiPanel { /** * Loads the tool groups of the selected integration for the core set (CAMEL-24834): the SQL tools when it has a * database, the guidance for its tracing and circuit breakers. Read again only when another integration is selected - * or the selected one reloaded (the groups then only grow: what it had before still counts), or the SQL mode - * changed, so the tools and the prompt stay the same from question to question and a local model's prompt cache - * keeps working. While an integration has no groups yet its status is read again, since one that just started may - * not have written it completely. The full set is not affected. + * or the selected one reloaded (the groups then only grow: what it had before still counts), so the tools and the + * prompt stay the same from question to question and a local model's prompt cache keeps working. While an + * integration has no groups yet its status is read again, since one that just started may not have written it + * completely. The full set is not affected. */ private void refreshToolGroups() { - boolean writes = sqlWritesForTesting != null ? sqlWritesForTesting : TuiSettings.load().isAiSqlWrites(); if (!useCoreTools()) { - sqlWrites = writes; return; } AppStatusSource source = appStatusSource != null ? appStatusSource : facadeStatusSource(); String pid = source != null ? source.selectedPid() : null; int reloads = source != null ? source.reloadCount() : 0; boolean samePid = Objects.equals(pid, toolGroupsPid); - boolean reread = !samePid || reloads != toolGroupsReloads || toolGroups.groups().isEmpty(); - if (!reread && writes == toolGroups.sqlWrites()) { + if (samePid && reloads == toolGroupsReloads && !toolGroups.groups().isEmpty()) { return; } - if (reread) { - AppFeatures read = pid != null ? source.features() : AppFeatures.none(); - toolGroupsFeatures = samePid ? toolGroupsFeatures.merge(read) : read; - toolGroupsPid = pid; - toolGroupsReloads = reloads; - } - toolGroups = TuiToolGroups.select(toolGroupsFeatures, writes); - sqlWrites = writes; + AppFeatures read = pid != null ? source.features() : AppFeatures.none(); + toolGroupsFeatures = samePid ? toolGroupsFeatures.merge(read) : read; + toolGroupsPid = pid; + toolGroupsReloads = reloads; + toolGroups = TuiToolGroups.select(toolGroupsFeatures); } private AppStatusSource facadeStatusSource() { @@ -3462,9 +3453,6 @@ class AiPanel { groups = toolGroups.groups().isEmpty() ? "; groups: none loaded" : "; groups: " + toolGroups.groupIds() + " (from the selected integration)"; - if (!sqlWrites) { - groups += "; SQL read-only"; - } } return (useCoreTools() ? "core" : "full") + " (" + active + " of " + total + " tools), mode " + mode + detail + groups; @@ -3893,10 +3881,6 @@ class AiPanel { if (toolRegistry == null) { return "Error: TUI tools not available"; } - String refused = refuseSqlWrite(name, args); - if (refused != null) { - return "Error: " + refused; - } try { return toolRegistry.execute(name, args); } catch (IllegalArgumentException e) { @@ -3906,26 +3890,6 @@ class AiPanel { } } - /** - * Keeps a local model (the core set) from writing to the database unless camel.tui.ai.sqlWrites is true: SQL that - * writes is refused, and so is tui_update_row, which the core set then does not even offer. - */ - private String refuseSqlWrite(String name, JsonObject args) { - if (sqlWrites || !useCoreTools()) { - return null; - } - if (TuiToolGroups.SQL_TOOL.equals(name)) { - String query = args != null && args.get("query") instanceof String q ? q : null; - String refused = query != null ? SqlReadOnlyGuard.check(query) : null; - return refused != null ? refused + " (" + TuiSettings.PROP_AI_SQL_WRITES + "=true)" : null; - } - if (TuiToolGroups.UPDATE_ROW_TOOL.equals(name)) { - return "read-only: " + name + " changes the database; ask the user to enable SQL writes (" - + TuiSettings.PROP_AI_SQL_WRITES + "=true)"; - } - return null; - } - // F8 intentionally excluded — it closes the panel and is handled above private static boolean isFunctionKey(KeyEvent ke) { KeyCode code = ke.code(); @@ -4098,10 +4062,6 @@ class AiPanel { this.appStatusSource = source; } - void setSqlWritesForTesting(Boolean writes) { - this.sqlWritesForTesting = writes; - } - void refreshToolGroupsForTesting() { refreshToolGroups(); } @@ -4110,10 +4070,6 @@ class AiPanel { return toolGroups; } - String executeTuiToolForTesting(String name, JsonObject args) { - return executeTuiTool(name, args); - } - List<LlmClient.ToolDef> toolDefinitionsForTesting() { return buildTuiToolDefinitions(); } diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiSettings.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiSettings.java index 48226d1d24cb..37daa098baa8 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiSettings.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiSettings.java @@ -42,7 +42,6 @@ final class TuiSettings { static final String PROP_AI_TOOLS = "camel.tui.ai.tools"; static final String PROP_AI_OVERVIEW = "camel.tui.ai.overview"; static final String PROP_AI_ACP_COMMAND = "camel.tui.ai.acp.command"; - static final String PROP_AI_SQL_WRITES = "camel.tui.ai.sqlWrites"; static final String PROP_PROXY_HOST = "camel.tui.proxyHost"; static final String PROP_PROXY_PORT = "camel.tui.proxyPort"; static final String PROP_SHELL_HISTORY = "camel.tui.shell.history"; @@ -66,7 +65,6 @@ final class TuiSettings { private String aiTools; private String aiOverview; private String aiAcpCommand; - private String aiSqlWrites; private String shellHistory; private String aiPromptHistory; private String confirmActions; @@ -195,23 +193,6 @@ final class TuiSettings { this.aiAcpCommand = aiAcpCommand; } - /** - * Whether the AI panel lets a local model (the core tool set) write to the integration's database (CAMEL-24834): - * {@code false} (default) limits tui_execute_sql to reading and leaves tui_update_row out, {@code true} allows - * both. The full tool set is not limited. - */ - String getAiSqlWrites() { - return aiSqlWrites; - } - - void setAiSqlWrites(String aiSqlWrites) { - this.aiSqlWrites = aiSqlWrites; - } - - boolean isAiSqlWrites() { - return "true".equalsIgnoreCase(aiSqlWrites); - } - String getShellHistory() { return shellHistory; } @@ -308,7 +289,6 @@ final class TuiSettings { settings.aiTools = trimToNull(TuiUserConfig.read(PROP_AI_TOOLS)); settings.aiOverview = trimToNull(TuiUserConfig.read(PROP_AI_OVERVIEW)); settings.aiAcpCommand = trimToNull(TuiUserConfig.read(PROP_AI_ACP_COMMAND)); - settings.aiSqlWrites = trimToNull(TuiUserConfig.read(PROP_AI_SQL_WRITES)); settings.shellHistory = trimToNull(TuiUserConfig.read(PROP_SHELL_HISTORY)); settings.aiPromptHistory = trimToNull(TuiUserConfig.read(PROP_AI_PROMPT_HISTORY)); settings.confirmActions = trimToNull(TuiUserConfig.read(PROP_CONFIRM_ACTIONS)); @@ -342,7 +322,6 @@ final class TuiSettings { TuiUserConfig.write(PROP_AI_TOOLS, aiTools); TuiUserConfig.write(PROP_AI_OVERVIEW, aiOverview); TuiUserConfig.write(PROP_AI_ACP_COMMAND, aiAcpCommand); - TuiUserConfig.write(PROP_AI_SQL_WRITES, aiSqlWrites); TuiUserConfig.write(PROP_SHELL_HISTORY, shellHistory); TuiUserConfig.write(PROP_AI_PROMPT_HISTORY, aiPromptHistory); TuiUserConfig.write(PROP_CONFIRM_ACTIONS, confirmActions); diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiToolGroups.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiToolGroups.java index 5a22971bab8d..448841972cff 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiToolGroups.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiToolGroups.java @@ -38,12 +38,11 @@ final class TuiToolGroups { /** * The groups of an integration as the panel uses them. * - * @param groups the loaded groups - * @param tools the tools the groups add to the core set - * @param guidance one line per group, appended to the system prompt - * @param sqlWrites whether SQL may write + * @param groups the loaded groups + * @param tools the tools the groups add to the core set + * @param guidance one line per group, appended to the system prompt */ - record Selection(List<ToolGroup> groups, List<String> tools, List<String> guidance, boolean sqlWrites) { + record Selection(List<ToolGroup> groups, List<String> tools, List<String> guidance) { Selection { groups = List.copyOf(groups); @@ -52,7 +51,7 @@ final class TuiToolGroups { } static Selection none() { - return new Selection(List.of(), List.of(), List.of(), false); + return new Selection(List.of(), List.of(), List.of()); } String groupIds() { @@ -63,32 +62,29 @@ final class TuiToolGroups { private TuiToolGroups() { } - static Selection select(AppFeatures features, boolean sqlWrites) { + static Selection select(AppFeatures features) { AppFeatures f = features != null ? features : AppFeatures.none(); List<ToolGroup> groups = ToolGroups.groups(f); List<String> tools = new ArrayList<>(); List<String> guidance = new ArrayList<>(); for (ToolGroup group : groups) { - tools.addAll(tools(group, sqlWrites)); - guidance.add(guidance(group, f, sqlWrites)); + tools.addAll(tools(group)); + guidance.add(guidance(group, f)); } - return new Selection(groups, tools, guidance, sqlWrites); + return new Selection(groups, tools, guidance); } - static List<String> tools(ToolGroup group, boolean sqlWrites) { + static List<String> tools(ToolGroup group) { if (group == ToolGroup.SQL) { - return sqlWrites ? List.of(SQL_TOOL, UPDATE_ROW_TOOL) : List.of(SQL_TOOL); + return List.of(SQL_TOOL, UPDATE_ROW_TOOL); } return List.of(); } - static String guidance(ToolGroup group, AppFeatures f, boolean sqlWrites) { + static String guidance(ToolGroup group, AppFeatures f) { return switch (group) { - case SQL -> "SQL: " + ToolGroups.describeSql(f) + ". " - + (sqlWrites - ? SQL_TOOL + " runs any statement, " + UPDATE_ROW_TOOL + " changes one row." - : "Read-only: " + SQL_TOOL + " runs SELECT only.") - + " Table names come from the SQL trace (tui_get_table tab 'SQL Trace'); don't guess a schema."; + case SQL -> "SQL: " + ToolGroups.describeSql(f) + + ". Table names come from the SQL trace (tui_get_table tab 'SQL Trace'); don't guess a schema."; case TRACING -> { List<String> parts = new ArrayList<>(); if (f.openTelemetry()) { diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelPromptBudgetTest.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelPromptBudgetTest.java index 6e9fb6e7619c..fca8515e5783 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelPromptBudgetTest.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelPromptBudgetTest.java @@ -59,10 +59,11 @@ class AiPanelPromptBudgetTest { // raised from 9450 for camel_project_overview and camel_save_project_summary (CAMEL-25143), measured ~9750: // full mode only (hosted models), and the panel's own /overview sends no tools at all static final int FULL_BUDGET_TOKENS = 9_850; - /** Measured ~5.0k tokens for 25 tools: the core set (~4.7k) plus every tool group (CAMEL-24834). */ - // the SQL group adds tui_execute_sql (~190 tokens), each group one guidance line in the prompt (~140 for all - // three); an integration rarely has all three, and the groups only load for the integration that needs them - static final int CORE_WITH_GROUPS_BUDGET_TOKENS = 5_300; + /** Measured ~5.2k tokens for 26 tools: the core set (~4.7k) plus every tool group (CAMEL-24834). */ + // the SQL group adds tui_execute_sql and tui_update_row (~385 tokens), each group one guidance line in the prompt + // (~130 for all three); an integration rarely has all three, and the groups only load for the integration that + // needs them + static final int CORE_WITH_GROUPS_BUDGET_TOKENS = 5_500; record Prefix(String mode, int tools, long promptChars, long toolChars) { @@ -116,10 +117,10 @@ class AiPanelPromptBudgetTest { } /** A core panel with every tool group loaded: datasources, OpenTelemetry, tracing, Micrometer, circuit breakers. */ - static AiPanel coreWithAllGroups(boolean sqlWrites) { + static AiPanel coreWithAllGroups() { AiPanelToolGroupsTest.FakeApp app = new AiPanelToolGroupsTest.FakeApp(); app.features = AiPanelToolGroupsTest.EVERYTHING; - AiPanel panel = AiPanelToolGroupsTest.panel(AiPanel.TOOL_MODE_CORE, app, sqlWrites); + AiPanel panel = AiPanelToolGroupsTest.panel(AiPanel.TOOL_MODE_CORE, app); panel.refreshToolGroupsForTesting(); return panel; } @@ -135,7 +136,7 @@ class AiPanelPromptBudgetTest { @Test void coreWithAllGroupsPrefixStaysWithinBudget() { - Prefix groups = measure("core+groups", coreWithAllGroups(false)); + Prefix groups = measure("core+groups", coreWithAllGroups()); System.out.println("AI panel static prefix: " + groups); assertTrue(groups.totalTokens() <= CORE_WITH_GROUPS_BUDGET_TOKENS, @@ -187,22 +188,19 @@ class AiPanelPromptBudgetTest { assertTrue(prompt.contains("camel_write_file"), "write files with the tool"); } - // CAMEL-24834: the guidance of the tool groups only names tools the model is given, with SQL writes or not - for (boolean sqlWrites : List.of(false, true)) { - AiPanel groups = coreWithAllGroups(sqlWrites); - String prompt = groups.systemPromptForTesting(); - int start = prompt.indexOf("The selected integration:"); - assertTrue(start > 0, "the guidance is appended at the end"); - Set<String> tools = groups.toolDefinitionsForTesting().stream().map(LlmClient.ToolDef::name) - .collect(Collectors.toSet()); - Matcher m = Pattern.compile("\\b(?:tui|camel)_[a-z_]+").matcher(prompt.substring(start)); - int named = 0; - while (m.find()) { - named++; - assertTrue(tools.contains(m.group()), m.group() + " is named in the guidance but not in the set"); - } - assertTrue(named >= 4, "the guidance names the tools to use"); - assertTrue(prompt.contains("tui_update_row") == sqlWrites, "tui_update_row only with SQL writes"); + // CAMEL-24834: the guidance of the tool groups only names tools the model is given + AiPanel groups = coreWithAllGroups(); + String prompt = groups.systemPromptForTesting(); + int start = prompt.indexOf("The selected integration:"); + assertTrue(start > 0, "the guidance is appended at the end"); + Set<String> tools = groups.toolDefinitionsForTesting().stream().map(LlmClient.ToolDef::name) + .collect(Collectors.toSet()); + Matcher m = Pattern.compile("\\b(?:tui|camel)_[a-z_]+").matcher(prompt.substring(start)); + int named = 0; + while (m.find()) { + named++; + assertTrue(tools.contains(m.group()), m.group() + " is named in the guidance but not in the set"); } + assertTrue(named >= 4, "the guidance names the tools to use"); } } diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelToolGroupsTest.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelToolGroupsTest.java index 9a8e635adb7a..53fb1bf07691 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelToolGroupsTest.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelToolGroupsTest.java @@ -22,7 +22,6 @@ import java.util.Map; import org.apache.camel.dsl.jbang.core.commands.LlmClient; import org.apache.camel.dsl.jbang.core.commands.ai.AppFeatures; import org.apache.camel.dsl.jbang.core.commands.ai.ToolGroup; -import org.apache.camel.util.json.JsonObject; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -69,12 +68,11 @@ class AiPanelToolGroupsTest { } } - static AiPanel panel(String mode, FakeApp app, boolean sqlWrites) { + static AiPanel panel(String mode, FakeApp app) { AiPanel panel = new AiPanel(); panel.setToolRegistryForTesting(new TuiToolRegistry(null)); panel.setToolModeForTesting(mode); panel.setAppStatusSourceForTesting(app); - panel.setSqlWritesForTesting(sqlWrites); return panel; } @@ -83,29 +81,24 @@ class AiPanelToolGroupsTest { } @Test - void theSqlGroupAddsTheQueryToolOnly() { - AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, new FakeApp(), false); + void theSqlGroupAddsTheSqlTools() { + AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, new FakeApp()); assertFalse(toolNames(panel).contains("tui_execute_sql"), "no group before the first question"); panel.refreshToolGroupsForTesting(); assertEquals(List.of(ToolGroup.SQL), panel.toolGroupsForTesting().groups()); - assertTrue(toolNames(panel).contains("tui_execute_sql")); - assertFalse(toolNames(panel).contains("tui_update_row"), "writes are off"); - assertTrue(panel.systemPromptForTesting().contains("Read-only: tui_execute_sql runs SELECT only")); + assertTrue(toolNames(panel).containsAll(List.of("tui_execute_sql", "tui_update_row"))); + assertTrue(panel.systemPromptForTesting().contains( + "- SQL: datasource(s) orders (HikariCP), used by sql endpoints. Table names come from the SQL trace" + + " (tui_get_table tab 'SQL Trace'); don't guess a schema.\n")); assertTrue(panel.describeToolModeForTesting().contains("groups: sql (from the selected integration)"), panel.describeToolModeForTesting()); - assertTrue(panel.describeToolModeForTesting().contains("SQL read-only")); - - AiPanel writes = panel(AiPanel.TOOL_MODE_CORE, new FakeApp(), true); - writes.refreshToolGroupsForTesting(); - assertTrue(toolNames(writes).containsAll(List.of("tui_execute_sql", "tui_update_row"))); - assertFalse(writes.describeToolModeForTesting().contains("SQL read-only")); } @Test void anotherIntegrationGetsItsOwnGroups() { FakeApp app = new FakeApp(); - AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app, false); + AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app); panel.refreshToolGroupsForTesting(); assertEquals(List.of(ToolGroup.SQL), panel.toolGroupsForTesting().groups()); @@ -121,7 +114,7 @@ class AiPanelToolGroupsTest { @Test void aReloadAddsToTheGroups() { FakeApp app = new FakeApp(); - AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app, false); + AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app); panel.refreshToolGroupsForTesting(); app.reloads = 1; @@ -136,7 +129,7 @@ class AiPanelToolGroupsTest { @Test void theGroupsAreCachedOtherwise() { FakeApp app = new FakeApp(); - AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app, false); + AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app); panel.refreshToolGroupsForTesting(); String prompt = panel.systemPromptForTesting(); List<LlmClient.ToolDef> tools = panel.toolDefinitionsForTesting(); @@ -148,12 +141,6 @@ class AiPanelToolGroupsTest { assertEquals(1, app.reads, "same integration, no reload: the status is not read again"); assertEquals(prompt, panel.systemPromptForTesting(), "the prompt stays byte-identical"); assertEquals(tools, panel.toolDefinitionsForTesting()); - - // turning SQL writes on changes the tools without reading the status - panel.setSqlWritesForTesting(true); - panel.refreshToolGroupsForTesting(); - assertEquals(1, app.reads); - assertTrue(toolNames(panel).contains("tui_update_row")); } @Test @@ -161,7 +148,7 @@ class AiPanelToolGroupsTest { // one that just started may not have written its status completely yet FakeApp app = new FakeApp(); app.features = AppFeatures.none(); - AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app, false); + AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, app); panel.refreshToolGroupsForTesting(); assertTrue(panel.toolGroupsForTesting().groups().isEmpty()); assertTrue(panel.describeToolModeForTesting().contains("groups: none loaded")); @@ -175,8 +162,8 @@ class AiPanelToolGroupsTest { void theFullSetIsUnchanged() { FakeApp app = new FakeApp(); app.features = EVERYTHING; - AiPanel plain = panel(AiPanel.TOOL_MODE_FULL, new FakeApp(), false); - AiPanel panel = panel(AiPanel.TOOL_MODE_FULL, app, false); + AiPanel plain = panel(AiPanel.TOOL_MODE_FULL, new FakeApp()); + AiPanel panel = panel(AiPanel.TOOL_MODE_FULL, app); String before = panel.systemPromptForTesting(); panel.refreshToolGroupsForTesting(); @@ -185,34 +172,5 @@ class AiPanelToolGroupsTest { assertFalse(panel.systemPromptForTesting().contains("The selected integration")); assertEquals(toolNames(plain), toolNames(panel)); assertTrue(toolNames(panel).contains("tui_update_row")); - // and writing is not limited there - String answer = panel.executeTuiToolForTesting("tui_execute_sql", query("DELETE FROM orders")); - assertFalse(answer.contains("read-only"), answer); - } - - @Test - void theCoreSetRefusesSqlThatWrites() { - AiPanel panel = panel(AiPanel.TOOL_MODE_CORE, new FakeApp(), false); - panel.refreshToolGroupsForTesting(); - - String answer = panel.executeTuiToolForTesting("tui_execute_sql", query("INSERT INTO orders VALUES (1)")); - assertTrue(answer.startsWith("Error: read-only: "), answer); - assertTrue(answer.contains("camel.tui.ai.sqlWrites=true"), answer); - answer = panel.executeTuiToolForTesting("tui_update_row", new JsonObject()); - assertTrue(answer.startsWith("Error: read-only: tui_update_row"), answer); - // a read goes on to the tool (which has no integration here) - answer = panel.executeTuiToolForTesting("tui_execute_sql", query("SELECT * FROM orders")); - assertFalse(answer.contains("read-only"), answer); - - AiPanel writes = panel(AiPanel.TOOL_MODE_CORE, new FakeApp(), true); - writes.refreshToolGroupsForTesting(); - answer = writes.executeTuiToolForTesting("tui_execute_sql", query("INSERT INTO orders VALUES (1)")); - assertFalse(answer.contains("read-only"), answer); - } - - private static JsonObject query(String sql) { - JsonObject args = new JsonObject(); - args.put("query", sql); - return args; } }
