diff --git a/backend/windmill-worker/src/ai_executor.rs b/backend/windmill-worker/src/ai_executor.rs index 63c2b37339..456a34fcb0 100644 --- a/backend/windmill-worker/src/ai_executor.rs +++ b/backend/windmill-worker/src/ai_executor.rs @@ -279,8 +279,13 @@ fn narrow_roster( // An MCP entry is added with no summary and the roster displays it by its // resource path, so that path is the only name the form can offer for it. Match // it too, or naming one tool would be the sole way to keep a whole server. + // Both sides are stripped: the roster stores the path as authored, so the name + // the picker offers carries the `$res:` prefix while `McpToolSource` does not. let path = mcp.resource_path.trim_start_matches("$res:"); - let named = named || enabled.iter().any(|n| n == path); + let named = named + || enabled + .iter() + .any(|n| n.trim_start_matches("$res:") == path); if named { enabled_mcp_paths.insert(path.to_string()); } @@ -324,7 +329,12 @@ fn unmatched_enabled_tools_message( let unmatched: Vec<&str> = enabled_tools .iter() .map(|name| name.as_str()) - .filter(|name| !advertised.contains(name)) + // An MCP server is named by the path the roster shows, which carries the `$res:` the + // matched paths are stripped of, so the two are compared without it. + .filter(|name| { + let bare = name.trim_start_matches("$res:"); + !advertised.iter().any(|a| a == name || *a == bare) + }) .collect(); if unmatched.is_empty() { return None; @@ -811,6 +821,8 @@ pub async fn handle_ai_agent_job( .filter(|t| t.mcp_source.is_some()) .map(|t| t.def.function.name.as_str()), ); + // A server named by its path matched something real, so the log must not call it unmatched. + matchable.extend(enabled_mcp_paths.iter().map(|p| p.as_str())); windmill_common::feature_usage::log_feature_usage( "ai_agent", "dynamic_tools", @@ -2070,6 +2082,15 @@ mod tests { let (dropped, paths) = narrow_roster(unnamed(), Some(&["u/test/other".to_string()])); assert!(dropped.is_empty()); assert!(paths.is_empty()); + + // The roster stores the path as authored, so the name the picker offers is the `$res:` form + // while the loader's own key is not — a run that selects a server from the form sends this. + let selected = ["$res:u/test/gh".to_string()]; + let (kept, paths) = narrow_roster(unnamed(), Some(&selected)); + assert_eq!(kept.len(), 1); + assert_eq!(paths.into_iter().collect::>(), ["u/test/gh"]); + // And having matched, it is not reported as having named nothing. + assert!(unmatched_enabled_tools_message(&selected, &["u/test/gh"]).is_none()); } /// The two sides of the server-entry match are different types, and getting it wrong advertises diff --git a/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte b/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte index 7625214c77..da78d2d50f 100644 --- a/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte +++ b/frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte @@ -171,8 +171,12 @@ // By the name the roster shows, not the summary alone: an MCP entry is added without one and // displays as its resource path, so keying on `summary` would leave a whole server with no // name to pick. `narrow_roster` matches that path for the same reason. + // + // Stripped of `$res:`, which the roster keeps: a static input transform holding one is + // resolved to the resource's own value before the step runs, so the prefixed form would + // reach the worker as an object where a name is expected, and the step would fail outright. const names = tools - .map((tool) => toolDisplayName(tool)) + .map((tool) => toolDisplayName(tool)?.replace(/^\$res:/, '')) .filter((name): name is string => !!name) const properties = schemaProperties untrack(() => {