fix(mcp): 限制 server 名称字符并兜底 tool_name 清洗
OpenAI 要求 function name 匹配 ^[a-zA-Z0-9_-]+$,否则整个请求 400。MCP 工具名由 mcp_{server_key}_{tool_name} 拼成,两个输入源:server_key 用户可控(前端 addMcp/renameMcp 正则校验 + toast),tool_name 由 MCP server 上报(后端 sanitize_tool_name 替换非法字符为 _)。
前端:MCP 卡片头部改用 MapEntryHeader 支持点击重命名,状态指示灯移到上方独立行。顺手修了 MapEntryHeader 进入编辑时 val 未同步当前 name 的 bug。
后端:保留 server_key 和 tool_name 原值用于路由,仅清洗 LLM 可见的 full_name,并在发生清洗时打 warn 日志便于定位。
This commit is contained in:
parent
14d903e067
commit
6901659849
@ -8,6 +8,22 @@ use rmcp::model::Tool;
|
||||
use crate::mcp::client::McpClientManager;
|
||||
use crate::tools::traits::{Tool as PicoBotTool, ToolResult};
|
||||
|
||||
/// Sanitize a tool name to comply with OpenAI's function name pattern `^[a-zA-Z0-9_-]+$`.
|
||||
/// Any character outside [a-zA-Z0-9_-] (e.g. '.', ':', '/') is replaced with '_'.
|
||||
/// This is applied to the LLM-facing name only; `McpToolWrapper` retains the original
|
||||
/// `server_key` and `tool_name` for routing tool calls to the correct MCP server.
|
||||
fn sanitize_tool_name(name: &str) -> String {
|
||||
name.chars()
|
||||
.map(|c| {
|
||||
if c.is_ascii_alphanumeric() || c == '_' || c == '-' {
|
||||
c
|
||||
} else {
|
||||
'_'
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Wrapper that adapts an MCP tool to PicoBot's Tool trait
|
||||
#[derive(Clone)]
|
||||
pub struct McpToolWrapper {
|
||||
@ -27,7 +43,18 @@ impl McpToolWrapper {
|
||||
/// Create a new tool wrapper
|
||||
pub fn new(manager: Arc<McpClientManager>, server_key: String, tool_info: Tool) -> Self {
|
||||
let tool_name = tool_info.name.clone().into_owned();
|
||||
let full_name = format!("mcp_{}_{}", server_key, tool_name);
|
||||
let raw_name = format!("mcp_{}_{}", server_key, tool_name);
|
||||
let full_name = sanitize_tool_name(&raw_name);
|
||||
if full_name != raw_name {
|
||||
tracing::warn!(
|
||||
original = %raw_name,
|
||||
sanitized = %full_name,
|
||||
server_key = %server_key,
|
||||
tool_name = %tool_name,
|
||||
"MCP tool name contained characters invalid for OpenAI function name pattern \
|
||||
(^[a-zA-Z0-9_-]+$); sanitized to comply"
|
||||
);
|
||||
}
|
||||
Self {
|
||||
manager,
|
||||
server_key,
|
||||
@ -176,4 +203,45 @@ mod tests {
|
||||
assert_eq!(wrapper.original_name(), "echo");
|
||||
assert_eq!(wrapper.server_key(), "filesystem");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_wrapper_name_sanitizes_invalid_chars() {
|
||||
// OpenAI requires function names to match ^[a-zA-Z0-9_-]+$.
|
||||
// server_key and tool_name from MCP servers may contain '.', ':', '/', etc.
|
||||
let manager = Arc::new(McpClientManager::new());
|
||||
let schema: serde_json::Map<String, serde_json::Value> =
|
||||
serde_json::json!({"type": "object"})
|
||||
.as_object()
|
||||
.unwrap()
|
||||
.clone();
|
||||
let tool_info = Tool::new("tools.list:read", "Namespaced tool", schema);
|
||||
|
||||
let wrapper = McpToolWrapper::new(manager, "github.api".to_string(), tool_info);
|
||||
// mcp_github.api_tools.list:read → mcp_github_api_tools_list_read
|
||||
assert_eq!(wrapper.name(), "mcp_github_api_tools_list_read");
|
||||
// Original identifiers preserved for routing
|
||||
assert_eq!(wrapper.original_name(), "tools.list:read");
|
||||
assert_eq!(wrapper.server_key(), "github.api");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sanitize_tool_name_matches_openai_pattern() {
|
||||
let re = regex::Regex::new(r"^[a-zA-Z0-9_-]+$").unwrap();
|
||||
for input in [
|
||||
"mcp_filesystem_echo",
|
||||
"mcp_github.api_tools.list:read",
|
||||
"mcp_a/b@c d",
|
||||
"mcp_中文_tool",
|
||||
] {
|
||||
let sanitized = sanitize_tool_name(input);
|
||||
assert!(
|
||||
re.is_match(&sanitized),
|
||||
"sanitized name {:?} (from {:?}) does not match OpenAI pattern",
|
||||
sanitized,
|
||||
input
|
||||
);
|
||||
}
|
||||
// Empty stays empty
|
||||
assert_eq!(sanitize_tool_name(""), "");
|
||||
}
|
||||
}
|
||||
|
||||
@ -2062,13 +2062,22 @@ export function ConfigPage({ onClose, onSaveConnection, initialTab }: ConfigPage
|
||||
const entries = Object.entries(config.mcpServers);
|
||||
const statusFor = (key: string) => mcpStatus?.servers?.find((s) => s.key === key);
|
||||
const addMcp = () => {
|
||||
const name = prompt('MCP 服务器名称:')?.trim();
|
||||
if (name && !config.mcpServers[name]) {
|
||||
const name = prompt('MCP 服务器名称(仅字母、数字、下划线、连字符):')?.trim();
|
||||
if (!name) return;
|
||||
if (!/^[a-zA-Z0-9_-]+$/.test(name)) {
|
||||
setToast('名称只能包含字母、数字、下划线和连字符');
|
||||
setTimeout(() => setToast(''), 3000);
|
||||
return;
|
||||
}
|
||||
if (config.mcpServers[name]) {
|
||||
setToast('该名称已存在');
|
||||
setTimeout(() => setToast(''), 3000);
|
||||
return;
|
||||
}
|
||||
update('mcpServers', {
|
||||
...config.mcpServers,
|
||||
[name]: { type: 'stdio', is_active: true, command: '', args: [] },
|
||||
});
|
||||
}
|
||||
};
|
||||
const delMcp = (name: string) => {
|
||||
if (confirm(`删除 MCP 服务器 "${name}"?`)) {
|
||||
@ -2076,6 +2085,26 @@ export function ConfigPage({ onClose, onSaveConnection, initialTab }: ConfigPage
|
||||
update('mcpServers', rest);
|
||||
}
|
||||
};
|
||||
const renameMcp = (oldName: string, newName: string) => {
|
||||
const trimmed = newName.trim();
|
||||
if (trimmed === oldName || !trimmed) return;
|
||||
if (!/^[a-zA-Z0-9_-]+$/.test(trimmed)) {
|
||||
setToast('名称只能包含字母、数字、下划线和连字符');
|
||||
setTimeout(() => setToast(''), 3000);
|
||||
return;
|
||||
}
|
||||
if (config.mcpServers[trimmed]) {
|
||||
setToast('该名称已存在');
|
||||
setTimeout(() => setToast(''), 3000);
|
||||
return;
|
||||
}
|
||||
const entries = Object.entries(config.mcpServers);
|
||||
const newMap: Record<string, McpServerConfig> = {};
|
||||
for (const [k, v] of entries) {
|
||||
newMap[k === oldName ? trimmed : k] = v;
|
||||
}
|
||||
update('mcpServers', newMap);
|
||||
};
|
||||
const updMcp = (name: string, patch: Partial<McpServerConfig>) =>
|
||||
update('mcpServers', {
|
||||
...config.mcpServers,
|
||||
@ -2114,9 +2143,9 @@ export function ConfigPage({ onClose, onSaveConnection, initialTab }: ConfigPage
|
||||
key={name}
|
||||
className="rounded-xl border border-[var(--border-color)] bg-[var(--bg-secondary)]/60 overflow-hidden"
|
||||
>
|
||||
<div className="flex items-center gap-2 p-3 border-b border-[var(--border-color)]">
|
||||
{st ? (
|
||||
st.connected ? (
|
||||
{st && (
|
||||
<div className="flex items-center gap-2 px-4 py-2">
|
||||
{st.connected ? (
|
||||
<span className="inline-flex items-center gap-1 text-xs text-green-400">
|
||||
<span className="w-2 h-2 rounded-full bg-green-400" /> {st.tool_count} 工具
|
||||
</span>
|
||||
@ -2131,18 +2160,14 @@ export function ConfigPage({ onClose, onSaveConnection, initialTab }: ConfigPage
|
||||
<span className="inline-flex items-center gap-1 text-xs text-gray-400">
|
||||
<span className="w-2 h-2 rounded-full bg-gray-400" /> 未连接
|
||||
</span>
|
||||
)
|
||||
) : null}
|
||||
<span className="flex-1 text-sm font-medium text-[var(--text-primary)]">
|
||||
{name}
|
||||
</span>
|
||||
<button
|
||||
onClick={() => delMcp(name)}
|
||||
className="p-1 rounded text-[var(--text-muted)] hover:text-red-400 transition-colors"
|
||||
>
|
||||
<Trash2 className="h-3.5 w-3.5" />
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
<MapEntryHeader
|
||||
name={name}
|
||||
onDelete={() => delMcp(name)}
|
||||
onRename={(n) => renameMcp(name, n)}
|
||||
/>
|
||||
<div className="p-4 space-y-3">
|
||||
<Field label="传输类型">
|
||||
<select
|
||||
|
||||
@ -388,7 +388,12 @@ export function MapEntryHeader({
|
||||
) : (
|
||||
<span
|
||||
className="text-sm font-mono text-[var(--accent-cyan)] cursor-pointer"
|
||||
onClick={() => onRename && setEditing(true)}
|
||||
onClick={() => {
|
||||
if (onRename) {
|
||||
setVal(name);
|
||||
setEditing(true);
|
||||
}
|
||||
}}
|
||||
>
|
||||
{name}
|
||||
</span>
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user