From 75c9c80679caf6d290c5cc65a466a6cb32be18d8 Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Sun, 15 Mar 2026 18:26:53 +0300 Subject: [PATCH] bug fixes --- crates/openfang-api/src/channel_bridge.rs | 4 ++ crates/openfang-api/src/routes.rs | 18 +++++- crates/openfang-api/static/index_body.html | 4 +- crates/openfang-runtime/src/drivers/openai.rs | 38 +++++++----- crates/openfang-runtime/src/mcp.rs | 60 +++++++++++++++++++ crates/openfang-runtime/src/tool_runner.rs | 6 +- 6 files changed, 107 insertions(+), 23 deletions(-) diff --git a/crates/openfang-api/src/channel_bridge.rs b/crates/openfang-api/src/channel_bridge.rs index ccdf25dd..2f275674 100644 --- a/crates/openfang-api/src/channel_bridge.rs +++ b/crates/openfang-api/src/channel_bridge.rs @@ -73,6 +73,10 @@ impl ChannelBridgeHandle for KernelBridgeAdapter { .send_message(agent_id, message) .await .map_err(|e| format!("{e}"))?; + // Silent/NO_REPLY responses should not be forwarded to channels + if result.silent { + return Ok(String::new()); + } Ok(result.response) } diff --git a/crates/openfang-api/src/routes.rs b/crates/openfang-api/src/routes.rs index 2492231a..3fdc3ac9 100644 --- a/crates/openfang-api/src/routes.rs +++ b/crates/openfang-api/src/routes.rs @@ -374,8 +374,11 @@ pub async fn send_message( // Strip ... blocks from model output let cleaned = crate::ws::strip_think_tags(&result.response); - // Guard: ensure we never return an empty response to the client - let response = if cleaned.trim().is_empty() { + // If the agent intentionally returned a silent/NO_REPLY response, + // return an empty string — don't generate debug fallback text. + let response = if result.silent { + String::new() + } else if cleaned.trim().is_empty() { format!( "[The agent completed processing but returned no text response. ({} in / {} out | {} iter)]", result.total_usage.input_tokens, @@ -3431,12 +3434,14 @@ pub async fn clawhub_search( let cache_dir = state.kernel.config.home_dir.join(".cache").join("clawhub"); let client = openfang_skills::clawhub::ClawHubClient::new(cache_dir); + let skills_dir = state.kernel.config.home_dir.join("skills"); match client.search(&query, limit).await { Ok(results) => { let items: Vec = results .results .iter() .map(|e| { + let installed = skills_dir.join(&e.slug).exists(); serde_json::json!({ "slug": e.slug, "name": e.display_name, @@ -3444,6 +3449,7 @@ pub async fn clawhub_search( "version": e.version, "score": e.score, "updated_at": e.updated_at, + "installed": installed, }) }) .collect(); @@ -3508,12 +3514,18 @@ pub async fn clawhub_browse( let cache_dir = state.kernel.config.home_dir.join(".cache").join("clawhub"); let client = openfang_skills::clawhub::ClawHubClient::new(cache_dir); + let skills_dir = state.kernel.config.home_dir.join("skills"); match client.browse(sort, limit, cursor).await { Ok(results) => { let items: Vec = results .items .iter() - .map(clawhub_browse_entry_to_json) + .map(|entry| { + let mut json = clawhub_browse_entry_to_json(entry); + let installed = skills_dir.join(&entry.slug).exists(); + json["installed"] = serde_json::json!(installed); + json + }) .collect(); let resp = serde_json::json!({ "items": items, diff --git a/crates/openfang-api/static/index_body.html b/crates/openfang-api/static/index_body.html index 29ed4b76..488480d6 100644 --- a/crates/openfang-api/static/index_body.html +++ b/crates/openfang-api/static/index_body.html @@ -2293,7 +2293,7 @@
- + @@ -2316,7 +2316,7 @@ - + diff --git a/crates/openfang-runtime/src/drivers/openai.rs b/crates/openfang-runtime/src/drivers/openai.rs index 15a5a665..12e0cdf8 100644 --- a/crates/openfang-runtime/src/drivers/openai.rs +++ b/crates/openfang-runtime/src/drivers/openai.rs @@ -35,8 +35,10 @@ impl OpenAIDriver { } /// True if this provider is Moonshot/Kimi and requires reasoning_content on assistant messages with tool_calls. - fn kimi_needs_reasoning_content(&self, model: &str) -> bool { - self.base_url.contains("moonshot") || model.to_lowercase().contains("kimi") + fn needs_reasoning_content(&self, model: &str) -> bool { + self.base_url.contains("moonshot") + || model.to_lowercase().contains("kimi") + || model.to_lowercase().contains("reasoner") } /// Create a driver with additional HTTP headers (e.g. for Copilot IDE auth). @@ -295,6 +297,7 @@ impl LlmDriver for OpenAIDriver { (Role::Assistant, MessageContent::Blocks(blocks)) => { let mut text_parts = Vec::new(); let mut tool_calls = Vec::new(); + let mut reasoning_text = String::new(); for block in blocks { match block { ContentBlock::Text { text, .. } => text_parts.push(text.clone()), @@ -308,17 +311,16 @@ impl LlmDriver for OpenAIDriver { }, }); } - ContentBlock::Thinking { .. } => {} + ContentBlock::Thinking { thinking, .. } => { + reasoning_text = thinking.clone(); + } _ => {} } } let has_tool_calls = !tool_calls.is_empty(); + let needs_reasoning = self.needs_reasoning_content(&request.model); oai_messages.push(OaiMessage { role: "assistant".to_string(), - // ZHIPU (GLM) rejects assistant messages where content is - // null or omitted when tool_calls are present (error 1214). - // Always send an empty string so every OpenAI-compat - // provider gets a valid payload. content: if text_parts.is_empty() { if has_tool_calls { Some(OaiMessageContent::Text(String::new())) @@ -334,8 +336,8 @@ impl LlmDriver for OpenAIDriver { Some(tool_calls) }, tool_call_id: None, - reasoning_content: if has_tool_calls && self.kimi_needs_reasoning_content(&request.model) { - Some(String::new()) + reasoning_content: if needs_reasoning { + Some(if reasoning_text.is_empty() { String::new() } else { reasoning_text }) } else { None }, @@ -377,7 +379,7 @@ impl LlmDriver for OpenAIDriver { messages: oai_messages, max_tokens: mt, max_completion_tokens: mct, - temperature: if self.kimi_needs_reasoning_content(&request.model) { + temperature: if self.needs_reasoning_content(&request.model) { // Kimi with thinking disabled uses fixed 0.6 for multi-turn compatibility. Some(0.6) } else if temperature_must_be_one(&request.model) { @@ -391,7 +393,7 @@ impl LlmDriver for OpenAIDriver { tool_choice, stream: false, stream_options: None, - thinking: if self.kimi_needs_reasoning_content(&request.model) { + thinking: if self.needs_reasoning_content(&request.model) { Some(serde_json::json!({"type": "disabled"})) } else { None @@ -718,6 +720,7 @@ impl LlmDriver for OpenAIDriver { (Role::Assistant, MessageContent::Blocks(blocks)) => { let mut text_parts = Vec::new(); let mut tool_calls_out = Vec::new(); + let mut reasoning_text = String::new(); for block in blocks { match block { ContentBlock::Text { text, .. } => text_parts.push(text.clone()), @@ -731,11 +734,14 @@ impl LlmDriver for OpenAIDriver { }, }); } - ContentBlock::Thinking { .. } => {} + ContentBlock::Thinking { thinking, .. } => { + reasoning_text = thinking.clone(); + } _ => {} } } let has_tool_calls = !tool_calls_out.is_empty(); + let needs_reasoning = self.needs_reasoning_content(&request.model); oai_messages.push(OaiMessage { role: "assistant".to_string(), content: if text_parts.is_empty() { @@ -753,8 +759,8 @@ impl LlmDriver for OpenAIDriver { Some(tool_calls_out) }, tool_call_id: None, - reasoning_content: if has_tool_calls && self.kimi_needs_reasoning_content(&request.model) { - Some(String::new()) + reasoning_content: if needs_reasoning { + Some(if reasoning_text.is_empty() { String::new() } else { reasoning_text }) } else { None }, @@ -796,7 +802,7 @@ impl LlmDriver for OpenAIDriver { messages: oai_messages, max_tokens: mt, max_completion_tokens: mct, - temperature: if self.kimi_needs_reasoning_content(&request.model) { + temperature: if self.needs_reasoning_content(&request.model) { Some(0.6) } else if temperature_must_be_one(&request.model) { Some(1.0) @@ -809,7 +815,7 @@ impl LlmDriver for OpenAIDriver { tool_choice, stream: true, stream_options: Some(serde_json::json!({"include_usage": true})), - thinking: if self.kimi_needs_reasoning_content(&request.model) { + thinking: if self.needs_reasoning_content(&request.model) { Some(serde_json::json!({"type": "disabled"})) } else { None diff --git a/crates/openfang-runtime/src/mcp.rs b/crates/openfang-runtime/src/mcp.rs index f899ce3a..170cd74b 100644 --- a/crates/openfang-runtime/src/mcp.rs +++ b/crates/openfang-runtime/src/mcp.rs @@ -557,6 +557,10 @@ pub fn is_mcp_tool(name: &str) -> bool { } /// Extract server name from an MCP tool name. +/// +/// Falls back to first-underscore heuristic, but prefer +/// `extract_mcp_server_from_known()` which handles server names containing +/// hyphens (normalized to underscores) correctly. pub fn extract_mcp_server(tool_name: &str) -> Option<&str> { if !tool_name.starts_with("mcp_") { return None; @@ -565,6 +569,30 @@ pub fn extract_mcp_server(tool_name: &str) -> Option<&str> { rest.find('_').map(|pos| &rest[..pos]) } +/// Extract the original server name by matching against known server names. +/// +/// This handles server names with hyphens (e.g. "bocha-search") correctly — +/// the normalized prefix "mcp_bocha_search_" is matched against each known +/// server's normalized name, returning the original (unhyphenated) name. +pub fn extract_mcp_server_from_known<'a>( + tool_name: &str, + server_names: &[&'a str], +) -> Option<&'a str> { + if !tool_name.starts_with("mcp_") { + return None; + } + // Sort by length descending so longer (more specific) names match first + let mut sorted: Vec<&&str> = server_names.iter().collect(); + sorted.sort_by_key(|a| std::cmp::Reverse(a.len())); + for name in sorted { + let prefix = format!("mcp_{}_", normalize_name(name)); + if tool_name.starts_with(&prefix) { + return Some(name); + } + } + None +} + /// Strip the MCP namespace prefix from a tool name. fn strip_mcp_prefix<'a>(server: &str, tool_name: &'a str) -> Option<&'a str> { let prefix = format!("mcp_{}_", normalize_name(server)); @@ -627,6 +655,38 @@ mod tests { assert_eq!(extract_mcp_server("file_read"), None); } + #[test] + fn test_extract_mcp_server_from_known_with_hyphens() { + // Server "bocha-search" normalized to "bocha_search" in tool prefix + let servers = vec!["bocha-search", "github"]; + let tool = "mcp_bocha_search_bocha_web_search"; + assert_eq!( + extract_mcp_server_from_known(tool, &servers), + Some("bocha-search") + ); + // Simple server name still works + assert_eq!( + extract_mcp_server_from_known("mcp_github_create_issue", &servers), + Some("github") + ); + // Non-MCP tool returns None + assert_eq!(extract_mcp_server_from_known("file_read", &servers), None); + } + + #[test] + fn test_extract_mcp_server_from_known_longest_match() { + // "my-api" and "my-api-v2" — should match the longer one + let servers = vec!["my-api", "my-api-v2"]; + assert_eq!( + extract_mcp_server_from_known("mcp_my_api_v2_get_users", &servers), + Some("my-api-v2") + ); + assert_eq!( + extract_mcp_server_from_known("mcp_my_api_list_items", &servers), + Some("my-api") + ); + } + #[test] fn test_mcp_jsonrpc_initialize() { // Verify the initialize request structure diff --git a/crates/openfang-runtime/src/tool_runner.rs b/crates/openfang-runtime/src/tool_runner.rs index dd0bcf1e..aa7de97a 100644 --- a/crates/openfang-runtime/src/tool_runner.rs +++ b/crates/openfang-runtime/src/tool_runner.rs @@ -441,8 +441,10 @@ pub async fn execute_tool( // Fallback 1: MCP tools (mcp_{server}_{tool} prefix) if mcp::is_mcp_tool(other) { if let Some(mcp_conns) = mcp_connections { - if let Some(server_name) = mcp::extract_mcp_server(other) { - let mut conns = mcp_conns.lock().await; + let mut conns = mcp_conns.lock().await; + let known_names: Vec = conns.iter().map(|c| c.name().to_string()).collect(); + let known_refs: Vec<&str> = known_names.iter().map(|s| s.as_str()).collect(); + if let Some(server_name) = mcp::extract_mcp_server_from_known(other, &known_refs) { if let Some(conn) = conns.iter_mut().find(|c| c.name() == server_name) { debug!( tool = other,