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 40958cd861.

* use trunc suffix

* cleaning

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::<DB>().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()),

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<SchemaType>) -> String {
     let schema_obj = match schema_obj {
@@ -98,4 +113,4 @@ pub fn reverse_transform_key(transformed_key: &str, schema_obj: &Option<SchemaTy
     }
 
     transformed_key.to_string()
-}
+}

frontend/src/lib/components/settings/CreateToken.svelte

@@ -39,6 +39,9 @@
 		newTokenLabel = $bindable(undefined)
 	}: Props = $props()
 
+	// MCP clients do not allow names longer than 60 characters, here we use 55 because final tool name server side will add ~5 characters
+	const MAX_PATH_LENGTH = 55
+
 	let newToken = $state<string | undefined>(undefined)
 	let newMcpToken = $state<string | undefined>(undefined)
 	let newTokenExpiration = $state<number | undefined>(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}
 							</Alert>
 						{:else}
+							{#if longPathWarning}
+								<Alert type="warning" title="Some paths are too long" size="xs">
+									{longPathWarning}
+								</Alert>
+							{/if}
 							<span class="block text-xs text-tertiary"
 								>Scripts & Flows that will be available via MCP</span
 							>
 							<div class="flex flex-wrap gap-1">
-								{#if includedRunnables.length <= 5}
-									{#each includedRunnables as scriptOrFlow}
+								{#if validRunnables.length <= 5}
+									{#each validRunnables as scriptOrFlow}
 										<Badge rounded small color="blue">{scriptOrFlow}</Badge>
 									{/each}
 								{:else}
-									{#each includedRunnables.slice(0, 3) as scriptOrFlow}
+									{#each validRunnables.slice(0, 3) as scriptOrFlow}
 										<Badge rounded small color="blue">{scriptOrFlow}</Badge>
 									{/each}
 									<Badge rounded small color="dark-gray">
-										+{includedRunnables.length - 3} more
+										+{validRunnables.length - 3} more
 									</Badge>
 								{/if}
 							</div>