feat(mcp): 为 MCP 工具调用增加超时保护,默认 5 分钟
在 McpToolWrapper 适配层用 tokio::time::timeout 包裹 call_tool,防止外部 MCP server 挂起导致 agent loop 无限阻塞。超时时间通过 config.mcp_tool_timeout_secs 配置(默认 300 秒,0=不超时),前端 McpTab 设置页提供输入框。
This commit is contained in:
parent
7de9a8a054
commit
3e97ed903c
@ -78,6 +78,7 @@ impl InitWizard {
|
|||||||
tools: crate::config::ToolsConfig::default(),
|
tools: crate::config::ToolsConfig::default(),
|
||||||
memory_maintenance: crate::config::MemoryMaintenanceConfig::default(),
|
memory_maintenance: crate::config::MemoryMaintenanceConfig::default(),
|
||||||
mcp_servers: HashMap::new(),
|
mcp_servers: HashMap::new(),
|
||||||
|
mcp_tool_timeout_secs: 300,
|
||||||
image_context: crate::config::ImageContextConfig::default(),
|
image_context: crate::config::ImageContextConfig::default(),
|
||||||
subagents: crate::config::SubagentsConfig::default(),
|
subagents: crate::config::SubagentsConfig::default(),
|
||||||
experts: crate::config::ExpertsConfig::default(),
|
experts: crate::config::ExpertsConfig::default(),
|
||||||
@ -843,6 +844,7 @@ impl InitWizard {
|
|||||||
tools: existing.tools.clone(),
|
tools: existing.tools.clone(),
|
||||||
memory_maintenance: existing.memory_maintenance.clone(),
|
memory_maintenance: existing.memory_maintenance.clone(),
|
||||||
mcp_servers: existing.mcp_servers.clone(),
|
mcp_servers: existing.mcp_servers.clone(),
|
||||||
|
mcp_tool_timeout_secs: existing.mcp_tool_timeout_secs,
|
||||||
image_context: existing.image_context.clone(),
|
image_context: existing.image_context.clone(),
|
||||||
subagents: existing.subagents.clone(),
|
subagents: existing.subagents.clone(),
|
||||||
experts: existing.experts.clone(),
|
experts: existing.experts.clone(),
|
||||||
|
|||||||
@ -34,6 +34,9 @@ pub struct Config {
|
|||||||
pub memory_maintenance: MemoryMaintenanceConfig,
|
pub memory_maintenance: MemoryMaintenanceConfig,
|
||||||
#[serde(default, rename = "mcpServers")]
|
#[serde(default, rename = "mcpServers")]
|
||||||
pub mcp_servers: HashMap<String, crate::mcp::McpServerConfig>,
|
pub mcp_servers: HashMap<String, crate::mcp::McpServerConfig>,
|
||||||
|
/// MCP 工具调用超时时间(秒)。0 表示不超时。默认 300(5 分钟)。
|
||||||
|
#[serde(default = "default_mcp_tool_timeout_secs")]
|
||||||
|
pub mcp_tool_timeout_secs: u64,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub image_context: ImageContextConfig,
|
pub image_context: ImageContextConfig,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
@ -556,6 +559,10 @@ fn default_max_retries() -> u32 {
|
|||||||
3
|
3
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn default_mcp_tool_timeout_secs() -> u64 {
|
||||||
|
300
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Deserialize, Serialize)]
|
#[derive(Debug, Clone, Deserialize, Serialize)]
|
||||||
pub struct GatewayConfig {
|
pub struct GatewayConfig {
|
||||||
#[serde(default = "default_gateway_host")]
|
#[serde(default = "default_gateway_host")]
|
||||||
|
|||||||
@ -118,6 +118,7 @@ impl GatewayState {
|
|||||||
config.memory_maintenance.clone(),
|
config.memory_maintenance.clone(),
|
||||||
session_ttl_hours,
|
session_ttl_hours,
|
||||||
mcp_config,
|
mcp_config,
|
||||||
|
config.mcp_tool_timeout_secs,
|
||||||
Some(bus.clone()),
|
Some(bus.clone()),
|
||||||
Arc::new(crate::config::ModelResolver::from_config(&config)),
|
Arc::new(crate::config::ModelResolver::from_config(&config)),
|
||||||
config.compaction.clone(),
|
config.compaction.clone(),
|
||||||
|
|||||||
@ -56,6 +56,7 @@ pub(crate) fn build_session_manager(
|
|||||||
maintenance_config: MemoryMaintenanceConfig,
|
maintenance_config: MemoryMaintenanceConfig,
|
||||||
session_ttl_hours: Option<u64>,
|
session_ttl_hours: Option<u64>,
|
||||||
mcp_config: crate::mcp::McpConfig,
|
mcp_config: crate::mcp::McpConfig,
|
||||||
|
mcp_tool_timeout_secs: u64,
|
||||||
bus: Option<Arc<MessageBus>>,
|
bus: Option<Arc<MessageBus>>,
|
||||||
model_resolver: Arc<ModelResolver>,
|
model_resolver: Arc<ModelResolver>,
|
||||||
compaction_config: CompactionConfig,
|
compaction_config: CompactionConfig,
|
||||||
@ -84,6 +85,7 @@ pub(crate) fn build_session_manager(
|
|||||||
maintenance_config,
|
maintenance_config,
|
||||||
session_ttl_hours,
|
session_ttl_hours,
|
||||||
mcp_config,
|
mcp_config,
|
||||||
|
mcp_tool_timeout_secs,
|
||||||
bus,
|
bus,
|
||||||
model_resolver,
|
model_resolver,
|
||||||
compaction_config,
|
compaction_config,
|
||||||
@ -106,6 +108,7 @@ pub(crate) fn build_session_manager_with_sender(
|
|||||||
maintenance_config: MemoryMaintenanceConfig,
|
maintenance_config: MemoryMaintenanceConfig,
|
||||||
session_ttl_hours: Option<u64>,
|
session_ttl_hours: Option<u64>,
|
||||||
mcp_config: crate::mcp::McpConfig,
|
mcp_config: crate::mcp::McpConfig,
|
||||||
|
mcp_tool_timeout_secs: u64,
|
||||||
bus: Option<Arc<MessageBus>>,
|
bus: Option<Arc<MessageBus>>,
|
||||||
model_resolver: Arc<ModelResolver>,
|
model_resolver: Arc<ModelResolver>,
|
||||||
compaction_config: CompactionConfig,
|
compaction_config: CompactionConfig,
|
||||||
@ -192,6 +195,7 @@ pub(crate) fn build_session_manager_with_sender(
|
|||||||
manager.clone(),
|
manager.clone(),
|
||||||
server_key.clone(),
|
server_key.clone(),
|
||||||
tool_info,
|
tool_info,
|
||||||
|
mcp_tool_timeout_secs,
|
||||||
);
|
);
|
||||||
mcp_tools_for_subagents.push(wrapper);
|
mcp_tools_for_subagents.push(wrapper);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -747,6 +747,7 @@ impl SessionManager {
|
|||||||
maintenance_config,
|
maintenance_config,
|
||||||
session_ttl_hours,
|
session_ttl_hours,
|
||||||
mcp_config,
|
mcp_config,
|
||||||
|
300,
|
||||||
None,
|
None,
|
||||||
model_resolver,
|
model_resolver,
|
||||||
crate::config::CompactionConfig::default(),
|
crate::config::CompactionConfig::default(),
|
||||||
|
|||||||
@ -788,13 +788,14 @@ impl McpInitializer {
|
|||||||
pub async fn register_tools(
|
pub async fn register_tools(
|
||||||
&mut self,
|
&mut self,
|
||||||
registry: &mut crate::tools::ToolRegistry,
|
registry: &mut crate::tools::ToolRegistry,
|
||||||
|
timeout_secs: u64,
|
||||||
) -> anyhow::Result<()> {
|
) -> anyhow::Result<()> {
|
||||||
if let Some(manager) = self.manager.clone() {
|
if let Some(manager) = self.manager.clone() {
|
||||||
// Wait for connections to complete first
|
// Wait for connections to complete first
|
||||||
self.wait_for_connections().await?;
|
self.wait_for_connections().await?;
|
||||||
|
|
||||||
tracing::info!("Registering MCP tools after connections completed");
|
tracing::info!("Registering MCP tools after connections completed");
|
||||||
crate::mcp::register_mcp_tools(manager, registry).await?;
|
crate::mcp::register_mcp_tools(manager, registry, timeout_secs).await?;
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|||||||
@ -37,11 +37,18 @@ pub struct McpToolWrapper {
|
|||||||
full_name: String,
|
full_name: String,
|
||||||
/// Tool information from MCP server
|
/// Tool information from MCP server
|
||||||
tool_info: Tool,
|
tool_info: Tool,
|
||||||
|
/// Tool call timeout in seconds (0 = no timeout)
|
||||||
|
timeout_secs: u64,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl McpToolWrapper {
|
impl McpToolWrapper {
|
||||||
/// Create a new tool wrapper
|
/// Create a new tool wrapper
|
||||||
pub fn new(manager: Arc<McpClientManager>, server_key: String, tool_info: Tool) -> Self {
|
pub fn new(
|
||||||
|
manager: Arc<McpClientManager>,
|
||||||
|
server_key: String,
|
||||||
|
tool_info: Tool,
|
||||||
|
timeout_secs: u64,
|
||||||
|
) -> Self {
|
||||||
let tool_name = tool_info.name.clone().into_owned();
|
let tool_name = tool_info.name.clone().into_owned();
|
||||||
let raw_name = format!("mcp_{}_{}", server_key, tool_name);
|
let raw_name = format!("mcp_{}_{}", server_key, tool_name);
|
||||||
let full_name = sanitize_tool_name(&raw_name);
|
let full_name = sanitize_tool_name(&raw_name);
|
||||||
@ -61,6 +68,7 @@ impl McpToolWrapper {
|
|||||||
tool_name,
|
tool_name,
|
||||||
full_name,
|
full_name,
|
||||||
tool_info,
|
tool_info,
|
||||||
|
timeout_secs,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -98,10 +106,33 @@ impl PicoBotTool for McpToolWrapper {
|
|||||||
"Calling MCP tool"
|
"Calling MCP tool"
|
||||||
);
|
);
|
||||||
|
|
||||||
let result = self
|
let call = self
|
||||||
.manager
|
.manager
|
||||||
.call_tool(&self.server_key, &self.tool_name, args)
|
.call_tool(&self.server_key, &self.tool_name, args);
|
||||||
.await?;
|
|
||||||
|
let result = if self.timeout_secs > 0 {
|
||||||
|
tokio::time::timeout(
|
||||||
|
std::time::Duration::from_secs(self.timeout_secs),
|
||||||
|
call,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map_err(|_| {
|
||||||
|
tracing::warn!(
|
||||||
|
server_key = %self.server_key,
|
||||||
|
tool = %self.tool_name,
|
||||||
|
timeout_secs = self.timeout_secs,
|
||||||
|
"MCP tool call timed out"
|
||||||
|
);
|
||||||
|
anyhow::anyhow!(
|
||||||
|
"MCP tool '{}' on server '{}' timed out after {}s",
|
||||||
|
self.tool_name,
|
||||||
|
self.server_key,
|
||||||
|
self.timeout_secs
|
||||||
|
)
|
||||||
|
})??
|
||||||
|
} else {
|
||||||
|
call.await?
|
||||||
|
};
|
||||||
|
|
||||||
// Convert MCP CallToolResult to PicoBot ToolResult
|
// Convert MCP CallToolResult to PicoBot ToolResult
|
||||||
let output = extract_text_content(&result);
|
let output = extract_text_content(&result);
|
||||||
@ -147,11 +178,17 @@ fn extract_text_content(result: &rmcp::model::CallToolResult) -> String {
|
|||||||
pub async fn register_mcp_tools(
|
pub async fn register_mcp_tools(
|
||||||
manager: Arc<McpClientManager>,
|
manager: Arc<McpClientManager>,
|
||||||
registry: &mut crate::tools::registry::ToolRegistry,
|
registry: &mut crate::tools::registry::ToolRegistry,
|
||||||
|
timeout_secs: u64,
|
||||||
) -> anyhow::Result<()> {
|
) -> anyhow::Result<()> {
|
||||||
let all_tools = manager.all_tools().await;
|
let all_tools = manager.all_tools().await;
|
||||||
|
|
||||||
for (server_key, tool_info) in all_tools {
|
for (server_key, tool_info) in all_tools {
|
||||||
let wrapper = McpToolWrapper::new(manager.clone(), server_key.clone(), tool_info);
|
let wrapper = McpToolWrapper::new(
|
||||||
|
manager.clone(),
|
||||||
|
server_key.clone(),
|
||||||
|
tool_info,
|
||||||
|
timeout_secs,
|
||||||
|
);
|
||||||
|
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
name = %wrapper.name(),
|
name = %wrapper.name(),
|
||||||
@ -198,7 +235,7 @@ mod tests {
|
|||||||
.clone();
|
.clone();
|
||||||
let tool_info = Tool::new("echo", "Echo tool", schema);
|
let tool_info = Tool::new("echo", "Echo tool", schema);
|
||||||
|
|
||||||
let wrapper = McpToolWrapper::new(manager, "filesystem".to_string(), tool_info);
|
let wrapper = McpToolWrapper::new(manager, "filesystem".to_string(), tool_info, 300);
|
||||||
assert_eq!(wrapper.name(), "mcp_filesystem_echo");
|
assert_eq!(wrapper.name(), "mcp_filesystem_echo");
|
||||||
assert_eq!(wrapper.original_name(), "echo");
|
assert_eq!(wrapper.original_name(), "echo");
|
||||||
assert_eq!(wrapper.server_key(), "filesystem");
|
assert_eq!(wrapper.server_key(), "filesystem");
|
||||||
@ -216,7 +253,7 @@ mod tests {
|
|||||||
.clone();
|
.clone();
|
||||||
let tool_info = Tool::new("tools.list:read", "Namespaced tool", schema);
|
let tool_info = Tool::new("tools.list:read", "Namespaced tool", schema);
|
||||||
|
|
||||||
let wrapper = McpToolWrapper::new(manager, "github.api".to_string(), tool_info);
|
let wrapper = McpToolWrapper::new(manager, "github.api".to_string(), tool_info, 300);
|
||||||
// mcp_github.api_tools.list:read → mcp_github_api_tools_list_read
|
// mcp_github.api_tools.list:read → mcp_github_api_tools_list_read
|
||||||
assert_eq!(wrapper.name(), "mcp_github_api_tools_list_read");
|
assert_eq!(wrapper.name(), "mcp_github_api_tools_list_read");
|
||||||
// Original identifiers preserved for routing
|
// Original identifiers preserved for routing
|
||||||
|
|||||||
@ -58,6 +58,18 @@ export function McpTab({ config, update, setToast }: Props) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="space-y-4">
|
<div className="space-y-4">
|
||||||
|
<div className="rounded-xl border border-[var(--border-color)] bg-[var(--bg-secondary)]/60 p-4">
|
||||||
|
<Field label="工具调用超时(秒)" hint="0 = 不超时,默认 300(5 分钟)">
|
||||||
|
<input
|
||||||
|
type="number"
|
||||||
|
value={config.mcp_tool_timeout_secs ?? 300}
|
||||||
|
onChange={(e) => update('mcp_tool_timeout_secs', Number(e.target.value))}
|
||||||
|
className={inputCls}
|
||||||
|
min={0}
|
||||||
|
step={30}
|
||||||
|
/>
|
||||||
|
</Field>
|
||||||
|
</div>
|
||||||
{mcpStatus && mcpStatus.enabled && (
|
{mcpStatus && mcpStatus.enabled && (
|
||||||
<div className="flex items-center gap-3 p-3 rounded-lg bg-[var(--bg-tertiary)] text-xs">
|
<div className="flex items-center gap-3 p-3 rounded-lg bg-[var(--bg-tertiary)] text-xs">
|
||||||
<div className="flex items-center gap-1.5">
|
<div className="flex items-center gap-1.5">
|
||||||
|
|||||||
@ -257,6 +257,7 @@ export interface AppConfig {
|
|||||||
client: ClientConfig;
|
client: ClientConfig;
|
||||||
channels: Record<string, ChannelConfig>;
|
channels: Record<string, ChannelConfig>;
|
||||||
mcpServers: Record<string, McpServerConfig>;
|
mcpServers: Record<string, McpServerConfig>;
|
||||||
|
mcp_tool_timeout_secs: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type TabId =
|
export type TabId =
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user