From cc2afdb264b0eaa353e5f2736c98e475337b71f7 Mon Sep 17 00:00:00 2001 From: centdix <40307056+centdix@users.noreply.github.com> Date: Mon, 29 Sep 2025 12:42:02 +0200 Subject: [PATCH] fix(mcp): filter out tools with long names (#6692) * filter out tools with too long names * do not advertise tool change ability * add comment * use id for names * Revert "use id for names" This reverts commit 40958cd86105cd30698641fbdf7aabb7a78c459b. * use trunc suffix * cleaning --- backend/windmill-api/src/mcp/server.rs | 17 +++++++- .../windmill-api/src/mcp/utils/transform.rs | 39 +++++++++++++------ .../components/settings/CreateToken.svelte | 27 +++++++++++-- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/backend/windmill-api/src/mcp/server.rs b/backend/windmill-api/src/mcp/server.rs index fa8644b5b8..8a09459202 100644 --- a/backend/windmill-api/src/mcp/server.rs +++ b/backend/windmill-api/src/mcp/server.rs @@ -151,6 +151,22 @@ impl ServerHandler for Runner { check_scopes(authed)?; + if request.name.ends_with("_TRUNC") { + return Ok(CallToolResult::error( + vec![ + Annotated::new( + RawContent::Text(RawTextContent { + text: + "Tool path is too long. Consider shortening it to make it compatible with MCP." + .to_string(), + meta: None, + }), + None + ), + ] + )); + } + let db = http_parts.extensions.get::().ok_or_else(|| { tracing::error!("DB Axum extension not found"); ErrorData::internal_error("DB Axum extension not found", None) @@ -435,7 +451,6 @@ impl ServerHandler for Runner { protocol_version: ProtocolVersion::default(), capabilities: ServerCapabilities::builder() .enable_tools() - .enable_tool_list_changed() .build(), server_info: Implementation::from_build_env(), instructions: Some("This server provides a list of scripts and flows the user can run on Windmill. Each flow and script is a tool callable with their respective arguments.".to_string()), diff --git a/backend/windmill-api/src/mcp/utils/transform.rs b/backend/windmill-api/src/mcp/utils/transform.rs index c40fe37263..d4ef52fd3f 100644 --- a/backend/windmill-api/src/mcp/utils/transform.rs +++ b/backend/windmill-api/src/mcp/utils/transform.rs @@ -5,22 +5,34 @@ use super::models::SchemaType; +// MCP clients do not allow names longer than 60 characters +const MAX_PATH_LENGTH: usize = 60; + /// Transform the path for workspace scripts/flows -/// -/// This function takes a path and a type string and formats the transformed -/// path with the type prefix. This is used when listing, because we can't -/// have names with slashes. Because we replace slashes with underscores, +/// +/// This function takes a path and a type string and formats the transformed +/// path with the type prefix. This is used when listing, because we can't +/// have names with slashes. Because we replace slashes with underscores, /// we also need to escape underscores. pub fn transform_path(path: &str, type_str: &str) -> String { let escaped_path = path.replace('_', "__").replace('/', "_"); // first letter of type_str is used as prefix, only one letter to avoid reaching 60 char name limit - format!("{}-{}", &type_str[..1], escaped_path) + let transformed_path = format!("{}-{}", &type_str[..1], escaped_path); + if transformed_path.len() > MAX_PATH_LENGTH { + let suffix = "_TRUNC"; + return format!( + "{}{}", + &transformed_path[..MAX_PATH_LENGTH - suffix.len()], + suffix + ); + } + transformed_path } /// Reverse the transformation of a path /// -/// This function takes a transformed path and reverses the transformation -/// applied by `transform_path`. It checks if the path starts with "h" +/// This function takes a transformed path and reverses the transformation +/// applied by `transform_path`. It checks if the path starts with "h" /// (indicating a Hub script) and removes the prefix if present. /// It then determines the type of the item (script or flow) based on the prefix. /// This is used in call_tool to get the original path, and the type of the item. @@ -54,7 +66,10 @@ pub fn reverse_transform(transformed_path: &str) -> Result<(&str, String, bool), parts[0].to_string() } else { const TEMP_PLACEHOLDER: &str = "@@UNDERSCORE@@"; - mangled_path.replace("__", TEMP_PLACEHOLDER).replace('_', "/").replace(TEMP_PLACEHOLDER, "_") + mangled_path + .replace("__", TEMP_PLACEHOLDER) + .replace('_', "/") + .replace(TEMP_PLACEHOLDER, "_") }; Ok((type_str, original_path, is_hub)) @@ -64,7 +79,7 @@ pub fn reverse_transform(transformed_path: &str) -> Result<(&str, String, bool), /// /// This function takes a key and replaces spaces with underscores. /// It also removes any characters that are not alphanumeric or underscores. -/// This is used when listing, because we can't have names with spaces +/// This is used when listing, because we can't have names with spaces /// or special characters in the schema properties. pub fn apply_key_transformation(key: &str) -> String { key.replace(' ', "_") @@ -75,8 +90,8 @@ pub fn apply_key_transformation(key: &str) -> String { /// Reverse the transformation of a key /// -/// This function takes a transformed key and a schema object and reverses -/// the transformation applied by `apply_key_transformation`. This can be +/// This function takes a transformed key and a schema object and reverses +/// the transformation applied by `apply_key_transformation`. This can be /// subject to collisions, but it's unlikely and is ok for our use case. pub fn reverse_transform_key(transformed_key: &str, schema_obj: &Option) -> String { let schema_obj = match schema_obj { @@ -98,4 +113,4 @@ pub fn reverse_transform_key(transformed_key: &str, schema_obj: &Option(undefined) let newMcpToken = $state(undefined) let newTokenExpiration = $state(undefined) @@ -124,6 +127,17 @@ : 'Create your first scripts or flows to make them available via MCP.' ) const noScriptsOrFlowsAvailableWarning = $derived(includedRunnables.length === 0 ? warning : '') + const longPathRunnables = $derived( + includedRunnables.filter((path) => path.length > MAX_PATH_LENGTH) + ) + const validRunnables = $derived( + includedRunnables.filter((path) => path.length <= MAX_PATH_LENGTH) + ) + const longPathWarning = $derived( + longPathRunnables.length > 0 + ? `${longPathRunnables.length} script(s)/flow(s) have paths longer than 60 characters and will be excluded from MCP tools. Consider shortening the paths: ${longPathRunnables.slice(0, 3).join(', ')}${longPathRunnables.length > 3 ? ` and ${longPathRunnables.length - 3} more` : ''}` + : '' + ) $effect(() => { if (mcpCreationMode) { @@ -406,20 +420,25 @@ {noScriptsOrFlowsAvailableWarning} {:else} + {#if longPathWarning} + + {longPathWarning} + + {/if} Scripts & Flows that will be available via MCP
- {#if includedRunnables.length <= 5} - {#each includedRunnables as scriptOrFlow} + {#if validRunnables.length <= 5} + {#each validRunnables as scriptOrFlow} {scriptOrFlow} {/each} {:else} - {#each includedRunnables.slice(0, 3) as scriptOrFlow} + {#each validRunnables.slice(0, 3) as scriptOrFlow} {scriptOrFlow} {/each} - +{includedRunnables.length - 3} more + +{validRunnables.length - 3} more {/if}