From 63d20d1eb8702e96d44dd87f1f073ede1fe8c8bc Mon Sep 17 00:00:00 2001 From: xiaoxixi Date: Tue, 14 Jul 2026 11:00:39 +0800 Subject: [PATCH] refactor: eliminate build warnings and legacy evaluator Resolve strict Clippy findings across all targets, preserve public API compatibility with scoped lint exceptions, and fix sourced messages retaining media references. Replace meval and its future-incompatible nom dependency with a bounded internal expression parser and regression tests. --- Cargo.toml | 1 - src/agent/agent_loop.rs | 16 +- src/agent/context_compressor.rs | 31 ++-- src/agent/media_handler.rs | 6 + src/agent/sub_agent.rs | 34 ++-- src/bus/message.rs | 2 +- src/channels/cli_chat.rs | 2 +- src/channels/feishu.rs | 110 ++++++------- src/config/mod.rs | 4 +- src/memory/types.rs | 8 +- src/providers/anthropic.rs | 2 + src/providers/openai.rs | 13 +- src/session/mod.rs | 2 + src/session/session.rs | 46 +++--- src/session/session_id.rs | 7 +- src/storage/memory.rs | 2 +- src/storage/mod.rs | 4 +- src/tools/browser.rs | 33 ++-- src/tools/calculator.rs | 2 +- src/tools/chat_manager.rs | 10 +- src/tools/delegate.rs | 10 +- src/tools/expression.rs | 277 ++++++++++++++++++++++++++++++++ src/tools/mod.rs | 9 +- src/tools/pty.rs | 8 +- src/tools/send_message.rs | 2 +- tests/test_request_format.rs | 2 +- tests/test_scheduler.rs | 5 +- 27 files changed, 461 insertions(+), 187 deletions(-) create mode 100644 src/tools/expression.rs diff --git a/Cargo.toml b/Cargo.toml index e59fffd..ade2fef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,6 @@ base64 = "0.22" tempfile = "3" cron = "0.16" chrono-tz = "0.10" -meval = "0.2" ratatui = "0.30" crossterm = { version = "0.29", features = ["event-stream"] } termimad = "0.34" diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index a0b41be..88468b5 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -362,16 +362,16 @@ impl AgentLoop { let end = messages.len().saturating_sub(keep_recent); let start = 1; // protect system message at [0] if present let mut modified = 0; - for i in start..end { - if messages[i].role != "tool" { + for message in messages.iter_mut().take(end).skip(start) { + if message.role != "tool" { continue; } - if messages[i].content.len() <= max_chars { + if message.content.len() <= max_chars { continue; } - let tool_name = messages[i].tool_name.as_deref().unwrap_or("unknown"); - let chars = messages[i].content.len(); - messages[i].content = format!( + let tool_name = message.tool_name.as_deref().unwrap_or("unknown"); + let chars = message.content.len(); + message.content = format!( "[Tool output ({}) — {} chars, omitted from context]", tool_name, chars ); @@ -810,14 +810,14 @@ mod tests { fn test_should_execute_in_parallel_single_tool() { // Would need a proper setup with AgentLoop to test fully // For now, just verify the logic: single tool should return false - let calls = vec![ToolCall { + let calls = [ToolCall { id: "1".to_string(), name: "test".to_string(), arguments: serde_json::json!({}), }]; // If there's only 1 tool, should return false regardless - assert_eq!(calls.len() <= 1, true); + assert!(calls.len() <= 1); } #[test] diff --git a/src/agent/context_compressor.rs b/src/agent/context_compressor.rs index dcb3225..5d0b018 100644 --- a/src/agent/context_compressor.rs +++ b/src/agent/context_compressor.rs @@ -403,18 +403,17 @@ impl ContextCompressor { // Strip tool_calls from any assistant in the head whose results // were dropped (previously in the middle section). for msg in &mut truncated[..self.config.protect_first_n] { - if msg.role == "assistant" { - if let Some(ref tcs) = msg.tool_calls - && !tcs.is_empty() - { - let names: Vec<&str> = tcs.iter().map(|tc| tc.name.as_str()).collect(); - msg.content = format!( - "{}\n\n[Tool calls ({}) — results dropped during truncation]", - msg.content, - names.join(", ") - ); - msg.tool_calls = None; - } + if msg.role == "assistant" + && let Some(ref tcs) = msg.tool_calls + && !tcs.is_empty() + { + let names: Vec<&str> = tcs.iter().map(|tc| tc.name.as_str()).collect(); + msg.content = format!( + "{}\n\n[Tool calls ({}) — results dropped during truncation]", + msg.content, + names.join(", ") + ); + msg.tool_calls = None; } } @@ -564,9 +563,7 @@ impl ContextCompressor { // Add last user and everything after (protected) let last_user_idx = user_indices[user_indices.len() - 1]; - for i in last_user_idx..history.len() { - new_messages.push(history[i].clone()); - } + new_messages.extend_from_slice(&history[last_user_idx..]); // Remove orphan tool results whose declaring tool_calls were compressed away Self::repair_tool_pairs(&mut new_messages); @@ -786,7 +783,7 @@ mod tests { let mut messages = vec![ ChatMessage::user("Hello"), - ChatMessage::tool("call1", "bash", &"x".repeat(200)), + ChatMessage::tool("call1", "bash", "x".repeat(200)), ]; let modified = compressor.fast_trim_tool_results(&mut messages, 2); @@ -820,7 +817,7 @@ mod tests { let messages = vec![ ChatMessage::user("Hi"), - ChatMessage::tool("call1", "bash", &"x".repeat(3000)), + ChatMessage::tool("call1", "bash", "x".repeat(3000)), ]; let result = compressor diff --git a/src/agent/media_handler.rs b/src/agent/media_handler.rs index 5b1b72c..96b412c 100644 --- a/src/agent/media_handler.rs +++ b/src/agent/media_handler.rs @@ -67,6 +67,12 @@ pub struct MediaHandlerRegistry { handlers: HashMap>, } +impl Default for MediaHandlerRegistry { + fn default() -> Self { + Self::new() + } +} + impl MediaHandlerRegistry { pub fn new() -> Self { Self { diff --git a/src/agent/sub_agent.rs b/src/agent/sub_agent.rs index bc7c0e3..b42e2fa 100644 --- a/src/agent/sub_agent.rs +++ b/src/agent/sub_agent.rs @@ -158,12 +158,10 @@ impl SubAgentManager { fn get_skills_prompt(&self, tools: &ToolRegistry) -> Option { let has_get_skill = tools.iter().iter().any(|(name, _)| name == "get_skill"); - if has_get_skill { - if let Some(ref loader) = self.skills_loader { - let prompt = loader.build_skills_prompt(); - if !prompt.is_empty() { - return Some(prompt); - } + if has_get_skill && let Some(ref loader) = self.skills_loader { + let prompt = loader.build_skills_prompt(); + if !prompt.is_empty() { + return Some(prompt); } } None @@ -300,7 +298,7 @@ impl SubAgentManager { .collect(); let results = futures_util::future::join_all(futures).await; - Ok(results.into_iter().collect::, _>>()?) + results.into_iter().collect::, _>>() } pub async fn run_background( @@ -395,12 +393,12 @@ impl SubAgentManager { } let mut provider = create_provider(provider_config.clone()).ok(); - if let Some(ref mut p) = provider { - if let Some(ref s) = storage { - p.set_storage(s.clone()); - } + if let Some(ref mut p) = provider + && let Some(ref s) = storage + { + p.set_storage(s.clone()); } - let provider_result: Option> = provider.map(|p| Arc::from(p)); + let provider_result: Option> = provider.map(Arc::from); let result = match provider_result { Some(provider) => { @@ -566,12 +564,12 @@ impl SubAgentManager { pub async fn cancel_by_session(&self, session_id: &str) { // Cancel all running tasks for a session by checking DB - if let Some(ref s) = self.storage { - if let Ok(tasks) = s.list_background_tasks(session_id).await { - for task in &tasks { - if task.status == "pending" || task.status == "running" { - let _ = self.cancel_task(&task.id).await; - } + if let Some(ref s) = self.storage + && let Ok(tasks) = s.list_background_tasks(session_id).await + { + for task in &tasks { + if task.status == "pending" || task.status == "running" { + let _ = self.cancel_task(&task.id).await; } } } diff --git a/src/bus/message.rs b/src/bus/message.rs index 93315b7..b297eb9 100644 --- a/src/bus/message.rs +++ b/src/bus/message.rs @@ -286,7 +286,7 @@ pub struct OutboundMessage { impl OutboundMessage { pub fn is_stream_delta(&self) -> bool { - self.metadata.get("_stream_delta").is_some() + self.metadata.contains_key("_stream_delta") } } diff --git a/src/channels/cli_chat.rs b/src/channels/cli_chat.rs index 17a25e8..da34e7b 100644 --- a/src/channels/cli_chat.rs +++ b/src/channels/cli_chat.rs @@ -73,7 +73,7 @@ impl CliChatChannel { Ok((id, _title)) => id, Err(e) => { tracing::error!(error = %e, "Failed to create initial session"); - UnifiedSessionId::new("cli_chat", &chat_id, &crate::util::short_id()).to_string() + UnifiedSessionId::new("cli_chat", &chat_id, crate::util::short_id()).to_string() } }; diff --git a/src/channels/feishu.rs b/src/channels/feishu.rs index e05eac0..218c10e 100644 --- a/src/channels/feishu.rs +++ b/src/channels/feishu.rs @@ -1357,52 +1357,6 @@ impl FeishuChannel { } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn collect_post_image_keys_finds_nested_images() { - let content = serde_json::json!({ - "zh_cn": { - "title": "", - "content": [[ - {"tag": "img", "image_key": "img_v3_001"}, - {"tag": "text", "text": "这是哪里？"}, - {"tag": "img", "image_key": "img_v3_002"}, - {"tag": "img", "image_key": "img_v3_001"} - ]] - } - }) - .to_string(); - - assert_eq!( - collect_post_image_keys(&content), - vec!["img_v3_001".to_string(), "img_v3_002".to_string()] - ); - } - - #[test] - fn parse_post_content_preserves_image_positions() { - let content = serde_json::json!({ - "zh_cn": { - "title": "", - "content": [[ - {"tag": "text", "text": "这是一张图："}, - {"tag": "img", "image_key": "img_v3_001"}, - {"tag": "text", "text": "看完继续说"} - ]] - } - }) - .to_string(); - - assert_eq!( - parse_post_content(&content), - "这是一张图：[image]看完继续说" - ); - } -} - fn parse_post_content(content: &str) -> String { /// Extract text from a single post element (text, link, at-mention). fn extract_element(el: &serde_json::Value, out: &mut Vec) { @@ -1732,13 +1686,8 @@ fn collect_list_items(items: &[serde_json::Value], lines: &mut Vec, dept collect_list_items(children, lines, depth + 1); } } else if let Some(children_arr) = item.as_array().and_then(|arr| { - arr.iter().find_map(|child| { - if child.as_object().and_then(|o| o.get("children")).is_some() { - Some(child) - } else { - None - } - }) + arr.iter() + .find(|child| child.as_object().and_then(|o| o.get("children")).is_some()) }) && let Some(children) = children_arr .as_object() .and_then(|o| o.get("children")) @@ -1819,13 +1768,12 @@ fn resolve_image_ext(content_type: &str) -> &str { } fn resolve_file_ext(content_json: &serde_json::Value) -> String { - if let Some(name) = content_json.get("file_name").and_then(|v| v.as_str()) { - if let Some(ext) = std::path::Path::new(name) + if let Some(name) = content_json.get("file_name").and_then(|v| v.as_str()) + && let Some(ext) = std::path::Path::new(name) .extension() .and_then(|e| e.to_str()) - { - return ext.to_string(); - } + { + return ext.to_string(); } String::new() } @@ -2271,3 +2219,49 @@ impl Channel for FeishuChannel { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn collect_post_image_keys_finds_nested_images() { + let content = serde_json::json!({ + "zh_cn": { + "title": "", + "content": [[ + {"tag": "img", "image_key": "img_v3_001"}, + {"tag": "text", "text": "这是哪里？"}, + {"tag": "img", "image_key": "img_v3_002"}, + {"tag": "img", "image_key": "img_v3_001"} + ]] + } + }) + .to_string(); + + assert_eq!( + collect_post_image_keys(&content), + vec!["img_v3_001".to_string(), "img_v3_002".to_string()] + ); + } + + #[test] + fn parse_post_content_preserves_image_positions() { + let content = serde_json::json!({ + "zh_cn": { + "title": "", + "content": [[ + {"tag": "text", "text": "这是一张图："}, + {"tag": "img", "image_key": "img_v3_001"}, + {"tag": "text", "text": "看完继续说"} + ]] + } + }) + .to_string(); + + assert_eq!( + parse_post_content(&content), + "这是一张图：[image]看完继续说" + ); + } +} diff --git a/src/config/mod.rs b/src/config/mod.rs index f67284f..ee4e7c7 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -19,10 +19,10 @@ pub fn get_default_workspace_dir() -> PathBuf { /// Expand ~ in path to user home directory pub fn expand_path(path: &str) -> PathBuf { - if path.starts_with("~/") { + if let Some(path) = path.strip_prefix("~/") { dirs::home_dir() .unwrap_or_else(|| PathBuf::from(".")) - .join(&path[2..]) + .join(path) } else { PathBuf::from(path) } diff --git a/src/memory/types.rs b/src/memory/types.rs index 3987d21..7487772 100644 --- a/src/memory/types.rs +++ b/src/memory/types.rs @@ -18,7 +18,7 @@ impl MemoryCategory { } } - pub fn from_str(s: &str) -> Option { + pub fn parse(s: &str) -> Option { match s { "knowledge" => Some(Self::Knowledge), "timeline" => Some(Self::Timeline), @@ -78,13 +78,13 @@ mod tests { #[test] fn test_memory_category_from_str() { assert_eq!( - MemoryCategory::from_str("knowledge"), + MemoryCategory::parse("knowledge"), Some(MemoryCategory::Knowledge) ); assert_eq!( - MemoryCategory::from_str("timeline"), + MemoryCategory::parse("timeline"), Some(MemoryCategory::Timeline) ); - assert_eq!(MemoryCategory::from_str("invalid"), None); + assert_eq!(MemoryCategory::parse("invalid"), None); } } diff --git a/src/providers/anthropic.rs b/src/providers/anthropic.rs index f76ef72..67a389d 100644 --- a/src/providers/anthropic.rs +++ b/src/providers/anthropic.rs @@ -87,6 +87,8 @@ pub struct AnthropicProvider { } impl AnthropicProvider { + // Keep this constructor aligned with OpenAIProvider and LLMProviderConfig. + #[allow(clippy::too_many_arguments)] pub fn new( name: String, api_key: String, diff --git a/src/providers/openai.rs b/src/providers/openai.rs index cc157cd..6693590 100644 --- a/src/providers/openai.rs +++ b/src/providers/openai.rs @@ -46,6 +46,9 @@ pub struct OpenAIProvider { } impl OpenAIProvider { + // Provider construction mirrors the independently configurable fields in + // LLMProviderConfig; grouping them again would only duplicate that API. + #[allow(clippy::too_many_arguments)] pub fn new( name: String, api_key: String, @@ -112,10 +115,10 @@ impl OpenAIProvider { "role": m.role, "content": convert_content_blocks(&m.content) }); - if m.role == "assistant" { - if let Some(ref rc) = m.reasoning_content { - msg["reasoning_content"] = json!(rc); - } + if m.role == "assistant" + && let Some(ref rc) = m.reasoning_content + { + msg["reasoning_content"] = json!(rc); } msg } @@ -358,7 +361,7 @@ impl LLMProvider for OpenAIProvider { prompt_tokens: usage.prompt_tokens, completion_tokens: usage.completion_tokens, total_tokens: usage.total_tokens, - cached_tokens: cached_tokens, + cached_tokens, cache_read_input_tokens: None, cache_creation_input_tokens: None, }, diff --git a/src/session/mod.rs b/src/session/mod.rs index bcc099b..c3686f4 100644 --- a/src/session/mod.rs +++ b/src/session/mod.rs @@ -1,6 +1,8 @@ pub mod commands; pub mod error; pub mod events; +// The public `session::session` path is retained for API compatibility. +#[allow(clippy::module_inception)] pub mod session; pub mod session_id; diff --git a/src/session/session.rs b/src/session/session.rs index 093abfd..a17f56c 100644 --- a/src/session/session.rs +++ b/src/session/session.rs @@ -531,11 +531,9 @@ impl Session { media_refs: Vec, source: MessageSource, ) -> ChatMessage { - if media_refs.is_empty() { - ChatMessage::user_with_source(content, source) - } else { - ChatMessage::user_with_source(content, source) - } + let mut message = ChatMessage::user_with_source(content, source); + message.media_refs = media_refs; + message } /// 将 session 元数据写回 Storage @@ -861,7 +859,7 @@ impl Session { /// Repair damaged tool call chains after restoring from storage. /// Handles cases where the gateway crashed mid-loop, leaving assistant /// tool_calls without corresponding tool result messages. -fn repair_tool_call_chains(messages: &mut Vec) { +fn repair_tool_call_chains(messages: &mut [ChatMessage]) { let mut i = 0; while i < messages.len() { let calls = match &messages[i].tool_calls { @@ -2665,6 +2663,24 @@ impl OutboundMessenger for SessionManager { } } +fn format_task_notification( + task_id: &str, + status: &crate::agent::TaskStatus, + summary: &str, +) -> String { + match status { + crate::agent::TaskStatus::Completed => format!( + "📋 后台任务完成\n\n任务 ID: {}\n\n结果:\n{}", + task_id, summary + ), + crate::agent::TaskStatus::Failed(err) => { + format!("📋 后台任务失败\n\n任务 ID: {}\n错误: {}", task_id, err) + } + crate::agent::TaskStatus::Cancelled => format!("📋 后台任务已取消\n\n任务 ID: {}", task_id), + crate::agent::TaskStatus::TimedOut => format!("📋 后台任务超时\n\n任务 ID: {}", task_id), + } +} + #[cfg(test)] mod tests { use super::*; @@ -2689,21 +2705,3 @@ mod tests { } } } - -fn format_task_notification( - task_id: &str, - status: &crate::agent::TaskStatus, - summary: &str, -) -> String { - match status { - crate::agent::TaskStatus::Completed => format!( - "📋 后台任务完成\n\n任务 ID: {}\n\n结果:\n{}", - task_id, summary - ), - crate::agent::TaskStatus::Failed(err) => { - format!("📋 后台任务失败\n\n任务 ID: {}\n错误: {}", task_id, err) - } - crate::agent::TaskStatus::Cancelled => format!("📋 后台任务已取消\n\n任务 ID: {}", task_id), - crate::agent::TaskStatus::TimedOut => format!("📋 后台任务超时\n\n任务 ID: {}", task_id), - } -} diff --git a/src/session/session_id.rs b/src/session/session_id.rs index 0db2c9c..8a0f6eb 100644 --- a/src/session/session_id.rs +++ b/src/session/session_id.rs @@ -55,11 +55,6 @@ impl UnifiedSessionId { }) } - /// Convert to string format "channel:chat_id:dialog_id" - pub fn to_string(&self) -> String { - format!("{}:{}:{}", self.channel, self.chat_id, self.dialog_id) - } - /// Get the session key without dialog_id (channel:chat_id) /// This is used to group all dialogs within a chat pub fn chat_scope(&self) -> String { @@ -69,7 +64,7 @@ impl UnifiedSessionId { impl std::fmt::Display for UnifiedSessionId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.to_string()) + write!(f, "{}:{}:{}", self.channel, self.chat_id, self.dialog_id) } } diff --git a/src/storage/memory.rs b/src/storage/memory.rs index 4010c00..d536040 100644 --- a/src/storage/memory.rs +++ b/src/storage/memory.rs @@ -282,7 +282,7 @@ fn parse_memory_rows(rows: &[sqlx::sqlite::SqliteRow]) -> Result("category")?) + category: MemoryCategory::parse(&row.try_get::("category")?) .unwrap_or(MemoryCategory::Knowledge), importance: row.try_get::("importance")?, session_id: row.try_get::, _>("session_id")?, diff --git a/src/storage/mod.rs b/src/storage/mod.rs index f42534a..076a574 100644 --- a/src/storage/mod.rs +++ b/src/storage/mod.rs @@ -1390,8 +1390,8 @@ mod tests { chat_id: "sid123".to_string(), dialog_id: format!("dialog{}", i), title: format!("会话{}", i), - created_at: i as i64 * 1000, - last_active_at: i as i64 * 1000, + created_at: i * 1000, + last_active_at: i * 1000, message_count: i, routing_info: None, archived_at: None, diff --git a/src/tools/browser.rs b/src/tools/browser.rs index a247a1b..08e9846 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -39,11 +39,11 @@ struct BrowserState { impl Drop for BrowserTool { fn drop(&mut self) { - if let Ok(mut driver) = self.driver.lock() { - if let Some(ref mut child) = driver.take() { - tracing::debug!("Stopping chromedriver process"); - let _ = child.start_kill(); - } + if let Ok(mut driver) = self.driver.lock() + && let Some(ref mut child) = driver.take() + { + tracing::debug!("Stopping chromedriver process"); + let _ = child.start_kill(); } } } @@ -454,10 +454,7 @@ impl BrowserState { } => { let client = self.active_client()?; let result: Value = client - .execute( - &snapshot_script(interactive_only, compact, depth.map(i64::from)), - vec![], - ) + .execute(&snapshot_script(interactive_only, compact, depth), vec![]) .await?; let output = serde_json::to_string_pretty(&result)?; Ok(ToolResult { @@ -826,11 +823,11 @@ impl BrowserState { if let Some(client) = self.client.take() { let _ = client.close().await; } - if let Ok(mut guard) = driver.lock() { - if let Some(ref mut child) = guard.take() { - tracing::debug!("Stopping chromedriver process"); - let _ = child.start_kill(); - } + if let Ok(mut guard) = driver.lock() + && let Some(ref mut child) = guard.take() + { + tracing::debug!("Stopping chromedriver process"); + let _ = child.start_kill(); } } @@ -957,10 +954,10 @@ fn launch_chromedriver( } fn kill_driver_guard(driver: &std::sync::Mutex>) { - if let Ok(mut guard) = driver.lock() { - if let Some(ref mut child) = guard.take() { - let _ = child.start_kill(); - } + if let Ok(mut guard) = driver.lock() + && let Some(ref mut child) = guard.take() + { + let _ = child.start_kill(); } } diff --git a/src/tools/calculator.rs b/src/tools/calculator.rs index 2b42e8d..a11ef1d 100644 --- a/src/tools/calculator.rs +++ b/src/tools/calculator.rs @@ -380,7 +380,7 @@ fn calc_evaluate(args: &serde_json::Value) -> Result { .and_then(|v| v.as_str()) .ok_or_else(|| "Missing required parameter: expression".to_string())?; - meval::eval_str(expression) + super::expression::evaluate(expression) .map(format_num) .map_err(|e| format!("Expression evaluation error: {e}")) } diff --git a/src/tools/chat_manager.rs b/src/tools/chat_manager.rs index ff465d3..06497d9 100644 --- a/src/tools/chat_manager.rs +++ b/src/tools/chat_manager.rs @@ -299,8 +299,8 @@ mod tests { chat_id: format!("sid{}", i), dialog_id: format!("dialog{}", i), title: format!("会话{}", i), - created_at: now - i * 3600_000, - last_active_at: now - i * 3600_000, + created_at: now - i * 3_600_000, + last_active_at: now - i * 3_600_000, message_count: i * 5, routing_info: None, archived_at: None, @@ -350,7 +350,7 @@ mod tests { let msg = crate::storage::message::MessageMeta { id: format!("msg{}", i), session_id: session_id.to_string(), - seq: i as i64 + 1, + seq: i + 1, role: if i == 0 { "user".to_string() } else { @@ -412,7 +412,7 @@ mod tests { let msg = crate::storage::message::MessageMeta { id: format!("msg{}", i), session_id: session_id.to_string(), - seq: i as i64 + 1, + seq: i + 1, role: if i % 2 == 0 { "user".to_string() } else { @@ -472,7 +472,7 @@ mod tests { let msg = crate::storage::message::MessageMeta { id: format!("msg{}", i), session_id: session_id.to_string(), - seq: i as i64 + 1, + seq: i + 1, role: "user".to_string(), content: format!("消息内容 {}", i), reasoning_content: None, diff --git a/src/tools/delegate.rs b/src/tools/delegate.rs index 7e3f521..fc8827e 100644 --- a/src/tools/delegate.rs +++ b/src/tools/delegate.rs @@ -303,11 +303,11 @@ impl DelegateTool { if let Some(ref error) = task.error { output.push_str(&format!("\n错误: {}", error)); } - if let Some(started) = task.started_at { - if let Some(finished) = task.finished_at { - let duration = (finished - started) as f64 / 1000.0; - output.push_str(&format!("\n耗时: {:.1}s", duration)); - } + if let Some(started) = task.started_at + && let Some(finished) = task.finished_at + { + let duration = (finished - started) as f64 / 1000.0; + output.push_str(&format!("\n耗时: {:.1}s", duration)); } Ok(ToolResult { success: true, diff --git a/src/tools/expression.rs b/src/tools/expression.rs new file mode 100644 index 0000000..42d482c --- /dev/null +++ b/src/tools/expression.rs @@ -0,0 +1,277 @@ +/// Evaluate a self-contained mathematical expression without executing code or +/// resolving external variables. +pub(super) fn evaluate(input: &str) -> Result { + const MAX_EXPRESSION_BYTES: usize = 4096; + if input.len() > MAX_EXPRESSION_BYTES { + return Err(format!( + "expression exceeds the {MAX_EXPRESSION_BYTES}-byte limit" + )); + } + let mut parser = Parser { + input, + position: 0, + depth: 0, + }; + let value = parser.parse_expression()?; + parser.skip_whitespace(); + if parser.position != input.len() { + return Err(parser.error("unexpected trailing input")); + } + Ok(value) +} + +struct Parser<'a> { + input: &'a str, + position: usize, + depth: usize, +} + +impl Parser<'_> { + fn parse_expression(&mut self) -> Result { + let mut value = self.parse_term()?; + loop { + if self.consume(b'+') { + value += self.parse_term()?; + } else if self.consume(b'-') { + value -= self.parse_term()?; + } else { + return Ok(value); + } + } + } + + fn parse_term(&mut self) -> Result { + let mut value = self.parse_unary()?; + loop { + if self.consume(b'*') { + value *= self.parse_unary()?; + } else if self.consume(b'/') { + value /= self.parse_unary()?; + } else if self.consume(b'%') { + value %= self.parse_unary()?; + } else { + return Ok(value); + } + } + } + + fn parse_unary(&mut self) -> Result { + if self.consume(b'+') { + self.nested(Self::parse_unary) + } else if self.consume(b'-') { + Ok(-self.nested(Self::parse_unary)?) + } else { + self.parse_power() + } + } + + fn parse_power(&mut self) -> Result { + let base = self.parse_primary()?; + if self.consume(b'^') { + Ok(base.powf(self.nested(Self::parse_unary)?)) + } else { + Ok(base) + } + } + + fn parse_primary(&mut self) -> Result { + self.skip_whitespace(); + match self.peek() { + Some(b'(') => { + self.position += 1; + let value = self.nested(Self::parse_expression)?; + if !self.consume(b')') { + return Err(self.error("expected ')'")); + } + Ok(value) + } + Some(byte) if byte.is_ascii_digit() || byte == b'.' => self.parse_number(), + Some(byte) if byte.is_ascii_alphabetic() || byte == b'_' => self.parse_identifier(), + Some(_) => Err(self.error("expected a number, constant, function, or '('")), + None => Err(self.error("unexpected end of expression")), + } + } + + fn parse_number(&mut self) -> Result { + self.skip_whitespace(); + let start = self.position; + let mut digits = 0; + while self.peek().is_some_and(|byte| byte.is_ascii_digit()) { + self.position += 1; + digits += 1; + } + if self.peek() == Some(b'.') { + self.position += 1; + while self.peek().is_some_and(|byte| byte.is_ascii_digit()) { + self.position += 1; + digits += 1; + } + } + if digits == 0 { + return Err(self.error("invalid number")); + } + if matches!(self.peek(), Some(b'e' | b'E')) { + self.position += 1; + if matches!(self.peek(), Some(b'+' | b'-')) { + self.position += 1; + } + let exponent_start = self.position; + while self.peek().is_some_and(|byte| byte.is_ascii_digit()) { + self.position += 1; + } + if self.position == exponent_start { + return Err(self.error("invalid numeric exponent")); + } + } + + self.input[start..self.position] + .parse::() + .map_err(|_| self.error("invalid number")) + } + + fn parse_identifier(&mut self) -> Result { + self.skip_whitespace(); + let start = self.position; + while self + .peek() + .is_some_and(|byte| byte.is_ascii_alphanumeric() || byte == b'_') + { + self.position += 1; + } + let name = self.input[start..self.position].to_ascii_lowercase(); + self.skip_whitespace(); + if self.peek() != Some(b'(') { + return match name.as_str() { + "pi" => Ok(std::f64::consts::PI), + "e" => Ok(std::f64::consts::E), + _ => Err(self.error(&format!("unknown constant or variable '{name}'"))), + }; + } + + self.position += 1; + let mut arguments = Vec::new(); + self.skip_whitespace(); + if self.peek() != Some(b')') { + loop { + arguments.push(self.nested(Self::parse_expression)?); + if self.consume(b',') { + continue; + } + break; + } + } + if !self.consume(b')') { + return Err(self.error("expected ')' after function arguments")); + } + apply_function(&name, &arguments).map_err(|message| self.error(&message)) + } + + fn consume(&mut self, expected: u8) -> bool { + self.skip_whitespace(); + if self.peek() == Some(expected) { + self.position += 1; + true + } else { + false + } + } + + fn skip_whitespace(&mut self) { + while self.peek().is_some_and(|byte| byte.is_ascii_whitespace()) { + self.position += 1; + } + } + + fn peek(&self) -> Option { + self.input.as_bytes().get(self.position).copied() + } + + fn nested(&mut self, parse: fn(&mut Self) -> Result) -> Result { + const MAX_PARSE_DEPTH: usize = 128; + if self.depth >= MAX_PARSE_DEPTH { + return Err(self.error("expression nesting limit exceeded")); + } + self.depth += 1; + let result = parse(self); + self.depth -= 1; + result + } + + fn error(&self, message: &str) -> String { + format!("{message} at byte {}", self.position) + } +} + +fn apply_function(name: &str, arguments: &[f64]) -> Result { + let unary = |function: fn(f64) -> f64| match arguments { + [value] => Ok(function(*value)), + _ => Err(format!("function '{name}' expects one argument")), + }; + match name { + "sqrt" => unary(f64::sqrt), + "abs" => unary(f64::abs), + "exp" => unary(f64::exp), + "ln" => unary(f64::ln), + "log2" => unary(f64::log2), + "log10" => unary(f64::log10), + "sin" => unary(f64::sin), + "cos" => unary(f64::cos), + "tan" => unary(f64::tan), + "asin" => unary(f64::asin), + "acos" => unary(f64::acos), + "atan" => unary(f64::atan), + "sinh" => unary(f64::sinh), + "cosh" => unary(f64::cosh), + "tanh" => unary(f64::tanh), + "asinh" => unary(f64::asinh), + "acosh" => unary(f64::acosh), + "atanh" => unary(f64::atanh), + "floor" => unary(f64::floor), + "ceil" => unary(f64::ceil), + "round" => unary(f64::round), + "signum" => unary(f64::signum), + "atan2" => match arguments { + [y, x] => Ok(y.atan2(*x)), + _ => Err("function 'atan2' expects two arguments".to_string()), + }, + "min" => arguments + .iter() + .copied() + .reduce(f64::min) + .ok_or_else(|| "function 'min' expects at least one argument".to_string()), + "max" => arguments + .iter() + .copied() + .reduce(f64::max) + .ok_or_else(|| "function 'max' expects at least one argument".to_string()), + _ => Err(format!("unknown function '{name}'")), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn respects_precedence_and_right_associative_power() { + assert_eq!(evaluate("15*3+5^(2+1)").unwrap(), 170.0); + assert_eq!(evaluate("2^3^2").unwrap(), 512.0); + assert_eq!(evaluate("-2^2").unwrap(), -4.0); + } + + #[test] + fn supports_constants_functions_and_scientific_notation() { + assert_eq!(evaluate("sqrt(1.44e2)").unwrap(), 12.0); + assert_eq!(evaluate("max(1, 2, 3) + min(4, 5)").unwrap(), 7.0); + assert!((evaluate("sin(pi / 2)").unwrap() - 1.0).abs() < f64::EPSILON); + } + + #[test] + fn rejects_unknown_names_and_trailing_input() { + assert!(evaluate("unknown").is_err()); + assert!(evaluate("1 + 2 garbage").is_err()); + assert!(evaluate("sqrt() ").is_err()); + assert!(evaluate(&"(".repeat(129)).is_err()); + assert!(evaluate(&"1+".repeat(3000)).is_err()); + } +} diff --git a/src/tools/mod.rs b/src/tools/mod.rs index cbed3f2..2266f9c 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -5,6 +5,7 @@ pub mod chat_manager; pub mod content_search; pub mod cron; pub mod delegate; +mod expression; pub mod file_edit; pub mod file_read; pub mod file_search; @@ -77,10 +78,10 @@ pub fn create_default_tools( registry.register(TimelineRecallTool::new(memory.clone())); registry.register(MemoryForgetTool::new(memory.clone())); - if let Some(cfg) = browser_config { - if cfg.enabled { - registry.register(BrowserTool::new(cfg)); - } + if let Some(cfg) = browser_config + && cfg.enabled + { + registry.register(BrowserTool::new(cfg)); } if let Some(mgr) = sub_agent_manager { diff --git a/src/tools/pty.rs b/src/tools/pty.rs index c3e93f4..8ac8641 100644 --- a/src/tools/pty.rs +++ b/src/tools/pty.rs @@ -146,6 +146,12 @@ pub struct PtyManager { sessions: Mutex>>>, } +impl Default for PtyManager { + fn default() -> Self { + Self::new() + } +} + impl PtyManager { pub fn new() -> Self { Self { @@ -199,7 +205,7 @@ impl PtyManager { .map_err(|e| format!("Failed to open PTY: {}", e))?; let mut cmd = portable_pty::CommandBuilder::new("bash"); - cmd.args(&["-c", command]); + cmd.args(["-c", command]); cmd.cwd(cwd); let child = pty_pair diff --git a/src/tools/send_message.rs b/src/tools/send_message.rs index 6cb0de9..9a507ee 100644 --- a/src/tools/send_message.rs +++ b/src/tools/send_message.rs @@ -169,7 +169,7 @@ fn parse_files_arg(args: &serde_json::Value) -> Vec { files .iter() .filter_map(|v| v.as_str()) - .map(|path| path_to_media_item(path)) + .map(path_to_media_item) .collect() } diff --git a/tests/test_request_format.rs b/tests/test_request_format.rs index 750fc4a..dd327af 100644 --- a/tests/test_request_format.rs +++ b/tests/test_request_format.rs @@ -18,7 +18,7 @@ fn test_message_special_characters() { /// Test that multi-line system prompt is preserved #[test] fn test_multiline_system_prompt() { - let messages = vec![ + let messages = [ Message::system( "You are a helpful assistant.\n\nFollow these rules:\n1. Be kind\n2. Be accurate", ), diff --git a/tests/test_scheduler.rs b/tests/test_scheduler.rs index 21c4c6e..fe323fe 100644 --- a/tests/test_scheduler.rs +++ b/tests/test_scheduler.rs @@ -1,6 +1,5 @@ -/// Integration tests for the scheduled tasks (cron) system. -/// Run with: cargo test --test test_scheduler -use serde_json::json; +//! Integration tests for the scheduled tasks (cron) system. +//! Run with: `cargo test --test test_scheduler`. /// Verify that Schedule types (de)serialize correctly. #[tokio::test]