From cc93ef45716ad4611990ff71726bc5bd4d28da7f Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Tue, 10 Mar 2026 00:40:45 +0300 Subject: [PATCH] community fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix tool name mapping so LLM-hallucinated aliases (fs-write, fsRead, writeFile, etc.) normalize to canonical names before capability check and dispatch (#349). Fix provider keys not loading after dashboard save by creating fresh drivers that read current env vars instead of stale boot-time cache (#465, #458, #355). Fix Moonshot/kimi model IDs and provider inference (#428). Add Telegram message reactions for agent lifecycle feedback (#435). Add configurable api_url for Telegram proxy support (#477). Add Discord ignore_bots config option (#403). Fix openfang init EPERM crash with 7-browser fallback on Linux (#389). Add text-based tool call parsing for models without native function calling — [TOOL_CALL], , bare JSON patterns (#354, #332). Fix pre-existing Windows test failures with cross-platform paths. 1948 tests pass, 0 clippy warnings. --- Cargo.lock | 28 +- Cargo.toml | 2 +- crates/openfang-api/src/channel_bridge.rs | 1 + crates/openfang-api/src/routes.rs | 76 ++- crates/openfang-channels/src/bridge.rs | 38 +- crates/openfang-channels/src/discord.rs | 45 ++ crates/openfang-channels/src/telegram.rs | 129 +++- crates/openfang-cli/src/main.rs | 49 +- crates/openfang-kernel/src/kernel.rs | 77 ++- crates/openfang-runtime/src/agent_loop.rs | 638 ++++++++++++++++++- crates/openfang-runtime/src/model_catalog.rs | 11 +- crates/openfang-runtime/src/tool_runner.rs | 93 ++- crates/openfang-types/src/config.rs | 23 + crates/openfang-types/src/tool_compat.rs | 64 ++ 14 files changed, 1180 insertions(+), 94 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe4af0c2..12bc362e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3792,7 +3792,7 @@ dependencies = [ [[package]] name = "openfang-api" -version = "0.3.35" +version = "0.3.36" dependencies = [ "async-trait", "axum", @@ -3829,7 +3829,7 @@ dependencies = [ [[package]] name = "openfang-channels" -version = "0.3.35" +version = "0.3.36" dependencies = [ "async-trait", "axum", @@ -3861,7 +3861,7 @@ dependencies = [ [[package]] name = "openfang-cli" -version = "0.3.35" +version = "0.3.36" dependencies = [ "clap", "clap_complete", @@ -3888,7 +3888,7 @@ dependencies = [ [[package]] name = "openfang-desktop" -version = "0.3.35" +version = "0.3.36" dependencies = [ "axum", "open", @@ -3914,7 +3914,7 @@ dependencies = [ [[package]] name = "openfang-extensions" -version = "0.3.35" +version = "0.3.36" dependencies = [ "aes-gcm", "argon2", @@ -3942,7 +3942,7 @@ dependencies = [ [[package]] name = "openfang-hands" -version = "0.3.35" +version = "0.3.36" dependencies = [ "chrono", "dashmap", @@ -3959,7 +3959,7 @@ dependencies = [ [[package]] name = "openfang-kernel" -version = "0.3.35" +version = "0.3.36" dependencies = [ "async-trait", "chrono", @@ -3996,7 +3996,7 @@ dependencies = [ [[package]] name = "openfang-memory" -version = "0.3.35" +version = "0.3.36" dependencies = [ "async-trait", "chrono", @@ -4015,7 +4015,7 @@ dependencies = [ [[package]] name = "openfang-migrate" -version = "0.3.35" +version = "0.3.36" dependencies = [ "chrono", "dirs 6.0.0", @@ -4034,7 +4034,7 @@ dependencies = [ [[package]] name = "openfang-runtime" -version = "0.3.35" +version = "0.3.36" dependencies = [ "anyhow", "async-trait", @@ -4068,7 +4068,7 @@ dependencies = [ [[package]] name = "openfang-skills" -version = "0.3.35" +version = "0.3.36" dependencies = [ "chrono", "hex", @@ -4091,7 +4091,7 @@ dependencies = [ [[package]] name = "openfang-types" -version = "0.3.35" +version = "0.3.36" dependencies = [ "async-trait", "chrono", @@ -4110,7 +4110,7 @@ dependencies = [ [[package]] name = "openfang-wire" -version = "0.3.35" +version = "0.3.36" dependencies = [ "async-trait", "chrono", @@ -8772,7 +8772,7 @@ checksum = "b9cc00251562a284751c9973bace760d86c0276c471b4be569fe6b068ee97a56" [[package]] name = "xtask" -version = "0.3.35" +version = "0.3.36" [[package]] name = "yoke" diff --git a/Cargo.toml b/Cargo.toml index ec3fae55..434313c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ members = [ ] [workspace.package] -version = "0.3.36" +version = "0.3.37" edition = "2021" license = "Apache-2.0 OR MIT" repository = "https://github.com/RightNow-AI/openfang" diff --git a/crates/openfang-api/src/channel_bridge.rs b/crates/openfang-api/src/channel_bridge.rs index 11693b00..5a78a048 100644 --- a/crates/openfang-api/src/channel_bridge.rs +++ b/crates/openfang-api/src/channel_bridge.rs @@ -1066,6 +1066,7 @@ pub async fn start_channel_bridge_with_config( token, tg_config.allowed_users.clone(), poll_interval, + tg_config.api_url.clone(), )); adapters.push((adapter, tg_config.default_agent.clone())); } diff --git a/crates/openfang-api/src/routes.rs b/crates/openfang-api/src/routes.rs index 0c4f73a8..5a904828 100644 --- a/crates/openfang-api/src/routes.rs +++ b/crates/openfang-api/src/routes.rs @@ -6744,14 +6744,33 @@ pub async fn set_provider_key( // Auto-switch default provider if current default has no working key. // This fixes the common case where a user adds e.g. a Gemini key via dashboard // but their agent still tries to use the previous provider (which has no key). - let current_provider = &state.kernel.config.default_model.provider; - let current_key_env = &state.kernel.config.default_model.api_key_env; + // + // Read the effective default from the hot-reload override (if set) rather than + // the stale boot-time config — a previous set_provider_key call may have already + // switched the default. + let (current_provider, current_key_env) = { + let guard = state + .kernel + .default_model_override + .read() + .unwrap_or_else(|e| e.into_inner()); + match guard.as_ref() { + Some(dm) => (dm.provider.clone(), dm.api_key_env.clone()), + None => ( + state.kernel.config.default_model.provider.clone(), + state.kernel.config.default_model.api_key_env.clone(), + ), + } + }; let current_has_key = if current_key_env.is_empty() { false } else { - std::env::var(current_key_env).is_ok() + std::env::var(¤t_key_env) + .ok() + .filter(|v| !v.is_empty()) + .is_some() }; - let switched = if !current_has_key && *current_provider != name { + let switched = if !current_has_key && current_provider != name { // Find a default model for the newly-keyed provider let default_model = { let catalog = state.kernel.model_catalog.read().unwrap_or_else(|e| e.into_inner()); @@ -6771,10 +6790,57 @@ pub async fn set_provider_key( } else { let _ = std::fs::write(&config_path, update_toml); } + + // Hot-update the in-memory default model override so resolve_driver() + // immediately creates drivers for the new provider — no restart needed. + { + let new_dm = openfang_types::config::DefaultModelConfig { + provider: name.clone(), + model: model_id, + api_key_env: env_var.clone(), + base_url: None, + }; + let mut guard = state + .kernel + .default_model_override + .write() + .unwrap_or_else(|e| e.into_inner()); + *guard = Some(new_dm); + } true } else { false } + } else if current_provider == name { + // User is saving a key for the CURRENT default provider. The env var is + // already set (set_var above), but we must ensure default_model_override + // has the correct api_key_env so resolve_driver reads the right variable. + let needs_update = { + let guard = state + .kernel + .default_model_override + .read() + .unwrap_or_else(|e| e.into_inner()); + match guard.as_ref() { + Some(dm) => dm.api_key_env != env_var, + None => state.kernel.config.default_model.api_key_env != env_var, + } + }; + if needs_update { + let mut guard = state + .kernel + .default_model_override + .write() + .unwrap_or_else(|e| e.into_inner()); + let base = guard + .clone() + .unwrap_or_else(|| state.kernel.config.default_model.clone()); + *guard = Some(openfang_types::config::DefaultModelConfig { + api_key_env: env_var.clone(), + ..base + }); + } + false } else { false }; @@ -6783,7 +6849,7 @@ pub async fn set_provider_key( if switched { resp["switched_default"] = serde_json::json!(true); resp["message"] = serde_json::json!( - format!("API key saved. Default provider switched to '{}'. Restart the daemon to apply.", name) + format!("API key saved and default provider switched to '{}'.", name) ); } diff --git a/crates/openfang-channels/src/bridge.rs b/crates/openfang-channels/src/bridge.rs index e687de46..3527dbd0 100644 --- a/crates/openfang-channels/src/bridge.rs +++ b/crates/openfang-channels/src/bridge.rs @@ -5,7 +5,10 @@ use crate::formatter; use crate::router::AgentRouter; -use crate::types::{ChannelAdapter, ChannelContent, ChannelMessage, ChannelUser}; +use crate::types::{ + default_phase_emoji, AgentPhase, ChannelAdapter, ChannelContent, ChannelMessage, ChannelUser, + LifecycleReaction, +}; use async_trait::async_trait; use dashmap::DashMap; use openfang_types::message::ContentBlock; @@ -380,6 +383,25 @@ async fn send_response( } } +/// Send a lifecycle reaction (best-effort, non-blocking for supported adapters). +/// +/// Silently ignores errors — reactions are non-critical UX polish. +/// For Telegram, the underlying HTTP call is already fire-and-forget (spawned internally), +/// so this await returns almost immediately. +async fn send_lifecycle_reaction( + adapter: &dyn ChannelAdapter, + user: &ChannelUser, + message_id: &str, + phase: AgentPhase, +) { + let reaction = LifecycleReaction { + emoji: default_phase_emoji(&phase).to_string(), + phase, + remove_previous: true, + }; + let _ = adapter.send_reaction(user, message_id, &reaction).await; +} + /// Dispatch a single incoming message — handles bot commands or routes to an agent. /// /// Applies per-channel policies (DM/group filtering, rate limiting, formatting, threading). @@ -698,15 +720,22 @@ async fn dispatch_message( // Send typing indicator (best-effort) let _ = adapter.send_typing(&message.sender).await; + // Lifecycle reaction: ⏳ Queued → 🤔 Thinking → ✅ Done / ❌ Error + let msg_id = &message.platform_message_id; + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Queued).await; + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Thinking).await; + // Send to agent and relay response match handle.send_message(agent_id, &text).await { Ok(response) => { + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Done).await; send_response(adapter, &message.sender, response, thread_id, output_format).await; handle .record_delivery(agent_id, ct_str, &message.sender.platform_id, true, None) .await; } Err(e) => { + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Error).await; warn!("Agent error for {agent_id}: {e}"); let err_msg = format!("Agent error: {e}"); send_response( @@ -880,14 +909,21 @@ async fn dispatch_with_blocks( let _ = adapter.send_typing(&message.sender).await; + // Lifecycle reaction: ⏳ Queued → 🤔 Thinking → ✅ Done / ❌ Error + let msg_id = &message.platform_message_id; + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Queued).await; + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Thinking).await; + match handle.send_message_with_blocks(agent_id, blocks).await { Ok(response) => { + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Done).await; send_response(adapter, &message.sender, response, thread_id, output_format).await; handle .record_delivery(agent_id, ct_str, &message.sender.platform_id, true, None) .await; } Err(e) => { + send_lifecycle_reaction(adapter, &message.sender, msg_id, AgentPhase::Error).await; warn!("Agent error for {agent_id}: {e}"); let err_msg = format!("Agent error: {e}"); send_response( diff --git a/crates/openfang-channels/src/discord.rs b/crates/openfang-channels/src/discord.rs index acf05c80..a9171696 100644 --- a/crates/openfang-channels/src/discord.rs +++ b/crates/openfang-channels/src/discord.rs @@ -612,6 +612,51 @@ mod tests { assert!(msg.is_none()); } + #[tokio::test] + async fn test_parse_discord_ignore_bots_false_allows_other_bots() { + let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); + let d = serde_json::json!({ + "id": "msg1", + "channel_id": "ch1", + "content": "Bot message", + "author": { + "id": "other_bot", + "username": "somebot", + "discriminator": "0", + "bot": true + }, + "timestamp": "2024-01-01T00:00:00+00:00" + }); + + // With ignore_bots=false, other bots' messages should be allowed + let msg = parse_discord_message(&d, &bot_id, &[], &[], false).await; + assert!(msg.is_some()); + let msg = msg.unwrap(); + assert_eq!(msg.sender.display_name, "somebot"); + assert!(matches!(msg.content, ChannelContent::Text(ref t) if t == "Bot message")); + } + + #[tokio::test] + async fn test_parse_discord_ignore_bots_false_still_filters_self() { + let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); + let d = serde_json::json!({ + "id": "msg1", + "channel_id": "ch1", + "content": "My own message", + "author": { + "id": "bot123", + "username": "openfang", + "discriminator": "0", + "bot": true + }, + "timestamp": "2024-01-01T00:00:00+00:00" + }); + + // Even with ignore_bots=false, the bot's own messages must still be filtered + let msg = parse_discord_message(&d, &bot_id, &[], &[], false).await; + assert!(msg.is_none()); + } + #[tokio::test] async fn test_parse_discord_message_guild_filter() { let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); diff --git a/crates/openfang-channels/src/telegram.rs b/crates/openfang-channels/src/telegram.rs index 6620f711..059f663c 100644 --- a/crates/openfang-channels/src/telegram.rs +++ b/crates/openfang-channels/src/telegram.rs @@ -5,6 +5,7 @@ use crate::types::{ split_message, ChannelAdapter, ChannelContent, ChannelMessage, ChannelType, ChannelUser, + LifecycleReaction, }; use async_trait::async_trait; use futures::Stream; @@ -23,6 +24,9 @@ const INITIAL_BACKOFF: Duration = Duration::from_secs(1); /// Telegram long-polling timeout (seconds) — sent as the `timeout` parameter to getUpdates. const LONG_POLL_TIMEOUT: u64 = 30; +/// Default Telegram Bot API base URL. +const DEFAULT_API_URL: &str = "https://api.telegram.org"; + /// Telegram Bot API adapter using long-polling. pub struct TelegramAdapter { /// SECURITY: Bot token is zeroized on drop to prevent memory disclosure. @@ -30,6 +34,8 @@ pub struct TelegramAdapter { client: reqwest::Client, allowed_users: Vec, poll_interval: Duration, + /// Base URL for Telegram Bot API (supports proxies/mirrors). + api_base_url: String, shutdown_tx: Arc>, shutdown_rx: watch::Receiver, } @@ -39,13 +45,24 @@ impl TelegramAdapter { /// /// `token` is the raw bot token (read from env by the caller). /// `allowed_users` is the list of Telegram user IDs allowed to interact (empty = allow all). - pub fn new(token: String, allowed_users: Vec, poll_interval: Duration) -> Self { + /// `api_url` overrides the Telegram Bot API base URL (for proxies/mirrors). + pub fn new( + token: String, + allowed_users: Vec, + poll_interval: Duration, + api_url: Option, + ) -> Self { let (shutdown_tx, shutdown_rx) = watch::channel(false); + let api_base_url = api_url + .unwrap_or_else(|| DEFAULT_API_URL.to_string()) + .trim_end_matches('/') + .to_string(); Self { token: Zeroizing::new(token), client: reqwest::Client::new(), allowed_users, poll_interval, + api_base_url, shutdown_tx: Arc::new(shutdown_tx), shutdown_rx, } @@ -53,7 +70,7 @@ impl TelegramAdapter { /// Validate the bot token by calling `getMe`. pub async fn validate_token(&self) -> Result> { - let url = format!("https://api.telegram.org/bot{}/getMe", self.token.as_str()); + let url = format!("{}/bot{}/getMe", self.api_base_url, self.token.as_str()); let resp: serde_json::Value = self.client.get(&url).send().await?.json().await?; if resp["ok"].as_bool() != Some(true) { @@ -75,7 +92,8 @@ impl TelegramAdapter { text: &str, ) -> Result<(), Box> { let url = format!( - "https://api.telegram.org/bot{}/sendMessage", + "{}/bot{}/sendMessage", + self.api_base_url, self.token.as_str() ); @@ -111,7 +129,8 @@ impl TelegramAdapter { caption: Option<&str>, ) -> Result<(), Box> { let url = format!( - "https://api.telegram.org/bot{}/sendPhoto", + "{}/bot{}/sendPhoto", + self.api_base_url, self.token.as_str() ); let mut body = serde_json::json!({ @@ -138,7 +157,8 @@ impl TelegramAdapter { filename: &str, ) -> Result<(), Box> { let url = format!( - "https://api.telegram.org/bot{}/sendDocument", + "{}/bot{}/sendDocument", + self.api_base_url, self.token.as_str() ); let body = serde_json::json!({ @@ -161,7 +181,8 @@ impl TelegramAdapter { voice_url: &str, ) -> Result<(), Box> { let url = format!( - "https://api.telegram.org/bot{}/sendVoice", + "{}/bot{}/sendVoice", + self.api_base_url, self.token.as_str() ); let body = serde_json::json!({ @@ -184,7 +205,8 @@ impl TelegramAdapter { lon: f64, ) -> Result<(), Box> { let url = format!( - "https://api.telegram.org/bot{}/sendLocation", + "{}/bot{}/sendLocation", + self.api_base_url, self.token.as_str() ); let body = serde_json::json!({ @@ -203,7 +225,8 @@ impl TelegramAdapter { /// Call `sendChatAction` to show "typing..." indicator. async fn api_send_typing(&self, chat_id: i64) -> Result<(), Box> { let url = format!( - "https://api.telegram.org/bot{}/sendChatAction", + "{}/bot{}/sendChatAction", + self.api_base_url, self.token.as_str() ); let body = serde_json::json!({ @@ -213,6 +236,37 @@ impl TelegramAdapter { let _ = self.client.post(&url).json(&body).send().await?; Ok(()) } + + /// Call `setMessageReaction` on the Telegram API (fire-and-forget). + /// + /// Sets or replaces the bot's emoji reaction on a message. Each new call + /// automatically replaces the previous reaction, so there is no need to + /// explicitly remove old ones. + fn fire_reaction(&self, chat_id: i64, message_id: i64, emoji: &str) { + let url = format!( + "{}/bot{}/setMessageReaction", + self.api_base_url, + self.token.as_str() + ); + let body = serde_json::json!({ + "chat_id": chat_id, + "message_id": message_id, + "reaction": [{"type": "emoji", "emoji": emoji}], + }); + let client = self.client.clone(); + tokio::spawn(async move { + match client.post(&url).json(&body).send().await { + Ok(resp) if !resp.status().is_success() => { + let body_text = resp.text().await.unwrap_or_default(); + debug!("Telegram setMessageReaction failed: {body_text}"); + } + Err(e) => { + debug!("Telegram setMessageReaction error: {e}"); + } + _ => {} + } + }); + } } #[async_trait] @@ -238,7 +292,8 @@ impl ChannelAdapter for TelegramAdapter { // still be active on Telegram's side for ~30s, causing 409 errors. { let delete_url = format!( - "https://api.telegram.org/bot{}/deleteWebhook", + "{}/bot{}/deleteWebhook", + self.api_base_url, self.token.as_str() ); match self @@ -259,6 +314,7 @@ impl ChannelAdapter for TelegramAdapter { let client = self.client.clone(); let allowed_users = self.allowed_users.clone(); let poll_interval = self.poll_interval; + let api_base_url = self.api_base_url.clone(); let mut shutdown = self.shutdown_rx.clone(); tokio::spawn(async move { @@ -272,7 +328,7 @@ impl ChannelAdapter for TelegramAdapter { } // Build getUpdates request - let url = format!("https://api.telegram.org/bot{}/getUpdates", token.as_str()); + let url = format!("{}/bot{}/getUpdates", api_base_url, token.as_str()); let mut params = serde_json::json!({ "timeout": LONG_POLL_TIMEOUT, "allowed_updates": ["message", "edited_message"], @@ -371,7 +427,7 @@ impl ChannelAdapter for TelegramAdapter { } // Parse the message - let msg = match parse_telegram_update(update, &allowed_users, token.as_str(), &client).await { + let msg = match parse_telegram_update(update, &allowed_users, token.as_str(), &client, &api_base_url).await { Some(m) => m, None => continue, // filtered out or unparseable }; @@ -441,6 +497,23 @@ impl ChannelAdapter for TelegramAdapter { self.api_send_typing(chat_id).await } + async fn send_reaction( + &self, + user: &ChannelUser, + message_id: &str, + reaction: &LifecycleReaction, + ) -> Result<(), Box> { + let chat_id: i64 = user + .platform_id + .parse() + .map_err(|_| format!("Invalid Telegram chat_id: {}", user.platform_id))?; + let msg_id: i64 = message_id + .parse() + .map_err(|_| format!("Invalid Telegram message_id: {message_id}"))?; + self.fire_reaction(chat_id, msg_id, &reaction.emoji); + Ok(()) + } + async fn stop(&self) -> Result<(), Box> { let _ = self.shutdown_tx.send(true); Ok(()) @@ -454,8 +527,9 @@ async fn telegram_get_file_url( token: &str, client: &reqwest::Client, file_id: &str, + api_base_url: &str, ) -> Option { - let url = format!("https://api.telegram.org/bot{token}/getFile"); + let url = format!("{api_base_url}/bot{token}/getFile"); let resp = client .post(&url) .json(&serde_json::json!({"file_id": file_id})) @@ -468,7 +542,7 @@ async fn telegram_get_file_url( } let file_path = body["result"]["file_path"].as_str()?; Some(format!( - "https://api.telegram.org/file/bot{token}/{file_path}" + "{api_base_url}/file/bot{token}/{file_path}" )) } @@ -477,6 +551,7 @@ async fn parse_telegram_update( allowed_users: &[String], token: &str, client: &reqwest::Client, + api_base_url: &str, ) -> Option { let message = update .get("message") @@ -541,7 +616,7 @@ async fn parse_telegram_update( .and_then(|p| p["file_id"].as_str()) .unwrap_or(""); let caption = message["caption"].as_str().map(String::from); - match telegram_get_file_url(token, client, file_id).await { + match telegram_get_file_url(token, client, file_id, api_base_url).await { Some(url) => ChannelContent::Image { url, caption }, None => ChannelContent::Text(format!( "[Photo received{}]", @@ -554,14 +629,14 @@ async fn parse_telegram_update( .as_str() .unwrap_or("document") .to_string(); - match telegram_get_file_url(token, client, file_id).await { + match telegram_get_file_url(token, client, file_id, api_base_url).await { Some(url) => ChannelContent::File { url, filename }, None => ChannelContent::Text(format!("[Document received: {filename}]")), } } else if message.get("voice").is_some() { let file_id = message["voice"]["file_id"].as_str().unwrap_or(""); let duration = message["voice"]["duration"].as_u64().unwrap_or(0) as u32; - match telegram_get_file_url(token, client, file_id).await { + match telegram_get_file_url(token, client, file_id, api_base_url).await { Some(url) => ChannelContent::Voice { url, duration_seconds: duration, @@ -685,7 +760,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); assert_eq!(msg.channel, ChannelType::Telegram); assert_eq!(msg.sender.display_name, "Alice Smith"); assert_eq!(msg.sender.platform_id, "111222333"); @@ -717,7 +792,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); match &msg.content { ChannelContent::Command { name, args } => { assert_eq!(name, "agent"); @@ -749,17 +824,17 @@ mod tests { let client = test_client(); // Empty allowed_users = allow all - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await; + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await; assert!(msg.is_some()); // Non-matching allowed_users = filter out let blocked: Vec = vec!["111".to_string(), "222".to_string()]; - let msg = parse_telegram_update(&update, &blocked, "fake:token", &client).await; + let msg = parse_telegram_update(&update, &blocked, "fake:token", &client, DEFAULT_API_URL).await; assert!(msg.is_none()); // Matching allowed_users = allow let allowed: Vec = vec!["999".to_string()]; - let msg = parse_telegram_update(&update, &allowed, "fake:token", &client).await; + let msg = parse_telegram_update(&update, &allowed, "fake:token", &client, DEFAULT_API_URL).await; assert!(msg.is_some()); } @@ -785,7 +860,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); assert_eq!(msg.channel, ChannelType::Telegram); assert_eq!(msg.sender.display_name, "Alice Smith"); assert!(matches!(msg.content, ChannelContent::Text(ref t) if t == "Edited message!")); @@ -821,7 +896,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); match &msg.content { ChannelContent::Command { name, args } => { assert_eq!(name, "agents"); @@ -845,7 +920,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); assert!(matches!(msg.content, ChannelContent::Location { .. })); } @@ -869,7 +944,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); // With a fake token, getFile will fail, so we get a text fallback match &msg.content { ChannelContent::Text(t) => { @@ -904,7 +979,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); match &msg.content { ChannelContent::Text(t) => { assert!(t.contains("Document received")); @@ -935,7 +1010,7 @@ mod tests { }); let client = test_client(); - let msg = parse_telegram_update(&update, &[], "fake:token", &client).await.unwrap(); + let msg = parse_telegram_update(&update, &[], "fake:token", &client, DEFAULT_API_URL).await.unwrap(); match &msg.content { ChannelContent::Text(t) => { assert!(t.contains("Voice message")); diff --git a/crates/openfang-cli/src/main.rs b/crates/openfang-cli/src/main.rs index 6c6e53fa..beade43c 100644 --- a/crates/openfang-cli/src/main.rs +++ b/crates/openfang-cli/src/main.rs @@ -1301,10 +1301,18 @@ fn launch_desktop_app(_openfang_dir: &std::path::Path) { ui::blank(); if let Some(base) = find_daemon() { let url = format!("{base}/"); - let _ = open_in_browser(&url); - // Always print the URL — browser launch may silently fail - // (e.g., Chromium sandbox EPERM in containers) + if !open_in_browser(&url) { + // Browser launch failed entirely (e.g., sandbox EPERM, + // no display server, container environment). + ui::hint("Could not open a browser automatically."); + } + // Always print the URL so the user can open it manually, + // even when open_in_browser reported success — the spawned + // opener may still fail asynchronously. ui::hint(&format!("Dashboard: {url}")); + } else { + ui::hint("Daemon is not running. Start it with: openfang start"); + ui::hint("Then open: http://127.0.0.1:4200"); } } } @@ -2974,16 +2982,31 @@ pub(crate) fn open_in_browser(url: &str) -> bool { } #[cfg(target_os = "linux")] { - // Detach from parent to avoid inheriting sandbox restrictions. - // Some Chromium-based browsers fail with EPERM when launched from - // restricted environments (containers, snaps, flatpaks). - std::process::Command::new("xdg-open") - .arg(url) - .stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - .is_ok() + // Try multiple openers in order. xdg-open is the standard, but it + // (or the browser it launches) can fail with EPERM in sandboxed + // environments (containers, Snap, Flatpak, user-namespace + // restrictions). Fall through to alternatives if any opener fails. + let openers = [ + "xdg-open", + "sensible-browser", + "x-www-browser", + "firefox", + "google-chrome", + "chromium", + "chromium-browser", + ]; + for opener in &openers { + let result = std::process::Command::new(opener) + .arg(url) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn(); + if result.is_ok() { + return true; + } + } + false } #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "linux")))] { diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index f835dbd6..1dde28da 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -4000,25 +4000,36 @@ impl OpenFangKernel { /// Resolve the LLM driver for an agent. /// - /// If the agent's manifest specifies a different provider than the kernel default, - /// a dedicated driver is created. Otherwise the kernel's default driver is reused. + /// Always creates a fresh driver using current environment variables so that + /// API keys saved via the dashboard (`set_provider_key`) take effect immediately + /// without requiring a daemon restart. Uses the hot-reloaded default model + /// override when available. /// If fallback models are configured, wraps the primary in a `FallbackDriver`. fn resolve_driver(&self, manifest: &AgentManifest) -> KernelResult> { let agent_provider = &manifest.model.provider; - let default_provider = &self.config.default_model.provider; - // If agent uses same provider as kernel default and has no custom overrides, reuse + // Use the effective default model: hot-reloaded override takes priority + // over the boot-time config. This ensures that when a user saves a new + // API key via the dashboard and the default provider is switched, + // resolve_driver sees the updated provider/model/api_key_env. + let override_guard = self + .default_model_override + .read() + .unwrap_or_else(|e: std::sync::PoisonError<_>| e.into_inner()); + let effective_default = override_guard + .as_ref() + .unwrap_or(&self.config.default_model); + let default_provider = &effective_default.provider; + let has_custom_key = manifest.model.api_key_env.is_some(); let has_custom_url = manifest.model.base_url.is_some(); - let primary = if agent_provider == default_provider && !has_custom_key && !has_custom_url { - Arc::clone(&self.default_driver) - } else { - // Create a dedicated driver for this agent. - // - // IMPORTANT: When the agent's provider differs from the default, - // we must NOT pass the default provider's API key. Instead, pass None - // so create_driver() can look up the correct env var for the target provider. + // Always create a fresh driver by reading current env vars. + // This ensures API keys saved at runtime (via dashboard POST + // /api/providers/{name}/key which calls std::env::set_var) are + // picked up immediately — the boot-time default_driver cache is + // only used as a final fallback when driver creation fails. + let primary = { let api_key = if has_custom_key { // Agent explicitly set an API key env var — use it manifest @@ -4027,8 +4038,13 @@ impl OpenFangKernel { .as_ref() .and_then(|env| std::env::var(env).ok()) } else if agent_provider == default_provider { - // Same provider — use default key - std::env::var(&self.config.default_model.api_key_env).ok() + // Same provider as effective default — use its env var + if !effective_default.api_key_env.is_empty() { + std::env::var(&effective_default.api_key_env).ok() + } else { + let env_var = self.config.resolve_api_key_env(agent_provider); + std::env::var(&env_var).ok() + } } else { // Different provider — check auth profiles, provider_api_keys, // and convention-based env var. For custom providers (not in the @@ -4041,8 +4057,7 @@ impl OpenFangKernel { let base_url = if has_custom_url { manifest.model.base_url.clone() } else if agent_provider == default_provider { - self.config - .default_model + effective_default .base_url .clone() .or_else(|| self.config.provider_urls.get(agent_provider.as_str()).cloned()) @@ -4057,9 +4072,27 @@ impl OpenFangKernel { base_url, }; - drivers::create_driver(&driver_config).map_err(|e| { - KernelError::BootFailed(format!("Agent LLM driver init failed: {e}")) - })? + match drivers::create_driver(&driver_config) { + Ok(d) => d, + Err(e) => { + // If fresh driver creation fails (e.g. key not yet set for this + // provider), fall back to the boot-time default driver. This + // keeps existing agents working while the user is still + // configuring providers via the dashboard. + if agent_provider == default_provider && !has_custom_key && !has_custom_url { + debug!( + provider = %agent_provider, + error = %e, + "Fresh driver creation failed, falling back to boot-time default" + ); + Arc::clone(&self.default_driver) + } else { + return Err(KernelError::BootFailed(format!( + "Agent LLM driver init failed: {e}" + ))); + } + } + } }; // If fallback models are configured, wrap in FallbackDriver @@ -4851,6 +4884,10 @@ fn infer_provider_from_model(model: &str) -> Option { | "openrouter" | "volcengine" | "doubao" | "dashscope" => { return Some(prefix.to_string()); } + // "kimi" is a brand alias for moonshot + "kimi" => { + return Some("moonshot".to_string()); + } _ => {} } } @@ -4884,6 +4921,8 @@ fn infer_provider_from_model(model: &str) -> Option { Some("qianfan".to_string()) } else if lower.starts_with("abab") { Some("minimax".to_string()) + } else if lower.starts_with("moonshot") || lower.starts_with("kimi") { + Some("moonshot".to_string()) } else { None } diff --git a/crates/openfang-runtime/src/agent_loop.rs b/crates/openfang-runtime/src/agent_loop.rs index abd8ca1a..bd47150d 100644 --- a/crates/openfang-runtime/src/agent_loop.rs +++ b/crates/openfang-runtime/src/agent_loop.rs @@ -1799,12 +1799,20 @@ pub async fn run_agent_loop_streaming( Err(OpenFangError::MaxIterationsExceeded(max_iterations)) } -/// Recover tool calls that LLMs (Groq/Llama, DeepSeek) output as plain text -/// instead of the proper `tool_calls` API field. +/// Recover tool calls that LLMs output as plain text instead of the proper +/// `tool_calls` API field. Covers Groq/Llama, DeepSeek, Qwen, and Ollama models. /// -/// Parses patterns like `{"key":"value"}` from -/// the model's text output, validates tool names against the available tools, -/// and returns synthetic `ToolCall` entries. +/// Supported patterns: +/// 1. `{"key":"value"}` +/// 2. `tool_name{"key":"value"}` +/// 3. `tool_name{"key":"value"}` +/// 4. Markdown code blocks containing `tool_name {"key":"value"}` +/// 5. Backtick-wrapped `tool_name {"key":"value"}` +/// 6. `[TOOL_CALL]...[/TOOL_CALL]` blocks (JSON or arrow syntax) — issue #354 +/// 7. `{"name":"tool","arguments":{...}}` — Qwen3, issue #332 +/// 8. Bare JSON `{"name":"tool","arguments":{...}}` objects (last resort, only if no tags found) +/// +/// Validates tool names against available tools and returns synthetic `ToolCall` entries. fn recover_text_tool_calls(text: &str, available_tools: &[ToolDefinition]) -> Vec { let mut calls = Vec::new(); let tool_names: Vec<&str> = available_tools.iter().map(|t| t.name.as_str()).collect(); @@ -2053,9 +2061,348 @@ fn recover_text_tool_calls(text: &str, available_tools: &[ToolDefinition]) -> Ve } } + // Pattern 6: [TOOL_CALL]...[/TOOL_CALL] blocks (Ollama models like Qwen, issue #354) + // Handles both JSON args and custom `{tool => "name", args => {--key "value"}}` syntax. + search_from = 0; + while let Some(start) = text[search_from..].find("[TOOL_CALL]") { + let abs_start = search_from + start; + let after_tag = abs_start + "[TOOL_CALL]".len(); + + let Some(close_offset) = text[after_tag..].find("[/TOOL_CALL]") else { + search_from = after_tag; + continue; + }; + let inner = text[after_tag..after_tag + close_offset].trim(); + search_from = after_tag + close_offset + "[/TOOL_CALL]".len(); + + // Try standard JSON first: {"name":"tool","arguments":{...}} + if let Some((tool_name, input)) = parse_json_tool_call_object(inner, &tool_names) { + if !calls + .iter() + .any(|c| c.name == tool_name && c.input == input) + { + info!( + tool = tool_name.as_str(), + "Recovered tool call from [TOOL_CALL] block (JSON)" + ); + calls.push(ToolCall { + id: format!("recovered_{}", uuid::Uuid::new_v4()), + name: tool_name, + input, + }); + } + continue; + } + + // Custom arrow syntax: {tool => "name", args => {--key "value"}} + if let Some((tool_name, input)) = + parse_arrow_syntax_tool_call(inner, &tool_names) + { + if !calls + .iter() + .any(|c| c.name == tool_name && c.input == input) + { + info!( + tool = tool_name.as_str(), + "Recovered tool call from [TOOL_CALL] block (arrow syntax)" + ); + calls.push(ToolCall { + id: format!("recovered_{}", uuid::Uuid::new_v4()), + name: tool_name, + input, + }); + } + } + } + + // Pattern 7: JSON (Qwen3 models on Ollama, issue #332) + search_from = 0; + while let Some(start) = text[search_from..].find("") { + let abs_start = search_from + start; + let after_tag = abs_start + "".len(); + + let Some(close_offset) = text[after_tag..].find("") else { + search_from = after_tag; + continue; + }; + let inner = text[after_tag..after_tag + close_offset].trim(); + search_from = after_tag + close_offset + "".len(); + + if let Some((tool_name, input)) = parse_json_tool_call_object(inner, &tool_names) { + if !calls + .iter() + .any(|c| c.name == tool_name && c.input == input) + { + info!( + tool = tool_name.as_str(), + "Recovered tool call from block" + ); + calls.push(ToolCall { + id: format!("recovered_{}", uuid::Uuid::new_v4()), + name: tool_name, + input, + }); + } + } + } + + // Pattern 8: Bare JSON tool call objects in text (common Ollama fallback) + // Matches: {"name":"tool_name","arguments":{"key":"value"}} not already inside tags + // Only try this if no calls were found by tag-based patterns, to avoid false positives. + if calls.is_empty() { + // Scan for JSON objects that look like tool calls + let mut scan_from = 0; + while let Some(brace_start) = text[scan_from..].find('{') { + let abs_brace = scan_from + brace_start; + // Try to parse a JSON object starting here + if let Some((tool_name, input)) = + try_parse_bare_json_tool_call(&text[abs_brace..], &tool_names) + { + if !calls + .iter() + .any(|c| c.name == tool_name && c.input == input) + { + info!( + tool = tool_name.as_str(), + "Recovered tool call from bare JSON object in text" + ); + calls.push(ToolCall { + id: format!("recovered_{}", uuid::Uuid::new_v4()), + name: tool_name, + input, + }); + } + } + scan_from = abs_brace + 1; + } + } + calls } +/// Parse a JSON object that represents a tool call. +/// Supports formats: +/// - `{"name":"tool","arguments":{"key":"value"}}` +/// - `{"name":"tool","parameters":{"key":"value"}}` +/// - `{"function":"tool","arguments":{"key":"value"}}` +/// - `{"tool":"tool_name","args":{"key":"value"}}` +fn parse_json_tool_call_object( + text: &str, + tool_names: &[&str], +) -> Option<(String, serde_json::Value)> { + let obj: serde_json::Value = serde_json::from_str(text).ok()?; + let obj = obj.as_object()?; + + // Extract tool name from various field names + let name = obj + .get("name") + .or_else(|| obj.get("function")) + .or_else(|| obj.get("tool")) + .and_then(|v| v.as_str())?; + + if !tool_names.contains(&name) { + return None; + } + + // Extract arguments from various field names + let args = obj + .get("arguments") + .or_else(|| obj.get("parameters")) + .or_else(|| obj.get("args")) + .or_else(|| obj.get("input")) + .cloned() + .unwrap_or(serde_json::json!({})); + + // If arguments is a string (some models stringify it), try to parse it + let args = if let Some(s) = args.as_str() { + serde_json::from_str(s).unwrap_or(serde_json::json!({})) + } else { + args + }; + + Some((name.to_string(), args)) +} + +/// Parse the custom arrow syntax used by some Ollama models: +/// `{tool => "name", args => {--key "value"}}` or `{tool => "name", args => {"key":"value"}}` +fn parse_arrow_syntax_tool_call( + text: &str, + tool_names: &[&str], +) -> Option<(String, serde_json::Value)> { + // Extract tool name: look for `tool => "name"` or `tool=>"name"` + let tool_marker_pos = text.find("tool")?; + let after_tool = &text[tool_marker_pos + 4..]; + // Skip whitespace and `=>` + let after_arrow = after_tool.trim_start(); + let after_arrow = after_arrow.strip_prefix("=>")?; + let after_arrow = after_arrow.trim_start(); + + // Extract quoted tool name + let tool_name = if let Some(stripped) = after_arrow.strip_prefix('"') { + let end_quote = stripped.find('"')?; + &stripped[..end_quote] + } else { + // Unquoted: take until comma, whitespace, or '}' + let end = after_arrow + .find(|c: char| c == ',' || c == '}' || c.is_whitespace()) + .unwrap_or(after_arrow.len()); + &after_arrow[..end] + }; + + if tool_name.is_empty() || !tool_names.contains(&tool_name) { + return None; + } + + // Extract args: look for `args => {` or `args=>{` + let args_value = if let Some(args_pos) = text.find("args") { + let after_args = &text[args_pos + 4..]; + let after_args = after_args.trim_start(); + let after_args = after_args.strip_prefix("=>")?; + let after_args = after_args.trim_start(); + + if after_args.starts_with('{') { + // Try standard JSON parse first + if let Ok(v) = serde_json::from_str::(after_args) { + v + } else { + // Parse `--key "value"` / `--key value` style args + parse_dash_dash_args(after_args) + } + } else { + serde_json::json!({}) + } + } else { + serde_json::json!({}) + }; + + Some((tool_name.to_string(), args_value)) +} + +/// Parse `{--key "value", --flag}` or `{--command "ls -F /"}` style arguments +/// into a JSON object. +fn parse_dash_dash_args(text: &str) -> serde_json::Value { + let mut map = serde_json::Map::new(); + + // Strip outer braces — find matching close brace + let inner = if text.starts_with('{') { + let mut depth = 0; + let mut end = text.len(); + for (i, c) in text.char_indices() { + match c { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + end = i; + break; + } + } + _ => {} + } + } + text[1..end].trim() + } else { + text.trim() + }; + + // Parse --key "value" or --key value pairs + let mut remaining = inner; + while let Some(dash_pos) = remaining.find("--") { + remaining = &remaining[dash_pos + 2..]; + + // Extract key: runs until whitespace, '=', '"', or end + let key_end = remaining + .find(|c: char| c.is_whitespace() || c == '=' || c == '"') + .unwrap_or(remaining.len()); + let key = &remaining[..key_end]; + if key.is_empty() { + continue; + } + remaining = &remaining[key_end..]; + remaining = remaining.trim_start(); + + // Skip optional '=' + if remaining.starts_with('=') { + remaining = remaining[1..].trim_start(); + } + + // Extract value + if remaining.starts_with('"') { + // Quoted value — find closing quote + if let Some(end_quote) = remaining[1..].find('"') { + let value = &remaining[1..1 + end_quote]; + map.insert( + key.to_string(), + serde_json::Value::String(value.to_string()), + ); + remaining = &remaining[2 + end_quote..]; + } else { + // Unclosed quote — take rest + let value = &remaining[1..]; + map.insert( + key.to_string(), + serde_json::Value::String(value.to_string()), + ); + break; + } + } else { + // Unquoted value — take until next --, comma, }, or end + let val_end = remaining + .find([',', '}']) + .or_else(|| remaining.find("--")) + .unwrap_or(remaining.len()); + let value = remaining[..val_end].trim(); + if !value.is_empty() { + map.insert( + key.to_string(), + serde_json::Value::String(value.to_string()), + ); + } else { + // Flag with no value — set to true + map.insert(key.to_string(), serde_json::Value::Bool(true)); + } + remaining = &remaining[val_end..]; + } + + // Skip comma separator + remaining = remaining.trim_start(); + if remaining.starts_with(',') { + remaining = remaining[1..].trim_start(); + } + } + + serde_json::Value::Object(map) +} + +/// Try to parse a bare JSON object as a tool call. +/// The JSON must have a "name"/"function"/"tool" field matching a known tool. +fn try_parse_bare_json_tool_call( + text: &str, + tool_names: &[&str], +) -> Option<(String, serde_json::Value)> { + // Find the end of this JSON object by counting braces + let mut depth = 0; + let mut end = 0; + for (i, c) in text.char_indices() { + match c { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + end = i + 1; + break; + } + } + _ => {} + } + } + if end == 0 { + return None; + } + + parse_json_tool_call_object(&text[..end], tool_names) +} + #[cfg(test)] mod tests { use super::*; @@ -2997,6 +3344,287 @@ mod tests { assert_eq!(calls.len(), 1); } + // --- Pattern 6: [TOOL_CALL]...[/TOOL_CALL] tests (issue #354) --- + + #[test] + fn test_recover_tool_call_block_json() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute shell command".into(), + input_schema: serde_json::json!({}), + }]; + let text = "[TOOL_CALL]\n{\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls -la\"}}\n[/TOOL_CALL]"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[0].input["command"], "ls -la"); + } + + #[test] + fn test_recover_tool_call_block_arrow_syntax() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute shell command".into(), + input_schema: serde_json::json!({}), + }]; + // Exact format from issue #354 + let text = "[TOOL_CALL]\n{tool => \"shell_exec\", args => {\n--command \"ls -F /\"\n}}\n[/TOOL_CALL]"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[0].input["command"], "ls -F /"); + } + + #[test] + fn test_recover_tool_call_block_unknown_tool() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "[TOOL_CALL]\n{\"name\": \"hack_system\", \"arguments\": {\"cmd\": \"rm -rf /\"}}\n[/TOOL_CALL]"; + let calls = recover_text_tool_calls(text, &tools); + assert!(calls.is_empty()); + } + + #[test] + fn test_recover_tool_call_block_multiple() { + let tools = vec![ + ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }, + ToolDefinition { + name: "file_read".into(), + description: "Read".into(), + input_schema: serde_json::json!({}), + }, + ]; + let text = "[TOOL_CALL]\n{\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls\"}}\n[/TOOL_CALL]\nSome text.\n[TOOL_CALL]\n{\"name\": \"file_read\", \"arguments\": {\"path\": \"/tmp/test.txt\"}}\n[/TOOL_CALL]"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 2); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[1].name, "file_read"); + } + + #[test] + fn test_recover_tool_call_block_unclosed() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + // Unclosed [TOOL_CALL] — pattern 6 skips it, but pattern 8 (bare JSON) + // still finds the valid JSON tool call object. + let text = "[TOOL_CALL]\n{\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1, "Bare JSON fallback should recover this"); + assert_eq!(calls[0].name, "shell_exec"); + } + + // --- Pattern 7: JSON tests (Qwen3, issue #332) --- + + #[test] + fn test_recover_tool_call_xml_basic() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "\n{\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls -la\"}}\n"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[0].input["command"], "ls -la"); + } + + #[test] + fn test_recover_tool_call_xml_with_surrounding_text() { + let tools = vec![ToolDefinition { + name: "web_search".into(), + description: "Search".into(), + input_schema: serde_json::json!({}), + }]; + let text = "I'll search for that.\n\n\n{\"name\": \"web_search\", \"arguments\": {\"query\": \"rust async\"}}\n\n\nLet me get results."; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "web_search"); + assert_eq!(calls[0].input["query"], "rust async"); + } + + #[test] + fn test_recover_tool_call_xml_function_field() { + let tools = vec![ToolDefinition { + name: "file_read".into(), + description: "Read".into(), + input_schema: serde_json::json!({}), + }]; + let text = "{\"function\": \"file_read\", \"arguments\": {\"path\": \"/etc/hosts\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "file_read"); + } + + #[test] + fn test_recover_tool_call_xml_parameters_field() { + let tools = vec![ToolDefinition { + name: "web_fetch".into(), + description: "Fetch".into(), + input_schema: serde_json::json!({}), + }]; + let text = "{\"name\": \"web_fetch\", \"parameters\": {\"url\": \"https://example.com\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "web_fetch"); + assert_eq!(calls[0].input["url"], "https://example.com"); + } + + #[test] + fn test_recover_tool_call_xml_stringified_args() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "{\"name\": \"shell_exec\", \"arguments\": \"{\\\"command\\\": \\\"pwd\\\"}\"}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[0].input["command"], "pwd"); + } + + #[test] + fn test_recover_tool_call_xml_unknown_tool() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "{\"name\": \"hack_system\", \"arguments\": {\"cmd\": \"rm -rf /\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert!(calls.is_empty()); + } + + #[test] + fn test_recover_tool_call_xml_multiple() { + let tools = vec![ + ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }, + ToolDefinition { + name: "web_search".into(), + description: "Search".into(), + input_schema: serde_json::json!({}), + }, + ]; + let text = "{\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls\"}}\n{\"name\": \"web_search\", \"arguments\": {\"query\": \"rust\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 2); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[1].name, "web_search"); + } + + // --- Pattern 8: Bare JSON tool call object tests --- + + #[test] + fn test_recover_bare_json_tool_call() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "I'll run that: {\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls -la\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "shell_exec"); + assert_eq!(calls[0].input["command"], "ls -la"); + } + + #[test] + fn test_recover_bare_json_no_false_positive() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "The config looks like {\"debug\": true, \"level\": \"info\"}"; + let calls = recover_text_tool_calls(text, &tools); + assert!(calls.is_empty()); + } + + #[test] + fn test_recover_bare_json_skipped_when_tags_found() { + let tools = vec![ToolDefinition { + name: "shell_exec".into(), + description: "Execute".into(), + input_schema: serde_json::json!({}), + }]; + let text = "{\"command\":\"ls\"} {\"name\": \"shell_exec\", \"arguments\": {\"command\": \"pwd\"}}"; + let calls = recover_text_tool_calls(text, &tools); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].input["command"], "ls"); + } + + // --- Helper function tests --- + + #[test] + fn test_parse_dash_dash_args_basic() { + let result = parse_dash_dash_args("{--command \"ls -F /\"}"); + assert_eq!(result["command"], "ls -F /"); + } + + #[test] + fn test_parse_dash_dash_args_multiple() { + let result = parse_dash_dash_args("{--file \"test.txt\", --verbose}"); + assert_eq!(result["file"], "test.txt"); + assert_eq!(result["verbose"], true); + } + + #[test] + fn test_parse_dash_dash_args_unquoted_value() { + let result = parse_dash_dash_args("{--count 5}"); + assert_eq!(result["count"], "5"); + } + + #[test] + fn test_parse_json_tool_call_object_standard() { + let tool_names = vec!["shell_exec"]; + let result = parse_json_tool_call_object( + "{\"name\": \"shell_exec\", \"arguments\": {\"command\": \"ls\"}}", + &tool_names, + ); + assert!(result.is_some()); + let (name, args) = result.unwrap(); + assert_eq!(name, "shell_exec"); + assert_eq!(args["command"], "ls"); + } + + #[test] + fn test_parse_json_tool_call_object_function_field() { + let tool_names = vec!["web_fetch"]; + let result = parse_json_tool_call_object( + "{\"function\": \"web_fetch\", \"parameters\": {\"url\": \"https://x.com\"}}", + &tool_names, + ); + assert!(result.is_some()); + let (name, args) = result.unwrap(); + assert_eq!(name, "web_fetch"); + assert_eq!(args["url"], "https://x.com"); + } + + #[test] + fn test_parse_json_tool_call_object_unknown_tool() { + let tool_names = vec!["shell_exec"]; + let result = parse_json_tool_call_object( + "{\"name\": \"unknown\", \"arguments\": {}}", + &tool_names, + ); + assert!(result.is_none()); + } + // --- End-to-end integration test: text-as-tool-call recovery through agent loop --- /// Mock driver that simulates a Groq/Llama model outputting tool calls as text. diff --git a/crates/openfang-runtime/src/model_catalog.rs b/crates/openfang-runtime/src/model_catalog.rs index 573eed2e..8f4d3a80 100644 --- a/crates/openfang-runtime/src/model_catalog.rs +++ b/crates/openfang-runtime/src/model_catalog.rs @@ -790,7 +790,8 @@ fn builtin_aliases() -> HashMap { ("qwen", "qwen-plus"), ("glm", "glm-5-20250605"), ("ernie", "ernie-4.5-8k"), - ("kimi", "kimi-k2-0711"), + ("kimi", "kimi-k2"), + ("moonshot", "moonshot-v1-128k"), ("minimax", "MiniMax-M2.5"), ("minimax-m2.5", "MiniMax-M2.5"), ("minimax-m2.1", "MiniMax-M2.1"), @@ -2910,7 +2911,7 @@ fn builtin_models() -> Vec { aliases: vec![], }, ModelCatalogEntry { - id: "kimi-k2-0711".into(), + id: "kimi-k2".into(), display_name: "Kimi K2".into(), provider: "moonshot".into(), tier: ModelTier::Frontier, @@ -2921,10 +2922,10 @@ fn builtin_models() -> Vec { supports_tools: true, supports_vision: true, supports_streaming: true, - aliases: vec!["kimi-k2".into()], + aliases: vec![], }, ModelCatalogEntry { - id: "kimi-k2.5-0711".into(), + id: "kimi-k2.5".into(), display_name: "Kimi K2.5".into(), provider: "moonshot".into(), tier: ModelTier::Frontier, @@ -2935,7 +2936,7 @@ fn builtin_models() -> Vec { supports_tools: true, supports_vision: true, supports_streaming: true, - aliases: vec!["kimi-k2.5".into()], + aliases: vec![], }, // ══════════════════════════════════════════════════════════════ // Baidu Qianfan / ERNIE (3) diff --git a/crates/openfang-runtime/src/tool_runner.rs b/crates/openfang-runtime/src/tool_runner.rs index 9f98bae9..cf05a801 100644 --- a/crates/openfang-runtime/src/tool_runner.rs +++ b/crates/openfang-runtime/src/tool_runner.rs @@ -9,6 +9,7 @@ use crate::web_search::{parse_ddg_results, WebToolsContext}; use openfang_skills::registry::SkillRegistry; use openfang_types::taint::{TaintLabel, TaintSink, TaintedValue}; use openfang_types::tool::{ToolDefinition, ToolResult}; +use openfang_types::tool_compat::normalize_tool_name; use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -123,6 +124,10 @@ pub async fn execute_tool( docker_config: Option<&openfang_types::config::DockerSandboxConfig>, process_manager: Option<&crate::process_manager::ProcessManager>, ) -> ToolResult { + // Normalize the tool name through compat mappings so LLM-hallucinated aliases + // (e.g. "fs-write" → "file_write") resolve to the canonical OpenFang name. + let tool_name = normalize_tool_name(tool_name); + // Capability enforcement: reject tools not in the allowed list if let Some(allowed) = allowed_tools { if !allowed.iter().any(|t| t == tool_name) { @@ -3262,10 +3267,13 @@ mod tests { #[tokio::test] async fn test_file_read_missing() { + let bad_path = std::env::temp_dir() + .join("openfang_test_nonexistent_99999") + .join("file.txt"); let result = execute_tool( "test-id", "file_read", - &serde_json::json!({"path": "/nonexistent/file.txt"}), + &serde_json::json!({"path": bad_path.to_str().unwrap()}), None, None, None, @@ -3282,7 +3290,7 @@ mod tests { None, // process_manager ) .await; - assert!(result.is_error); + assert!(result.is_error, "Expected error but got: {}", result.content); } #[tokio::test] @@ -3471,10 +3479,14 @@ mod tests { #[tokio::test] async fn test_capability_enforcement_allowed() { let allowed = vec!["file_read".to_string()]; + // Use a cross-platform nonexistent path + let bad_path = std::env::temp_dir() + .join("openfang_test_nonexistent_12345") + .join("file.txt"); let result = execute_tool( "test-id", "file_read", - &serde_json::json!({"path": "/nonexistent/file.txt"}), + &serde_json::json!({"path": bad_path.to_str().unwrap()}), None, Some(&allowed), None, @@ -3492,8 +3504,81 @@ mod tests { ) .await; // Should fail for file-not-found, NOT for permission denied + assert!(result.is_error, "Expected error but got: {}", result.content); + assert!( + result.content.contains("Failed to read") || result.content.contains("not found") || result.content.contains("No such file"), + "Unexpected error: {}", result.content + ); + } + + #[tokio::test] + async fn test_capability_enforcement_aliased_tool_name() { + // Agent has "file_write" in allowed tools, but LLM calls "fs-write". + // After normalization, this should pass the capability check. + let allowed = vec![ + "file_read".to_string(), + "file_write".to_string(), + "file_list".to_string(), + "shell_exec".to_string(), + ]; + let result = execute_tool( + "test-id", + "fs-write", // LLM-hallucinated alias + &serde_json::json!({"path": "/nonexistent/file.txt", "content": "hello"}), + None, + Some(&allowed), + None, + None, + None, + None, + None, + None, + None, + None, // media_engine + None, // exec_policy + None, // tts_engine + None, // docker_config + None, // process_manager + ) + .await; + // Should NOT be "Permission denied" — it should normalize to file_write + // and pass the capability check. It will fail for other reasons (path validation). + assert!( + !result.content.contains("Permission denied"), + "fs-write should normalize to file_write and pass capability check, got: {}", + result.content + ); + } + + #[tokio::test] + async fn test_capability_enforcement_aliased_denied() { + // Agent does NOT have file_write, and LLM calls "fs-write" — should be denied. + let allowed = vec!["file_read".to_string()]; + let result = execute_tool( + "test-id", + "fs-write", + &serde_json::json!({"path": "/tmp/test.txt", "content": "hello"}), + None, + Some(&allowed), + None, + None, + None, + None, + None, + None, + None, + None, // media_engine + None, // exec_policy + None, // tts_engine + None, // docker_config + None, // process_manager + ) + .await; assert!(result.is_error); - assert!(result.content.contains("Failed to read")); + assert!( + result.content.contains("Permission denied"), + "fs-write should normalize to file_write which is not in allowed list" + ); } // --- Schedule parser tests --- diff --git a/crates/openfang-types/src/config.rs b/crates/openfang-types/src/config.rs index 9d2e25ee..99aca824 100644 --- a/crates/openfang-types/src/config.rs +++ b/crates/openfang-types/src/config.rs @@ -1593,6 +1593,10 @@ pub struct TelegramConfig { pub default_agent: Option, /// Polling interval in seconds. pub poll_interval_secs: u64, + /// Custom Telegram Bot API base URL for proxies or mirrors. + /// Defaults to `https://api.telegram.org` when not set. + #[serde(default)] + pub api_url: Option, /// Per-channel behavior overrides. #[serde(default)] pub overrides: ChannelOverrides, @@ -1605,6 +1609,7 @@ impl Default for TelegramConfig { allowed_users: vec![], default_agent: None, poll_interval_secs: 1, + api_url: None, overrides: ChannelOverrides::default(), } } @@ -3324,6 +3329,24 @@ mod tests { assert_eq!(dc.bot_token_env, "DISCORD_BOT_TOKEN"); assert!(dc.allowed_guilds.is_empty()); assert_eq!(dc.intents, 37376); + assert!(dc.ignore_bots); + } + + #[test] + fn test_discord_config_ignore_bots_deserialization() { + let toml_str = r#" + bot_token_env = "DISCORD_BOT_TOKEN" + ignore_bots = false + "#; + let dc: DiscordConfig = toml::from_str(toml_str).unwrap(); + assert!(!dc.ignore_bots); + + // Default (field omitted) should be true + let toml_str2 = r#" + bot_token_env = "DISCORD_BOT_TOKEN" + "#; + let dc2: DiscordConfig = toml::from_str(toml_str2).unwrap(); + assert!(dc2.ignore_bots); } #[test] diff --git a/crates/openfang-types/src/tool_compat.rs b/crates/openfang-types/src/tool_compat.rs index d1d9b225..eebe132f 100644 --- a/crates/openfang-types/src/tool_compat.rs +++ b/crates/openfang-types/src/tool_compat.rs @@ -24,10 +24,31 @@ pub fn map_tool_name(openclaw_name: &str) -> Option<&'static str> { "sessions_send" | "agent_message" => Some("agent_send"), "sessions_list" | "agents_list" | "agent_list" => Some("agent_list"), "sessions_spawn" => Some("agent_send"), + + // LLM-hallucinated aliases (fs-* style names) + "fs-read" | "fs_read" | "fsRead" | "readFile" => Some("file_read"), + "fs-write" | "fs_write" | "fsWrite" | "writeFile" => Some("file_write"), + "fs-list" | "fs_list" | "fsList" | "listFiles" | "list_dir" | "ls" => Some("file_list"), + "fs-exec" | "run" | "run_command" | "runCommand" | "execute" | "shell" => { + Some("shell_exec") + } + _ => None, } } +/// Normalize a tool name to its canonical OpenFang form. +/// +/// If the name is already a known OpenFang tool, returns it as-is. +/// Otherwise, tries to map it through [`map_tool_name`]. +/// Returns the original name if no mapping is found. +pub fn normalize_tool_name(name: &str) -> &str { + if is_known_openfang_tool(name) { + return name; + } + map_tool_name(name).unwrap_or(name) +} + /// Check if a tool name is a known OpenFang built-in tool. pub fn is_known_openfang_tool(name: &str) -> bool { matches!( @@ -104,11 +125,54 @@ mod tests { assert_eq!(map_tool_name("agent_list"), Some("agent_list")); assert_eq!(map_tool_name("sessions_spawn"), Some("agent_send")); + // LLM-hallucinated fs-* aliases + assert_eq!(map_tool_name("fs-read"), Some("file_read")); + assert_eq!(map_tool_name("fs_read"), Some("file_read")); + assert_eq!(map_tool_name("fsRead"), Some("file_read")); + assert_eq!(map_tool_name("readFile"), Some("file_read")); + assert_eq!(map_tool_name("fs-write"), Some("file_write")); + assert_eq!(map_tool_name("fs_write"), Some("file_write")); + assert_eq!(map_tool_name("fsWrite"), Some("file_write")); + assert_eq!(map_tool_name("writeFile"), Some("file_write")); + assert_eq!(map_tool_name("fs-list"), Some("file_list")); + assert_eq!(map_tool_name("fs_list"), Some("file_list")); + assert_eq!(map_tool_name("fsList"), Some("file_list")); + assert_eq!(map_tool_name("listFiles"), Some("file_list")); + assert_eq!(map_tool_name("list_dir"), Some("file_list")); + assert_eq!(map_tool_name("ls"), Some("file_list")); + assert_eq!(map_tool_name("fs-exec"), Some("shell_exec")); + assert_eq!(map_tool_name("run"), Some("shell_exec")); + assert_eq!(map_tool_name("run_command"), Some("shell_exec")); + assert_eq!(map_tool_name("runCommand"), Some("shell_exec")); + assert_eq!(map_tool_name("execute"), Some("shell_exec")); + assert_eq!(map_tool_name("shell"), Some("shell_exec")); + // Unknown assert_eq!(map_tool_name("unknown_tool"), None); assert_eq!(map_tool_name(""), None); } + #[test] + fn test_normalize_tool_name() { + // Known OpenFang tools pass through unchanged + assert_eq!(normalize_tool_name("file_read"), "file_read"); + assert_eq!(normalize_tool_name("file_write"), "file_write"); + assert_eq!(normalize_tool_name("shell_exec"), "shell_exec"); + assert_eq!(normalize_tool_name("web_search"), "web_search"); + + // Aliases get normalized to canonical names + assert_eq!(normalize_tool_name("fs-read"), "file_read"); + assert_eq!(normalize_tool_name("fs-write"), "file_write"); + assert_eq!(normalize_tool_name("fs-list"), "file_list"); + assert_eq!(normalize_tool_name("fs-exec"), "shell_exec"); + assert_eq!(normalize_tool_name("Read"), "file_read"); + assert_eq!(normalize_tool_name("Bash"), "shell_exec"); + + // Unknown names pass through unchanged + assert_eq!(normalize_tool_name("my_custom_tool"), "my_custom_tool"); + assert_eq!(normalize_tool_name("mcp_server_tool"), "mcp_server_tool"); + } + #[test] fn test_is_known_openfang_tool() { // All 23 built-in tools + location_get