mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix: name a websearch tool that carries no summary of its own
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c900952eff
commit
f0c949938d
@@ -243,15 +243,29 @@ fn overlay_tool_inputs(
|
||||
}
|
||||
}
|
||||
|
||||
/// The name a run enables a roster entry by: the name the model is shown, except for an MCP server,
|
||||
/// which the model is shown nothing of and which is named by the resource it points at.
|
||||
/// What a websearch entry is named by when it carries no summary of its own. A summary is a name
|
||||
/// only for an entry the model is shown; web search reaches the model as a provider capability
|
||||
/// rather than a tool, so its summary is a label the editor happens to write and JSON authored
|
||||
/// anywhere else may leave out, and an entry with no name at all could not be enabled.
|
||||
const WEBSEARCH_ENABLED_NAME: &str = "web_search";
|
||||
|
||||
/// The name a run enables a roster entry by: the name the model is shown, except for an entry the
|
||||
/// model is shown nothing of, which is named by whatever identifies it instead. An MCP server is
|
||||
/// named by the resource it points at, web search by its label or `WEBSEARCH_ENABLED_NAME`.
|
||||
///
|
||||
/// The path is bare. The roster stores it as authored, `$res:` and all, but a name is an argument
|
||||
/// value and one carrying that prefix is resolved to the resource itself before the worker is
|
||||
/// handed its args, so the prefixed form is not something the list can hold.
|
||||
/// The MCP path is bare. The roster stores it as authored, `$res:` and all, but a name is an
|
||||
/// argument value and one carrying that prefix is resolved to the resource itself before the worker
|
||||
/// is handed its args, so the prefixed form is not something the list can hold.
|
||||
fn tool_enabled_name(tool: &AgentTool) -> Option<&str> {
|
||||
match &tool.value {
|
||||
ToolValue::Mcp(mcp) => Some(mcp.resource_path.trim_start_matches("$res:")),
|
||||
ToolValue::Websearch(_) => Some(
|
||||
tool.summary
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
.unwrap_or(WEBSEARCH_ENABLED_NAME),
|
||||
),
|
||||
_ => tool.summary.as_deref(),
|
||||
}
|
||||
}
|
||||
@@ -1962,6 +1976,14 @@ mod tests {
|
||||
}),
|
||||
}
|
||||
}
|
||||
fn websearch(id: &str, summary: Option<&str>) -> AgentTool {
|
||||
AgentTool {
|
||||
id: id.to_string(),
|
||||
summary: summary.map(str::to_string),
|
||||
description: None,
|
||||
value: ToolValue::Websearch(windmill_common::flows::WebsearchToolValue {}),
|
||||
}
|
||||
}
|
||||
let roster = || {
|
||||
vec![
|
||||
named("a", "get_user"),
|
||||
@@ -1972,6 +1994,9 @@ mod tests {
|
||||
let names = |tools: &[AgentTool]| -> Vec<String> {
|
||||
tools.iter().filter_map(|t| t.summary.clone()).collect()
|
||||
};
|
||||
let ids = |tools: &[AgentTool]| -> Vec<String> {
|
||||
tools.iter().map(|t| t.id.clone()).collect()
|
||||
};
|
||||
|
||||
// No list at all: the whole roster, as every agent written before the field expects.
|
||||
assert_eq!(
|
||||
@@ -2011,6 +2036,26 @@ mod tests {
|
||||
["get_user"]
|
||||
);
|
||||
assert!(narrow_roster(roster(), Some(&["u/test/other".to_string()])).is_empty());
|
||||
|
||||
// Web search reaches the model as a provider capability rather than a tool, so its summary
|
||||
// is a label the editor writes and JSON authored anywhere else may leave out. Without the
|
||||
// fallback such an entry has no name, and a run that narrows could not keep web search.
|
||||
let mut with_websearch = roster();
|
||||
with_websearch.push(websearch("w", None));
|
||||
assert_eq!(
|
||||
ids(&narrow_roster(
|
||||
with_websearch,
|
||||
Some(&[WEBSEARCH_ENABLED_NAME.to_string()])
|
||||
)),
|
||||
["w"]
|
||||
);
|
||||
// A label of its own still names it, which is what the editor writes.
|
||||
let mut labelled = roster();
|
||||
labelled.push(websearch("w", Some("Web Search")));
|
||||
assert_eq!(
|
||||
ids(&narrow_roster(labelled, Some(&["Web Search".to_string()]))),
|
||||
["w"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// `agentToolUtils` reaches the copilot bundle, and Monaco's CSS with it, through this one import.
|
||||
// Only `createAiAgentTool` reads it, and nothing below does.
|
||||
vi.mock('../aiProviderStorage', () => ({ loadStoredConfig: () => undefined }))
|
||||
|
||||
import { toolEnabledName, WEBSEARCH_ENABLED_NAME } from './agentToolUtils'
|
||||
|
||||
/**
|
||||
* The names this returns are the ones `enabled_tools` holds and `tool_enabled_name` in
|
||||
* `ai_executor.rs` matches against, so the two have to agree: a name only one side produces
|
||||
* silently drops the tool from every run that narrows.
|
||||
*/
|
||||
describe('toolEnabledName', () => {
|
||||
it('names a flow module tool by the name the model is shown', () => {
|
||||
expect(
|
||||
toolEnabledName({ id: 'a', summary: 'get_user', value: { tool_type: 'flowmodule' } } as any)
|
||||
).toBe('get_user')
|
||||
})
|
||||
|
||||
it('names an MCP server by its bare path, never the summary two servers may share', () => {
|
||||
// `$res:` and all is how the roster stores it, but a name carrying that prefix is resolved to
|
||||
// the resource itself before the worker sees it, so the list can only hold the bare path.
|
||||
expect(
|
||||
toolEnabledName({
|
||||
id: 'm',
|
||||
summary: 'github',
|
||||
value: { tool_type: 'mcp', resource_path: '$res:u/admin/gh' }
|
||||
} as any)
|
||||
).toBe('u/admin/gh')
|
||||
})
|
||||
|
||||
it('falls back to a constant for web search authored without a label', () => {
|
||||
// The editor always writes one and offers no way to clear it; JSON authored anywhere else may
|
||||
// carry none, and an entry with no name could not be enabled at all.
|
||||
expect(toolEnabledName({ id: 'w', value: { tool_type: 'websearch' } } as any)).toBe(
|
||||
WEBSEARCH_ENABLED_NAME
|
||||
)
|
||||
expect(
|
||||
toolEnabledName({ id: 'w', summary: 'Web Search', value: { tool_type: 'websearch' } } as any)
|
||||
).toBe('Web Search')
|
||||
})
|
||||
})
|
||||
@@ -99,11 +99,17 @@ export function toolDisplayName(tool: AgentTool): string | undefined {
|
||||
return tool?.summary || value?.path || value?.resource_path || undefined
|
||||
}
|
||||
|
||||
/** The name `enabled_tools` holds a tool by: the name the model is shown, except for an MCP server,
|
||||
* which the model is shown nothing of and which is named by the resource it points at. Its summary
|
||||
* is a label two entries may share, so naming one would enable both.
|
||||
/** What web search is named by with no summary of its own, mirroring `WEBSEARCH_ENABLED_NAME` in
|
||||
* `ai_executor.rs`. The editor writes a label and offers no way to clear it, but JSON authored
|
||||
* anywhere else may carry none, and an entry with no name could not be enabled at all. */
|
||||
export const WEBSEARCH_ENABLED_NAME = 'web_search'
|
||||
|
||||
/** The name `enabled_tools` holds a tool by: the name the model is shown, except for an entry the
|
||||
* model is shown nothing of, which is named by whatever identifies it instead. An MCP server is
|
||||
* named by the resource it points at, since its summary is a label two entries may share and
|
||||
* naming one would enable both; web search by its label, else `WEBSEARCH_ENABLED_NAME`.
|
||||
*
|
||||
* The path is offered bare. It is stored with the `$res:` it was authored with, and an
|
||||
* The MCP path is offered bare. It is stored with the `$res:` it was authored with, and an
|
||||
* `enabled_tools` entry carrying that prefix is resolved to the resource's own value before the
|
||||
* step runs, reaching the worker as an object where a name is expected. Mirrors
|
||||
* `tool_enabled_name` in `ai_executor.rs`. */
|
||||
@@ -112,6 +118,9 @@ export function toolEnabledName(tool: AgentTool): string | undefined {
|
||||
if (value?.tool_type === 'mcp') {
|
||||
return (value?.resource_path as string | undefined)?.replace(/^\$res:/, '') || undefined
|
||||
}
|
||||
if (value?.tool_type === 'websearch') {
|
||||
return tool?.summary?.trim() || WEBSEARCH_ENABLED_NAME
|
||||
}
|
||||
return toolDisplayName(tool)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user