From 40958cd86105cd30698641fbdf7aabb7a78c459b Mon Sep 17 00:00:00 2001 From: centdix Date: Sun, 28 Sep 2025 20:02:08 +0000 Subject: [PATCH] use id for names --- backend/windmill-api/src/mcp/server.rs | 59 ++++-- .../windmill-api/src/mcp/tools/flow_tools.rs | 13 +- .../windmill-api/src/mcp/tools/hub_tools.rs | 15 +- .../src/mcp/tools/script_tools.rs | 13 +- .../windmill-api/src/mcp/utils/database.rs | 195 +++++++++++++++++- backend/windmill-api/src/mcp/utils/models.rs | 20 +- 6 files changed, 273 insertions(+), 42 deletions(-) diff --git a/backend/windmill-api/src/mcp/server.rs b/backend/windmill-api/src/mcp/server.rs index aad0817b11..bd54ee734e 100644 --- a/backend/windmill-api/src/mcp/server.rs +++ b/backend/windmill-api/src/mcp/server.rs @@ -32,8 +32,8 @@ use super::tools::endpoint_tools::{ }; use super::utils::{ database::{ - check_scopes, get_hub_script_schema, get_item_schema, get_items, get_resources_types, - get_scripts_from_hub, + check_scopes, get_flow_path_and_schema_by_id, get_hub_script_schema, get_item_schema, + get_items, get_resources_types, get_script_path_and_schema_by_hash, get_scripts_from_hub, }, models::{ FlowInfo, ResourceInfo, ResourceType, SchemaType, ScriptInfo, ToolableItem, WorkspaceId, @@ -72,12 +72,20 @@ impl Runner { resources_types: &Vec, ) -> Result { let is_hub = item.is_hub(); - let path = item.get_path_or_id(); + let tool_id = item.get_id(); let item_type = item.item_type(); + + // Use path for title if summary is empty, otherwise use summary + let title = if item.get_summary().is_empty() || item.get_summary() == "No summary" { + item.get_path() + } else { + item.get_summary().to_string() + }; + let description = format!( "This is a {} named `{}` with the following description: `{}`.{}", item_type, - item.get_summary(), + title, item.get_description(), if is_hub { format!( @@ -101,13 +109,13 @@ impl Runner { let input_schema_map = match serde_json::to_value(schema_obj) { Ok(Value::Object(map)) => map, Ok(_) => { - tracing::warn!("Schema object for tool '{}' did not serialize to a JSON object, using empty schema.", path); + tracing::warn!("Schema object for tool '{}' did not serialize to a JSON object, using empty schema.", tool_id); serde_json::Map::new() } Err(e) => { tracing::error!( "Failed to serialize schema object for tool '{}': {}. Using empty schema.", - path, + tool_id, e ); serde_json::Map::new() @@ -115,14 +123,14 @@ impl Runner { }; Ok(Tool { - name: Cow::Owned(path), + name: Cow::Owned(tool_id), description: Some(Cow::Owned(description)), input_schema: Arc::new(input_schema_map), - title: Some(item.get_summary().to_string()), + title: Some(title.clone()), output_schema: None, icons: None, annotations: Some(ToolAnnotations { - title: Some(item.get_summary().to_string()), + title: Some(title), read_only_hint: Some(false), // Can modify environment destructive_hint: Some(true), // Can potentially be destructive idempotent_hint: Some(false), // Are not guaranteed to be idempotent @@ -195,14 +203,27 @@ impl ServerHandler for Runner { } // Continue with script/flow logic - let (tool_type, path, is_hub) = reverse_transform(&request.name).map_err(|e| { - ErrorData::internal_error(format!("Failed to reverse transform path: {}", e), None) - })?; - - let item_schema = if is_hub { - get_hub_script_schema(&format!("hub/{}", path), db).await? + let (tool_type, path, item_schema, is_hub) = if request.name.starts_with("script:") { + let id = &request.name[7..]; // Remove "script:" prefix + let item_data = + get_script_path_and_schema_by_hash(id, user_db, authed, &workspace_id).await?; + ("script", item_data.path, item_data.schema, false) + } else if request.name.starts_with("flow:") { + let version_id = &request.name[5..]; // Remove "flow:" prefix + let item_data = + get_flow_path_and_schema_by_id(version_id, user_db, authed, &workspace_id).await?; + ("flow", item_data.path, item_data.schema, false) } else { - get_item_schema(&path, user_db, authed, &workspace_id, &tool_type).await? + // Fall back to old transform method for hub scripts and other tools + let (tool_type, path, is_hub) = reverse_transform(&request.name).map_err(|e| { + ErrorData::internal_error(format!("Failed to reverse transform path: {}", e), None) + })?; + let item_schema = if is_hub { + get_hub_script_schema(&format!("hub/{}", path), db).await? + } else { + get_item_schema(&path, user_db, authed, &workspace_id, &tool_type).await? + }; + (tool_type, path, item_schema, is_hub) }; let schema_obj = if let Some(ref s) = item_schema { @@ -384,7 +405,7 @@ impl ServerHandler for Runner { let mut tools: Vec = Vec::new(); for script in scripts { - if script.get_path_or_id().len() <= MAX_PATH_LENGTH { + if script.get_id().len() <= MAX_PATH_LENGTH { tools.push( Runner::create_tool_from_item( &script, @@ -400,7 +421,7 @@ impl ServerHandler for Runner { } for flow in flows { - if flow.get_path_or_id().len() <= MAX_PATH_LENGTH { + if flow.get_id().len() <= MAX_PATH_LENGTH { tools.push( Runner::create_tool_from_item( &flow, @@ -416,7 +437,7 @@ impl ServerHandler for Runner { } for hub_script in hub_scripts { - if hub_script.get_path_or_id().len() <= MAX_PATH_LENGTH { + if hub_script.get_id().len() <= MAX_PATH_LENGTH { tools.push( Runner::create_tool_from_item( &hub_script, diff --git a/backend/windmill-api/src/mcp/tools/flow_tools.rs b/backend/windmill-api/src/mcp/tools/flow_tools.rs index 03b47472bf..60e83770be 100644 --- a/backend/windmill-api/src/mcp/tools/flow_tools.rs +++ b/backend/windmill-api/src/mcp/tools/flow_tools.rs @@ -3,15 +3,18 @@ //! Contains functionality for converting Windmill flows into MCP tools. use super::super::utils::{ - models::{FlowInfo, ToolableItem, SchemaType}, + models::{FlowInfo, SchemaType, ToolableItem}, schema::convert_schema_to_schema_type, - transform::transform_path, }; /// Implementation of ToolableItem for FlowInfo impl ToolableItem for FlowInfo { - fn get_path_or_id(&self) -> String { - transform_path(&self.path, "flow") + fn get_path(&self) -> String { + self.path.clone() + } + + fn get_id(&self) -> String { + format!("flow:{}", self.id) } fn get_summary(&self) -> &str { @@ -37,4 +40,4 @@ impl ToolableItem for FlowInfo { fn get_integration_type(&self) -> Option { None } -} \ No newline at end of file +} diff --git a/backend/windmill-api/src/mcp/tools/hub_tools.rs b/backend/windmill-api/src/mcp/tools/hub_tools.rs index b81b973b30..d2778af6f1 100644 --- a/backend/windmill-api/src/mcp/tools/hub_tools.rs +++ b/backend/windmill-api/src/mcp/tools/hub_tools.rs @@ -2,13 +2,18 @@ //! //! Contains functionality for integrating Windmill Hub scripts as MCP tools. -use super::super::utils::{ - models::{HubScriptInfo, ToolableItem, SchemaType}, -}; +use super::super::utils::models::{HubScriptInfo, SchemaType, ToolableItem}; /// Implementation of ToolableItem for HubScriptInfo impl ToolableItem for HubScriptInfo { - fn get_path_or_id(&self) -> String { + fn get_path(&self) -> String { + // Hub scripts don't have a traditional path, use the ID format + let id = self.version_id; + let summary = self.summary.as_deref().unwrap_or("No summary"); + format!("hub/{}-{}", id, summary.replace(" ", "_")) + } + + fn get_id(&self) -> String { let id = self.version_id; let summary = self.summary.as_deref().unwrap_or("No summary"); format!("hs-{}-{}", id, summary.replace(" ", "_")) @@ -40,4 +45,4 @@ impl ToolableItem for HubScriptInfo { fn get_integration_type(&self) -> Option { self.app.clone() } -} \ No newline at end of file +} diff --git a/backend/windmill-api/src/mcp/tools/script_tools.rs b/backend/windmill-api/src/mcp/tools/script_tools.rs index 716fae3a87..3fc0d5daa4 100644 --- a/backend/windmill-api/src/mcp/tools/script_tools.rs +++ b/backend/windmill-api/src/mcp/tools/script_tools.rs @@ -3,15 +3,18 @@ //! Contains functionality for converting Windmill scripts into MCP tools. use super::super::utils::{ - models::{ScriptInfo, ToolableItem, SchemaType}, + models::{SchemaType, ScriptInfo, ToolableItem}, schema::convert_schema_to_schema_type, - transform::transform_path, }; /// Implementation of ToolableItem for ScriptInfo impl ToolableItem for ScriptInfo { - fn get_path_or_id(&self) -> String { - transform_path(&self.path, "script") + fn get_path(&self) -> String { + self.path.clone() + } + + fn get_id(&self) -> String { + format!("script:{}", self.hash) } fn get_summary(&self) -> &str { @@ -37,4 +40,4 @@ impl ToolableItem for ScriptInfo { fn get_integration_type(&self) -> Option { None } -} \ No newline at end of file +} diff --git a/backend/windmill-api/src/mcp/utils/database.rs b/backend/windmill-api/src/mcp/utils/database.rs index 39aac1ae6e..ed9901c785 100644 --- a/backend/windmill-api/src/mcp/utils/database.rs +++ b/backend/windmill-api/src/mcp/utils/database.rs @@ -144,7 +144,14 @@ pub async fn get_items sqlx::FromRow<'a, sqlx::postgres::PgRow> + Sen scope_path: Option<&str>, ) -> Result, ErrorData> { let mut sqlb = SqlBuilder::select_from(&format!("{} as o", item_type)); - let fields = vec!["o.path", "o.summary", "o.description", "o.schema"]; + let fields = if item_type == "script" { + vec!["o.path", "o.hash", "o.summary", "o.description", "o.schema"] + } else { + // For flows, we need to join with flow_version to get the latest version ID + sqlb.join(&format!("{}_version as fv", item_type)) + .on(&format!("fv.workspace_id = o.workspace_id AND fv.path = o.path AND fv.id = (SELECT MAX(id) FROM {}_version WHERE workspace_id = o.workspace_id AND path = o.path)", item_type)); + vec!["o.path", "fv.id", "o.summary", "o.description", "o.schema"] + }; sqlb.fields(&fields); if scope_type == "favorites" { sqlb.join("favorite") @@ -260,3 +267,189 @@ pub async fn get_hub_script_schema(path: &str, db: &DB) -> Result } } } + +/// Get script path by hash +pub async fn get_script_path_by_hash( + hash: &str, + user_db: &UserDB, + authed: &ApiAuthed, + workspace_id: &str, +) -> Result { + let hash_i64 = hash + .parse::() + .map_err(|_| ErrorData::internal_error("Invalid hash format", None))?; + + let mut sqlb = SqlBuilder::select_from("script as o"); + sqlb.fields(&["o.path"]); + sqlb.and_where("o.hash = ?".bind(&hash_i64)); + sqlb.and_where("o.workspace_id = ?".bind(&workspace_id)); + sqlb.and_where("o.archived = false"); + sqlb.and_where("o.draft_only IS NOT TRUE"); + sqlb.and_where("(o.no_main_func IS NOT TRUE OR o.no_main_func IS NULL)"); + + let sql = sqlb.sql().map_err(|_e| { + tracing::error!("failed to build sql: {}", _e); + ErrorData::internal_error("failed to build sql", None) + })?; + + let mut tx = user_db + .clone() + .begin(authed) + .await + .map_err(|_e| ErrorData::internal_error("failed to begin transaction", None))?; + + let row = sqlx::query_scalar::<_, String>(&sql) + .fetch_one(&mut *tx) + .await + .map_err(|_e| { + tracing::error!("failed to fetch script path by hash: {}", _e); + ErrorData::internal_error("failed to fetch script path by hash", None) + })?; + + tx.commit() + .await + .map_err(|_e| ErrorData::internal_error("failed to commit transaction", None))?; + + Ok(row) +} + +/// Get flow path by version id +pub async fn get_flow_path_by_id( + version_id: &str, + user_db: &UserDB, + authed: &ApiAuthed, + workspace_id: &str, +) -> Result { + let mut sqlb = SqlBuilder::select_from("flow_version as fv"); + sqlb.join("flow as o") + .on("o.workspace_id = fv.workspace_id AND o.path = fv.path"); + sqlb.fields(&["o.path"]); + sqlb.and_where( + "fv.id = ?".bind( + &version_id + .parse::() + .map_err(|_| ErrorData::internal_error("Invalid version ID format", None))?, + ), + ); + sqlb.and_where("fv.workspace_id = ?".bind(&workspace_id)); + sqlb.and_where("o.archived = false"); + sqlb.and_where("o.draft_only IS NOT TRUE"); + + let sql = sqlb.sql().map_err(|_e| { + tracing::error!("failed to build sql: {}", _e); + ErrorData::internal_error("failed to build sql", None) + })?; + + let mut tx = user_db + .clone() + .begin(authed) + .await + .map_err(|_e| ErrorData::internal_error("failed to begin transaction", None))?; + + let row = sqlx::query_scalar::<_, String>(&sql) + .fetch_one(&mut *tx) + .await + .map_err(|_e| { + tracing::error!("failed to fetch flow path by version id: {}", _e); + ErrorData::internal_error("failed to fetch flow path by version id", None) + })?; + + tx.commit() + .await + .map_err(|_e| ErrorData::internal_error("failed to commit transaction", None))?; + + Ok(row) +} + +/// Get script path and schema by hash +pub async fn get_script_path_and_schema_by_hash( + hash: &str, + user_db: &UserDB, + authed: &ApiAuthed, + workspace_id: &str, +) -> Result { + let hash_i64 = hash + .parse::() + .map_err(|_| ErrorData::internal_error("Invalid hash format", None))?; + + let mut sqlb = SqlBuilder::select_from("script as o"); + sqlb.fields(&["o.path", "o.schema"]); + sqlb.and_where("o.hash = ?".bind(&hash_i64)); + sqlb.and_where("o.workspace_id = ?".bind(&workspace_id)); + sqlb.and_where("o.archived = false"); + sqlb.and_where("o.draft_only IS NOT TRUE"); + sqlb.and_where("(o.no_main_func IS NOT TRUE OR o.no_main_func IS NULL)"); + + let sql = sqlb.sql().map_err(|_e| { + tracing::error!("failed to build sql: {}", _e); + ErrorData::internal_error("failed to build sql", None) + })?; + + let mut tx = user_db + .clone() + .begin(authed) + .await + .map_err(|_e| ErrorData::internal_error("failed to begin transaction", None))?; + + let row = sqlx::query_as::<_, ItemPathAndSchema>(&sql) + .fetch_one(&mut *tx) + .await + .map_err(|_e| { + tracing::error!("failed to fetch script path and schema by hash: {}", _e); + ErrorData::internal_error("failed to fetch script path and schema by hash", None) + })?; + + tx.commit() + .await + .map_err(|_e| ErrorData::internal_error("failed to commit transaction", None))?; + + Ok(row) +} + +/// Get flow path and schema by version id +pub async fn get_flow_path_and_schema_by_id( + version_id: &str, + user_db: &UserDB, + authed: &ApiAuthed, + workspace_id: &str, +) -> Result { + let mut sqlb = SqlBuilder::select_from("flow_version as fv"); + sqlb.join("flow as o") + .on("o.workspace_id = fv.workspace_id AND o.path = fv.path"); + sqlb.fields(&["o.path", "fv.schema"]); + sqlb.and_where( + "fv.id = ?".bind( + &version_id + .parse::() + .map_err(|_| ErrorData::internal_error("Invalid version ID format", None))?, + ), + ); + sqlb.and_where("fv.workspace_id = ?".bind(&workspace_id)); + sqlb.and_where("o.archived = false"); + sqlb.and_where("o.draft_only IS NOT TRUE"); + + let sql = sqlb.sql().map_err(|_e| { + tracing::error!("failed to build sql: {}", _e); + ErrorData::internal_error("failed to build sql", None) + })?; + + let mut tx = user_db + .clone() + .begin(authed) + .await + .map_err(|_e| ErrorData::internal_error("failed to begin transaction", None))?; + + let row = sqlx::query_as::<_, ItemPathAndSchema>(&sql) + .fetch_one(&mut *tx) + .await + .map_err(|_e| { + tracing::error!("failed to fetch flow path and schema by version id: {}", _e); + ErrorData::internal_error("failed to fetch flow path and schema by version id", None) + })?; + + tx.commit() + .await + .map_err(|_e| ErrorData::internal_error("failed to commit transaction", None))?; + + Ok(row) +} diff --git a/backend/windmill-api/src/mcp/utils/models.rs b/backend/windmill-api/src/mcp/utils/models.rs index 453b1731fd..d2fb2f87b2 100644 --- a/backend/windmill-api/src/mcp/utils/models.rs +++ b/backend/windmill-api/src/mcp/utils/models.rs @@ -39,11 +39,7 @@ pub struct SchemaType { impl Default for SchemaType { fn default() -> Self { - Self { - r#type: "object".to_string(), - properties: HashMap::new(), - required: vec![], - } + Self { r#type: "object".to_string(), properties: HashMap::new(), required: vec![] } } } @@ -51,6 +47,7 @@ impl Default for SchemaType { #[derive(Serialize, FromRow, Debug)] pub struct ScriptInfo { pub path: String, + pub hash: i64, // Script hash is stored as bigint in the database pub summary: Option, pub description: Option, pub schema: Option, @@ -60,6 +57,7 @@ pub struct ScriptInfo { #[derive(Serialize, FromRow, Debug)] pub struct FlowInfo { pub path: String, + pub id: i64, // This is the flow_version.id, not a string pub summary: Option, pub description: Option, pub schema: Option, @@ -86,13 +84,21 @@ pub struct ItemSchema { pub schema: Option, } +/// Path and schema holder for database queries +#[derive(Serialize, FromRow)] +pub struct ItemPathAndSchema { + pub path: String, + pub schema: Option, +} + /// Trait for objects that can be converted to MCP tools pub trait ToolableItem { - fn get_path_or_id(&self) -> String; + fn get_path(&self) -> String; + fn get_id(&self) -> String; fn get_summary(&self) -> &str; fn get_description(&self) -> &str; fn get_schema(&self) -> SchemaType; fn is_hub(&self) -> bool; fn item_type(&self) -> &'static str; fn get_integration_type(&self) -> Option; -} \ No newline at end of file +}