From a22e16ff83b0214ff1acfaa7e00d8c2b5e89f11f Mon Sep 17 00:00:00 2001 From: wsp1911 Date: Mon, 11 May 2026 20:11:29 +0800 Subject: [PATCH 1/2] refactor(mcp): align tool naming with metadata-driven server resolution - align dynamic MCP tool wire names with Claude Code style normalization - add authoritative mcp_info metadata and registry mapping for MCP tools - expose MCP metadata through desktop tool APIs and shared frontend types - switch MCP UI routing and tool grouping from name parsing to registry-backed metadata - add focused MCP naming tests and fix affected frontend tool DTO usage --- src/apps/desktop/src/api/mcp_api.rs | 9 +- src/apps/desktop/src/api/tool_api.rs | 88 ++++++++++--------- .../core/src/agentic/tools/framework.rs | 6 ++ src/crates/core/src/agentic/tools/registry.rs | 44 ++++++++-- .../core/src/service/mcp/adapter/tool.rs | 17 +++- src/crates/core/src/service/mcp/mod.rs | 6 ++ src/crates/core/src/service/mcp/tool_info.rs | 8 ++ src/crates/core/src/service/mcp/tool_name.rs | 56 ++++++++++++ .../agents/components/CreateAgentPage.tsx | 2 +- .../app/scenes/agents/hooks/useAgentsList.ts | 2 + .../profile/views/TemplateConfigPage.tsx | 33 ++++--- .../flow_chat/tool-cards/MCPToolDisplay.tsx | 43 ++++++--- .../infrastructure/api/service-api/ToolAPI.ts | 9 +- .../src/infrastructure/mcp/toolInfoCache.ts | 19 ++++ src/web-ui/src/shared/types/agent-api.ts | 7 ++ 15 files changed, 262 insertions(+), 87 deletions(-) create mode 100644 src/crates/core/src/service/mcp/tool_info.rs create mode 100644 src/crates/core/src/service/mcp/tool_name.rs create mode 100644 src/web-ui/src/infrastructure/mcp/toolInfoCache.ts diff --git a/src/apps/desktop/src/api/mcp_api.rs b/src/apps/desktop/src/api/mcp_api.rs index b14df51fd..f97360a6f 100644 --- a/src/apps/desktop/src/api/mcp_api.rs +++ b/src/apps/desktop/src/api/mcp_api.rs @@ -503,7 +503,7 @@ pub struct McpUiResourcePermissions { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct FetchMCPAppResourceRequest { - /// MCP server ID (e.g. from tool name mcp__{server_id}__{tool_name}) + /// Authoritative MCP server ID for the tool/app. pub server_id: String, /// Full resource URI, e.g. "ui://my-server/widget" pub resource_uri: String, @@ -541,13 +541,14 @@ pub async fn get_mcp_tool_ui_uri( _state: State<'_, AppState>, tool_name: String, ) -> Result, String> { - if !tool_name.starts_with("mcp__") { - return Ok(None); - } let registry = bitfun_core::agentic::tools::registry::get_global_tool_registry(); let guard = registry.read().await; + let is_mcp_tool = guard.get_mcp_tool_info(&tool_name).is_some(); let tool = guard.get_tool(&tool_name); drop(guard); + if !is_mcp_tool { + return Ok(None); + } Ok(tool.and_then(|t| t.ui_resource_uri())) } diff --git a/src/apps/desktop/src/api/tool_api.rs b/src/apps/desktop/src/api/tool_api.rs index dfb4dbcee..d511aeb9d 100644 --- a/src/apps/desktop/src/api/tool_api.rs +++ b/src/apps/desktop/src/api/tool_api.rs @@ -4,6 +4,7 @@ use log::error; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::path::PathBuf; +use std::sync::Arc; use bitfun_core::agentic::{ tools::framework::ToolUseContext, @@ -26,6 +27,20 @@ pub struct ToolExecutionRequest { pub safe_mode: Option, } +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GetToolInfoRequest { + pub tool_name: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct McpToolInfo { + pub server_id: String, + pub server_name: String, + pub tool_name: String, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ToolInfo { pub name: String, @@ -34,6 +49,8 @@ pub struct ToolInfo { pub is_readonly: bool, pub is_concurrency_safe: bool, pub needs_permissions: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub mcp_info: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -159,6 +176,31 @@ async fn build_tool_context(workspace_path: Option<&str>) -> ToolUseContext { } } +fn to_mcp_tool_info(info: bitfun_core::service::mcp::McpToolInfo) -> McpToolInfo { + McpToolInfo { + server_id: info.server_id, + server_name: info.server_name, + tool_name: info.tool_name, + } +} + +async fn build_tool_info(tool: &Arc) -> ToolInfo { + let description = tool + .description() + .await + .unwrap_or_else(|_| "No description available".to_string()); + + ToolInfo { + name: tool.name().to_string(), + description, + input_schema: tool.input_schema_for_model().await, + is_readonly: tool.is_readonly(), + is_concurrency_safe: tool.is_concurrency_safe(None), + needs_permissions: tool.needs_permissions(None), + mcp_info: tool.mcp_info().map(to_mcp_tool_info), + } +} + fn has_explicit_workspace_path(workspace_path: Option<&str>) -> bool { workspace_path.is_some_and(|path| !path.trim().is_empty()) } @@ -202,19 +244,7 @@ pub async fn get_all_tools_info() -> Result, String> { let mut tool_infos = Vec::new(); for tool in tools { - let description = tool - .description() - .await - .unwrap_or_else(|_| "No description available".to_string()); - - tool_infos.push(ToolInfo { - name: tool.name().to_string(), - description, - input_schema: tool.input_schema_for_model().await, - is_readonly: tool.is_readonly(), - is_concurrency_safe: tool.is_concurrency_safe(None), - needs_permissions: tool.needs_permissions(None), - }); + tool_infos.push(build_tool_info(&tool).await); } Ok(tool_infos) @@ -229,43 +259,19 @@ pub async fn get_readonly_tools_info() -> Result, String> { let mut tool_infos = Vec::new(); for tool in tools { - let description = tool - .description() - .await - .unwrap_or_else(|_| "No description available".to_string()); - - tool_infos.push(ToolInfo { - name: tool.name().to_string(), - description, - input_schema: tool.input_schema_for_model().await, - is_readonly: tool.is_readonly(), - is_concurrency_safe: tool.is_concurrency_safe(None), - needs_permissions: tool.needs_permissions(None), - }); + tool_infos.push(build_tool_info(&tool).await); } Ok(tool_infos) } #[tauri::command] -pub async fn get_tool_info(tool_name: String) -> Result, String> { +pub async fn get_tool_info(request: GetToolInfoRequest) -> Result, String> { let tools = get_all_tools().await; for tool in tools { - if tool.name() == tool_name { - let description = tool - .description() - .await - .unwrap_or_else(|_| "No description available".to_string()); - - return Ok(Some(ToolInfo { - name: tool.name().to_string(), - description, - input_schema: tool.input_schema_for_model().await, - is_readonly: tool.is_readonly(), - is_concurrency_safe: tool.is_concurrency_safe(None), - needs_permissions: tool.needs_permissions(None), - })); + if tool.name() == request.tool_name { + return Ok(Some(build_tool_info(&tool).await)); } } diff --git a/src/crates/core/src/agentic/tools/framework.rs b/src/crates/core/src/agentic/tools/framework.rs index 2df6dfec0..7f4a0b09e 100644 --- a/src/crates/core/src/agentic/tools/framework.rs +++ b/src/crates/core/src/agentic/tools/framework.rs @@ -14,6 +14,7 @@ use crate::agentic::workspace::WorkspaceServices; use crate::agentic::WorkspaceBinding; use crate::infrastructure::get_path_manager_arc; use crate::service::git::{GitDiffParams, GitService}; +use crate::service::mcp::McpToolInfo; use crate::service::remote_ssh::workspace_state::remote_workspace_runtime_root; use crate::service::{get_workspace_runtime_service_arc, WorkspaceRuntimeContext}; use crate::util::errors::BitFunResult; @@ -606,6 +607,11 @@ pub trait Tool: Send + Sync { None } + /// Stable MCP metadata for dynamic tools. + fn mcp_info(&self) -> Option { + None + } + /// User friendly name fn user_facing_name(&self) -> String { self.name().to_string() diff --git a/src/crates/core/src/agentic/tools/registry.rs b/src/crates/core/src/agentic/tools/registry.rs index a6f878b7f..47650377d 100644 --- a/src/crates/core/src/agentic/tools/registry.rs +++ b/src/crates/core/src/agentic/tools/registry.rs @@ -2,6 +2,7 @@ use crate::agentic::tools::framework::Tool; use crate::agentic::tools::implementations::*; +use crate::service::mcp::McpToolInfo; use crate::util::errors::BitFunResult; use bitfun_runtime_ports::{DynamicToolDescriptor, DynamicToolProvider, ToolDecorator}; use indexmap::IndexMap; @@ -22,6 +23,7 @@ impl ToolDecorator for SnapshotToolDecorator { /// Tool registry - manages all available tools (using IndexMap to maintain registration order) pub struct ToolRegistry { tools: IndexMap, + mcp_tools: IndexMap, tool_decorator: ToolDecoratorRef, } @@ -45,6 +47,7 @@ impl ToolRegistry { pub fn with_tool_decorator(tool_decorator: ToolDecoratorRef) -> Self { let mut registry = Self { tools: IndexMap::new(), + mcp_tools: IndexMap::new(), tool_decorator, }; @@ -93,8 +96,18 @@ impl ToolRegistry { /// Remove all tools from the MCP server pub fn unregister_mcp_server_tools(&mut self, server_id: &str) { - let prefix = format!("mcp__{}__", server_id); - self.unregister_tools_by_prefix(&prefix); + let to_remove: Vec = self + .mcp_tools + .iter() + .filter(|(_, info)| info.server_id == server_id) + .map(|(tool_name, _)| tool_name.clone()) + .collect(); + + for key in to_remove { + info!("Unregistering dynamic tool: tool_name={}", key); + self.tools.shift_remove(&key); + self.mcp_tools.shift_remove(&key); + } } /// Remove all tools whose registry name starts with the given prefix. @@ -110,6 +123,7 @@ impl ToolRegistry { for key in to_remove { info!("Unregistering dynamic tool: tool_name={}", key); self.tools.shift_remove(&key); + self.mcp_tools.shift_remove(&key); } count } @@ -190,6 +204,11 @@ impl ToolRegistry { // subsequent lookup returns the same runtime implementation. let tool = self.tool_decorator.decorate(tool); let name = tool.name().to_string(); + if let Some(mcp_info) = tool.mcp_info() { + self.mcp_tools.insert(name.clone(), mcp_info); + } else { + self.mcp_tools.shift_remove(&name); + } self.tools.insert(name, tool); } @@ -198,6 +217,10 @@ impl ToolRegistry { self.tools.get(name).cloned() } + pub fn get_mcp_tool_info(&self, name: &str) -> Option { + self.mcp_tools.get(name).cloned() + } + /// Get all tool names pub fn get_tool_names(&self) -> Vec { self.tools.keys().cloned().collect() @@ -220,12 +243,16 @@ impl DynamicToolProvider for ToolRegistry { ) -> bitfun_runtime_ports::PortResult> { let mut descriptors = Vec::new(); - for tool in self.tools.values() { - let Some(provider_id) = tool - .dynamic_provider_id() - .filter(|value| !value.is_empty()) - .map(ToOwned::to_owned) - else { + for (name, tool) in self.tools.iter() { + let provider_id = if let Some(mcp_info) = self.mcp_tools.get(name) { + Some(mcp_info.server_id.clone()) + } else { + tool.dynamic_provider_id() + .filter(|value| !value.is_empty()) + .map(ToOwned::to_owned) + }; + + let Some(provider_id) = provider_id else { continue; }; let description = tool.description().await.map_err(|error| { @@ -344,7 +371,6 @@ mod tests { Some("github__enterprise/prod") ); } - #[test] fn registry_exposes_controlhub_and_computer_use() { let registry = create_tool_registry(); diff --git a/src/crates/core/src/service/mcp/adapter/tool.rs b/src/crates/core/src/service/mcp/adapter/tool.rs index 939f694dd..3f0bb2b8c 100644 --- a/src/crates/core/src/service/mcp/adapter/tool.rs +++ b/src/crates/core/src/service/mcp/adapter/tool.rs @@ -5,6 +5,7 @@ use crate::agentic::tools::framework::{ Tool, ToolRenderOptions, ToolResult, ToolUseContext, ValidationResult, }; +use crate::service::mcp::{build_mcp_tool_name, McpToolInfo}; use crate::service::mcp::protocol::{MCPTool, MCPToolResult}; use crate::service::mcp::server::MCPConnection; use crate::util::errors::BitFunResult; @@ -17,7 +18,7 @@ use std::sync::Arc; pub struct MCPToolWrapper { mcp_tool: MCPTool, connection: Arc, - provider_id: String, + server_id: String, server_name: String, full_name: String, } @@ -32,11 +33,11 @@ impl MCPToolWrapper { server_id: String, server_name: String, ) -> Self { - let full_name = format!("mcp__{}__{}", server_id, mcp_tool.name); + let full_name = build_mcp_tool_name(&server_id, &mcp_tool.name); Self { mcp_tool, connection, - provider_id: server_id, + server_id, server_name, full_name, } @@ -127,13 +128,21 @@ impl Tool for MCPToolWrapper { } fn dynamic_provider_id(&self) -> Option<&str> { - Some(&self.provider_id) + Some(&self.server_id) } fn user_facing_name(&self) -> String { format!("{} ({})", self.tool_title(), self.server_name) } + fn mcp_info(&self) -> Option { + Some(McpToolInfo { + server_id: self.server_id.clone(), + server_name: self.server_name.clone(), + tool_name: self.mcp_tool.name.clone(), + }) + } + async fn is_enabled(&self) -> bool { true } diff --git a/src/crates/core/src/service/mcp/mod.rs b/src/crates/core/src/service/mcp/mod.rs index 843550e23..673a9cb19 100644 --- a/src/crates/core/src/service/mcp/mod.rs +++ b/src/crates/core/src/service/mcp/mod.rs @@ -14,6 +14,8 @@ pub mod auth; pub mod config; pub mod protocol; pub mod server; +mod tool_info; +mod tool_name; use std::sync::Arc; use std::sync::OnceLock; @@ -34,6 +36,10 @@ pub use adapter::{ }; pub use config::{ConfigLocation, MCPConfigService}; +pub use tool_info::McpToolInfo; +pub use tool_name::{ + build_mcp_tool_name, normalize_name_for_mcp, MCP_TOOL_DELIMITER, MCP_TOOL_PREFIX, +}; /// MCP service interface. pub struct MCPService { diff --git a/src/crates/core/src/service/mcp/tool_info.rs b/src/crates/core/src/service/mcp/tool_info.rs new file mode 100644 index 000000000..35f73b2e7 --- /dev/null +++ b/src/crates/core/src/service/mcp/tool_info.rs @@ -0,0 +1,8 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct McpToolInfo { + pub server_id: String, + pub server_name: String, + pub tool_name: String, +} diff --git a/src/crates/core/src/service/mcp/tool_name.rs b/src/crates/core/src/service/mcp/tool_name.rs new file mode 100644 index 000000000..a626e189b --- /dev/null +++ b/src/crates/core/src/service/mcp/tool_name.rs @@ -0,0 +1,56 @@ +//! Shared MCP tool-name helpers. + +pub const MCP_TOOL_PREFIX: &str = "mcp__"; +pub const MCP_TOOL_DELIMITER: &str = "__"; + +/// Normalize MCP server/tool names to a wire-safe format aligned with claude-code. +pub fn normalize_name_for_mcp(name: &str) -> String { + name.chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '_' || ch == '-' { + ch + } else { + '_' + } + }) + .collect() +} + +pub fn build_mcp_tool_name(server_id: &str, tool_name: &str) -> String { + format!( + "{}{}{}{}", + MCP_TOOL_PREFIX, + normalize_name_for_mcp(server_id), + MCP_TOOL_DELIMITER, + normalize_name_for_mcp(tool_name) + ) +} + +#[cfg(test)] +mod tests { + use super::{build_mcp_tool_name, normalize_name_for_mcp}; + + #[test] + fn normalize_name_for_mcp_replaces_spaces_and_symbols() { + assert_eq!( + normalize_name_for_mcp("Acme Search / Primary"), + "Acme_Search___Primary" + ); + } + + #[test] + fn normalize_name_for_mcp_keeps_ascii_word_chars_and_hyphen() { + assert_eq!( + normalize_name_for_mcp("github-enterprise_v2"), + "github-enterprise_v2" + ); + } + + #[test] + fn build_mcp_tool_name_normalizes_both_segments() { + assert_eq!( + build_mcp_tool_name("Claude Code", "search repos"), + "mcp__Claude_Code__search_repos" + ); + } +} diff --git a/src/web-ui/src/app/scenes/agents/components/CreateAgentPage.tsx b/src/web-ui/src/app/scenes/agents/components/CreateAgentPage.tsx index 95d7c491d..0c4857ad5 100644 --- a/src/web-ui/src/app/scenes/agents/components/CreateAgentPage.tsx +++ b/src/web-ui/src/app/scenes/agents/components/CreateAgentPage.tsx @@ -48,7 +48,7 @@ const CreateAgentPage: React.FC = () => { if (!name) return null; return { name, - isReadonly: Boolean(tool?.isReadonly ?? tool?.is_readonly), + isReadonly: Boolean(tool?.is_readonly), }; }) .filter((tool): tool is SubagentEditorToolInfo => Boolean(tool)); diff --git a/src/web-ui/src/app/scenes/agents/hooks/useAgentsList.ts b/src/web-ui/src/app/scenes/agents/hooks/useAgentsList.ts index c172540a2..39805793f 100644 --- a/src/web-ui/src/app/scenes/agents/hooks/useAgentsList.ts +++ b/src/web-ui/src/app/scenes/agents/hooks/useAgentsList.ts @@ -5,6 +5,7 @@ import { SubagentAPI } from '@/infrastructure/api/service-api/SubagentAPI'; import { configAPI } from '@/infrastructure/api/service-api/ConfigAPI'; import type { ModeConfigItem, ModeSkillInfo } from '@/infrastructure/config/types'; import { useNotification } from '@/shared/notification-system'; +import type { McpToolInfo } from '@/shared/types/agent-api'; import type { AgentWithCapabilities } from '../agentsStore'; import { enrichCapabilities } from '../utils'; import { STATIC_HIDDEN_AGENT_IDS, isAgentInOverviewZone } from '../agentVisibility'; @@ -18,6 +19,7 @@ export interface ToolInfo { name: string; description: string; is_readonly: boolean; + mcp_info?: McpToolInfo; } interface UseAgentsListOptions { diff --git a/src/web-ui/src/app/scenes/profile/views/TemplateConfigPage.tsx b/src/web-ui/src/app/scenes/profile/views/TemplateConfigPage.tsx index be87e88f2..6b9236016 100644 --- a/src/web-ui/src/app/scenes/profile/views/TemplateConfigPage.tsx +++ b/src/web-ui/src/app/scenes/profile/views/TemplateConfigPage.tsx @@ -18,14 +18,19 @@ import { configManager } from '@/infrastructure/config/services/ConfigManager'; import type { AIModelConfig, ModeConfigItem, ModeSkillInfo } from '@/infrastructure/config/types'; import { MCPAPI, type MCPServerInfo } from '@/infrastructure/api/service-api/MCPAPI'; import { notificationService } from '@/shared/notification-system'; +import type { McpToolInfo } from '@/shared/types/agent-api'; import { createLogger } from '@/shared/utils/logger'; -import { isMcpToolName, parseMcpToolName } from '@/infrastructure/mcp/toolName'; import { useNurseryStore } from '../nurseryStore'; import { formatTokenCount } from './useTokenEstimate'; const log = createLogger('TemplateConfigPage'); -interface ToolInfo { name: string; description: string; is_readonly: boolean; } +interface ToolInfo { + name: string; + description: string; + is_readonly: boolean; + mcp_info?: McpToolInfo; +} type TemplateDetail = | { type: 'tool'; tool: ToolInfo; isMcp: boolean } @@ -34,16 +39,16 @@ type TemplateDetail = type ModelSlot = 'primary' | 'fast'; -function isMcpTool(name: string): boolean { - return isMcpToolName(name); +function isMcpTool(tool: ToolInfo): boolean { + return Boolean(tool.mcp_info); } -function getMcpServerName(toolName: string): string { - return parseMcpToolName(toolName)?.serverId ?? toolName; +function getMcpServerName(tool: ToolInfo): string { + return tool.mcp_info?.server_id ?? tool.name; } -function getMcpShortName(toolName: string): string { - return parseMcpToolName(toolName)?.toolName ?? toolName; +function getMcpShortName(tool: ToolInfo): string { + return tool.mcp_info?.tool_name ?? tool.name; } type CtxSegKey = 'systemPrompt' | 'toolInjection' | 'rules' | 'memories'; @@ -171,7 +176,7 @@ const TemplateConfigPage: React.FC = () => { // Split tools into built-in vs MCP const builtinTools = useMemo( - () => availableTools.filter((t) => !isMcpTool(t.name)), + () => availableTools.filter((tool) => !isMcpTool(tool)), [availableTools], ); @@ -189,8 +194,8 @@ const TemplateConfigPage: React.FC = () => { const mcpToolsByServer = useMemo(() => { const map = new Map(); for (const tool of availableTools) { - if (!isMcpTool(tool.name)) continue; - const server = getMcpServerName(tool.name); + if (!isMcpTool(tool)) continue; + const server = getMcpServerName(tool); if (!map.has(server)) map.set(server, []); map.get(server)!.push(tool); } @@ -415,7 +420,7 @@ const TemplateConfigPage: React.FC = () => {
{tools.map((tool) => { const enabled = agenticConfig?.enabled_tools?.includes(tool.name) ?? false; - const displayName = isMcp ? getMcpShortName(tool.name) : tool.name; + const displayName = isMcp ? getMcpShortName(tool) : tool.name; const selected = detail?.type === 'tool' && detail.tool.name === tool.name; return (
{ if (detail.type === 'tool') { const { tool, isMcp } = detail; - const displayName = isMcp ? getMcpShortName(tool.name) : tool.name; + const displayName = isMcp ? getMcpShortName(tool) : tool.name; const enabled = agenticConfig?.enabled_tools?.includes(tool.name) ?? false; return (