starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-21 00:02:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: name an mcp server by its bare path on the one side that can hold it

Browse Source 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
hugocasa
2026-09-14 09:45:36 +02:00
co-authored by Claude Opus 5
parent 2caa145ac4
commit 979af21ddd
3 changed files with 31 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-worker/src/ai_executor.rs
+16 -36
View File
@@ -276,16 +276,12 @@ fn narrow_roster(
.is_some_and(|s| enabled.iter().any(|n| n == s));
match &t.value {
ToolValue::Mcp(mcp) => {
// 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.
// An MCP entry is added with no summary and shows as its resource path, so that
// path is the only name the form can offer for the server as a whole. It is
// matched bare: a name carrying the `$res:` the roster stores would be resolved
// to the resource itself before the worker is handed its args.
let path = mcp.resource_path.trim_start_matches("$res:");
let named = named
|| enabled
.iter()
.any(|n| n.trim_start_matches("$res:") == path);
let named = named || enabled.iter().any(|n| n == path);
if named {
enabled_mcp_paths.insert(path.to_string());
}
@@ -329,12 +325,7 @@ fn unmatched_enabled_tools_message(
let unmatched: Vec<&str> = enabled_tools
.iter()
.map(|name| name.as_str())
// 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)
})
.filter(|name| !advertised.contains(name))
.collect();
if unmatched.is_empty() {
return None;
@@ -2067,30 +2058,19 @@ mod tests {
assert_eq!(names(&kept), ["github"]);
assert!(paths.is_empty());
// An MCP entry is added with no summary, and the form offers it by the path the roster
// displays it as. Without this the only way to keep such a server would be naming one of
// the tools it has not been asked for yet.
let unnamed = || {
vec![AgentTool {
summary: None,
..mcp("c", "github", "$res:u/test/gh")
}]
};
let (kept, paths) = narrow_roster(unnamed(), Some(&["u/test/gh".to_string()]));
assert_eq!(kept.len(), 1);
assert_eq!(paths.into_iter().collect::<Vec<_>>(), ["u/test/gh"]);
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));
// An MCP entry is added with no summary, and the form names it by its resource path, bare.
// Without this the only way to keep such a server would be naming one of the tools it has
// not been asked for yet.
let unnamed = || vec![AgentTool { summary: None, ..mcp("c", "github", "$res:u/test/gh") }];
let named_server = ["u/test/gh".to_string()];
let (kept, paths) = narrow_roster(unnamed(), Some(&named_server));
assert_eq!(kept.len(), 1);
assert_eq!(paths.into_iter().collect::<Vec<_>>(), ["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());
assert!(unmatched_enabled_tools_message(&named_server, &["u/test/gh"]).is_none());
let (dropped, paths) = narrow_roster(unnamed(), Some(&["u/test/other".to_string()]));
assert!(dropped.is_empty());
assert!(paths.is_empty());
}
/// The two sides of the server-entry match are different types, and getting it wrong advertises
frontend/src/lib/components/flows/agentToolUtils.ts
+10
View File
@@ -99,6 +99,16 @@ export function toolDisplayName(tool: AgentTool): string | undefined {
return tool?.summary || value?.path || value?.resource_path || undefined
}
/** The name `enabled_tools` holds a tool by, which is what the roster shows it as except for an MCP
* server displayed by the resource path it was authored with: a name carrying that `$res:` is
* resolved to the resource's own value before the step runs, so it would reach the worker as an
* object where a name is expected and fail the step outright. */
export function toolEnabledName(tool: AgentTool): string | undefined {
const name = toolDisplayName(tool)
const value = tool?.value as Record<string, any>
return name && name === value?.resource_path ? name.replace(/^\$res:/, '') : name
}
/**
* Create an AI Agent tool (nested agent)
*/
frontend/src/lib/components/flows/content/AiAgentStepInputs.svelte
+5 -9
View File
@@ -46,7 +46,7 @@
import { Plus, X } from 'lucide-svelte'
import type { PickableProperties } from '../previousResults'
import type { FlowCopilotContext } from '$lib/components/copilot/flow'
import { toolDisplayName, type AgentTool } from '../agentToolUtils'
import { toolEnabledName, type AgentTool } from '../agentToolUtils'
import {
AGENT_FIELDS,
AGENT_FIELD_GROUPS,
@@ -168,15 +168,11 @@
// field's shape from; `flowInfers` hands every step its own copy, so this stays this step's.
// A linked step gets the resource's roster here, which is the one it narrows.
$effect(() => {
// 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.
// By what each tool is named, not the summary alone: an MCP entry is added without one and is
// named by 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.
const names = tools
.map((tool) => toolDisplayName(tool)?.replace(/^\$res:/, ''))
.map((tool) => toolEnabledName(tool))
.filter((name): name is string => !!name)
const properties = schemaProperties
untrack(() => {