From e47c7716fac856f46a5436695e8eb3ea068607a6 Mon Sep 17 00:00:00 2001 From: wsp Date: Thu, 12 Feb 2026 18:04:33 +0800 Subject: [PATCH 1/6] fix: enhance openai streaming robustness --- .../src/agentic/execution/stream_processor.rs | 24 +- .../src/stream_handler/openai.rs | 181 +++++++++-- .../ai/ai_stream_handlers/src/types/openai.rs | 282 ++++++++++++++++-- .../core/src/infrastructure/ai/client.rs | 57 +++- 4 files changed, 482 insertions(+), 62 deletions(-) diff --git a/src/crates/core/src/agentic/execution/stream_processor.rs b/src/crates/core/src/agentic/execution/stream_processor.rs index 05fd608d..8f8a87ef 100644 --- a/src/crates/core/src/agentic/execution/stream_processor.rs +++ b/src/crates/core/src/agentic/execution/stream_processor.rs @@ -691,26 +691,30 @@ impl StreamProcessor { } // Handle different types of response content - // Normalize empty strings to None so the if-else chain correctly - // falls through (some models send empty text alongside reasoning content) + // Normalize empty strings to None + // (some models send empty text alongside reasoning content) let text = response.text.filter(|t| !t.is_empty()); let reasoning_content = response.reasoning_content.filter(|t| !t.is_empty()); - if let Some(tool_call) = response.tool_call { - self.send_thinking_end_if_needed(&mut ctx).await; - if let Some(err) = self.check_cancellation(&mut ctx, cancellation_token, "processing tool call").await { + if let Some(thinking_content) = reasoning_content { + self.handle_thinking_chunk(&mut ctx, thinking_content).await; + if let Some(err) = self.check_cancellation(&mut ctx, cancellation_token, "processing thinking chunk").await { return err; } - self.handle_tool_call_chunk(&mut ctx, tool_call).await; - } else if let Some(text) = text { + } + + if let Some(text) = text { self.send_thinking_end_if_needed(&mut ctx).await; self.handle_text_chunk(&mut ctx, text).await; if let Some(err) = self.check_cancellation(&mut ctx, cancellation_token, "processing text chunk").await { return err; } - } else if let Some(thinking_content) = reasoning_content { - self.handle_thinking_chunk(&mut ctx, thinking_content).await; - if let Some(err) = self.check_cancellation(&mut ctx, cancellation_token, "processing thinking chunk").await { + } + + if let Some(tool_call) = response.tool_call { + self.send_thinking_end_if_needed(&mut ctx).await; + self.handle_tool_call_chunk(&mut ctx, tool_call).await; + if let Some(err) = self.check_cancellation(&mut ctx, cancellation_token, "processing tool call").await { return err; } } diff --git a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs index b8cc2bf3..87893a5c 100644 --- a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs +++ b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs @@ -1,14 +1,35 @@ use crate::types::openai::OpenAISSEData; use crate::types::unified::UnifiedResponse; -use anyhow::{anyhow, Error, Result}; +use anyhow::{anyhow, Result}; use eventsource_stream::Eventsource; use futures::StreamExt; -use log::{trace, warn}; +use log::{error, trace, warn}; use reqwest::Response; +use serde_json::Value; use std::time::Duration; use tokio::sync::mpsc; use tokio::time::timeout; +const OPENAI_CHAT_COMPLETION_CHUNK_OBJECT: &str = "chat.completion.chunk"; + +fn is_valid_chat_completion_chunk_weak(event_json: &Value) -> bool { + matches!( + event_json.get("object").and_then(|value| value.as_str()), + Some(OPENAI_CHAT_COMPLETION_CHUNK_OBJECT) + ) +} + +fn extract_sse_api_error_message(event_json: &Value) -> Option { + let error = event_json.get("error")?; + if let Some(message) = error.get("message").and_then(|value| value.as_str()) { + return Some(message.to_string()); + } + if let Some(message) = error.as_str() { + return Some(message.to_string()); + } + Some("An error occurred during streaming".to_string()) +} + /// Convert a byte stream into a structured response stream /// /// # Arguments @@ -22,37 +43,32 @@ pub async fn handle_openai_stream( ) { let mut stream = response.bytes_stream().eventsource(); let idle_timeout = Duration::from_secs(600); - let mut received_done = false; - let response_error: Option = None; loop { let sse_event = timeout(idle_timeout, stream.next()).await; let sse = match sse_event { Ok(Some(Ok(sse))) => sse, Ok(None) => { - if !received_done { - let error = response_error - .unwrap_or(anyhow!("SSE stream closed before response completed")); - warn!("SSE stream ended unexpectedly: {}", error); - let _ = tx_event.send(Err(error)); - } + let error_msg = "SSE stream closed before response completed"; + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } Ok(Some(Err(e))) => { - let error = anyhow!("SSE stream error: {}", e); - let _ = tx_event.send(Err(error)); + let error_msg = format!("SSE stream error: {}", e); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } Err(_) => { - warn!("SSE stream timeout after {}s", idle_timeout.as_secs()); - let _ = tx_event.send(Err(anyhow!( - "SSE stream timeout: idle timeout waiting for SSE" - ))); + let error_msg = format!("SSE stream timeout after {}s", idle_timeout.as_secs()); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } }; - let raw = sse.data; + let raw = sse.data.clone(); trace!("sse data: {:?}", raw); if let Some(ref tx) = tx_raw_sse { @@ -60,19 +76,136 @@ pub async fn handle_openai_stream( } if raw == "[DONE]" { - received_done = true; + return; + } + + let event_json: Value = match serde_json::from_str(&raw) { + Ok(json) => json, + Err(e) => { + let _ = tx_event.send(Err(anyhow!("SSE parsing error: {}, data: {}", e, &raw))); + return; + } + }; + + if let Some(api_error_message) = extract_sse_api_error_message(&event_json) { + let error_msg = format!("SSE API error: {}, data: {}", api_error_message, raw); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); + return; + } + + if !is_valid_chat_completion_chunk_weak(&event_json) { + warn!( + "Skipping non-standard OpenAI SSE event; object={}", + event_json + .get("object") + .and_then(|value| value.as_str()) + .unwrap_or("") + ); continue; } - let sse_data: OpenAISSEData = match serde_json::from_str(&raw) { + let sse_data: OpenAISSEData = match serde_json::from_value(event_json) { Ok(event) => event, Err(e) => { - let _ = tx_event.send(Err(anyhow!("SSE parsing error: {}, data: {}", e, &raw))); - continue; + let error_msg = format!("SSE data schema error: {}, data: {}", e, &raw); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); + return; } }; - let unified_response: UnifiedResponse = sse_data.into(); - trace!("unified_response: {:?}", unified_response); - let _ = tx_event.send(Ok(unified_response)); + + let tool_call_count = sse_data.first_choice_tool_call_count(); + if tool_call_count > 1 { + warn!( + "OpenAI SSE chunk contains {} tool calls in the first choice; splitting and sending sequentially", + tool_call_count + ); + } + + let has_empty_choices = sse_data.is_choices_empty(); + let unified_responses = sse_data.into_unified_responses(); + trace!("unified responses: {:?}", unified_responses); + if unified_responses.is_empty() { + if has_empty_choices { + warn!( + "Ignoring OpenAI SSE chunk with empty choices and no usage payload: {}", + raw + ); + // Ignore keepalive/metadata chunks with empty choices and no usage payload. + continue; + } + // Defensive fallback: this should be unreachable if OpenAISSEData::into_unified_responses + // keeps returning at least one event for all non-empty-choices chunks. + let error_msg = format!("OpenAI SSE chunk produced no unified events, data: {}", raw); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); + return; + } + + for unified_response in unified_responses { + let _ = tx_event.send(Ok(unified_response)); + } + } +} + +#[cfg(test)] +mod tests { + use super::{extract_sse_api_error_message, is_valid_chat_completion_chunk_weak}; + + #[test] + fn weak_filter_accepts_chat_completion_chunk() { + let event = serde_json::json!({ + "object": "chat.completion.chunk" + }); + assert!(is_valid_chat_completion_chunk_weak(&event)); + } + + #[test] + fn weak_filter_rejects_non_standard_object() { + let event = serde_json::json!({ + "object": "" + }); + assert!(!is_valid_chat_completion_chunk_weak(&event)); + } + + #[test] + fn weak_filter_rejects_missing_object() { + let event = serde_json::json!({ + "id": "chatcmpl_test" + }); + assert!(!is_valid_chat_completion_chunk_weak(&event)); + } + + #[test] + fn extracts_api_error_message_from_object_shape() { + let event = serde_json::json!({ + "error": { + "message": "provider error" + } + }); + assert_eq!( + extract_sse_api_error_message(&event).as_deref(), + Some("provider error") + ); + } + + #[test] + fn extracts_api_error_message_from_string_shape() { + let event = serde_json::json!({ + "error": "provider error" + }); + assert_eq!( + extract_sse_api_error_message(&event).as_deref(), + Some("provider error") + ); + } + + #[test] + fn returns_none_when_no_error_payload_exists() { + let event = serde_json::json!({ + "object": "chat.completion.chunk" + }); + assert!(extract_sse_api_error_message(&event).is_none()); } } diff --git a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/openai.rs b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/openai.rs index ed7af8c1..57fde206 100644 --- a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/openai.rs +++ b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/openai.rs @@ -3,7 +3,7 @@ use serde::Deserialize; #[derive(Debug, Deserialize)] struct PromptTokensDetails { - cached_tokens: u32, + cached_tokens: Option, } #[derive(Debug, Deserialize)] @@ -20,13 +20,9 @@ impl From for UnifiedTokenUsage { prompt_token_count: usage.prompt_tokens, candidates_token_count: usage.completion_tokens, total_token_count: usage.total_tokens, - cached_content_token_count: if let Some(prompt_tokens_details) = - usage.prompt_tokens_details - { - Some(prompt_tokens_details.cached_tokens) - } else { - None - }, + cached_content_token_count: usage + .prompt_tokens_details + .and_then(|prompt_tokens_details| prompt_tokens_details.cached_tokens), } } } @@ -91,24 +87,260 @@ pub struct OpenAISSEData { usage: Option, } -impl From for UnifiedResponse { - fn from(data: OpenAISSEData) -> Self { - let choices0 = data.choices.get(0).unwrap(); - let text = choices0.delta.content.clone(); - let reasoning_content = choices0.delta.reasoning_content.clone(); - let finish_reason = choices0.finish_reason.clone(); - let tool_call = choices0.delta.tool_calls.as_ref().and_then(|tool_calls| { - tool_calls - .get(0) - .map(|tool_call| UnifiedToolCall::from(tool_call.clone())) - }); - Self { - text, - reasoning_content, - thinking_signature: None, - tool_call, - usage: data.usage.map(|usage| usage.into()), +impl OpenAISSEData { + pub fn is_choices_empty(&self) -> bool { + self.choices.is_empty() + } + + pub fn first_choice_tool_call_count(&self) -> usize { + self.choices + .first() + .and_then(|choice| choice.delta.tool_calls.as_ref()) + .map(|tool_calls| tool_calls.len()) + .unwrap_or(0) + } + + pub fn into_unified_responses(self) -> Vec { + let mut usage = self.usage.map(|usage| usage.into()); + + let Some(first_choice) = self.choices.into_iter().next() else { + // OpenAI can emit `choices: []` for the final usage chunk. + return usage + .map(|usage_data| { + vec![UnifiedResponse { + usage: Some(usage_data), + ..Default::default() + }] + }) + .unwrap_or_default(); + }; + + let Choice { + delta, finish_reason, + .. + } = first_choice; + let mut finish_reason = finish_reason; + let Delta { + reasoning_content, + content, + tool_calls, + .. + } = delta; + + let mut responses = Vec::new(); + + if content.is_some() || reasoning_content.is_some() { + responses.push(UnifiedResponse { + text: content, + reasoning_content, + thinking_signature: None, + tool_call: None, + usage: usage.take(), + finish_reason: finish_reason.take(), + }); + } + + if let Some(tool_calls) = tool_calls { + for tool_call in tool_calls { + let is_first_event = responses.is_empty(); + responses.push(UnifiedResponse { + text: None, + reasoning_content: None, + thinking_signature: None, + tool_call: Some(UnifiedToolCall::from(tool_call)), + usage: if is_first_event { usage.take() } else { None }, + finish_reason: if is_first_event { + finish_reason.take() + } else { + None + }, + }); + } } + + if responses.is_empty() { + responses.push(UnifiedResponse { + text: None, + reasoning_content: None, + thinking_signature: None, + tool_call: None, + usage, + finish_reason, + }); + } + + responses + } +} + +impl From for UnifiedResponse { + fn from(data: OpenAISSEData) -> Self { + data.into_unified_responses() + .into_iter() + .next() + .unwrap_or_default() + } +} + +#[cfg(test)] +mod tests { + use super::OpenAISSEData; + + #[test] + fn splits_multiple_tool_calls_in_first_choice() { + let raw = r#"{ + "id": "chatcmpl_test", + "created": 123, + "model": "gpt-test", + "choices": [{ + "index": 0, + "delta": { + "tool_calls": [ + { + "index": 0, + "id": "call_1", + "type": "function", + "function": { + "name": "tool_a", + "arguments": "{\"a\":1}" + } + }, + { + "index": 1, + "id": "call_2", + "type": "function", + "function": { + "name": "tool_b", + "arguments": "{\"b\":2}" + } + } + ] + }, + "finish_reason": "tool_calls" + }], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 5, + "total_tokens": 15, + "prompt_tokens_details": { + "cached_tokens": 3 + } + } + }"#; + + let sse_data: OpenAISSEData = serde_json::from_str(raw).expect("valid openai sse data"); + let responses = sse_data.into_unified_responses(); + + assert_eq!(responses.len(), 2); + assert_eq!( + responses[0] + .tool_call + .as_ref() + .and_then(|tool| tool.id.as_deref()), + Some("call_1") + ); + assert_eq!( + responses[1] + .tool_call + .as_ref() + .and_then(|tool| tool.id.as_deref()), + Some("call_2") + ); + assert_eq!(responses[0].finish_reason.as_deref(), Some("tool_calls")); + assert!(responses[1].finish_reason.is_none()); + assert!(responses[0].usage.is_some()); + assert!(responses[1].usage.is_none()); + } + + #[test] + fn handles_empty_choices_with_usage_chunk() { + let raw = r#"{ + "id": "chatcmpl_test", + "created": 123, + "model": "gpt-test", + "choices": [], + "usage": { + "prompt_tokens": 7, + "completion_tokens": 3, + "total_tokens": 10 + } + }"#; + + let sse_data: OpenAISSEData = serde_json::from_str(raw).expect("valid openai sse data"); + let responses = sse_data.into_unified_responses(); + + assert_eq!(responses.len(), 1); + assert!(responses[0].usage.is_some()); + assert!(responses[0].text.is_none()); + assert!(responses[0].tool_call.is_none()); + } + + #[test] + fn handles_empty_choices_without_usage_chunk() { + let raw = r#"{ + "id": "chatcmpl_test", + "created": 123, + "model": "gpt-test", + "choices": [], + "usage": null + }"#; + + let sse_data: OpenAISSEData = serde_json::from_str(raw).expect("valid openai sse data"); + let responses = sse_data.into_unified_responses(); + + assert!(responses.is_empty()); + } + + #[test] + fn preserves_text_when_tool_calls_exist_in_same_chunk() { + let raw = r#"{ + "id": "chatcmpl_test", + "created": 123, + "model": "gpt-test", + "choices": [{ + "index": 0, + "delta": { + "content": "hello", + "tool_calls": [ + { + "index": 0, + "id": "call_1", + "type": "function", + "function": { + "name": "tool_a", + "arguments": "{\"a\":1}" + } + } + ] + }, + "finish_reason": "tool_calls" + }], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 5, + "total_tokens": 15 + } + }"#; + + let sse_data: OpenAISSEData = serde_json::from_str(raw).expect("valid openai sse data"); + let responses = sse_data.into_unified_responses(); + + assert_eq!(responses.len(), 2); + assert_eq!(responses[0].text.as_deref(), Some("hello")); + assert!(responses[0].tool_call.is_none()); + assert!(responses[0].usage.is_some()); + assert_eq!(responses[0].finish_reason.as_deref(), Some("tool_calls")); + + assert!(responses[1].text.is_none()); + assert_eq!( + responses[1] + .tool_call + .as_ref() + .and_then(|tool| tool.id.as_deref()), + Some("call_1") + ); + assert!(responses[1].usage.is_none()); + assert!(responses[1].finish_reason.is_none()); } } diff --git a/src/crates/core/src/infrastructure/ai/client.rs b/src/crates/core/src/infrastructure/ai/client.rs index 35d5505c..173a62a7 100644 --- a/src/crates/core/src/infrastructure/ai/client.rs +++ b/src/crates/core/src/infrastructure/ai/client.rs @@ -112,6 +112,45 @@ impl AIClient { &self.config.format } + /// Whether to inject the GLM-specific `tool_stream` request field. + /// + /// `tool_stream` is only required by GLM; other providers can do tool streaming without this field. + /// Current rule: inject only for pure-version GLM models (no suffix) with version >= 4.6. + fn supports_glm_tool_stream(model_name: &str) -> bool { + Self::parse_glm_major_minor(model_name) + .map(|(major, minor)| major > 4 || (major == 4 && minor >= 6)) + .unwrap_or(false) + } + + /// Parse strict `glm-[.]` from model names like: + /// - glm-4.6 + /// - glm-5 + /// + /// Models with non-numeric suffixes are treated as not requiring this GLM-specific field, e.g.: + /// - glm-4.6-flash + /// - glm-4.5v + fn parse_glm_major_minor(model_name: &str) -> Option<(u32, u32)> { + let version_part = model_name.strip_prefix("glm-")?; + + if version_part.is_empty() { + return None; + } + + let mut parts = version_part.split('.'); + let major: u32 = parts.next()?.parse().ok()?; + let minor: u32 = match parts.next() { + Some(v) => v.parse().ok()?, + None => 0, + }; + + // Only allow one numeric segment after the decimal point. + if parts.next().is_some() { + return None; + } + + Some((major, minor)) + } + /// Determine whether to use merge mode /// /// true: apply default headers first, then custom headers (custom can override) @@ -215,7 +254,7 @@ impl AIClient { let model_name = self.config.model.to_lowercase(); - if model_name == "glm-4.6" || model_name == "glm-4.7" || model_name == "glm-5" { + if Self::supports_glm_tool_stream(&model_name) { request_body["tool_stream"] = serde_json::Value::Bool(true); } @@ -236,6 +275,18 @@ impl AIClient { } } + // This client currently consumes only the first choice in stream handling. + // Remove custom n override and keep provider defaults. + if let Some(request_obj) = request_body.as_object_mut() { + if let Some(existing_n) = request_obj.remove("n") { + warn!( + target: "ai::openai_stream_request", + "Removed custom request field n={} because the stream processor only handles the first choice", + existing_n + ); + } + } + debug!(target: "ai::openai_stream_request", "OpenAI stream request body (excluding tools):\n{}", serde_json::to_string_pretty(&request_body).unwrap_or_else(|_| "serialization failed".to_string()) @@ -275,8 +326,8 @@ impl AIClient { let model_name = self.config.model.to_lowercase(); - // TODO: Zhipu tool streaming currently only supports the OpenAI format - if model_name == "glm-4.6" || model_name == "glm-4.7" || model_name == "glm-5" { + // GLM-specific extension: only set `tool_stream` when the model requires it. + if Self::supports_glm_tool_stream(&model_name) { request_body["tool_stream"] = serde_json::Value::Bool(true); } From 5cbc507364d2cafddaf68e78dbae5946835f9d4e Mon Sep 17 00:00:00 2001 From: wsp Date: Thu, 12 Feb 2026 18:21:23 +0800 Subject: [PATCH 2/6] refactor: clear usage of unwrap and expect --- src/apps/cli/src/main.rs | 30 +- src/apps/desktop/src/api/agentic_api.rs | 41 +- src/apps/desktop/src/api/config_api.rs | 244 +++-- .../desktop/src/api/context_upload_api.rs | 51 +- src/apps/desktop/src/api/skill_api.rs | 8 +- src/apps/desktop/src/api/subagent_api.rs | 16 +- src/apps/desktop/src/lib.rs | 54 +- src/apps/desktop/src/logging.rs | 7 +- src/apps/desktop/src/theme.rs | 8 +- src/crates/api-layer/src/handlers.rs | 7 +- src/crates/core/build.rs | 11 +- .../core/src/agentic/agents/registry.rs | 84 +- src/crates/core/src/agentic/core/message.rs | 7 +- .../core/src/agentic/core/messages_helper.rs | 10 +- .../agentic/session/compression_manager.rs | 27 +- .../src/agentic/session/session_manager.rs | 12 +- .../core/src/agentic/tools/framework.rs | 22 +- .../implementations/analyze_image_tool.rs | 122 +-- .../tools/implementations/code_review_tool.rs | 29 +- .../agentic/tools/implementations/log_tool.rs | 14 +- .../tools/implementations/todo_write_tool.rs | 7 +- .../tool-runtime/src/search/grep_search.rs | 35 +- .../core/src/agentic/util/list_files.rs | 18 +- .../src/infrastructure/ai/client_factory.rs | 57 +- .../filesystem/file_operations.rs | 322 ++++--- .../infrastructure/filesystem/file_tree.rs | 880 ++++++++++-------- .../infrastructure/filesystem/file_watcher.rs | 81 +- .../infrastructure/filesystem/path_manager.rs | 158 ++-- src/crates/core/src/service/config/manager.rs | 13 +- .../core/src/service/config/providers.rs | 29 +- .../core/src/service/lsp/workspace_manager.rs | 18 +- .../core/src/service/mcp/adapter/context.rs | 3 +- .../core/src/service/mcp/adapter/resource.rs | 3 +- .../core/src/service/mcp/adapter/tool.rs | 5 +- .../core/src/service/mcp/protocol/jsonrpc.rs | 28 +- .../core/src/service/mcp/server/connection.rs | 18 +- .../core/src/service/mcp/server/manager.rs | 10 +- src/crates/core/src/service/system/command.rs | 4 +- .../core/src/service/terminal/src/api.rs | 12 +- .../service/terminal/src/pty/data_bufferer.rs | 15 +- .../terminal/src/session/serializer.rs | 6 +- .../service/terminal/src/session/singleton.rs | 13 +- .../core/src/service/workspace/manager.rs | 13 +- src/crates/core/src/util/process_manager.rs | 43 +- 44 files changed, 1519 insertions(+), 1076 deletions(-) diff --git a/src/apps/cli/src/main.rs b/src/apps/cli/src/main.rs index 27913f47..7286a17d 100644 --- a/src/apps/cli/src/main.rs +++ b/src/apps/cli/src/main.rs @@ -12,7 +12,7 @@ mod modes; mod agent; use clap::{Parser, Subcommand}; -use anyhow::Result; +use anyhow::{Context, Result}; use config::CliConfig; use modes::chat::ChatMode; @@ -160,7 +160,15 @@ async fn main() -> Result<()> { { tracing_subscriber::fmt() .with_max_level(log_level) - .with_writer(move || file.try_clone().unwrap()) + .with_writer(move || -> Box { + match file.try_clone() { + Ok(cloned) => Box::new(cloned), + Err(e) => { + eprintln!("Warning: Failed to clone log file handle: {}", e); + Box::new(std::io::sink()) + } + } + }) .with_ansi(false) .with_target(false) .init(); @@ -227,7 +235,7 @@ async fn main() -> Result<()> { bitfun_core::service::config::initialize_global_config() .await - .expect("Failed to initialize global config service"); + .context("Failed to initialize global config service")?; tracing::info!("Global config service initialized"); let config_service = bitfun_core::service::config::get_global_config_service() @@ -249,12 +257,12 @@ async fn main() -> Result<()> { use bitfun_core::infrastructure::ai::AIClientFactory; AIClientFactory::initialize_global() .await - .expect("Failed to initialize global AIClientFactory"); + .context("Failed to initialize global AIClientFactory")?; tracing::info!("Global AI client factory initialized"); let agentic_system = agent::agentic_system::init_agentic_system() .await - .expect("Failed to initialize agentic system"); + .context("Failed to initialize agentic system")?; tracing::info!("Agentic system initialized"); if let Some(ref mut term) = startup_terminal { @@ -296,7 +304,7 @@ async fn main() -> Result<()> { bitfun_core::service::config::initialize_global_config() .await - .expect("Failed to initialize global config service"); + .context("Failed to initialize global config service")?; tracing::info!("Global config service initialized"); let config_service = bitfun_core::service::config::get_global_config_service() @@ -319,12 +327,12 @@ async fn main() -> Result<()> { use bitfun_core::infrastructure::ai::AIClientFactory; AIClientFactory::initialize_global() .await - .expect("Failed to initialize global AIClientFactory"); + .context("Failed to initialize global AIClientFactory")?; tracing::info!("Global AI client factory initialized"); let agentic_system = agent::agentic_system::init_agentic_system() .await - .expect("Failed to initialize agentic system"); + .context("Failed to initialize agentic system")?; tracing::info!("Agentic system initialized"); let mut exec_mode = ExecMode::new( @@ -407,7 +415,7 @@ async fn main() -> Result<()> { bitfun_core::service::config::initialize_global_config() .await - .expect("Failed to initialize global config service"); + .context("Failed to initialize global config service")?; tracing::info!("Global config service initialized"); let config_service = bitfun_core::service::config::get_global_config_service() @@ -427,12 +435,12 @@ async fn main() -> Result<()> { use bitfun_core::infrastructure::ai::AIClientFactory; AIClientFactory::initialize_global() .await - .expect("Failed to initialize global AIClientFactory"); + .context("Failed to initialize global AIClientFactory")?; tracing::info!("Global AI client factory initialized"); let agentic_system = agent::agentic_system::init_agentic_system() .await - .expect("Failed to initialize agentic system"); + .context("Failed to initialize agentic system")?; tracing::info!("Agentic system initialized"); ui::render_loading(&mut terminal, "System initialized, starting chat interface...")?; diff --git a/src/apps/desktop/src/api/agentic_api.rs b/src/apps/desktop/src/api/agentic_api.rs index 6c65c82c..fefde152 100644 --- a/src/apps/desktop/src/api/agentic_api.rs +++ b/src/apps/desktop/src/api/agentic_api.rs @@ -1,5 +1,6 @@ //! Agentic API +use log::warn; use serde::{Deserialize, Serialize}; use std::sync::Arc; use tauri::{AppHandle, State}; @@ -141,7 +142,8 @@ pub async fn create_session( coordinator: State<'_, Arc>, request: CreateSessionRequest, ) -> Result { - let config = request.config + let config = request + .config .map(|c| SessionConfig { max_context_tokens: c.max_context_tokens.unwrap_or(128128), auto_compact: c.auto_compact.unwrap_or(true), @@ -152,7 +154,7 @@ pub async fn create_session( compression_threshold: c.compression_threshold.unwrap_or(0.8), }) .unwrap_or_default(); - + let session = coordinator .create_session_with_id( request.session_id, @@ -177,7 +179,12 @@ pub async fn start_dialog_turn( request: StartDialogTurnRequest, ) -> Result { let _stream = coordinator - .start_dialog_turn(request.session_id, request.user_input, request.turn_id, request.agent_type) + .start_dialog_turn( + request.session_id, + request.user_input, + request.turn_id, + request.agent_type, + ) .await .map_err(|e| format!("Failed to start dialog turn: {}", e))?; @@ -269,11 +276,7 @@ pub async fn list_sessions( agent_type: summary.agent_type, state: format!("{:?}", summary.state), turn_count: summary.turn_count, - created_at: summary - .created_at - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs(), + created_at: system_time_to_unix_secs(summary.created_at), }) .collect(); @@ -375,11 +378,7 @@ fn session_to_response(session: Session) -> SessionResponse { agent_type: session.agent_type, state: format!("{:?}", session.state), turn_count: session.dialog_turn_ids.len(), - created_at: session - .created_at - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs(), + created_at: system_time_to_unix_secs(session.created_at), } } @@ -426,10 +425,16 @@ fn message_to_dto(message: Message) -> MessageDTO { id: message.id, role: role.to_string(), content, - timestamp: message - .timestamp - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs(), + timestamp: system_time_to_unix_secs(message.timestamp), + } +} + +fn system_time_to_unix_secs(time: std::time::SystemTime) -> u64 { + match time.duration_since(std::time::UNIX_EPOCH) { + Ok(duration) => duration.as_secs(), + Err(err) => { + warn!("Failed to convert SystemTime to unix timestamp: {}", err); + 0 + } } } diff --git a/src/apps/desktop/src/api/config_api.rs b/src/apps/desktop/src/api/config_api.rs index 24306690..d00d37d7 100644 --- a/src/apps/desktop/src/api/config_api.rs +++ b/src/apps/desktop/src/api/config_api.rs @@ -1,10 +1,10 @@ //! Configuration API -use tauri::State; use crate::api::app_state::AppState; +use log::{error, info, warn}; +use serde::{Deserialize, Serialize}; use serde_json::Value; -use serde::Deserialize; -use log::{info, warn, error}; +use tauri::State; #[derive(Debug, Deserialize)] pub struct GetConfigRequest { @@ -22,14 +22,21 @@ pub struct ResetConfigRequest { pub path: Option, } +fn to_json_value(value: T, context: &str) -> Result { + serde_json::to_value(value).map_err(|e| format!("Failed to serialize {}: {}", context, e)) +} + #[tauri::command] pub async fn get_config( state: State<'_, AppState>, request: GetConfigRequest, ) -> Result { let config_service = &state.config_service; - - match config_service.get_config::(request.path.as_deref()).await { + + match config_service + .get_config::(request.path.as_deref()) + .await + { Ok(config) => Ok(config), Err(e) => { error!("Failed to get config: path={:?}, error={}", request.path, e); @@ -44,17 +51,24 @@ pub async fn set_config( request: SetConfigRequest, ) -> Result { let config_service = &state.config_service; - - match config_service.set_config(&request.path, request.value).await { + + match config_service + .set_config(&request.path, request.value) + .await + { Ok(_) => { - if request.path.starts_with("ai.models") || - request.path.starts_with("ai.default_models") || - request.path.starts_with("ai.agent_models") || - request.path.starts_with("ai.proxy") { + if request.path.starts_with("ai.models") + || request.path.starts_with("ai.default_models") + || request.path.starts_with("ai.agent_models") + || request.path.starts_with("ai.proxy") + { state.ai_client_factory.invalidate_cache(); - info!("AI config changed, cache invalidated: path={}", request.path); + info!( + "AI config changed, cache invalidated: path={}", + request.path + ); } - + Ok("Configuration set successfully".to_string()) } Err(e) => { @@ -70,7 +84,7 @@ pub async fn reset_config( request: ResetConfigRequest, ) -> Result { let config_service = &state.config_service; - + match config_service.reset_config(request.path.as_deref()).await { Ok(_) => { let message = if let Some(path) = &request.path { @@ -78,33 +92,37 @@ pub async fn reset_config( } else { "All configurations reset successfully".to_string() }; - + let should_invalidate = match &request.path { Some(path) => path.starts_with("ai"), None => true, }; if should_invalidate { state.ai_client_factory.invalidate_cache(); - info!("AI config reset, cache invalidated: path={:?}", request.path); + info!( + "AI config reset, cache invalidated: path={:?}", + request.path + ); } - + Ok(message) } Err(e) => { - error!("Failed to reset config: path={:?}, error={}", request.path, e); + error!( + "Failed to reset config: path={:?}, error={}", + request.path, e + ); Err(format!("Failed to reset config: {}", e)) } } } #[tauri::command] -pub async fn export_config( - state: State<'_, AppState>, -) -> Result { +pub async fn export_config(state: State<'_, AppState>) -> Result { let config_service = &state.config_service; - + match config_service.export_config().await { - Ok(export_data) => Ok(serde_json::to_value(export_data).unwrap()), + Ok(export_data) => Ok(to_json_value(export_data, "export config data")?), Err(e) => { error!("Failed to export config: {}", e); Err(format!("Failed to export config: {}", e)) @@ -113,20 +131,17 @@ pub async fn export_config( } #[tauri::command] -pub async fn import_config( - state: State<'_, AppState>, - config: Value, -) -> Result { +pub async fn import_config(state: State<'_, AppState>, config: Value) -> Result { let config_service = &state.config_service; - - let export_data: bitfun_core::service::config::ConfigExport = serde_json::from_value(config) - .map_err(|e| format!("Invalid config format: {}", e))?; - + + let export_data: bitfun_core::service::config::ConfigExport = + serde_json::from_value(config).map_err(|e| format!("Invalid config format: {}", e))?; + match config_service.import_config(export_data).await { Ok(result) => { state.ai_client_factory.invalidate_cache(); info!("Config imported, AI client cache invalidated"); - Ok(serde_json::to_value(result).unwrap()) + Ok(to_json_value(result, "import config result")?) } Err(e) => { error!("Failed to import config: {}", e); @@ -136,13 +151,14 @@ pub async fn import_config( } #[tauri::command] -pub async fn validate_config( - state: State<'_, AppState>, -) -> Result { +pub async fn validate_config(state: State<'_, AppState>) -> Result { let config_service = &state.config_service; - + match config_service.validate_config().await { - Ok(validation_result) => Ok(serde_json::to_value(validation_result).unwrap()), + Ok(validation_result) => Ok(to_json_value( + validation_result, + "config validation result", + )?), Err(e) => { error!("Failed to validate config: {}", e); Err(format!("Failed to validate config: {}", e)) @@ -151,11 +167,9 @@ pub async fn validate_config( } #[tauri::command] -pub async fn reload_config( - state: State<'_, AppState>, -) -> Result { +pub async fn reload_config(state: State<'_, AppState>) -> Result { let config_service = &state.config_service; - + match config_service.reload().await { Ok(_) => { info!("Config reloaded"); @@ -169,9 +183,7 @@ pub async fn reload_config( } #[tauri::command] -pub async fn sync_config_to_global( - _state: State<'_, AppState>, -) -> Result { +pub async fn sync_config_to_global(_state: State<'_, AppState>) -> Result { match bitfun_core::service::config::reload_global_config().await { Ok(_) => { info!("Config synced to global service"); @@ -190,9 +202,7 @@ pub async fn get_global_config_health() -> Result { } #[tauri::command] -pub async fn get_mode_configs( - state: State<'_, AppState>, -) -> Result { +pub async fn get_mode_configs(state: State<'_, AppState>) -> Result { use bitfun_core::service::config::types::ModeConfig; use std::collections::HashMap; @@ -224,19 +234,26 @@ pub async fn get_mode_configs( } if needs_save { - if let Err(e) = config_service.set_config("ai.mode_configs", serde_json::to_value(&mode_configs).unwrap()).await { - warn!("Failed to save initialized mode configs: {}", e); + match to_json_value(&mode_configs, "mode configs") { + Ok(mode_configs_value) => { + if let Err(e) = config_service + .set_config("ai.mode_configs", mode_configs_value) + .await + { + warn!("Failed to save initialized mode configs: {}", e); + } + } + Err(e) => { + warn!("Failed to serialize initialized mode configs: {}", e); + } } } - Ok(serde_json::to_value(mode_configs).unwrap()) + Ok(to_json_value(mode_configs, "mode configs")?) } #[tauri::command] -pub async fn get_mode_config( - state: State<'_, AppState>, - mode_id: String, -) -> Result { +pub async fn get_mode_config(state: State<'_, AppState>, mode_id: String) -> Result { use bitfun_core::service::config::types::ModeConfig; let config_service = &state.config_service; @@ -261,8 +278,21 @@ pub async fn get_mode_config( enabled: true, default_tools: default_tools, }; - if let Err(e) = config_service.set_config(&path, serde_json::to_value(&new_config).unwrap()).await { - warn!("Failed to save initial mode config: mode_id={}, error={}", mode_id, e); + match to_json_value(&new_config, "initial mode config") { + Ok(new_config_value) => { + if let Err(e) = config_service.set_config(&path, new_config_value).await { + warn!( + "Failed to save initial mode config: mode_id={}, error={}", + mode_id, e + ); + } + } + Err(e) => { + warn!( + "Failed to serialize initial mode config: mode_id={}, error={}", + mode_id, e + ); + } } new_config } else { @@ -276,7 +306,7 @@ pub async fn get_mode_config( } }; - Ok(serde_json::to_value(config).unwrap()) + Ok(to_json_value(config, "mode config")?) } #[tauri::command] @@ -287,19 +317,28 @@ pub async fn set_mode_config( ) -> Result { let config_service = &state.config_service; let path = format!("ai.mode_configs.{}", mode_id); - + match config_service.set_config(&path, config).await { Ok(_) => { if let Err(e) = bitfun_core::service::config::reload_global_config().await { - warn!("Failed to reload global config after mode config change: mode_id={}, error={}", mode_id, e); + warn!( + "Failed to reload global config after mode config change: mode_id={}, error={}", + mode_id, e + ); } else { - info!("Global config reloaded after mode config change: mode_id={}", mode_id); + info!( + "Global config reloaded after mode config change: mode_id={}", + mode_id + ); } - + Ok(format!("Mode '{}' configuration set successfully", mode_id)) } Err(e) => { - error!("Failed to set mode config: mode_id={}, error={}", mode_id, e); + error!( + "Failed to set mode config: mode_id={}, error={}", + mode_id, e + ); Err(format!("Failed to set mode config: {}", e)) } } @@ -328,40 +367,51 @@ pub async fn reset_mode_config( let config_service = &state.config_service; let path = format!("ai.mode_configs.{}", mode_id); + let default_config_value = to_json_value(&default_config, "default mode config")?; - match config_service.set_config(&path, serde_json::to_value(&default_config).unwrap()).await { + match config_service.set_config(&path, default_config_value).await { Ok(_) => { if let Err(e) = bitfun_core::service::config::reload_global_config().await { - warn!("Failed to reload global config after mode config reset: mode_id={}, error={}", mode_id, e); + warn!( + "Failed to reload global config after mode config reset: mode_id={}, error={}", + mode_id, e + ); } else { - info!("Global config reloaded after mode config reset: mode_id={}", mode_id); + info!( + "Global config reloaded after mode config reset: mode_id={}", + mode_id + ); } - Ok(format!("Mode '{}' configuration reset successfully", mode_id)) + Ok(format!( + "Mode '{}' configuration reset successfully", + mode_id + )) } Err(e) => { - error!("Failed to reset mode config: mode_id={}, error={}", mode_id, e); + error!( + "Failed to reset mode config: mode_id={}, error={}", + mode_id, e + ); Err(format!("Failed to reset mode config: {}", e)) } } } #[tauri::command] -pub async fn get_subagent_configs( - state: State<'_, AppState>, -) -> Result { +pub async fn get_subagent_configs(state: State<'_, AppState>) -> Result { use bitfun_core::service::config::types::SubAgentConfig; use std::collections::HashMap; - + let config_service = &state.config_service; let mut subagent_configs: HashMap = config_service .get_config(Some("ai.subagent_configs")) .await .unwrap_or_default(); - + let all_subagents = state.agent_registry.get_subagents_info().await; let mut needs_save = false; - + for subagent in all_subagents { let subagent_id = subagent.id; if !subagent_configs.contains_key(&subagent_id) { @@ -369,14 +419,24 @@ pub async fn get_subagent_configs( needs_save = true; } } - + if needs_save { - if let Err(e) = config_service.set_config("ai.subagent_configs", serde_json::to_value(&subagent_configs).unwrap()).await { - warn!("Failed to save initialized subagent configs: {}", e); + match to_json_value(&subagent_configs, "subagent configs") { + Ok(subagent_configs_value) => { + if let Err(e) = config_service + .set_config("ai.subagent_configs", subagent_configs_value) + .await + { + warn!("Failed to save initialized subagent configs: {}", e); + } + } + Err(e) => { + warn!("Failed to serialize initialized subagent configs: {}", e); + } } } - - Ok(serde_json::to_value(subagent_configs).unwrap()) + + Ok(to_json_value(subagent_configs, "subagent configs")?) } #[tauri::command] @@ -386,40 +446,46 @@ pub async fn set_subagent_config( enabled: bool, ) -> Result { use bitfun_core::service::config::types::SubAgentConfig; - + let config_service = &state.config_service; let config = SubAgentConfig { enabled }; let path = format!("ai.subagent_configs.{}", subagent_id); - - match config_service.set_config(&path, serde_json::to_value(&config).unwrap()).await { + let config_value = to_json_value(&config, "subagent config")?; + + match config_service.set_config(&path, config_value).await { Ok(_) => { if let Err(e) = bitfun_core::service::config::reload_global_config().await { warn!("Failed to reload global config after subagent config change: subagent_id={}, error={}", subagent_id, e); } else { info!("Global config reloaded after subagent config change: subagent_id={}, enabled={}", subagent_id, enabled); } - - Ok(format!("SubAgent '{}' configuration set successfully", subagent_id)) + + Ok(format!( + "SubAgent '{}' configuration set successfully", + subagent_id + )) } Err(e) => { - error!("Failed to set subagent config: subagent_id={}, enabled={}, error={}", subagent_id, enabled, e); + error!( + "Failed to set subagent config: subagent_id={}, enabled={}, error={}", + subagent_id, enabled, e + ); Err(format!("Failed to set SubAgent config: {}", e)) } } } #[tauri::command] -pub async fn sync_tool_configs( - _state: State<'_, AppState>, -) -> Result { +pub async fn sync_tool_configs(_state: State<'_, AppState>) -> Result { match bitfun_core::service::config::tool_config_sync::sync_tool_configs().await { Ok(report) => { - info!("Tool configs synced: new_tools={}, deleted_tools={}, updated_modes={}", + info!( + "Tool configs synced: new_tools={}, deleted_tools={}, updated_modes={}", report.new_tools.len(), report.deleted_tools.len(), report.updated_modes.len() ); - Ok(serde_json::to_value(report).unwrap()) + Ok(to_json_value(report, "tool config sync report")?) } Err(e) => { error!("Failed to sync tool configs: {}", e); diff --git a/src/apps/desktop/src/api/context_upload_api.rs b/src/apps/desktop/src/api/context_upload_api.rs index 0f82ac41..57493c2e 100644 --- a/src/apps/desktop/src/api/context_upload_api.rs +++ b/src/apps/desktop/src/api/context_upload_api.rs @@ -1,14 +1,13 @@ //! Temporary Image Storage API -use log::debug; +use bitfun_core::agentic::tools::image_context::{ + ImageContextData as CoreImageContextData, ImageContextProvider, +}; use dashmap::DashMap; +use log::{debug, warn}; use once_cell::sync::Lazy; use serde::{Deserialize, Serialize}; use std::time::{SystemTime, UNIX_EPOCH}; -use bitfun_core::agentic::tools::image_context::{ - ImageContextProvider, - ImageContextData as CoreImageContextData -}; static IMAGE_STORAGE: Lazy> = Lazy::new(DashMap::new); @@ -47,13 +46,9 @@ pub struct UploadImageContextRequest { } #[tauri::command] -pub async fn upload_image_contexts( - request: UploadImageContextRequest, -) -> Result<(), String> { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_secs(); +pub async fn upload_image_contexts(request: UploadImageContextRequest) -> Result<(), String> { + let timestamp = + current_unix_timestamp().map_err(|e| format!("Failed to get current timestamp: {}", e))?; for image in request.images { let image_id = image.id.clone(); @@ -67,9 +62,7 @@ pub async fn upload_image_contexts( } pub fn get_image_context(image_id: &str) -> Option { - IMAGE_STORAGE - .get(image_id) - .map(|entry| entry.0.clone()) + IMAGE_STORAGE.get(image_id).map(|entry| entry.0.clone()) } pub fn remove_image_context(image_id: &str) { @@ -79,17 +72,23 @@ pub fn remove_image_context(image_id: &str) { } fn cleanup_expired_images(max_age_secs: u64) { - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_secs(); - + let now = match current_unix_timestamp() { + Ok(timestamp) => timestamp, + Err(e) => { + warn!( + "Failed to cleanup expired images due to timestamp error: {}", + e + ); + return; + } + }; + let expired_keys: Vec = IMAGE_STORAGE .iter() - .filter(|entry| now - entry.value().1 > max_age_secs) + .filter(|entry| now.saturating_sub(entry.value().1) > max_age_secs) .map(|entry| entry.key().clone()) .collect(); - + for key in expired_keys { IMAGE_STORAGE.remove(&key); debug!("Cleaned up expired image: image_id={}", key); @@ -103,7 +102,7 @@ impl ImageContextProvider for GlobalImageContextProvider { fn get_image(&self, image_id: &str) -> Option { get_image_context(image_id).map(|data| data.into()) } - + fn remove_image(&self, image_id: &str) { remove_image_context(image_id); } @@ -112,3 +111,9 @@ impl ImageContextProvider for GlobalImageContextProvider { pub fn create_image_context_provider() -> GlobalImageContextProvider { GlobalImageContextProvider } + +fn current_unix_timestamp() -> Result { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) +} diff --git a/src/apps/desktop/src/api/skill_api.rs b/src/apps/desktop/src/api/skill_api.rs index f1055ea3..8b972c82 100644 --- a/src/apps/desktop/src/api/skill_api.rs +++ b/src/apps/desktop/src/api/skill_api.rs @@ -31,7 +31,8 @@ pub async fn get_skill_configs( let all_skills = registry.get_all_skills().await; - Ok(serde_json::to_value(all_skills).unwrap()) + serde_json::to_value(all_skills) + .map_err(|e| format!("Failed to serialize skill configs: {}", e)) } #[tauri::command] @@ -136,7 +137,10 @@ pub async fn add_skill( return Err(validation.error.unwrap_or("Invalid skill path".to_string())); } - let skill_name = validation.name.as_ref().unwrap(); + let skill_name = validation + .name + .as_ref() + .ok_or_else(|| "Skill name missing after validation".to_string())?; let source = Path::new(&source_path); let target_dir = if level == "project" { diff --git a/src/apps/desktop/src/api/subagent_api.rs b/src/apps/desktop/src/api/subagent_api.rs index ddbdfcaf..2462ad36 100644 --- a/src/apps/desktop/src/api/subagent_api.rs +++ b/src/apps/desktop/src/api/subagent_api.rs @@ -158,7 +158,10 @@ pub async fn create_subagent( .chain(subagents.iter().map(|s| s.id.as_str().to_lowercase())) .collect(); if existing.contains(name.to_lowercase().as_str()) { - return Err(format!("Name '{}' conflicts with existing mode or Sub Agent", name)); + return Err(format!( + "Name '{}' conflicts with existing mode or Sub Agent", + name + )); } let pm = state.workspace_service.path_manager(); @@ -171,7 +174,8 @@ pub async fn create_subagent( } }; - std::fs::create_dir_all(&agents_dir).map_err(|e| format!("Failed to create directory: {}", e))?; + std::fs::create_dir_all(&agents_dir) + .map_err(|e| format!("Failed to create directory: {}", e))?; let tools = request.tools.filter(|t| !t.is_empty()).unwrap_or_else(|| { vec![ @@ -201,7 +205,9 @@ pub async fn create_subagent( path_str.clone(), kind, ); - subagent.save_to_file(None, None).map_err(|e| e.to_string())?; + subagent + .save_to_file(None, None) + .map_err(|e| e.to_string())?; let custom_config = CustomSubagentConfig { enabled: subagent.enabled, @@ -274,8 +280,10 @@ pub async fn update_subagent_config( if let Some(enabled) = request.enabled { let config = SubAgentConfig { enabled }; let path = format!("ai.subagent_configs.{}", subagent_id); + let config_value = serde_json::to_value(&config) + .map_err(|e| format!("Failed to serialize subagent config: {}", e))?; config_service - .set_config(&path, serde_json::to_value(&config).unwrap()) + .set_config(&path, config_value) .await .map_err(|e| format!("Failed to update enabled status: {}", e))?; } diff --git a/src/apps/desktop/src/lib.rs b/src/apps/desktop/src/lib.rs index 0010ea02..914cfb8a 100644 --- a/src/apps/desktop/src/lib.rs +++ b/src/apps/desktop/src/lib.rs @@ -60,25 +60,37 @@ pub async fn run() { eprintln!("=== BitFun Desktop Starting ==="); - bitfun_core::service::config::initialize_global_config() - .await - .expect("Failed to initialize global config service"); - - AIClientFactory::initialize_global() - .await - .expect("Failed to initialize global AIClientFactory"); - - let (coordinator, event_queue, event_router, ai_client_factory) = init_agentic_system() - .await - .expect("Failed to initialize agentic system"); + if let Err(e) = bitfun_core::service::config::initialize_global_config().await { + log::error!("Failed to initialize global config service: {}", e); + return; + } + + if let Err(e) = AIClientFactory::initialize_global().await { + log::error!("Failed to initialize global AIClientFactory: {}", e); + return; + } + + let (coordinator, event_queue, event_router, ai_client_factory) = + match init_agentic_system().await { + Ok(state) => state, + Err(e) => { + log::error!("Failed to initialize agentic system: {}", e); + return; + } + }; - init_function_agents(ai_client_factory.clone()) - .await - .expect("Failed to initialize function agents"); + if let Err(e) = init_function_agents(ai_client_factory.clone()).await { + log::error!("Failed to initialize function agents: {}", e); + return; + } - let app_state = AppState::new_async() - .await - .expect("Failed to initialize AppState"); + let app_state = match AppState::new_async().await { + Ok(state) => state, + Err(e) => { + log::error!("Failed to initialize AppState: {}", e); + return; + } + }; let coordinator_state = CoordinatorState { coordinator: coordinator.clone(), @@ -90,7 +102,7 @@ pub async fn run() { setup_panic_hook(); - tauri::Builder::default() + let run_result = tauri::Builder::default() .plugin( tauri_plugin_log::Builder::new() .level(log_config.level) @@ -492,8 +504,10 @@ pub async fn run() { i18n_get_config, i18n_set_config, ]) - .run(tauri::generate_context!()) - .expect("error while running tauri application"); + .run(tauri::generate_context!()); + if let Err(e) = run_result { + log::error!("Error while running tauri application: {}", e); + } } async fn init_agentic_system() -> anyhow::Result<( diff --git a/src/apps/desktop/src/logging.rs b/src/apps/desktop/src/logging.rs index 1a2593b9..418b272a 100644 --- a/src/apps/desktop/src/logging.rs +++ b/src/apps/desktop/src/logging.rs @@ -201,7 +201,12 @@ async fn do_cleanup_log_sessions( logs_root: &PathBuf, max_sessions: usize, ) -> Result<(), std::io::Error> { - let regex = regex::Regex::new(SESSION_DIR_PATTERN).expect("Invalid session dir pattern"); + let regex = regex::Regex::new(SESSION_DIR_PATTERN).map_err(|e| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("Invalid session dir pattern: {}", e), + ) + })?; let mut entries = tokio::fs::read_dir(logs_root).await?; let mut session_dirs: Vec = Vec::new(); diff --git a/src/apps/desktop/src/theme.rs b/src/apps/desktop/src/theme.rs index fd6473c0..58096aa5 100644 --- a/src/apps/desktop/src/theme.rs +++ b/src/apps/desktop/src/theme.rs @@ -220,7 +220,13 @@ pub fn create_main_window(app_handle: &tauri::AppHandle) { let init_script = theme.generate_init_script(); let main_url = if cfg!(debug_assertions) { - WebviewUrl::External("http://localhost:1422".parse().expect("Invalid dev URL")) + match "http://localhost:1422".parse() { + Ok(url) => WebviewUrl::External(url), + Err(e) => { + error!("Invalid dev URL, fallback to app URL: {}", e); + WebviewUrl::App("index.html".into()) + } + } } else { WebviewUrl::App("index.html".into()) }; diff --git a/src/crates/api-layer/src/handlers.rs b/src/crates/api-layer/src/handlers.rs index 2c84c161..9d165cef 100644 --- a/src/crates/api-layer/src/handlers.rs +++ b/src/crates/api-layer/src/handlers.rs @@ -35,7 +35,8 @@ pub async fn handle_execute_agent_task( ) -> Result { info!( "Executing agent task: agent_type={}, message_length={}", - request.agent_type, request.user_message.len() + request.agent_type, + request.user_message.len() ); Ok(ExecuteAgentResponse { @@ -109,7 +110,9 @@ mod tests { #[tokio::test] async fn test_health_check() { let state = CoreAppState::new(); - let response = handle_health_check(&state).await.unwrap(); + let response = handle_health_check(&state) + .await + .expect("health check should always succeed"); assert_eq!(response.status, "healthy"); } } diff --git a/src/crates/core/build.rs b/src/crates/core/build.rs index e7f2a833..13949992 100644 --- a/src/crates/core/build.rs +++ b/src/crates/core/build.rs @@ -61,8 +61,10 @@ fn embed_agents_prompt_data() -> Result<(), Box> { fn check_extension(path: &std::path::Path) -> bool { if let Some(ext) = path.extension() { - let ext = ext.to_str().unwrap(); - ext == "txt" || ext == "md" + match ext.to_str() { + Some(ext) => ext == "txt" || ext == "md", + None => false, + } } else { false } @@ -152,7 +154,10 @@ fn generate_embedded_prompts_code( let mut file = fs::File::create(&dest_path)?; writeln!(file, "// Embedded Agent Prompt data")?; - writeln!(file, "// This file is automatically generated by the build script, do not modify manually")?; + writeln!( + file, + "// This file is automatically generated by the build script, do not modify manually" + )?; writeln!(file)?; writeln!(file, "use std::collections::HashMap;")?; writeln!(file, "use once_cell::sync::Lazy;")?; diff --git a/src/crates/core/src/agentic/agents/registry.rs b/src/crates/core/src/agentic/agents/registry.rs index ee44f610..e5d1ac5a 100644 --- a/src/crates/core/src/agentic/agents/registry.rs +++ b/src/crates/core/src/agentic/agents/registry.rs @@ -148,6 +148,26 @@ pub struct AgentRegistry { } impl AgentRegistry { + fn read_agents(&self) -> std::sync::RwLockReadGuard<'_, HashMap> { + match self.agents.read() { + Ok(guard) => guard, + Err(poisoned) => { + warn!("Agent registry read lock poisoned, recovering"); + poisoned.into_inner() + } + } + } + + fn write_agents(&self) -> std::sync::RwLockWriteGuard<'_, HashMap> { + match self.agents.write() { + Ok(guard) => guard, + Err(poisoned) => { + warn!("Agent registry write lock poisoned, recovering"); + poisoned.into_inner() + } + } + } + /// Create a new agent registry with built-in agents pub fn new() -> Self { let mut agents = HashMap::new(); @@ -220,7 +240,7 @@ impl AgentRegistry { custom_config: Option, ) { let id = agent.id().to_string(); - let mut map = self.agents.write().expect("agents lock"); + let mut map = self.write_agents(); if map.contains_key(&id) { error!("Agent {} already registered, skip registration", id); return; @@ -238,57 +258,37 @@ impl AgentRegistry { /// Get a agent by ID (searches all categories including hidden) pub fn get_agent(&self, agent_type: &str) -> Option> { - self.agents - .read() - .expect("agents lock") - .get(agent_type) - .map(|e| e.agent.clone()) + self.read_agents().get(agent_type).map(|e| e.agent.clone()) } /// Check if an agent exists pub fn check_agent_exists(&self, agent_type: &str) -> bool { - self.agents - .read() - .expect("agents lock") - .contains_key(agent_type) + self.read_agents().contains_key(agent_type) } /// Get a mode by ID pub fn get_mode_agent(&self, agent_type: &str) -> Option> { - self.agents - .read() - .expect("agents lock") - .get(agent_type) - .and_then(|e| { - if e.category == AgentCategory::Mode { - Some(e.agent.clone()) - } else { - None - } - }) + self.read_agents().get(agent_type).and_then(|e| { + if e.category == AgentCategory::Mode { + Some(e.agent.clone()) + } else { + None + } + }) } /// check if a subagent exists with specified source (used for duplicate check before adding) pub fn has_subagent(&self, agent_id: &str, source: SubAgentSource) -> bool { - self.agents - .read() - .expect("agents lock") - .get(agent_id) - .map_or(false, |e| { - e.category == AgentCategory::SubAgent && e.subagent_source == Some(source) - }) + self.read_agents().get(agent_id).map_or(false, |e| { + e.category == AgentCategory::SubAgent && e.subagent_source == Some(source) + }) } /// get agent tools from config /// if not set, return default tools /// tool configuration synchronization is implemented through tool_config_sync, here only read configuration pub async fn get_agent_tools(&self, agent_type: &str) -> Vec { - let entry = self - .agents - .read() - .expect("agents lock") - .get(agent_type) - .cloned(); + let entry = self.read_agents().get(agent_type).cloned(); let Some(entry) = entry else { return Vec::new(); }; @@ -307,7 +307,7 @@ impl AgentRegistry { /// get all mode agent information (including enabled status, used for frontend mode selector etc.) pub async fn get_modes_info(&self) -> Vec { let mode_configs = get_mode_configs().await; - let map = self.agents.read().expect("agents lock"); + let map = self.read_agents(); let mut result: Vec = map .values() .filter(|e| e.category == AgentCategory::Mode) @@ -342,7 +342,7 @@ impl AgentRegistry { /// check if a subagent is readonly (used for TaskTool.is_concurrency_safe etc.) pub fn get_subagent_is_readonly(&self, id: &str) -> Option { - let map = self.agents.read().expect("agents lock"); + let map = self.read_agents(); let entry = map.get(id)?; if entry.category != AgentCategory::SubAgent { return None; @@ -355,7 +355,7 @@ impl AgentRegistry { /// - custom subagent: read enabled and model configuration from custom_config cache pub async fn get_subagents_info(&self) -> Vec { let subagent_configs = get_subagent_configs().await; - let map = self.agents.read().expect("agents lock"); + let map = self.read_agents(); let result: Vec = map .values() .filter(|e| e.category == AgentCategory::SubAgent) @@ -385,7 +385,7 @@ impl AgentRegistry { let valid_models = Self::get_valid_model_ids().await; let custom = CustomSubagentLoader::load_custom_subagents(workspace_root); - let mut map = self.agents.write().expect("agents lock"); + let mut map = self.write_agents(); // remove all non-built-in subagents map.retain(|_, e| { !(e.category == AgentCategory::SubAgent @@ -476,7 +476,7 @@ impl AgentRegistry { /// clear all custom subagents (project/user source), only keep built-in subagents. called when closing workspace. pub fn clear_custom_subagents(&self) { - let mut map = self.agents.write().expect("agents lock"); + let mut map = self.write_agents(); let before = map .values() .filter(|e| e.category == AgentCategory::SubAgent) @@ -498,7 +498,7 @@ impl AgentRegistry { /// get custom subagent configuration (used for updating configuration) /// only custom subagent is valid, return clone of CustomSubagentConfig pub fn get_custom_subagent_config(&self, agent_id: &str) -> Option { - let map = self.agents.read().expect("agents lock"); + let map = self.read_agents(); let entry = map.get(agent_id)?; if entry.category != AgentCategory::SubAgent { return None; @@ -514,7 +514,7 @@ impl AgentRegistry { enabled: Option, model: Option, ) -> BitFunResult<()> { - let mut map = self.agents.write().expect("agents lock"); + let mut map = self.write_agents(); let entry = map .get_mut(agent_id) .ok_or_else(|| BitFunError::agent(format!("Subagent not found: {}", agent_id)))?; @@ -559,7 +559,7 @@ impl AgentRegistry { /// remove single non-built-in subagent, return its file path (used for caller to delete file) /// only allow removing entries that are SubAgent and not Builtin pub fn remove_subagent(&self, agent_id: &str) -> BitFunResult> { - let mut map = self.agents.write().expect("agents lock"); + let mut map = self.write_agents(); let entry = map .get(agent_id) .ok_or_else(|| BitFunError::agent(format!("Subagent not found: {}", agent_id)))?; diff --git a/src/crates/core/src/agentic/core/message.rs b/src/crates/core/src/agentic/core/message.rs index e2fa3ce4..853574e8 100644 --- a/src/crates/core/src/agentic/core/message.rs +++ b/src/crates/core/src/agentic/core/message.rs @@ -167,7 +167,8 @@ impl From for AIMessage { } } else { // If no result_for_assistant, use serialized result - serde_json::to_string(&result).unwrap_or(format!("Tool {} execution completed", tool_name)) + serde_json::to_string(&result) + .unwrap_or(format!("Tool {} execution completed", tool_name)) }; Self { @@ -304,8 +305,8 @@ impl Message { /// Get message's token count pub fn get_tokens(&mut self) -> usize { - if self.metadata.tokens.is_some() { - return self.metadata.tokens.unwrap(); + if let Some(tokens) = self.metadata.tokens { + return tokens; } let tokens = TokenCounter::estimate_message_tokens(&AIMessage::from(&*self)); self.metadata.tokens = Some(tokens); diff --git a/src/crates/core/src/agentic/core/messages_helper.rs b/src/crates/core/src/agentic/core/messages_helper.rs index b8604a5a..203701e1 100644 --- a/src/crates/core/src/agentic/core/messages_helper.rs +++ b/src/crates/core/src/agentic/core/messages_helper.rs @@ -21,12 +21,14 @@ impl MessageHelper { .iter_mut() .for_each(|m| m.metadata.keep_thinking = true); } else { - let last_message_turn_id = messages.last().unwrap().metadata.turn_id.clone(); + let last_message_turn_id = messages.last().and_then(|m| m.metadata.turn_id.clone()); if let Some(last_turn_id) = last_message_turn_id { messages.iter_mut().for_each(|m| { - let cur_turn_id = m.metadata.turn_id.as_ref(); - m.metadata.keep_thinking = - cur_turn_id.is_some() && *cur_turn_id.unwrap() == last_turn_id; + m.metadata.keep_thinking = m + .metadata + .turn_id + .as_ref() + .is_some_and(|cur_turn_id| cur_turn_id == &last_turn_id); }) } else { // Find the index of the last user message (role is user and not ) from back to front diff --git a/src/crates/core/src/agentic/session/compression_manager.rs b/src/crates/core/src/agentic/session/compression_manager.rs index 54147032..dbc5a01c 100644 --- a/src/crates/core/src/agentic/session/compression_manager.rs +++ b/src/crates/core/src/agentic/session/compression_manager.rs @@ -182,8 +182,10 @@ impl CompressionManager { // If the last turn exceeds 30% but not 40%, keep the last turn let token_limit_last_turn = (context_window as f32 * self.config.keep_last_turn_ratio) as usize; - if *turns_tokens.last().unwrap() <= token_limit_last_turn { - turn_index_to_keep = turns_count - 1; + if let Some(last_turn_tokens) = turns_tokens.last() { + if *last_turn_tokens <= token_limit_last_turn { + turn_index_to_keep = turns_count - 1; + } } } debug!("Turn index to keep: {}", turn_index_to_keep); @@ -208,14 +210,21 @@ impl CompressionManager { return Ok(Vec::new()); } - let last_turn_messages = &turns.last().unwrap().messages; + let Some(last_turn_messages) = turns.last().map(|turn| &turn.messages) else { + debug!("No turns available after split, skipping last-turn extraction"); + return Ok(Vec::new()); + }; let last_user_message = { - let last_turn_first_message = last_turn_messages.first().unwrap().clone(); - if last_turn_first_message.role == MessageRole::User { - Some(last_turn_first_message) - } else { - None - } + last_turn_messages + .first() + .cloned() + .and_then(|first_message| { + if first_message.role == MessageRole::User { + Some(first_message) + } else { + None + } + }) }; let last_todo = MessageHelper::get_last_todo(&last_turn_messages); trace!("Last user message: {:?}", last_user_message); diff --git a/src/crates/core/src/agentic/session/session_manager.rs b/src/crates/core/src/agentic/session/session_manager.rs index 92aba44a..f65a7db9 100644 --- a/src/crates/core/src/agentic/session/session_manager.rs +++ b/src/crates/core/src/agentic/session/session_manager.rs @@ -133,7 +133,7 @@ impl SessionManager { info!("Session created: session_name={}", session.session_name); - Ok(self.sessions.get(&session_id).unwrap().clone()) + Ok(session) } /// Get session @@ -633,7 +633,10 @@ impl SessionManager { session.updated_at = SystemTime::now(); Ok(()) } else { - Err(BitFunError::NotFound(format!("Session not found: {}", session_id))) + Err(BitFunError::NotFound(format!( + "Session not found: {}", + session_id + ))) } } @@ -662,7 +665,10 @@ impl SessionManager { user_message.to_string() }; - let user_prompt = format!("User message: {}\n\nPlease generate session title:", truncated_message); + let user_prompt = format!( + "User message: {}\n\nPlease generate session title:", + truncated_message + ); // Construct messages (using AIClient's Message type) let messages = vec![ diff --git a/src/crates/core/src/agentic/tools/framework.rs b/src/crates/core/src/agentic/tools/framework.rs index f67d7d68..bb4aabd3 100644 --- a/src/crates/core/src/agentic/tools/framework.rs +++ b/src/crates/core/src/agentic/tools/framework.rs @@ -201,18 +201,18 @@ pub trait Tool: Send + Sync { ) -> BitFunResult>; async fn call(&self, input: &Value, context: &ToolUseContext) -> BitFunResult> { - if context.cancellation_token.is_none() { - return self.call_impl(input, context).await; - } - let cancellation_token = context.cancellation_token.as_ref().unwrap(); - tokio::select! { - result = self.call_impl(input, context) => { - result - } - - _ = cancellation_token.cancelled() => { - Err(crate::util::errors::BitFunError::Cancelled("Tool execution cancelled".to_string())) + if let Some(cancellation_token) = context.cancellation_token.as_ref() { + tokio::select! { + result = self.call_impl(input, context) => { + result + } + + _ = cancellation_token.cancelled() => { + Err(crate::util::errors::BitFunError::Cancelled("Tool execution cancelled".to_string())) + } } + } else { + self.call_impl(input, context).await } } } diff --git a/src/crates/core/src/agentic/tools/implementations/analyze_image_tool.rs b/src/crates/core/src/agentic/tools/implementations/analyze_image_tool.rs index 91676dd2..25d4c9c8 100644 --- a/src/crates/core/src/agentic/tools/implementations/analyze_image_tool.rs +++ b/src/crates/core/src/agentic/tools/implementations/analyze_image_tool.rs @@ -193,15 +193,21 @@ impl AnalyzeImageTool { .models .iter() .find(|m| { - m.enabled && m.capabilities.iter().any(|cap| { - matches!(cap, crate::service::config::types::ModelCapability::ImageUnderstanding) - }) + m.enabled + && m.capabilities.iter().any(|cap| { + matches!( + cap, + crate::service::config::types::ModelCapability::ImageUnderstanding + ) + }) }) - .ok_or_else(|| BitFunError::service( - "No image understanding model found.\n\ + .ok_or_else(|| { + BitFunError::service( + "No image understanding model found.\n\ Please configure an image understanding model in settings" - .to_string(), - ))? + .to_string(), + ) + })? .clone() }; @@ -252,52 +258,48 @@ impl AnalyzeImageTool { provider: &str, ) -> BitFunResult> { let message = match provider.to_lowercase().as_str() { - "openai" => { - Message { - role: "user".to_string(), - content: Some(serde_json::to_string(&json!([ - { - "type": "image_url", - "image_url": { - "url": format!("data:{};base64,{}", mime_type, base64_data) - } - }, - { - "type": "text", - "text": prompt + "openai" => Message { + role: "user".to_string(), + content: Some(serde_json::to_string(&json!([ + { + "type": "image_url", + "image_url": { + "url": format!("data:{};base64,{}", mime_type, base64_data) } - ]))?), - reasoning_content: None, - thinking_signature: None, - tool_calls: None, - tool_call_id: None, - name: None, - } - } - "anthropic" => { - Message { - role: "user".to_string(), - content: Some(serde_json::to_string(&json!([ - { - "type": "image", - "source": { - "type": "base64", - "media_type": mime_type, - "data": base64_data - } - }, - { - "type": "text", - "text": prompt + }, + { + "type": "text", + "text": prompt + } + ]))?), + reasoning_content: None, + thinking_signature: None, + tool_calls: None, + tool_call_id: None, + name: None, + }, + "anthropic" => Message { + role: "user".to_string(), + content: Some(serde_json::to_string(&json!([ + { + "type": "image", + "source": { + "type": "base64", + "media_type": mime_type, + "data": base64_data } - ]))?), - reasoning_content: None, - thinking_signature: None, - tool_calls: None, - tool_call_id: None, - name: None, - } - } + }, + { + "type": "text", + "text": prompt + } + ]))?), + reasoning_content: None, + thinking_signature: None, + tool_calls: None, + tool_call_id: None, + name: None, + }, _ => { return Err(BitFunError::validation(format!( "Unsupported provider: {}", @@ -636,6 +638,19 @@ Important Notes: &vision_model.provider, )?; + let custom_request_body = vision_model + .custom_request_body + .clone() + .map(|body| { + serde_json::from_str(&body).map_err(|e| { + BitFunError::parse(format!( + "Failed to parse custom request body for model {}: {}", + vision_model.name, e + )) + }) + }) + .transpose()?; + // Vision models cannot set max_tokens (e.g., glm-4v doesn't support this parameter) let model_config = ModelConfig { name: vision_model.name.clone(), @@ -650,10 +665,7 @@ Important Notes: custom_headers: vision_model.custom_headers.clone(), custom_headers_mode: vision_model.custom_headers_mode.clone(), skip_ssl_verify: vision_model.skip_ssl_verify, - custom_request_body: vision_model - .custom_request_body - .clone() - .map(|body| serde_json::from_str(&body).unwrap()), + custom_request_body, }; let ai_client = Arc::new(AIClient::new(model_config)); diff --git a/src/crates/core/src/agentic/tools/implementations/code_review_tool.rs b/src/crates/core/src/agentic/tools/implementations/code_review_tool.rs index 2af520c3..4b8c0196 100644 --- a/src/crates/core/src/agentic/tools/implementations/code_review_tool.rs +++ b/src/crates/core/src/agentic/tools/implementations/code_review_tool.rs @@ -124,15 +124,26 @@ impl CodeReviewTool { "confidence_note": "AI did not return complete review results" }); } else { - let summary = input.get_mut("summary").unwrap(); - if summary.get("overall_assessment").is_none() { - summary["overall_assessment"] = json!("None"); - } - if summary.get("risk_level").is_none() { - summary["risk_level"] = json!("low"); - } - if summary.get("recommended_action").is_none() { - summary["recommended_action"] = json!("approve"); + if let Some(summary) = input.get_mut("summary") { + if summary.get("overall_assessment").is_none() { + summary["overall_assessment"] = json!("None"); + } + if summary.get("risk_level").is_none() { + summary["risk_level"] = json!("low"); + } + if summary.get("recommended_action").is_none() { + summary["recommended_action"] = json!("approve"); + } + } else { + warn!( + "CodeReview tool summary field exists but is not mutable object, using default values" + ); + input["summary"] = json!({ + "overall_assessment": "None", + "risk_level": "low", + "recommended_action": "approve", + "confidence_note": "AI returned invalid summary format" + }); } } diff --git a/src/crates/core/src/agentic/tools/implementations/log_tool.rs b/src/crates/core/src/agentic/tools/implementations/log_tool.rs index b1213ca2..01b1b9d2 100644 --- a/src/crates/core/src/agentic/tools/implementations/log_tool.rs +++ b/src/crates/core/src/agentic/tools/implementations/log_tool.rs @@ -18,11 +18,11 @@ pub struct LogTool; /// LogTool input parameters #[derive(Debug, Clone, Serialize, Deserialize)] pub struct LogToolInput { - pub action: String, // Operation type: "read", "tail", "search", "analyze" + pub action: String, // Operation type: "read", "tail", "search", "analyze" pub log_path: Option, // Log file path - pub lines: Option, // Number of lines to read (for tail operation) - pub pattern: Option, // Search pattern (for search operation) - pub level: Option, // Log level filter: "error", "warn", "info", "debug" + pub lines: Option, // Number of lines to read (for tail operation) + pub pattern: Option, // Search pattern (for search operation) + pub level: Option, // Log level filter: "error", "warn", "info", "debug" } impl LogTool { @@ -312,9 +312,11 @@ The tool will return the log content or analysis results that you can use to dia } }; + let result_for_assistant = + serde_json::to_string_pretty(&result).unwrap_or_else(|_| result.to_string()); Ok(vec![ToolResult::Result { - data: result.clone(), - result_for_assistant: Some(serde_json::to_string_pretty(&result).unwrap()), + data: result, + result_for_assistant: Some(result_for_assistant), }]) } } diff --git a/src/crates/core/src/agentic/tools/implementations/todo_write_tool.rs b/src/crates/core/src/agentic/tools/implementations/todo_write_tool.rs index fd2f9587..be30d217 100644 --- a/src/crates/core/src/agentic/tools/implementations/todo_write_tool.rs +++ b/src/crates/core/src/agentic/tools/implementations/todo_write_tool.rs @@ -280,10 +280,9 @@ When in doubt, use this tool. Being proactive with task management demonstrates } // If no id, generate a new one if !obj.contains_key("id") { - let new_id = format!( - "todo_{}", - uuid::Uuid::new_v4().to_string().split('-').next().unwrap() - ); + let uuid = uuid::Uuid::new_v4().to_string(); + let short_id = uuid.split('-').next().unwrap_or("todo"); + let new_id = format!("todo_{}", short_id); obj.insert("id".to_string(), json!(new_id)); } } diff --git a/src/crates/core/src/agentic/tools/implementations/tool-runtime/src/search/grep_search.rs b/src/crates/core/src/agentic/tools/implementations/tool-runtime/src/search/grep_search.rs index e4da826e..62e822aa 100644 --- a/src/crates/core/src/agentic/tools/implementations/tool-runtime/src/search/grep_search.rs +++ b/src/crates/core/src/agentic/tools/implementations/tool-runtime/src/search/grep_search.rs @@ -52,6 +52,16 @@ struct GrepSink { last_line_number: Arc>>, } +fn lock_recover<'a, T>(mutex: &'a Mutex, name: &str) -> std::sync::MutexGuard<'a, T> { + match mutex.lock() { + Ok(guard) => guard, + Err(poisoned) => { + warn!("Mutex poisoned in grep search: {}", name); + poisoned.into_inner() + } + } +} + impl GrepSink { fn new( output_mode: OutputMode, @@ -76,21 +86,21 @@ impl GrepSink { } fn get_output(&self) -> String { - let output = self.output.lock().unwrap(); + let output = lock_recover(&self.output, "output"); String::from_utf8_lossy(&output).to_string() } fn get_line_count(&self) -> usize { - *self.line_count.lock().unwrap() + *lock_recover(&self.line_count, "line_count") } fn get_match_count(&self) -> usize { - *self.match_count.lock().unwrap() + *lock_recover(&self.match_count, "match_count") } fn should_stop(&self) -> bool { if let Some(limit) = self.head_limit { - let count = *self.line_count.lock().unwrap(); + let count = *lock_recover(&self.line_count, "line_count"); count >= limit } else { false @@ -98,7 +108,7 @@ impl GrepSink { } fn increment_line_count(&self) -> bool { - let mut count = self.line_count.lock().unwrap(); + let mut count = lock_recover(&self.line_count, "line_count"); *count += 1; if let Some(limit) = self.head_limit { *count <= limit @@ -109,7 +119,7 @@ impl GrepSink { fn write_line(&self, line: &[u8]) { if self.increment_line_count() { - let mut output = self.output.lock().unwrap(); + let mut output = lock_recover(&self.output, "output"); output.extend_from_slice(line); output.push(b'\n'); } @@ -123,11 +133,11 @@ impl GrepSink { return; } - let mut last_line = self.last_line_number.lock().unwrap(); + let mut last_line = lock_recover(&self.last_line_number, "last_line_number"); if let Some(last) = *last_line { // If current line number is not continuous with previous line (difference > 1), insert separator if current_line > last + 1 { - let mut output = self.output.lock().unwrap(); + let mut output = lock_recover(&self.output, "output"); output.extend_from_slice(b"--\n"); } } @@ -155,7 +165,7 @@ impl Sink for GrepSink { return Ok(false); } - *self.match_count.lock().unwrap() += 1; + *lock_recover(&self.match_count, "match_count") += 1; match self.output_mode { OutputMode::Content => { @@ -424,7 +434,7 @@ pub fn grep_search( // Add file type filter let mut types_builder = TypesBuilder::new(); types_builder.add_defaults(); - + types_builder .add("arkts", "*.ets") .map_err(|e| format!("Failed to add arkts type: {}", e))?; @@ -445,7 +455,10 @@ pub fn grep_search( types_builder .add(ftype, &glob_pattern) .map_err(|e| format!("Failed to add file type '{}': {}", ftype, e))?; - debug!("Auto-added file type '{}' with glob '{}'", ftype, glob_pattern); + debug!( + "Auto-added file type '{}' with glob '{}'", + ftype, glob_pattern + ); } // User specified type, use user-specified type diff --git a/src/crates/core/src/agentic/util/list_files.rs b/src/crates/core/src/agentic/util/list_files.rs index dd1f10f7..6cc17b6d 100644 --- a/src/crates/core/src/agentic/util/list_files.rs +++ b/src/crates/core/src/agentic/util/list_files.rs @@ -149,7 +149,9 @@ pub fn list_files( break; } - let entry = queue.pop_front().unwrap(); + let Some(entry) = queue.pop_front() else { + continue; + }; let entry_path = &entry.path; // Check if this is a special folder that should not be expanded @@ -384,15 +386,13 @@ pub fn format_files_list(files_list: Vec, dir_path: &str) -> String { // Extract the file/directory name (last component) let name = if child.is_dir { - format!( - "{}/", - child.path[..child.path.len() - 1] - .split('/') - .last() - .unwrap() - ) + let dir_name = child.path[..child.path.len() - 1] + .rsplit('/') + .next() + .unwrap_or(""); + format!("{}/", dir_name) } else { - child.path.split('/').last().unwrap().to_string() + child.path.rsplit('/').next().unwrap_or("").to_string() }; // Choose the appropriate connector diff --git a/src/crates/core/src/infrastructure/ai/client_factory.rs b/src/crates/core/src/infrastructure/ai/client_factory.rs index 8c8e1af3..94300f4b 100644 --- a/src/crates/core/src/infrastructure/ai/client_factory.rs +++ b/src/crates/core/src/infrastructure/ai/client_factory.rs @@ -6,12 +6,12 @@ //! 3. Invalidate cache when configuration changes //! 4. Provide global singleton access -use log::{debug, info}; use crate::infrastructure::ai::AIClient; use crate::service::config::{get_global_config_service, ConfigService}; use crate::util::errors::{BitFunError, BitFunResult}; use crate::util::types::AIConfig; use anyhow::{anyhow, Result}; +use log::{debug, info, warn}; use std::collections::HashMap; use std::sync::{Arc, OnceLock, RwLock}; @@ -79,11 +79,9 @@ impl AIClientFactory { match global_config.ai.default_models.fast { Some(fast_id) => fast_id, - None => { - global_config.ai.default_models.primary.ok_or_else(|| { - anyhow!("Fast model not configured and primary model not configured") - })? - } + None => global_config.ai.default_models.primary.ok_or_else(|| { + anyhow!("Fast model not configured and primary model not configured") + })?, } } _ => model_id.to_string(), @@ -93,19 +91,37 @@ impl AIClientFactory { } pub fn invalidate_cache(&self) { - let mut cache = self.client_cache.write().unwrap(); + let mut cache = match self.client_cache.write() { + Ok(cache) => cache, + Err(poisoned) => { + warn!("AI client cache write lock poisoned during invalidate_cache, recovering"); + poisoned.into_inner() + } + }; let count = cache.len(); cache.clear(); info!("AI client cache cleared (removed {} clients)", count); } pub fn get_cache_size(&self) -> usize { - let cache = self.client_cache.read().unwrap(); + let cache = match self.client_cache.read() { + Ok(cache) => cache, + Err(poisoned) => { + warn!("AI client cache read lock poisoned during get_cache_size, recovering"); + poisoned.into_inner() + } + }; cache.len() } pub fn invalidate_model(&self, model_id: &str) { - let mut cache = self.client_cache.write().unwrap(); + let mut cache = match self.client_cache.write() { + Ok(cache) => cache, + Err(poisoned) => { + warn!("AI client cache write lock poisoned during invalidate_model, recovering"); + poisoned.into_inner() + } + }; if cache.remove(model_id).is_some() { debug!("Client cache cleared for model: {}", model_id); } @@ -113,7 +129,15 @@ impl AIClientFactory { async fn get_or_create_client(&self, model_id: &str) -> Result> { { - let cache = self.client_cache.read().unwrap(); + let cache = match self.client_cache.read() { + Ok(cache) => cache, + Err(poisoned) => { + warn!( + "AI client cache read lock poisoned during get_or_create_client, recovering" + ); + poisoned.into_inner() + } + }; if let Some(client) = cache.get(model_id) { return Ok(client.clone()); } @@ -142,14 +166,21 @@ impl AIClientFactory { let client = Arc::new(AIClient::new_with_proxy(ai_config, proxy_config)); { - let mut cache = self.client_cache.write().unwrap(); + let mut cache = match self.client_cache.write() { + Ok(cache) => cache, + Err(poisoned) => { + warn!( + "AI client cache write lock poisoned during get_or_create_client, recovering" + ); + poisoned.into_inner() + } + }; cache.insert(model_id.to_string(), client.clone()); } debug!( "AI client created: model_id={}, name={}", - model_id, - model_config.name + model_id, model_config.name ); Ok(client) diff --git a/src/crates/core/src/infrastructure/filesystem/file_operations.rs b/src/crates/core/src/infrastructure/filesystem/file_operations.rs index 3027c812..514e4c48 100644 --- a/src/crates/core/src/infrastructure/filesystem/file_operations.rs +++ b/src/crates/core/src/infrastructure/filesystem/file_operations.rs @@ -3,9 +3,9 @@ //! Provides safe file read/write and operations use crate::util::errors::*; +use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; use tokio::fs; -use serde::{Serialize, Deserialize}; pub struct FileOperationService { max_file_size_mb: u64, @@ -91,29 +91,36 @@ impl FileOperationService { pub async fn read_file(&self, file_path: &str) -> BitFunResult { let path = Path::new(file_path); - + self.validate_file_access(path, false).await?; - + if !path.exists() { - return Err(BitFunError::service(format!("File does not exist: {}", file_path))); + return Err(BitFunError::service(format!( + "File does not exist: {}", + file_path + ))); } - + if path.is_dir() { - return Err(BitFunError::service(format!("Path is a directory: {}", file_path))); + return Err(BitFunError::service(format!( + "Path is a directory: {}", + file_path + ))); } - - let metadata = fs::metadata(path).await + + let metadata = fs::metadata(path) + .await .map_err(|e| BitFunError::service(format!("Failed to read file metadata: {}", e)))?; - + let file_size = metadata.len(); if file_size > self.max_file_size_mb * 1024 * 1024 { return Err(BitFunError::service(format!( - "File too large: {}MB (max: {}MB)", - file_size / (1024 * 1024), + "File too large: {}MB (max: {}MB)", + file_size / (1024 * 1024), self.max_file_size_mb ))); } - + match fs::read_to_string(path).await { Ok(content) => { let line_count = content.lines().count(); @@ -126,11 +133,12 @@ impl FileOperationService { }) } Err(_) => { - let bytes = fs::read(path).await + let bytes = fs::read(path) + .await .map_err(|e| BitFunError::service(format!("Failed to read file: {}", e)))?; - + let is_binary = self.is_binary_content(&bytes); - + if is_binary { use base64::Engine; let engine = base64::engine::general_purpose::STANDARD; @@ -156,34 +164,36 @@ impl FileOperationService { } pub async fn write_file( - &self, - file_path: &str, + &self, + file_path: &str, content: &str, options: FileOperationOptions, ) -> BitFunResult { let path = Path::new(file_path); - + self.validate_file_access(path, true).await?; - + let mut backup_created = false; let mut backup_path = None; - + if options.backup_on_overwrite && path.exists() { let backup_file_path = self.create_backup(path).await?; backup_created = true; backup_path = Some(backup_file_path); } - + if let Some(parent) = path.parent() { - fs::create_dir_all(parent).await - .map_err(|e| BitFunError::service(format!("Failed to create parent directory: {}", e)))?; + fs::create_dir_all(parent).await.map_err(|e| { + BitFunError::service(format!("Failed to create parent directory: {}", e)) + })?; } - - fs::write(path, content).await + + fs::write(path, content) + .await .map_err(|e| BitFunError::service(format!("Failed to write file: {}", e)))?; - + let bytes_written = content.len() as u64; - + Ok(FileWriteResult { bytes_written, backup_created, @@ -198,28 +208,30 @@ impl FileOperationService { options: FileOperationOptions, ) -> BitFunResult { let path = Path::new(file_path); - + self.validate_file_access(path, true).await?; - + let mut backup_created = false; let mut backup_path = None; - + if options.backup_on_overwrite && path.exists() { let backup_file_path = self.create_backup(path).await?; backup_created = true; backup_path = Some(backup_file_path); } - + if let Some(parent) = path.parent() { - fs::create_dir_all(parent).await - .map_err(|e| BitFunError::service(format!("Failed to create parent directory: {}", e)))?; + fs::create_dir_all(parent).await.map_err(|e| { + BitFunError::service(format!("Failed to create parent directory: {}", e)) + })?; } - - fs::write(path, data).await + + fs::write(path, data) + .await .map_err(|e| BitFunError::service(format!("Failed to write binary file: {}", e)))?; - + let bytes_written = data.len() as u64; - + Ok(FileWriteResult { bytes_written, backup_created, @@ -230,117 +242,138 @@ impl FileOperationService { pub async fn copy_file(&self, from: &str, to: &str) -> BitFunResult { let from_path = Path::new(from); let to_path = Path::new(to); - + self.validate_file_access(from_path, false).await?; self.validate_file_access(to_path, true).await?; - + if !from_path.exists() { - return Err(BitFunError::service(format!("Source file does not exist: {}", from))); + return Err(BitFunError::service(format!( + "Source file does not exist: {}", + from + ))); } - + if from_path.is_dir() { - return Err(BitFunError::service("Cannot copy directory as file".to_string())); + return Err(BitFunError::service( + "Cannot copy directory as file".to_string(), + )); } - + if let Some(parent) = to_path.parent() { - fs::create_dir_all(parent).await - .map_err(|e| BitFunError::service(format!("Failed to create target directory: {}", e)))?; + fs::create_dir_all(parent).await.map_err(|e| { + BitFunError::service(format!("Failed to create target directory: {}", e)) + })?; } - - let bytes_copied = fs::copy(from_path, to_path).await + + let bytes_copied = fs::copy(from_path, to_path) + .await .map_err(|e| BitFunError::service(format!("Failed to copy file: {}", e)))?; - + Ok(bytes_copied) } pub async fn move_file(&self, from: &str, to: &str) -> BitFunResult<()> { let from_path = Path::new(from); let to_path = Path::new(to); - + self.validate_file_access(from_path, true).await?; self.validate_file_access(to_path, true).await?; - + if !from_path.exists() { - return Err(BitFunError::service(format!("Source file does not exist: {}", from))); + return Err(BitFunError::service(format!( + "Source file does not exist: {}", + from + ))); } - + if let Some(parent) = to_path.parent() { - fs::create_dir_all(parent).await - .map_err(|e| BitFunError::service(format!("Failed to create target directory: {}", e)))?; + fs::create_dir_all(parent).await.map_err(|e| { + BitFunError::service(format!("Failed to create target directory: {}", e)) + })?; } - - fs::rename(from_path, to_path).await + + fs::rename(from_path, to_path) + .await .map_err(|e| BitFunError::service(format!("Failed to move file: {}", e)))?; - + Ok(()) } pub async fn delete_file(&self, file_path: &str) -> BitFunResult<()> { let path = Path::new(file_path); - + self.validate_file_access(path, true).await?; - + if !path.exists() { - return Err(BitFunError::service(format!("File does not exist: {}", file_path))); + return Err(BitFunError::service(format!( + "File does not exist: {}", + file_path + ))); } - + if path.is_dir() { - return Err(BitFunError::service("Cannot delete directory as file".to_string())); + return Err(BitFunError::service( + "Cannot delete directory as file".to_string(), + )); } - - fs::remove_file(path).await + + fs::remove_file(path) + .await .map_err(|e| BitFunError::service(format!("Failed to delete file: {}", e)))?; - + Ok(()) } pub async fn get_file_info(&self, file_path: &str) -> BitFunResult { let path = Path::new(file_path); - + self.validate_file_access(path, false).await?; - + if !path.exists() { - return Err(BitFunError::service(format!("File does not exist: {}", file_path))); + return Err(BitFunError::service(format!( + "File does not exist: {}", + file_path + ))); } - - let metadata = fs::metadata(path).await + + let metadata = fs::metadata(path) + .await .map_err(|e| BitFunError::service(format!("Failed to read file metadata: {}", e)))?; - - let file_name = path.file_name() + + let file_name = path + .file_name() .and_then(|n| n.to_str()) .unwrap_or("") .to_string(); - - let extension = path.extension() + + let extension = path + .extension() .and_then(|e| e.to_str()) .map(|s| s.to_string()); - + let mime_type = if !metadata.is_dir() { self.detect_mime_type(path) } else { None }; - - let created_at = metadata.created().ok() - .map(|t| { - let datetime: chrono::DateTime = t.into(); - datetime.format("%Y-%m-%d %H:%M:%S").to_string() - }); - - let modified_at = metadata.modified().ok() - .map(|t| { - let datetime: chrono::DateTime = t.into(); - datetime.format("%Y-%m-%d %H:%M:%S").to_string() - }); - - let accessed_at = metadata.accessed().ok() - .map(|t| { - let datetime: chrono::DateTime = t.into(); - datetime.format("%Y-%m-%d %H:%M:%S").to_string() - }); - + + let created_at = metadata.created().ok().map(|t| { + let datetime: chrono::DateTime = t.into(); + datetime.format("%Y-%m-%d %H:%M:%S").to_string() + }); + + let modified_at = metadata.modified().ok().map(|t| { + let datetime: chrono::DateTime = t.into(); + datetime.format("%Y-%m-%d %H:%M:%S").to_string() + }); + + let accessed_at = metadata.accessed().ok().map(|t| { + let datetime: chrono::DateTime = t.into(); + datetime.format("%Y-%m-%d %H:%M:%S").to_string() + }); + let permissions = self.get_permissions_string(path).await; - + Ok(FileInfo { path: file_path.to_string(), name: file_name, @@ -358,36 +391,42 @@ impl FileOperationService { pub async fn create_directory(&self, dir_path: &str) -> BitFunResult<()> { let path = Path::new(dir_path); - + self.validate_file_access(path, true).await?; - - fs::create_dir_all(path).await + + fs::create_dir_all(path) + .await .map_err(|e| BitFunError::service(format!("Failed to create directory: {}", e)))?; - + Ok(()) } pub async fn delete_directory(&self, dir_path: &str, recursive: bool) -> BitFunResult<()> { let path = Path::new(dir_path); - + self.validate_file_access(path, true).await?; - + if !path.exists() { - return Err(BitFunError::service(format!("Directory does not exist: {}", dir_path))); + return Err(BitFunError::service(format!( + "Directory does not exist: {}", + dir_path + ))); } - + if !path.is_dir() { return Err(BitFunError::service("Path is not a directory".to_string())); } - + if recursive { - fs::remove_dir_all(path).await - .map_err(|e| BitFunError::service(format!("Failed to delete directory recursively: {}", e)))?; + fs::remove_dir_all(path).await.map_err(|e| { + BitFunError::service(format!("Failed to delete directory recursively: {}", e)) + })?; } else { - fs::remove_dir(path).await + fs::remove_dir(path) + .await .map_err(|e| BitFunError::service(format!("Failed to delete directory: {}", e)))?; } - + Ok(()) } @@ -395,54 +434,65 @@ impl FileOperationService { for restricted in &self.restricted_paths { if path.starts_with(restricted) { return Err(BitFunError::service(format!( - "Access denied: path is in restricted list: {:?}", + "Access denied: path is in restricted list: {:?}", path ))); } } - + if let Some(allowed_extensions) = &self.allowed_extensions { if let Some(ext) = path.extension().and_then(|e| e.to_str()) { if !allowed_extensions.contains(&ext.to_lowercase()) { return Err(BitFunError::service(format!( - "File extension not allowed: {}", + "File extension not allowed: {}", ext ))); } } } - + if is_write { if let Some(parent) = path.parent() { if parent.exists() { - let metadata = fs::metadata(parent).await - .map_err(|e| BitFunError::service(format!("Failed to check parent directory permissions: {}", e)))?; - + let metadata = fs::metadata(parent).await.map_err(|e| { + BitFunError::service(format!( + "Failed to check parent directory permissions: {}", + e + )) + })?; + if metadata.permissions().readonly() { - return Err(BitFunError::service("Parent directory is read-only".to_string())); + return Err(BitFunError::service( + "Parent directory is read-only".to_string(), + )); } } } } - + Ok(()) } async fn create_backup(&self, path: &Path) -> BitFunResult { let timestamp = chrono::Utc::now().format("%Y%m%d_%H%M%S"); - let backup_name = format!("{}.backup_{}", - path.file_name().unwrap().to_string_lossy(), - timestamp); - + let file_name = path.file_name().ok_or_else(|| { + BitFunError::service(format!( + "Failed to create backup: path has no file name: {}", + path.display() + )) + })?; + let backup_name = format!("{}.backup_{}", file_name.to_string_lossy(), timestamp); + let backup_path = if let Some(parent) = path.parent() { parent.join(backup_name) } else { PathBuf::from(backup_name) }; - - fs::copy(path, &backup_path).await + + fs::copy(path, &backup_path) + .await .map_err(|e| BitFunError::service(format!("Failed to create backup: {}", e)))?; - + Ok(backup_path.to_string_lossy().to_string()) } @@ -453,15 +503,16 @@ impl FileOperationService { } else { data }; - + if sample.contains(&0) { return true; } - - let non_printable_count = sample.iter() + + let non_printable_count = sample + .iter() .filter(|&&b| b < 32 && b != 9 && b != 10 && b != 13) .count(); - + let non_printable_ratio = non_printable_count as f64 / sample.len() as f64; non_printable_ratio > 0.1 } @@ -502,26 +553,29 @@ impl FileOperationService { use std::os::unix::fs::PermissionsExt; let perms = metadata.permissions(); let mode = perms.mode(); - - let user = format!("{}{}{}", + + let user = format!( + "{}{}{}", if mode & 0o400 != 0 { "r" } else { "-" }, if mode & 0o200 != 0 { "w" } else { "-" }, if mode & 0o100 != 0 { "x" } else { "-" } ); - let group = format!("{}{}{}", + let group = format!( + "{}{}{}", if mode & 0o040 != 0 { "r" } else { "-" }, if mode & 0o020 != 0 { "w" } else { "-" }, if mode & 0o010 != 0 { "x" } else { "-" } ); - let other = format!("{}{}{}", + let other = format!( + "{}{}{}", if mode & 0o004 != 0 { "r" } else { "-" }, if mode & 0o002 != 0 { "w" } else { "-" }, if mode & 0o001 != 0 { "x" } else { "-" } ); - + Some(format!("{}{}{}", user, group, other)) } - + #[cfg(windows)] { let readonly = metadata.permissions().readonly(); diff --git a/src/crates/core/src/infrastructure/filesystem/file_tree.rs b/src/crates/core/src/infrastructure/filesystem/file_tree.rs index 4b6cfeae..3ae5dddb 100644 --- a/src/crates/core/src/infrastructure/filesystem/file_tree.rs +++ b/src/crates/core/src/infrastructure/filesystem/file_tree.rs @@ -2,18 +2,18 @@ //! //! Provides file tree building, directory scanning, and file search -use log::{warn}; use crate::util::errors::*; +use log::warn; -use std::path::{Path, PathBuf}; -use std::collections::{HashSet, HashMap}; -use std::sync::{Arc, Mutex}; -use std::sync::atomic::{AtomicBool, Ordering}; -use tokio::fs; -use serde::{Serialize, Deserialize}; use grep_regex::RegexMatcherBuilder; use grep_searcher::{Searcher, SearcherBuilder, Sink, SinkMatch}; use ignore::WalkBuilder; +use serde::{Deserialize, Serialize}; +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; +use tokio::fs; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileTreeNode { @@ -27,7 +27,7 @@ pub struct FileTreeNode { #[serde(rename = "lastModified")] pub last_modified: Option, pub extension: Option, - + pub depth: Option, pub is_symlink: Option, pub permissions: Option, @@ -36,12 +36,7 @@ pub struct FileTreeNode { } impl FileTreeNode { - pub fn new( - id: String, - name: String, - path: String, - is_directory: bool, - ) -> Self { + pub fn new(id: String, name: String, path: String, is_directory: bool) -> Self { Self { id, name, @@ -151,6 +146,18 @@ pub struct FileTreeService { options: FileTreeOptions, } +fn lock_search_results( + results: &Arc>>, +) -> std::sync::MutexGuard<'_, Vec> { + match results.lock() { + Ok(guard) => guard, + Err(poisoned) => { + warn!("File search results mutex was poisoned, recovering lock"); + poisoned.into_inner() + } + } +} + impl Default for FileTreeService { fn default() -> Self { Self::new(FileTreeOptions::default()) @@ -164,30 +171,34 @@ impl FileTreeService { pub async fn build_tree(&self, root_path: &str) -> Result, String> { let root_path_buf = PathBuf::from(root_path); - + if !root_path_buf.exists() { return Err("Directory does not exist".to_string()); } - + if !root_path_buf.is_dir() { return Err("Path is not a directory".to_string()); } - + let mut visited = HashSet::new(); - self.build_tree_recursive(&root_path_buf, &root_path_buf, &mut visited, 0).await + self.build_tree_recursive(&root_path_buf, &root_path_buf, &mut visited, 0) + .await } - pub async fn build_tree_with_stats(&self, root_path: &str) -> BitFunResult<(Vec, FileTreeStatistics)> { + pub async fn build_tree_with_stats( + &self, + root_path: &str, + ) -> BitFunResult<(Vec, FileTreeStatistics)> { let root_path_buf = PathBuf::from(root_path); - + if !root_path_buf.exists() { return Err(BitFunError::service("Directory does not exist".to_string())); } - + if !root_path_buf.is_dir() { return Err(BitFunError::service("Path is not a directory".to_string())); } - + let mut visited = HashSet::new(); let mut stats = FileTreeStatistics { total_files: 0, @@ -199,349 +210,388 @@ impl FileTreeService { symlinks_count: 0, hidden_files_count: 0, }; - - let nodes = self.build_tree_recursive_with_stats( - &root_path_buf, - &root_path_buf, - &mut visited, - 0, - &mut stats - ).await - .map_err(|e| BitFunError::service(e))?; - + + let nodes = self + .build_tree_recursive_with_stats( + &root_path_buf, + &root_path_buf, + &mut visited, + 0, + &mut stats, + ) + .await + .map_err(|e| BitFunError::service(e))?; + Ok((nodes, stats)) } fn build_tree_recursive<'a>( &'a self, - path: &'a PathBuf, - root_path: &'a PathBuf, - visited: &'a mut HashSet, - depth: u32 - ) -> std::pin::Pin, String>> + Send + 'a>> { + path: &'a PathBuf, + root_path: &'a PathBuf, + visited: &'a mut HashSet, + depth: u32, + ) -> std::pin::Pin< + Box, String>> + Send + 'a>, + > { Box::pin(async move { - if let Some(max_depth) = self.options.max_depth { - if depth > max_depth { - return Ok(vec![]); + if let Some(max_depth) = self.options.max_depth { + if depth > max_depth { + return Ok(vec![]); + } } - } - - // Prevent cycles - let canonical_path = match path.canonicalize() { - Ok(p) => p, - Err(_) => path.clone(), - }; - - if visited.contains(&canonical_path) { - return Ok(vec![]); - } - visited.insert(canonical_path); - - let mut nodes = Vec::new(); - - let mut read_dir = fs::read_dir(path).await - .map_err(|e| format!("Failed to read directory: {}", e))?; - - let mut entries = Vec::new(); - while let Some(entry) = read_dir.next_entry().await - .map_err(|e| format!("Failed to read directory entry: {}", e))? { - entries.push(entry); - } - - entries.sort_by(|a, b| { - let a_is_dir = a.path().is_dir(); - let b_is_dir = b.path().is_dir(); - match (a_is_dir, b_is_dir) { - (true, false) => std::cmp::Ordering::Less, - (false, true) => std::cmp::Ordering::Greater, - _ => a.file_name().cmp(&b.file_name()), + + // Prevent cycles + let canonical_path = match path.canonicalize() { + Ok(p) => p, + Err(_) => path.clone(), + }; + + if visited.contains(&canonical_path) { + return Ok(vec![]); } - }); - - for entry in entries { - let file_name = entry.file_name(); - let file_name_str = file_name.to_string_lossy(); - - if self.should_skip_file(&file_name_str) { - continue; + visited.insert(canonical_path); + + let mut nodes = Vec::new(); + + let mut read_dir = fs::read_dir(path) + .await + .map_err(|e| format!("Failed to read directory: {}", e))?; + + let mut entries = Vec::new(); + while let Some(entry) = read_dir + .next_entry() + .await + .map_err(|e| format!("Failed to read directory entry: {}", e))? + { + entries.push(entry); } - - let entry_path = entry.path(); - let relative_path = entry_path.strip_prefix(root_path) - .unwrap_or(&entry_path) - .to_string_lossy() - .to_string(); - - let file_type = match entry.file_type().await { - Ok(ft) => ft, - Err(_) => { - match std::fs::symlink_metadata(&entry_path) { + + entries.sort_by(|a, b| { + let a_is_dir = a.path().is_dir(); + let b_is_dir = b.path().is_dir(); + match (a_is_dir, b_is_dir) { + (true, false) => std::cmp::Ordering::Less, + (false, true) => std::cmp::Ordering::Greater, + _ => a.file_name().cmp(&b.file_name()), + } + }); + + for entry in entries { + let file_name = entry.file_name(); + let file_name_str = file_name.to_string_lossy(); + + if self.should_skip_file(&file_name_str) { + continue; + } + + let entry_path = entry.path(); + let relative_path = entry_path + .strip_prefix(root_path) + .unwrap_or(&entry_path) + .to_string_lossy() + .to_string(); + + let file_type = match entry.file_type().await { + Ok(ft) => ft, + Err(_) => match std::fs::symlink_metadata(&entry_path) { Ok(metadata) => metadata.file_type(), Err(e) => { - warn!("Failed to get file type, skipping: {} ({})", entry_path.display(), e); + warn!( + "Failed to get file type, skipping: {} ({})", + entry_path.display(), + e + ); continue; } + }, + }; + + let is_directory = file_type.is_dir(); + let is_symlink = file_type.is_symlink(); + + let metadata = entry.metadata().await.ok(); + let size = if is_directory { + None + } else { + metadata.as_ref().map(|m| m.len()) + }; + + if let (Some(size_bytes), Some(max_mb)) = (size, self.options.max_file_size_mb) { + if size_bytes > max_mb * 1024 * 1024 { + continue; } } - }; - - let is_directory = file_type.is_dir(); - let is_symlink = file_type.is_symlink(); - - let metadata = entry.metadata().await.ok(); - let size = if is_directory { - None - } else { - metadata.as_ref().map(|m| m.len()) - }; - - if let (Some(size_bytes), Some(max_mb)) = (size, self.options.max_file_size_mb) { - if size_bytes > max_mb * 1024 * 1024 { - continue; - } - } - - let last_modified = metadata.and_then(|m| { - m.modified().ok().and_then(|t| { - let datetime: chrono::DateTime = t.into(); - Some(datetime.format("%Y-%m-%d %H:%M:%S").to_string()) - }) - }); - - let extension = if !is_directory { - entry_path.extension().map(|ext| ext.to_string_lossy().to_string()) - } else { - None - }; - - let mime_type = if self.options.include_mime_types && !is_directory { - self.detect_mime_type(&entry_path) - } else { - None - }; - - let permissions = self.get_permissions_string(&entry_path).await; - - let mut node = FileTreeNode::new( - relative_path, - file_name_str.to_string(), - entry_path.to_string_lossy().to_string(), - is_directory, - ) - .with_metadata(size, last_modified) - .with_extension(extension) - .with_depth(depth) - .with_enhanced_info(is_symlink, permissions, mime_type, None); - - if is_directory { - if !is_symlink || self.options.follow_symlinks { - match self.build_tree_recursive(&entry_path, root_path, visited, depth + 1).await { - Ok(children) => { - node = node.with_children(children); - } - Err(_) => { - node = node.with_children(vec![]); + + let last_modified = metadata.and_then(|m| { + m.modified().ok().and_then(|t| { + let datetime: chrono::DateTime = t.into(); + Some(datetime.format("%Y-%m-%d %H:%M:%S").to_string()) + }) + }); + + let extension = if !is_directory { + entry_path + .extension() + .map(|ext| ext.to_string_lossy().to_string()) + } else { + None + }; + + let mime_type = if self.options.include_mime_types && !is_directory { + self.detect_mime_type(&entry_path) + } else { + None + }; + + let permissions = self.get_permissions_string(&entry_path).await; + + let mut node = FileTreeNode::new( + relative_path, + file_name_str.to_string(), + entry_path.to_string_lossy().to_string(), + is_directory, + ) + .with_metadata(size, last_modified) + .with_extension(extension) + .with_depth(depth) + .with_enhanced_info(is_symlink, permissions, mime_type, None); + + if is_directory { + if !is_symlink || self.options.follow_symlinks { + match self + .build_tree_recursive(&entry_path, root_path, visited, depth + 1) + .await + { + Ok(children) => { + node = node.with_children(children); + } + Err(_) => { + node = node.with_children(vec![]); + } } } } + + nodes.push(node); } - - nodes.push(node); - } - - Ok(nodes) + + Ok(nodes) }) } fn build_tree_recursive_with_stats<'a>( &'a self, - path: &'a PathBuf, - root_path: &'a PathBuf, - visited: &'a mut HashSet, + path: &'a PathBuf, + root_path: &'a PathBuf, + visited: &'a mut HashSet, depth: u32, stats: &'a mut FileTreeStatistics, - ) -> std::pin::Pin, String>> + Send + 'a>> { + ) -> std::pin::Pin< + Box, String>> + Send + 'a>, + > { Box::pin(async move { - if depth > stats.max_depth_reached { - stats.max_depth_reached = depth; - } - - if let Some(max_depth) = self.options.max_depth { - if depth > max_depth { - return Ok(vec![]); + if depth > stats.max_depth_reached { + stats.max_depth_reached = depth; } - } - - // Prevent cycles - let canonical_path = match path.canonicalize() { - Ok(p) => p, - Err(_) => path.clone(), - }; - - if visited.contains(&canonical_path) { - return Ok(vec![]); - } - visited.insert(canonical_path); - - let mut nodes = Vec::new(); - - let mut read_dir = fs::read_dir(path).await - .map_err(|e| format!("Failed to read directory: {}", e))?; - - let mut entries = Vec::new(); - while let Some(entry) = read_dir.next_entry().await - .map_err(|e| format!("Failed to read directory entry: {}", e))? { - entries.push(entry); - } - - entries.sort_by(|a, b| { - let a_is_dir = a.path().is_dir(); - let b_is_dir = b.path().is_dir(); - match (a_is_dir, b_is_dir) { - (true, false) => std::cmp::Ordering::Less, - (false, true) => std::cmp::Ordering::Greater, - _ => a.file_name().cmp(&b.file_name()), + + if let Some(max_depth) = self.options.max_depth { + if depth > max_depth { + return Ok(vec![]); + } } - }); - - for entry in entries { - let file_name = entry.file_name(); - let file_name_str = file_name.to_string_lossy(); - - if file_name_str.starts_with('.') { - stats.hidden_files_count += 1; + + // Prevent cycles + let canonical_path = match path.canonicalize() { + Ok(p) => p, + Err(_) => path.clone(), + }; + + if visited.contains(&canonical_path) { + return Ok(vec![]); } - - if self.should_skip_file(&file_name_str) { - continue; + visited.insert(canonical_path); + + let mut nodes = Vec::new(); + + let mut read_dir = fs::read_dir(path) + .await + .map_err(|e| format!("Failed to read directory: {}", e))?; + + let mut entries = Vec::new(); + while let Some(entry) = read_dir + .next_entry() + .await + .map_err(|e| format!("Failed to read directory entry: {}", e))? + { + entries.push(entry); } - - let entry_path = entry.path(); - let relative_path = entry_path.strip_prefix(root_path) - .unwrap_or(&entry_path) - .to_string_lossy() - .to_string(); - - let file_type = match entry.file_type().await { - Ok(ft) => ft, - Err(_) => { - match std::fs::symlink_metadata(&entry_path) { + + entries.sort_by(|a, b| { + let a_is_dir = a.path().is_dir(); + let b_is_dir = b.path().is_dir(); + match (a_is_dir, b_is_dir) { + (true, false) => std::cmp::Ordering::Less, + (false, true) => std::cmp::Ordering::Greater, + _ => a.file_name().cmp(&b.file_name()), + } + }); + + for entry in entries { + let file_name = entry.file_name(); + let file_name_str = file_name.to_string_lossy(); + + if file_name_str.starts_with('.') { + stats.hidden_files_count += 1; + } + + if self.should_skip_file(&file_name_str) { + continue; + } + + let entry_path = entry.path(); + let relative_path = entry_path + .strip_prefix(root_path) + .unwrap_or(&entry_path) + .to_string_lossy() + .to_string(); + + let file_type = match entry.file_type().await { + Ok(ft) => ft, + Err(_) => match std::fs::symlink_metadata(&entry_path) { Ok(metadata) => metadata.file_type(), Err(e) => { - warn!("Failed to get file type, skipping: {} ({})", entry_path.display(), e); + warn!( + "Failed to get file type, skipping: {} ({})", + entry_path.display(), + e + ); continue; } + }, + }; + + let is_directory = file_type.is_dir(); + let is_symlink = file_type.is_symlink(); + + if is_directory { + stats.total_directories += 1; + } else { + stats.total_files += 1; + } + + if is_symlink { + stats.symlinks_count += 1; + } + + let metadata = entry.metadata().await.ok(); + let size = if is_directory { + None + } else { + metadata.as_ref().map(|m| m.len()) + }; + + if let Some(file_size) = size { + stats.total_size_bytes += file_size; + + if file_size > 10 * 1024 * 1024 { + stats + .large_files + .push((entry_path.to_string_lossy().to_string(), file_size)); } } - }; - - let is_directory = file_type.is_dir(); - let is_symlink = file_type.is_symlink(); - - if is_directory { - stats.total_directories += 1; - } else { - stats.total_files += 1; - } - - if is_symlink { - stats.symlinks_count += 1; - } - - let metadata = entry.metadata().await.ok(); - let size = if is_directory { - None - } else { - metadata.as_ref().map(|m| m.len()) - }; - - if let Some(file_size) = size { - stats.total_size_bytes += file_size; - - if file_size > 10 * 1024 * 1024 { - stats.large_files.push((entry_path.to_string_lossy().to_string(), file_size)); + + if let (Some(size_bytes), Some(max_mb)) = (size, self.options.max_file_size_mb) { + if size_bytes > max_mb * 1024 * 1024 { + continue; + } } - } - - if let (Some(size_bytes), Some(max_mb)) = (size, self.options.max_file_size_mb) { - if size_bytes > max_mb * 1024 * 1024 { - continue; + + if !is_directory { + if let Some(ext) = entry_path.extension().and_then(|e| e.to_str()) { + *stats.file_type_counts.entry(ext.to_string()).or_insert(0) += 1; + } else { + *stats + .file_type_counts + .entry("no_extension".to_string()) + .or_insert(0) += 1; + } } - } - - if !is_directory { - if let Some(ext) = entry_path.extension().and_then(|e| e.to_str()) { - *stats.file_type_counts.entry(ext.to_string()).or_insert(0) += 1; + + let last_modified = metadata.and_then(|m| { + m.modified().ok().and_then(|t| { + let datetime: chrono::DateTime = t.into(); + Some(datetime.format("%Y-%m-%d %H:%M:%S").to_string()) + }) + }); + + let extension = if !is_directory { + entry_path + .extension() + .map(|ext| ext.to_string_lossy().to_string()) } else { - *stats.file_type_counts.entry("no_extension".to_string()).or_insert(0) += 1; - } - } - - let last_modified = metadata.and_then(|m| { - m.modified().ok().and_then(|t| { - let datetime: chrono::DateTime = t.into(); - Some(datetime.format("%Y-%m-%d %H:%M:%S").to_string()) - }) - }); - - let extension = if !is_directory { - entry_path.extension().map(|ext| ext.to_string_lossy().to_string()) - } else { - None - }; - - let mime_type = if self.options.include_mime_types && !is_directory { - self.detect_mime_type(&entry_path) - } else { - None - }; - - let permissions = self.get_permissions_string(&entry_path).await; - - let mut node = FileTreeNode::new( - relative_path, - file_name_str.to_string(), - entry_path.to_string_lossy().to_string(), - is_directory, - ) - .with_metadata(size, last_modified) - .with_extension(extension) - .with_depth(depth) - .with_enhanced_info(is_symlink, permissions, mime_type, None); - - if is_directory { - if !is_symlink || self.options.follow_symlinks { - match self.build_tree_recursive_with_stats( - &entry_path, - root_path, - visited, - depth + 1, - stats - ).await { - Ok(children) => { - node = node.with_children(children); - } - Err(_) => { - node = node.with_children(vec![]); + None + }; + + let mime_type = if self.options.include_mime_types && !is_directory { + self.detect_mime_type(&entry_path) + } else { + None + }; + + let permissions = self.get_permissions_string(&entry_path).await; + + let mut node = FileTreeNode::new( + relative_path, + file_name_str.to_string(), + entry_path.to_string_lossy().to_string(), + is_directory, + ) + .with_metadata(size, last_modified) + .with_extension(extension) + .with_depth(depth) + .with_enhanced_info(is_symlink, permissions, mime_type, None); + + if is_directory { + if !is_symlink || self.options.follow_symlinks { + match self + .build_tree_recursive_with_stats( + &entry_path, + root_path, + visited, + depth + 1, + stats, + ) + .await + { + Ok(children) => { + node = node.with_children(children); + } + Err(_) => { + node = node.with_children(vec![]); + } } } } + + nodes.push(node); } - - nodes.push(node); - } - - Ok(nodes) + + Ok(nodes) }) } fn should_skip_file(&self, file_name: &str) -> bool { // Skip hidden files and directories (unless explicitly included) // But .gitignore and .bitfun are always shown - if !self.options.include_hidden && file_name.starts_with('.') && file_name != ".gitignore" && file_name != ".bitfun" { + if !self.options.include_hidden + && file_name.starts_with('.') + && file_name != ".gitignore" + && file_name != ".bitfun" + { return true; } - + self.options.skip_patterns.iter().any(|pattern| { if pattern.contains('*') { let parts: Vec<&str> = pattern.split('*').collect(); @@ -558,45 +608,46 @@ impl FileTreeService { pub async fn get_directory_contents(&self, path: &str) -> Result, String> { let path_buf = PathBuf::from(path); - + if !path_buf.exists() { return Err("Directory does not exist".to_string()); } - + if !path_buf.is_dir() { return Err("Path is not a directory".to_string()); } - + let mut nodes = Vec::new(); - - let mut read_dir = fs::read_dir(&path_buf).await + + let mut read_dir = fs::read_dir(&path_buf) + .await .map_err(|e| format!("Failed to read directory: {}", e))?; - - while let Some(entry) = read_dir.next_entry().await - .map_err(|e| format!("Failed to read directory entry: {}", e))? { - + + while let Some(entry) = read_dir + .next_entry() + .await + .map_err(|e| format!("Failed to read directory entry: {}", e))? + { let file_name = entry.file_name(); let file_name_str = file_name.to_string_lossy(); - + if self.should_skip_file(&file_name_str) { continue; } - + let entry_path = entry.path(); - let is_directory = entry.file_type().await - .map(|t| t.is_dir()) - .unwrap_or(false); - + let is_directory = entry.file_type().await.map(|t| t.is_dir()).unwrap_or(false); + let node = FileTreeNode::new( entry_path.to_string_lossy().to_string(), file_name_str.to_string(), entry_path.to_string_lossy().to_string(), is_directory, ); - + nodes.push(node); } - + Ok(nodes) } @@ -610,7 +661,7 @@ impl FileTreeService { "json" => Some("application/json".to_string()), "xml" => Some("application/xml".to_string()), "yaml" | "yml" => Some("application/yaml".to_string()), - + "rs" => Some("text/rust".to_string()), "py" => Some("text/python".to_string()), "java" => Some("text/java".to_string()), @@ -621,23 +672,23 @@ impl FileTreeService { "php" => Some("text/php".to_string()), "rb" => Some("text/ruby".to_string()), "ts" => Some("application/typescript".to_string()), - + "png" => Some("image/png".to_string()), "jpg" | "jpeg" => Some("image/jpeg".to_string()), "gif" => Some("image/gif".to_string()), "svg" => Some("image/svg+xml".to_string()), "webp" => Some("image/webp".to_string()), - + "pdf" => Some("application/pdf".to_string()), "doc" | "docx" => Some("application/msword".to_string()), "xls" | "xlsx" => Some("application/excel".to_string()), "ppt" | "pptx" => Some("application/powerpoint".to_string()), - + "zip" => Some("application/zip".to_string()), "tar" => Some("application/tar".to_string()), "gz" => Some("application/gzip".to_string()), "rar" => Some("application/rar".to_string()), - + _ => None, } } else { @@ -652,26 +703,29 @@ impl FileTreeService { use std::os::unix::fs::PermissionsExt; let perms = metadata.permissions(); let mode = perms.mode(); - - let user = format!("{}{}{}", + + let user = format!( + "{}{}{}", if mode & 0o400 != 0 { "r" } else { "-" }, if mode & 0o200 != 0 { "w" } else { "-" }, if mode & 0o100 != 0 { "x" } else { "-" } ); - let group = format!("{}{}{}", + let group = format!( + "{}{}{}", if mode & 0o040 != 0 { "r" } else { "-" }, if mode & 0o020 != 0 { "w" } else { "-" }, if mode & 0o010 != 0 { "x" } else { "-" } ); - let other = format!("{}{}{}", + let other = format!( + "{}{}{}", if mode & 0o004 != 0 { "r" } else { "-" }, if mode & 0o002 != 0 { "w" } else { "-" }, if mode & 0o001 != 0 { "x" } else { "-" } ); - + Some(format!("{}{}{}", user, group, other)) } - + #[cfg(windows)] { let readonly = metadata.permissions().readonly(); @@ -695,9 +749,10 @@ impl FileTreeService { false, // case_sensitive false, // regex false, // whole_word - ).await + ) + .await } - + pub async fn search_files_with_options( &self, root_path: &str, @@ -708,13 +763,13 @@ impl FileTreeService { whole_word: bool, ) -> BitFunResult> { let root_path_buf = PathBuf::from(root_path); - + if !root_path_buf.exists() { return Err(BitFunError::service("Directory does not exist".to_string())); } - + let max_results = 10000; - + let filename_pattern = if use_regex { pattern.to_string() } else if whole_word { @@ -722,13 +777,13 @@ impl FileTreeService { } else { regex::escape(pattern) }; - + let results = Arc::new(Mutex::new(Vec::new())); let should_stop = Arc::new(AtomicBool::new(false)); - + let pattern = pattern.to_string(); let filename_pattern = Arc::new(filename_pattern); - + let walker = WalkBuilder::new(&root_path_buf) .hidden(false) .ignore(true) @@ -737,36 +792,37 @@ impl FileTreeService { .git_exclude(false) .threads(num_cpus::get().min(8)) .build_parallel(); - + walker.run(|| { let results = Arc::clone(&results); let should_stop = Arc::clone(&should_stop); let pattern = pattern.clone(); let filename_pattern = Arc::clone(&filename_pattern); let root_path_buf = root_path_buf.clone(); - + Box::new(move |entry| { if should_stop.load(Ordering::Relaxed) { return ignore::WalkState::Quit; } - + let entry = match entry { Ok(e) => e, Err(_) => return ignore::WalkState::Continue, }; - + let path = entry.path(); let is_dir = path.is_dir(); let is_file = path.is_file(); - + if path == root_path_buf { return ignore::WalkState::Continue; } - - let file_name = path.file_name() + + let file_name = path + .file_name() .map(|n| n.to_string_lossy().to_string()) .unwrap_or_default(); - + if is_dir { let dir_matches = if case_sensitive { if use_regex || whole_word { @@ -787,9 +843,9 @@ impl FileTreeService { file_name.to_lowercase().contains(&pattern.to_lowercase()) } }; - + if dir_matches { - let mut results_guard = results.lock().unwrap(); + let mut results_guard = lock_search_results(&results); if results_guard.len() < max_results { results_guard.push(FileSearchResult { path: path.to_string_lossy().to_string(), @@ -800,34 +856,36 @@ impl FileTreeService { matched_content: None, }); } - + if results_guard.len() >= max_results { should_stop.store(true, Ordering::Relaxed); return ignore::WalkState::Quit; } } - + return ignore::WalkState::Continue; } - + if !is_file { return ignore::WalkState::Continue; } - + if let Some(file_name) = path.file_name() { let file_name_str = file_name.to_string_lossy(); - if Self::should_skip_file_static(&file_name_str) || Self::is_binary_file_static(&file_name_str) { + if Self::should_skip_file_static(&file_name_str) + || Self::is_binary_file_static(&file_name_str) + { return ignore::WalkState::Continue; } } - + if let Ok(metadata) = path.metadata() { - let max_size = 10 * 1024 * 1024; + let max_size = 10 * 1024 * 1024; if metadata.len() > max_size { return ignore::WalkState::Continue; } } - + let filename_matches = if case_sensitive { if use_regex || whole_word { regex::Regex::new(&filename_pattern) @@ -847,9 +905,9 @@ impl FileTreeService { file_name.to_lowercase().contains(&pattern.to_lowercase()) } }; - + if filename_matches { - let mut results_guard = results.lock().unwrap(); + let mut results_guard = lock_search_results(&results); if results_guard.len() < max_results { results_guard.push(FileSearchResult { path: path.to_string_lossy().to_string(), @@ -860,15 +918,15 @@ impl FileTreeService { matched_content: None, }); } - + if results_guard.len() >= max_results { should_stop.store(true, Ordering::Relaxed); return ignore::WalkState::Quit; } } - + if search_content { - let results_len = results.lock().unwrap().len(); + let results_len = lock_search_results(&results).len(); if results_len < max_results { if let Err(e) = Self::search_file_content_static( path, @@ -883,24 +941,24 @@ impl FileTreeService { ) { warn!("Failed to search file content {}: {}", path.display(), e); } - - let results_len = results.lock().unwrap().len(); + + let results_len = lock_search_results(&results).len(); if results_len >= max_results { should_stop.store(true, Ordering::Relaxed); return ignore::WalkState::Quit; } } } - + ignore::WalkState::Continue }) }); - - let final_results = results.lock().unwrap().clone(); - + + let final_results = lock_search_results(&results).clone(); + Ok(final_results) } - + fn search_file_content_static( path: &Path, file_name: &str, @@ -915,7 +973,7 @@ impl FileTreeService { if should_stop.load(Ordering::Relaxed) { return Ok(()); } - + let search_pattern = if use_regex { pattern.to_string() } else if whole_word { @@ -923,16 +981,14 @@ impl FileTreeService { } else { regex::escape(pattern) }; - + let matcher = RegexMatcherBuilder::new() .case_insensitive(!case_sensitive) .build(&search_pattern) .map_err(|e| BitFunError::service(format!("Invalid regex pattern: {}", e)))?; - - let mut searcher = SearcherBuilder::new() - .line_number(true) - .build(); - + + let mut searcher = SearcherBuilder::new().line_number(true).build(); + let mut sink = FileContentSinkThreadSafe { path: path.to_path_buf(), file_name: file_name.to_string(), @@ -940,13 +996,14 @@ impl FileTreeService { max_results, should_stop, }; - - searcher.search_path(&matcher, path, &mut sink) + + searcher + .search_path(&matcher, path, &mut sink) .map_err(|e| BitFunError::service(format!("Search error: {}", e)))?; - + Ok(()) } - + fn should_skip_file_static(file_name: &str) -> bool { let skip_patterns = [ "node_modules", @@ -960,24 +1017,25 @@ impl FileTreeService { ".DS_Store", "Thumbs.db", ]; - - skip_patterns.iter().any(|pattern| file_name.contains(pattern)) + + skip_patterns + .iter() + .any(|pattern| file_name.contains(pattern)) } - + fn is_binary_file_static(file_name: &str) -> bool { let binary_extensions = [ - ".png", ".jpg", ".jpeg", ".gif", ".bmp", ".ico", ".svg", ".webp", - ".mp4", ".avi", ".mov", ".wmv", ".flv", ".mkv", - ".mp3", ".wav", ".flac", ".aac", ".ogg", - ".zip", ".tar", ".gz", ".7z", ".rar", ".bz2", - ".pdf", ".doc", ".docx", ".xls", ".xlsx", ".ppt", ".pptx", - ".woff", ".woff2", ".ttf", ".otf", ".eot", - ".exe", ".dll", ".so", ".dylib", ".bin", - ".pyc", ".class", ".o", ".a", ".lib", + ".png", ".jpg", ".jpeg", ".gif", ".bmp", ".ico", ".svg", ".webp", ".mp4", ".avi", + ".mov", ".wmv", ".flv", ".mkv", ".mp3", ".wav", ".flac", ".aac", ".ogg", ".zip", + ".tar", ".gz", ".7z", ".rar", ".bz2", ".pdf", ".doc", ".docx", ".xls", ".xlsx", ".ppt", + ".pptx", ".woff", ".woff2", ".ttf", ".otf", ".eot", ".exe", ".dll", ".so", ".dylib", + ".bin", ".pyc", ".class", ".o", ".a", ".lib", ]; - + let lower_name = file_name.to_lowercase(); - binary_extensions.iter().any(|ext| lower_name.ends_with(ext)) + binary_extensions + .iter() + .any(|ext| lower_name.ends_with(ext)) } } @@ -991,22 +1049,22 @@ struct FileContentSinkThreadSafe { impl Sink for FileContentSinkThreadSafe { type Error = std::io::Error; - + fn matched(&mut self, _searcher: &Searcher, mat: &SinkMatch<'_>) -> Result { if self.should_stop.load(Ordering::Relaxed) { return Ok(false); } - - let mut results = self.results.lock().unwrap(); - + + let mut results = lock_search_results(&self.results); + if results.len() >= self.max_results { self.should_stop.store(true, Ordering::Relaxed); return Ok(false); } - + let line_number = mat.line_number().unwrap_or(0) as usize; let matched_line = String::from_utf8_lossy(mat.bytes()).trim_end().to_string(); - + results.push(FileSearchResult { path: self.path.to_string_lossy().to_string(), name: self.file_name.clone(), @@ -1015,12 +1073,12 @@ impl Sink for FileContentSinkThreadSafe { line_number: Some(line_number), matched_content: Some(matched_line), }); - + let should_continue = results.len() < self.max_results; if !should_continue { self.should_stop.store(true, Ordering::Relaxed); } - + Ok(should_continue) } } diff --git a/src/crates/core/src/infrastructure/filesystem/file_watcher.rs b/src/crates/core/src/infrastructure/filesystem/file_watcher.rs index 3b586609..796a8b8c 100644 --- a/src/crates/core/src/infrastructure/filesystem/file_watcher.rs +++ b/src/crates/core/src/infrastructure/filesystem/file_watcher.rs @@ -2,6 +2,7 @@ //! //! Uses the notify crate to watch filesystem changes and send them to the frontend via Tauri events +use crate::infrastructure::events::EventEmitter; use log::{debug, error}; use notify::{Config, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher}; use serde::{Deserialize, Serialize}; @@ -9,7 +10,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex as StdMutex}; use tokio::sync::{Mutex, RwLock}; -use crate::infrastructure::events::EventEmitter; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileWatchEvent { @@ -67,6 +67,18 @@ pub struct FileWatcher { config: FileWatcherConfig, } +fn lock_event_buffer( + event_buffer: &StdMutex>, +) -> std::sync::MutexGuard<'_, Vec> { + match event_buffer.lock() { + Ok(buffer) => buffer, + Err(poisoned) => { + error!("File watcher event buffer mutex was poisoned, recovering lock"); + poisoned.into_inner() + } + } +} + impl FileWatcher { pub fn new(config: FileWatcherConfig) -> Self { Self { @@ -83,16 +95,23 @@ impl FileWatcher { *e = Some(emitter); } - pub async fn watch_path(&self, path: &str, config: Option) -> Result<(), String> { + pub async fn watch_path( + &self, + path: &str, + config: Option, + ) -> Result<(), String> { let path_buf = PathBuf::from(path); - + if !path_buf.exists() { return Err("Path does not exist".to_string()); } { let mut watched_paths = self.watched_paths.write().await; - watched_paths.insert(path_buf.clone(), config.unwrap_or_else(|| self.config.clone())); + watched_paths.insert( + path_buf.clone(), + config.unwrap_or_else(|| self.config.clone()), + ); } self.create_watcher().await?; @@ -102,7 +121,7 @@ impl FileWatcher { pub async fn unwatch_path(&self, path: &str) -> Result<(), String> { let path_buf = PathBuf::from(path); - + { let mut watched_paths = self.watched_paths.write().await; watched_paths.remove(&path_buf); @@ -115,7 +134,7 @@ impl FileWatcher { async fn create_watcher(&self) -> Result<(), String> { let watched_paths = self.watched_paths.read().await; - + if watched_paths.is_empty() { let mut watcher = self.watcher.lock().await; *watcher = None; @@ -133,7 +152,8 @@ impl FileWatcher { RecursiveMode::NonRecursive }; - watcher.watch(path, mode) + watcher + .watch(path, mode) .map_err(|e| format!("Failed to watch path {}: {}", path.display(), e))?; } @@ -149,7 +169,7 @@ impl FileWatcher { tokio::spawn(async move { let mut last_flush = std::time::Instant::now(); - + while let Ok(event) = rx.recv() { match event { Ok(event) => { @@ -159,12 +179,14 @@ impl FileWatcher { if let Some(file_event) = Self::convert_event(&event) { { - let mut buffer = event_buffer.lock().unwrap(); + let mut buffer = lock_event_buffer(&event_buffer); buffer.push(file_event); } let now = std::time::Instant::now(); - if now.duration_since(last_flush).as_millis() as u64 >= config.debounce_interval_ms { + if now.duration_since(last_flush).as_millis() as u64 + >= config.debounce_interval_ms + { Self::flush_events_static(&event_buffer, &emitter_arc).await; last_flush = now; } @@ -180,9 +202,12 @@ impl FileWatcher { Ok(()) } - async fn should_ignore_event(event: &Event, watched_paths: &Arc>>) -> bool { + async fn should_ignore_event( + event: &Event, + watched_paths: &Arc>>, + ) -> bool { let paths = watched_paths.read().await; - + let event_path = match event.paths.first() { Some(path) => path, None => return true, @@ -311,10 +336,10 @@ impl FileWatcher { async fn flush_events_static( event_buffer: &Arc>>, - emitter_arc: &Arc>>> + emitter_arc: &Arc>>>, ) { let events = { - let mut buffer = event_buffer.lock().unwrap(); + let mut buffer = lock_event_buffer(event_buffer); if buffer.is_empty() { return; } @@ -324,7 +349,7 @@ impl FileWatcher { let emitter_guard = emitter_arc.lock().await; if let Some(emitter) = emitter_guard.as_ref() { let mut event_array = Vec::new(); - + for event in &events { let kind = match event.kind { FileWatchEventKind::Create => "create", @@ -339,18 +364,21 @@ impl FileWatcher { "timestamp": event.timestamp })); continue; - }, + } FileWatchEventKind::Other => "other", }; - + event_array.push(serde_json::json!({ "path": event.path, "kind": kind, "timestamp": event.timestamp })); } - - if let Err(e) = emitter.emit("file-system-changed", serde_json::json!(event_array)).await { + + if let Err(e) = emitter + .emit("file-system-changed", serde_json::json!(event_array)) + .await + { error!("Failed to emit file-system-changed events: {}", e); } else { debug!("Emitted {} file system change events", event_array.len()); @@ -362,7 +390,8 @@ impl FileWatcher { pub async fn get_watched_paths(&self) -> Vec { let watched_paths = self.watched_paths.read().await; - watched_paths.keys() + watched_paths + .keys() .map(|path| path.to_string_lossy().to_string()) .collect() } @@ -371,9 +400,9 @@ impl FileWatcher { static GLOBAL_FILE_WATCHER: std::sync::OnceLock> = std::sync::OnceLock::new(); pub fn get_global_file_watcher() -> Arc { - GLOBAL_FILE_WATCHER.get_or_init(|| { - Arc::new(FileWatcher::new(FileWatcherConfig::default())) - }).clone() + GLOBAL_FILE_WATCHER + .get_or_init(|| Arc::new(FileWatcher::new(FileWatcherConfig::default()))) + .clone() } // Note: This function is called by the Tauri API layer; tauri::command is declared in the API layer. @@ -383,7 +412,7 @@ pub async fn start_file_watch(path: String, recursive: Option) -> Result<( if let Some(rec) = recursive { config.watch_recursively = rec; } - + watcher.watch_path(&path, Some(config)).await } @@ -401,8 +430,8 @@ pub async fn get_watched_paths() -> Result, String> { pub fn initialize_file_watcher(emitter: Arc) { let watcher = get_global_file_watcher(); - + tokio::spawn(async move { watcher.set_emitter(emitter).await; }); -} \ No newline at end of file +} diff --git a/src/crates/core/src/infrastructure/filesystem/path_manager.rs b/src/crates/core/src/infrastructure/filesystem/path_manager.rs index ace729fc..b42dcca1 100644 --- a/src/crates/core/src/infrastructure/filesystem/path_manager.rs +++ b/src/crates/core/src/infrastructure/filesystem/path_manager.rs @@ -2,11 +2,11 @@ //! //! Provides unified management for all app storage paths, supporting user, project, and temporary levels -use log::debug; +use crate::util::errors::*; +use log::{debug, error}; +use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; use std::sync::Arc; -use serde::{Serialize, Deserialize}; -use crate::util::errors::*; /// Storage level #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -47,12 +47,10 @@ impl PathManager { /// Create a new path manager pub fn new() -> BitFunResult { let user_root = Self::get_user_config_root()?; - - Ok(Self { - user_root, - }) + + Ok(Self { user_root }) } - + /// Get user config root directory /// /// - Windows: %APPDATA%\BitFun\ @@ -61,35 +59,35 @@ impl PathManager { fn get_user_config_root() -> BitFunResult { let config_dir = dirs::config_dir() .ok_or_else(|| BitFunError::config("Failed to get config directory".to_string()))?; - + Ok(config_dir.join("bitfun")) } - + /// Get user config root directory pub fn user_root(&self) -> &Path { &self.user_root } - + /// Get user config directory: ~/.config/bitfun/config/ pub fn user_config_dir(&self) -> PathBuf { self.user_root.join("config") } - + /// Get app config file path: ~/.config/bitfun/config/app.json pub fn app_config_file(&self) -> PathBuf { self.user_config_dir().join("app.json") } - + /// Get user agent directory: ~/.config/bitfun/agents/ pub fn user_agents_dir(&self) -> PathBuf { self.user_root.join("agents") } - + /// Get agent templates directory: ~/.config/bitfun/agents/templates/ pub fn agent_templates_dir(&self) -> PathBuf { self.user_agents_dir().join("templates") } - + /// Get user skills directory: /// - Windows: C:\Users\xxx\AppData\Roaming\BitFun\skills\ /// - macOS: ~/Library/Application Support/BitFun/skills/ @@ -114,17 +112,17 @@ impl PathManager { .join("skills") } } - + /// Get workspaces directory: ~/.config/bitfun/workspaces/ pub fn workspaces_dir(&self) -> PathBuf { self.user_root.join("workspaces") } - + /// Get cache root directory: ~/.config/bitfun/cache/ pub fn cache_root(&self) -> PathBuf { self.user_root.join("cache") } - + /// Get cache directory for a specific type pub fn cache_dir(&self, cache_type: CacheType) -> PathBuf { let subdir = match cache_type { @@ -135,148 +133,147 @@ impl PathManager { }; self.cache_root().join(subdir) } - + /// Get user data directory: ~/.config/bitfun/data/ pub fn user_data_dir(&self) -> PathBuf { self.user_root.join("data") } - + /// Get user-level rules directory: ~/.config/bitfun/data/rules/ pub fn user_rules_dir(&self) -> PathBuf { self.user_data_dir().join("rules") } - + /// Get history directory: ~/.config/bitfun/data/history/ pub fn history_dir(&self) -> PathBuf { self.user_data_dir().join("history") } - + /// Get snippets directory: ~/.config/bitfun/data/snippets/ pub fn snippets_dir(&self) -> PathBuf { self.user_data_dir().join("snippets") } - + /// Get templates directory: ~/.config/bitfun/data/templates/ pub fn templates_dir(&self) -> PathBuf { self.user_data_dir().join("templates") } - + /// Get logs directory: ~/.config/bitfun/logs/ pub fn logs_dir(&self) -> PathBuf { self.user_root.join("logs") } - + /// Get backups directory: ~/.config/bitfun/backups/ pub fn backups_dir(&self) -> PathBuf { self.user_root.join("backups") } - + /// Get temp directory: ~/.config/bitfun/temp/ pub fn temp_dir(&self) -> PathBuf { self.user_root.join("temp") } - + /// Get project config root directory: {project}/.bitfun/ pub fn project_root(&self, workspace_path: &Path) -> PathBuf { workspace_path.join(".bitfun") } - + /// Get project config file: {project}/.bitfun/config.json pub fn project_config_file(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("config.json") } - + /// Get project .gitignore file: {project}/.bitfun/.gitignore pub fn project_gitignore_file(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join(".gitignore") } - + /// Get project agent directory: {project}/.bitfun/agents/ pub fn project_agents_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("agents") } - + /// Get project-level rules directory: {project}/.bitfun/rules/ pub fn project_rules_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("rules") } - + /// Get project snapshots directory: {project}/.bitfun/snapshots/ pub fn project_snapshots_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("snapshots") } - + /// Get project sessions directory: {project}/.bitfun/sessions/ pub fn project_sessions_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("sessions") } - + /// Get project diffs cache directory: {project}/.bitfun/diffs/ pub fn project_diffs_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("diffs") } - + /// Get project checkpoints directory: {project}/.bitfun/checkpoints/ pub fn project_checkpoints_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("checkpoints") } - + /// Get project context directory: {project}/.bitfun/context/ pub fn project_context_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("context") } - + /// Get project local data directory: {project}/.bitfun/local/ pub fn project_local_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("local") } - + /// Get project local cache directory: {project}/.bitfun/local/cache/ pub fn project_cache_dir(&self, workspace_path: &Path) -> PathBuf { self.project_local_dir(workspace_path).join("cache") } - + /// Get project local logs directory: {project}/.bitfun/local/logs/ pub fn project_logs_dir(&self, workspace_path: &Path) -> PathBuf { self.project_local_dir(workspace_path).join("logs") } - + /// Get project local temp directory: {project}/.bitfun/local/temp/ pub fn project_temp_dir(&self, workspace_path: &Path) -> PathBuf { self.project_local_dir(workspace_path).join("temp") } - + /// Get project tasks directory: {project}/.bitfun/tasks/ pub fn project_tasks_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("tasks") } - + /// Get project plans directory: {project}/.bitfun/plans/ pub fn project_plans_dir(&self, workspace_path: &Path) -> PathBuf { self.project_root(workspace_path).join("plans") } - + /// Compute a hash of the workspace path (used for directory names) pub fn workspace_hash(workspace_path: &Path) -> String { use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; - + let mut hasher = DefaultHasher::new(); workspace_path.to_string_lossy().hash(&mut hasher); format!("{:x}", hasher.finish()) } - + /// Ensure directory exists pub async fn ensure_dir(&self, path: &Path) -> BitFunResult<()> { if !path.exists() { - tokio::fs::create_dir_all(path).await - .map_err(|e| BitFunError::service( - format!("Failed to create directory {:?}: {}", path, e) - ))?; + tokio::fs::create_dir_all(path).await.map_err(|e| { + BitFunError::service(format!("Failed to create directory {:?}: {}", path, e)) + })?; } Ok(()) } - + /// Initialize user-level directory structure pub async fn initialize_user_directories(&self) -> BitFunResult<()> { let dirs = vec![ @@ -298,15 +295,15 @@ impl PathManager { self.backups_dir(), self.temp_dir(), ]; - + for dir in dirs { self.ensure_dir(&dir).await?; } - + debug!("User-level directories initialized"); Ok(()) } - + /// Initialize project-level directory structure pub async fn initialize_project_directories(&self, workspace_path: &Path) -> BitFunResult<()> { let dirs = vec![ @@ -324,25 +321,28 @@ impl PathManager { self.project_temp_dir(workspace_path), self.project_tasks_dir(workspace_path), ]; - + for dir in dirs { self.ensure_dir(&dir).await?; } - + self.generate_project_gitignore(workspace_path).await?; - - debug!("Project-level directories initialized for {:?}", workspace_path); + + debug!( + "Project-level directories initialized for {:?}", + workspace_path + ); Ok(()) } - + /// Generate project-level .gitignore file async fn generate_project_gitignore(&self, workspace_path: &Path) -> BitFunResult<()> { let gitignore_path = self.project_gitignore_file(workspace_path); - + if gitignore_path.exists() { return Ok(()); } - + let content = r#"# BitFun local data (auto-generated) # Snapshots and cache @@ -364,12 +364,11 @@ temp/ # context/ # tasks/ "#; - - tokio::fs::write(&gitignore_path, content).await - .map_err(|e| BitFunError::service( - format!("Failed to create .gitignore: {}", e) - ))?; - + + tokio::fs::write(&gitignore_path, content) + .await + .map_err(|e| BitFunError::service(format!("Failed to create .gitignore: {}", e)))?; + debug!("Generated .gitignore for project"); Ok(()) } @@ -377,7 +376,18 @@ temp/ impl Default for PathManager { fn default() -> Self { - Self::new().expect("Failed to create PathManager") + match Self::new() { + Ok(manager) => manager, + Err(e) => { + error!( + "Failed to create PathManager from system config directory, using temp fallback: {}", + e + ); + Self { + user_root: std::env::temp_dir().join("bitfun"), + } + } + } } } @@ -394,7 +404,18 @@ fn init_global_path_manager() -> BitFunResult> { /// /// Return a shared Arc to the global PathManager instance pub fn get_path_manager_arc() -> Arc { - try_get_path_manager_arc().expect("Failed to create global PathManager") + GLOBAL_PATH_MANAGER + .get_or_init(|| match init_global_path_manager() { + Ok(manager) => manager, + Err(e) => { + error!( + "Failed to create global PathManager from config directory, using fallback: {}", + e + ); + Arc::new(PathManager::default()) + } + }) + .clone() } /// Try to get the global PathManager instance (Arc) @@ -403,4 +424,3 @@ pub fn try_get_path_manager_arc() -> BitFunResult> { .get_or_try_init(init_global_path_manager) .map(Arc::clone) } - diff --git a/src/crates/core/src/service/config/manager.rs b/src/crates/core/src/service/config/manager.rs index 51666310..c5a94620 100644 --- a/src/crates/core/src/service/config/manager.rs +++ b/src/crates/core/src/service/config/manager.rs @@ -4,7 +4,7 @@ use super::providers::ConfigProviderRegistry; use super::types::*; -use crate::infrastructure::{PathManager, try_get_path_manager_arc}; +use crate::infrastructure::{try_get_path_manager_arc, PathManager}; use crate::util::errors::*; use log::{debug, info, warn}; @@ -198,12 +198,7 @@ impl ConfigManager { fn add_default_agent_models_config( agent_models: &mut std::collections::HashMap, ) { - let agents_using_fast = vec![ - "Explore", - "FileFinder", - "GenerateDoc", - "CodeReview", - ]; + let agents_using_fast = vec!["Explore", "FileFinder", "GenerateDoc", "CodeReview"]; for key in agents_using_fast { if !agent_models.contains_key(key) { agent_models.insert(key.to_string(), "fast".to_string()); @@ -470,7 +465,9 @@ impl ConfigManager { return Ok(()); } - let last_key = keys.last().unwrap(); + let last_key = keys.last().ok_or_else(|| { + BitFunError::config(format!("Config path '{}' does not contain any keys", path)) + })?; let parent_keys = &keys[..keys.len() - 1]; let mut current = &mut config_value; diff --git a/src/crates/core/src/service/config/providers.rs b/src/crates/core/src/service/config/providers.rs index afc54439..402dd058 100644 --- a/src/crates/core/src/service/config/providers.rs +++ b/src/crates/core/src/service/config/providers.rs @@ -6,9 +6,22 @@ use super::types::*; use crate::util::errors::*; use async_trait::async_trait; -use log::info; +use log::{error, info}; use std::collections::HashMap; +fn serialize_default_config(section: &str, value: impl serde::Serialize) -> serde_json::Value { + match serde_json::to_value(value) { + Ok(serialized) => serialized, + Err(err) => { + error!( + "Failed to serialize default config section: section={}, error={}", + section, err + ); + serde_json::Value::Object(serde_json::Map::new()) + } + } +} + /// AI configuration provider. pub struct AIConfigProvider; @@ -19,7 +32,7 @@ impl ConfigProvider for AIConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(AIConfig::default()).unwrap() + serialize_default_config("ai", AIConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { @@ -152,7 +165,7 @@ impl ConfigProvider for ThemeConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(ThemeConfig::default()).unwrap() + serialize_default_config("theme", ThemeConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { @@ -219,7 +232,7 @@ impl ConfigProvider for ThemesConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(ThemesConfig::default()).unwrap() + serialize_default_config("themes", ThemesConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { @@ -268,7 +281,7 @@ impl ConfigProvider for EditorConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(EditorConfig::default()).unwrap() + serialize_default_config("editor", EditorConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { @@ -328,7 +341,7 @@ impl ConfigProvider for TerminalConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(TerminalConfig::default()).unwrap() + serialize_default_config("terminal", TerminalConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { @@ -384,7 +397,7 @@ impl ConfigProvider for WorkspaceConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(WorkspaceConfig::default()).unwrap() + serialize_default_config("workspace", WorkspaceConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { @@ -442,7 +455,7 @@ impl ConfigProvider for AppConfigProvider { } fn get_default_config(&self) -> serde_json::Value { - serde_json::to_value(AppConfig::default()).unwrap() + serialize_default_config("app", AppConfig::default()) } async fn validate_config(&self, config: &serde_json::Value) -> BitFunResult> { diff --git a/src/crates/core/src/service/lsp/workspace_manager.rs b/src/crates/core/src/service/lsp/workspace_manager.rs index 664975d1..11f244d2 100644 --- a/src/crates/core/src/service/lsp/workspace_manager.rs +++ b/src/crates/core/src/service/lsp/workspace_manager.rs @@ -560,12 +560,18 @@ impl WorkspaceLspManager { let mut states = self.server_states.write().await; if let Some(state) = states.get_mut(language) { state.status = ServerStatus::Running; - state.started_at = Some( - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap() - .as_secs(), - ); + state.started_at = match SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + { + Ok(duration) => Some(duration.as_secs()), + Err(e) => { + warn!( + "Failed to compute LSP server start timestamp: language={}, error={}", + language, e + ); + Some(0) + } + }; } info!("LSP server started: {}", language); } diff --git a/src/crates/core/src/service/mcp/adapter/context.rs b/src/crates/core/src/service/mcp/adapter/context.rs index baa44248..7e1004ab 100644 --- a/src/crates/core/src/service/mcp/adapter/context.rs +++ b/src/crates/core/src/service/mcp/adapter/context.rs @@ -8,6 +8,7 @@ use crate::service::mcp::server::MCPServerManager; use crate::util::errors::{BitFunError, BitFunResult}; use log::{debug, info, warn}; use serde_json::{json, Value}; +use std::cmp::Ordering; use std::collections::HashMap; use std::sync::Arc; @@ -58,7 +59,7 @@ impl ContextEnhancer { .collect::>(); let mut sorted = scored_resources; - sorted.sort_by(|a, b| b.2.partial_cmp(&a.2).unwrap()); + sorted.sort_by(|a, b| b.2.partial_cmp(&a.2).unwrap_or(Ordering::Equal)); let mut selected = Vec::new(); let mut total_size = 0; diff --git a/src/crates/core/src/service/mcp/adapter/resource.rs b/src/crates/core/src/service/mcp/adapter/resource.rs index 06a842d7..c9247043 100644 --- a/src/crates/core/src/service/mcp/adapter/resource.rs +++ b/src/crates/core/src/service/mcp/adapter/resource.rs @@ -4,6 +4,7 @@ use crate::service::mcp::protocol::{MCPResource, MCPResourceContent}; use serde_json::{json, Value}; +use std::cmp::Ordering; /// Resource adapter. pub struct ResourceAdapter; @@ -65,7 +66,7 @@ impl ResourceAdapter { .filter(|(_, score)| *score >= min_relevance) .collect(); - scored_resources.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap()); + scored_resources.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap_or(Ordering::Equal)); scored_resources.truncate(max_results); diff --git a/src/crates/core/src/service/mcp/adapter/tool.rs b/src/crates/core/src/service/mcp/adapter/tool.rs index 513f3d2b..7e8e0f8e 100644 --- a/src/crates/core/src/service/mcp/adapter/tool.rs +++ b/src/crates/core/src/service/mcp/adapter/tool.rs @@ -185,11 +185,10 @@ impl Tool for MCPToolWrapper { let result_value = serde_json::to_value(&result)?; + let result_for_assistant = self.render_result_for_assistant(&result_value); Ok(vec![ToolResult::Result { data: result_value, - result_for_assistant: Some( - self.render_result_for_assistant(&serde_json::to_value(&result).unwrap()), - ), + result_for_assistant: Some(result_for_assistant), }]) } } diff --git a/src/crates/core/src/service/mcp/protocol/jsonrpc.rs b/src/crates/core/src/service/mcp/protocol/jsonrpc.rs index 2725636d..9b6318b3 100644 --- a/src/crates/core/src/service/mcp/protocol/jsonrpc.rs +++ b/src/crates/core/src/service/mcp/protocol/jsonrpc.rs @@ -3,8 +3,22 @@ //! Helper functions and types for the JSON-RPC protocol. use super::types::*; +use log::warn; use serde_json::{json, Value}; +fn serialize_params(method: &str, params: impl serde::Serialize) -> Option { + match serde_json::to_value(params) { + Ok(value) => Some(value), + Err(err) => { + warn!( + "Failed to serialize MCP request params: method={}, error={}", + method, err + ); + None + } + } +} + /// Creates an `initialize` request. pub fn create_initialize_request( id: u64, @@ -25,7 +39,7 @@ pub fn create_initialize_request( MCPRequest::new( Value::Number(id.into()), "initialize".to_string(), - Some(serde_json::to_value(params).unwrap()), + serialize_params("initialize", params), ) } @@ -33,7 +47,7 @@ pub fn create_initialize_request( pub fn create_resources_list_request(id: u64, cursor: Option) -> MCPRequest { let params = if cursor.is_some() { let params = ResourcesListParams { cursor }; - Some(serde_json::to_value(params).unwrap()) + serialize_params("resources/list", params) } else { None }; @@ -50,7 +64,7 @@ pub fn create_resources_read_request(id: u64, uri: impl Into) -> MCPRequ MCPRequest::new( Value::Number(id.into()), "resources/read".to_string(), - Some(serde_json::to_value(params).unwrap()), + serialize_params("resources/read", params), ) } @@ -58,7 +72,7 @@ pub fn create_resources_read_request(id: u64, uri: impl Into) -> MCPRequ pub fn create_prompts_list_request(id: u64, cursor: Option) -> MCPRequest { let params = if cursor.is_some() { let params = PromptsListParams { cursor }; - Some(serde_json::to_value(params).unwrap()) + serialize_params("prompts/list", params) } else { None }; @@ -78,7 +92,7 @@ pub fn create_prompts_get_request( MCPRequest::new( Value::Number(id.into()), "prompts/get".to_string(), - Some(serde_json::to_value(params).unwrap()), + serialize_params("prompts/get", params), ) } @@ -86,7 +100,7 @@ pub fn create_prompts_get_request( pub fn create_tools_list_request(id: u64, cursor: Option) -> MCPRequest { let params = if cursor.is_some() { let params = ToolsListParams { cursor }; - Some(serde_json::to_value(params).unwrap()) + serialize_params("tools/list", params) } else { None }; @@ -106,7 +120,7 @@ pub fn create_tools_call_request( MCPRequest::new( Value::Number(id.into()), "tools/call".to_string(), - Some(serde_json::to_value(params).unwrap()), + serialize_params("tools/call", params), ) } diff --git a/src/crates/core/src/service/mcp/server/connection.rs b/src/crates/core/src/service/mcp/server/connection.rs index 1140ac61..c6f6dac0 100644 --- a/src/crates/core/src/service/mcp/server/connection.rs +++ b/src/crates/core/src/service/mcp/server/connection.rs @@ -15,7 +15,7 @@ use log::{debug, warn}; use serde_json::Value; use std::collections::HashMap; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tokio::process::ChildStdin; use tokio::sync::{mpsc, oneshot, RwLock}; @@ -151,14 +151,18 @@ impl MCPConnection { } } TransportType::Remote(transport) => { + let request_id = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| { + BitFunError::MCPError(format!( + "Failed to build request id for method {}: {}", + method, e + )) + })? + .as_millis() as u64; let request = MCPRequest { jsonrpc: "2.0".to_string(), - id: Value::Number(serde_json::Number::from( - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_millis() as u64, - )), + id: Value::Number(serde_json::Number::from(request_id)), method: method.clone(), params, }; diff --git a/src/crates/core/src/service/mcp/server/manager.rs b/src/crates/core/src/service/mcp/server/manager.rs index 5cd7d2e0..2eb269bb 100644 --- a/src/crates/core/src/service/mcp/server/manager.rs +++ b/src/crates/core/src/service/mcp/server/manager.rs @@ -354,12 +354,10 @@ impl MCPServerManager { self.config_service.save_server_config(&config).await?; let status = self.get_server_status(&config.id).await; - if status.is_ok() - && matches!( - status.unwrap(), - MCPServerStatus::Connected | MCPServerStatus::Healthy - ) - { + if matches!( + status, + Ok(MCPServerStatus::Connected | MCPServerStatus::Healthy) + ) { info!( "Restarting MCP server to apply new configuration: id={}", config.id diff --git a/src/crates/core/src/service/system/command.rs b/src/crates/core/src/service/system/command.rs index cdc0d551..b350b290 100644 --- a/src/crates/core/src/service/system/command.rs +++ b/src/crates/core/src/service/system/command.rs @@ -54,7 +54,9 @@ pub enum SystemError { /// ```rust /// let result = check_command("git"); /// if result.exists { -/// println!("Git path: {}", result.path.unwrap()); +/// if let Some(path) = result.path.as_deref() { +/// println!("Git path: {}", path); +/// } /// } /// ``` pub fn check_command(cmd: &str) -> CheckCommandResult { diff --git a/src/crates/core/src/service/terminal/src/api.rs b/src/crates/core/src/service/terminal/src/api.rs index 3adbdaef..d0dde2f0 100644 --- a/src/crates/core/src/service/terminal/src/api.rs +++ b/src/crates/core/src/service/terminal/src/api.rs @@ -245,11 +245,15 @@ impl TerminalApi { /// If the singleton is already initialized, it will use the existing instance. pub async fn new(config: TerminalConfig) -> Self { let session_manager = if is_session_manager_initialized() { - get_session_manager().expect("SessionManager should be initialized") + match get_session_manager() { + Some(manager) => manager, + None => panic!("SessionManager should be initialized"), + } } else { - init_session_manager(config) - .await - .expect("Failed to initialize SessionManager") + match init_session_manager(config).await { + Ok(manager) => manager, + Err(_) => panic!("Failed to initialize SessionManager"), + } }; Self { session_manager } diff --git a/src/crates/core/src/service/terminal/src/pty/data_bufferer.rs b/src/crates/core/src/service/terminal/src/pty/data_bufferer.rs index bd5b3a3c..4b014a31 100644 --- a/src/crates/core/src/service/terminal/src/pty/data_bufferer.rs +++ b/src/crates/core/src/service/terminal/src/pty/data_bufferer.rs @@ -206,7 +206,10 @@ mod tests { bufferer.buffer_data(1, b"hello").await; - let data = bufferer.recv().await.unwrap(); + let data = bufferer + .recv() + .await + .expect("expected buffered data when buffering is disabled"); assert_eq!(data.process_id, 1); assert_eq!(data.data, b"hello"); } @@ -228,7 +231,10 @@ mod tests { // Wait for flush tokio::time::sleep(Duration::from_millis(20)).await; - let data = bufferer.recv().await.unwrap(); + let data = bufferer + .recv() + .await + .expect("expected buffered data after flush interval"); assert_eq!(data.process_id, 1); assert_eq!(data.data, b"hello world"); } @@ -247,7 +253,10 @@ mod tests { // This should trigger immediate flush bufferer.buffer_data(1, b"0123456789AB").await; - let data = bufferer.recv().await.unwrap(); + let data = bufferer + .recv() + .await + .expect("expected immediate flush when max buffer size is exceeded"); assert_eq!(data.data.len(), 12); } } diff --git a/src/crates/core/src/service/terminal/src/session/serializer.rs b/src/crates/core/src/service/terminal/src/session/serializer.rs index 6dffa684..458cced7 100644 --- a/src/crates/core/src/service/terminal/src/session/serializer.rs +++ b/src/crates/core/src/service/terminal/src/session/serializer.rs @@ -182,8 +182,10 @@ mod tests { 24, ); - let serialized = SessionSerializer::serialize(&[session.clone()]).unwrap(); - let deserialized = SessionSerializer::deserialize(&serialized).unwrap(); + let serialized = SessionSerializer::serialize(&[session.clone()]) + .expect("serialize should succeed for a valid terminal session"); + let deserialized = SessionSerializer::deserialize(&serialized) + .expect("deserialize should succeed for serialized session payload"); assert_eq!(deserialized.len(), 1); assert_eq!(deserialized[0].id, session.id); diff --git a/src/crates/core/src/service/terminal/src/session/singleton.rs b/src/crates/core/src/service/terminal/src/session/singleton.rs index 2ae7a273..b32be262 100644 --- a/src/crates/core/src/service/terminal/src/session/singleton.rs +++ b/src/crates/core/src/service/terminal/src/session/singleton.rs @@ -37,11 +37,12 @@ static SESSION_MANAGER: OnceCell> = OnceCell::const_new(); pub async fn init_session_manager( config: TerminalConfig, ) -> Result, &'static str> { + let manager = Arc::new(SessionManager::new(config)); SESSION_MANAGER - .set(Arc::new(SessionManager::new(config))) + .set(manager.clone()) .map_err(|_| "SessionManager already initialized")?; - Ok(SESSION_MANAGER.get().unwrap().clone()) + Ok(manager) } /// Get the global SessionManager singleton. @@ -74,10 +75,10 @@ pub fn get_session_manager() -> Option> { /// let sessions = manager.list_sessions().await; /// ``` pub fn session_manager() -> Arc { - SESSION_MANAGER - .get() - .cloned() - .expect("SessionManager not initialized. Call init_session_manager first.") + match SESSION_MANAGER.get().cloned() { + Some(manager) => manager, + None => panic!("SessionManager not initialized. Call init_session_manager first."), + } } /// Check if the SessionManager singleton has been initialized. diff --git a/src/crates/core/src/service/workspace/manager.rs b/src/crates/core/src/service/workspace/manager.rs index a83f0998..dd71e91e 100644 --- a/src/crates/core/src/service/workspace/manager.rs +++ b/src/crates/core/src/service/workspace/manager.rs @@ -295,8 +295,10 @@ impl WorkspaceInfo { if let Ok(modified) = metadata.modified() { let modified_dt = chrono::DateTime::::from(modified); - if stats.last_modified.is_none() - || stats.last_modified.as_ref().unwrap() < &modified_dt + if stats + .last_modified + .as_ref() + .map_or(true, |last_modified| last_modified < &modified_dt) { stats.last_modified = Some(modified_dt); } @@ -459,7 +461,12 @@ impl WorkspaceManager { if let Some(workspace_id) = existing_workspace_id { self.set_current_workspace(workspace_id.clone())?; - return Ok(self.workspaces.get(&workspace_id).unwrap().clone()); + return self.workspaces.get(&workspace_id).cloned().ok_or_else(|| { + BitFunError::service(format!( + "Workspace '{}' disappeared after selecting it", + workspace_id + )) + }); } let workspace = WorkspaceInfo::new(path, ScanOptions::default()).await?; diff --git a/src/crates/core/src/util/process_manager.rs b/src/crates/core/src/util/process_manager.rs index 7837d613..f092e3b3 100644 --- a/src/crates/core/src/util/process_manager.rs +++ b/src/crates/core/src/util/process_manager.rs @@ -1,10 +1,10 @@ //! Unified process management to avoid Windows child process leaks use log::warn; +use once_cell::sync::Lazy; use std::process::Command; use std::sync::{Arc, Mutex}; use tokio::process::Command as TokioCommand; -use once_cell::sync::Lazy; #[cfg(windows)] use std::os::windows::process::CommandExt; @@ -28,42 +28,51 @@ impl ProcessManager { #[cfg(windows)] job: Arc::new(Mutex::new(None)), }; - + #[cfg(windows)] { if let Err(e) = manager.initialize_job() { warn!("Failed to initialize Windows Job object: {}", e); } } - + manager } - + #[cfg(windows)] fn initialize_job(&self) -> Result<(), Box> { - use win32job::{Job, ExtendedLimitInfo}; - + use win32job::{ExtendedLimitInfo, Job}; + let job = Job::create()?; - + // Terminate all child processes when the Job closes let mut info = ExtendedLimitInfo::new(); info.limit_kill_on_job_close(); job.set_extended_limit_info(&info)?; - + // Assign current process to Job so child processes inherit automatically if let Err(e) = job.assign_current_process() { warn!("Failed to assign current process to job: {}", e); } - - *self.job.lock().unwrap() = Some(job); - + + let mut job_guard = self.job.lock().map_err(|e| { + std::io::Error::other(format!("Failed to lock process manager job mutex: {}", e)) + })?; + *job_guard = Some(job); + Ok(()) } - + pub fn cleanup_all(&self) { #[cfg(windows)] { - let mut job_guard = self.job.lock().unwrap(); + let mut job_guard = match self.job.lock() { + Ok(guard) => guard, + Err(poisoned) => { + warn!("Process manager job mutex was poisoned during cleanup, recovering lock"); + poisoned.into_inner() + } + }; job_guard.take(); } } @@ -72,24 +81,24 @@ impl ProcessManager { /// Create synchronous Command (Windows automatically adds CREATE_NO_WINDOW) pub fn create_command>(program: S) -> Command { let mut cmd = Command::new(program.as_ref()); - + #[cfg(windows)] { cmd.creation_flags(CREATE_NO_WINDOW); } - + cmd } /// Create Tokio async Command (Windows automatically adds CREATE_NO_WINDOW) pub fn create_tokio_command>(program: S) -> TokioCommand { let mut cmd = TokioCommand::new(program.as_ref()); - + #[cfg(windows)] { cmd.creation_flags(CREATE_NO_WINDOW); } - + cmd } From 666377bb66037e610bffa8c3725b060820d4877f Mon Sep 17 00:00:00 2001 From: wsp Date: Thu, 12 Feb 2026 20:27:07 +0800 Subject: [PATCH 3/6] fix: enhance anthropic streaming robustness --- .../src/agentic/execution/stream_processor.rs | 6 +- .../src/stream_handler/anthropic.rs | 71 ++++++++++--------- .../src/stream_handler/openai.rs | 12 ++-- .../ai_stream_handlers/src/types/anthropic.rs | 42 +++++++++-- 4 files changed, 81 insertions(+), 50 deletions(-) diff --git a/src/crates/core/src/agentic/execution/stream_processor.rs b/src/crates/core/src/agentic/execution/stream_processor.rs index 8f8a87ef..c87fd00c 100644 --- a/src/crates/core/src/agentic/execution/stream_processor.rs +++ b/src/crates/core/src/agentic/execution/stream_processor.rs @@ -664,14 +664,16 @@ impl StreamProcessor { Ok(Some(Err(e))) => { let error_msg = format!("Stream processing error: {}", e); error!("{}", error_msg); - // Network errors/timeouts don't log SSE - // flush_sse_on_error(&sse_collector, &error_msg).await; + // log SSE for network errors + flush_sse_on_error(&sse_collector, &error_msg).await; self.graceful_shutdown_from_ctx(&mut ctx, error_msg.clone()).await; return Err(BitFunError::AIClient(error_msg)); } Err(_) => { let error_msg = format!("Stream data timeout (no data received for {} seconds)", chunk_timeout.as_secs()); error!("Stream data timeout ({} seconds), forcing termination", chunk_timeout.as_secs()); + // log SSE for timeout errors + flush_sse_on_error(&sse_collector, &error_msg).await; self.graceful_shutdown_from_ctx(&mut ctx, error_msg.clone()).await; return Err(BitFunError::AIClient(error_msg)); } diff --git a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/anthropic.rs b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/anthropic.rs index 0c1fc5d1..c00e3e5a 100644 --- a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/anthropic.rs +++ b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/anthropic.rs @@ -3,10 +3,10 @@ use crate::types::anthropic::{ MessageStart, Usage, }; use crate::types::unified::UnifiedResponse; -use anyhow::{anyhow, Error, Result}; +use anyhow::{anyhow, Result}; use eventsource_stream::Eventsource; use futures::StreamExt; -use log::{debug, trace}; +use log::{error, trace}; use reqwest::Response; use std::time::Duration; use tokio::sync::mpsc; @@ -25,8 +25,6 @@ pub async fn handle_anthropic_stream( ) { let mut stream = response.bytes_stream().eventsource(); let idle_timeout = Duration::from_secs(600); - let mut received_done = false; - let mut response_error: Option = None; let mut usage = Usage::default(); loop { @@ -34,30 +32,28 @@ pub async fn handle_anthropic_stream( let sse = match sse_event { Ok(Some(Ok(sse))) => sse, Ok(None) => { - if !received_done { - let error = response_error.unwrap_or(anyhow!( - "SSE Error: stream closed before response completed" - )); - let _ = tx_event.send(Err(error)); - } + let error_msg = "SSE Error: stream closed before response completed"; + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } Ok(Some(Err(e))) => { - let error_str = format!("SSE Error: {}", e); - debug!("{}", error_str); - let error = anyhow!(error_str); - let _ = tx_event.send(Err(error)); + let error_msg = format!("SSE Error: {}", e); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } Err(_) => { - let _ = tx_event.send(Err(anyhow!("SSE Timeout: idle timeout waiting for SSE"))); + let error_msg = "SSE Timeout: idle timeout waiting for SSE"; + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } }; + trace!("Anthropic SSE: {:?}", sse); let event_type = sse.event; let data = sse.data; - trace!("sse data: {:?}", data); if let Some(ref tx) = tx_raw_sse { let _ = tx.send(format!("[{}] {}", event_type, data)); @@ -69,20 +65,20 @@ pub async fn handle_anthropic_stream( Ok(message_start) => message_start, Err(e) => { let err_str = format!("SSE Parsing Error: {e}, data: {}", &data); - debug!("{}", err_str); - let _ = tx_event.send(Err(anyhow!(err_str))); + error!("{}", err_str); continue; } }; - usage.update(&message_start.message.usage); + if let Some(message_usage) = message_start.message.usage { + usage.update(&message_usage); + } } "content_block_start" => { let content_block_start: ContentBlockStart = match serde_json::from_str(&data) { Ok(content_block_start) => content_block_start, Err(e) => { let err_str = format!("SSE Parsing Error: {e}, data: {}", &data); - debug!("{}", err_str); - let _ = tx_event.send(Err(anyhow!(err_str))); + error!("{}", err_str); continue; } }; @@ -91,7 +87,7 @@ pub async fn handle_anthropic_stream( ContentBlock::ToolUse { .. } ) { let unified_response = UnifiedResponse::from(content_block_start); - trace!("unified_response: {:?}", unified_response); + trace!("Anthropic unified response: {:?}", unified_response); let _ = tx_event.send(Ok(unified_response)); } } @@ -100,18 +96,17 @@ pub async fn handle_anthropic_stream( Ok(content_block_delta) => content_block_delta, Err(e) => { let err_str = format!("SSE Parsing Error: {e}, data: {}", &data); - debug!("{}", err_str); - let _ = tx_event.send(Err(anyhow!(err_str))); + error!("{}", err_str); continue; } }; match UnifiedResponse::try_from(content_block_delta) { Ok(unified_response) => { - trace!("unified_response: {:?}", unified_response); + trace!("Anthropic unified response: {:?}", unified_response); let _ = tx_event.send(Ok(unified_response)); } Err(e) => { - debug!("Skipping invalid content_block_delta: {e}"); + error!("Skipping invalid content_block_delta: {}", e); } }; } @@ -120,15 +115,20 @@ pub async fn handle_anthropic_stream( Ok(message_delta) => message_delta, Err(e) => { let err_str = format!("SSE Parsing Error: {e}, data: {}", &data); - debug!("{}", err_str); - let _ = tx_event.send(Err(anyhow!(err_str))); + error!("{}", err_str); continue; } }; - usage.update(&message_delta.usage); - message_delta.usage = usage.clone(); + if let Some(delta_usage) = message_delta.usage.as_ref() { + usage.update(delta_usage); + } + message_delta.usage = if usage.is_empty() { + None + } else { + Some(usage.clone()) + }; let unified_response = UnifiedResponse::from(message_delta); - trace!("unified_response: {:?}", unified_response); + trace!("Anthropic unified response: {:?}", unified_response); let _ = tx_event.send(Ok(unified_response)); } "error" => { @@ -136,15 +136,16 @@ pub async fn handle_anthropic_stream( Ok(message_delta) => message_delta, Err(e) => { let err_str = format!("SSE Parsing Error: {e}, data: {}", &data); - debug!("{}", err_str); + error!("{}", err_str); let _ = tx_event.send(Err(anyhow!(err_str))); - continue; + return; } }; - response_error = Some(anyhow!(String::from(sse_error.error))) + let _ = tx_event.send(Err(anyhow!(String::from(sse_error.error)))); + return; } "message_stop" => { - received_done = true; + return; } _ => {} } diff --git a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs index 87893a5c..b4e50683 100644 --- a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs +++ b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/stream_handler/openai.rs @@ -68,13 +68,11 @@ pub async fn handle_openai_stream( } }; - let raw = sse.data.clone(); - trace!("sse data: {:?}", raw); - + let raw = sse.data; + trace!("OpenAI SSE: {:?}", raw); if let Some(ref tx) = tx_raw_sse { let _ = tx.send(raw.clone()); } - if raw == "[DONE]" { return; } @@ -82,7 +80,9 @@ pub async fn handle_openai_stream( let event_json: Value = match serde_json::from_str(&raw) { Ok(json) => json, Err(e) => { - let _ = tx_event.send(Err(anyhow!("SSE parsing error: {}, data: {}", e, &raw))); + let error_msg = format!("SSE parsing error: {}, data: {}", e, &raw); + error!("{}", error_msg); + let _ = tx_event.send(Err(anyhow!(error_msg))); return; } }; @@ -125,7 +125,7 @@ pub async fn handle_openai_stream( let has_empty_choices = sse_data.is_choices_empty(); let unified_responses = sse_data.into_unified_responses(); - trace!("unified responses: {:?}", unified_responses); + trace!("OpenAI unified responses: {:?}", unified_responses); if unified_responses.is_empty() { if has_empty_choices { warn!( diff --git a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/anthropic.rs b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/anthropic.rs index 562c8808..049c27bb 100644 --- a/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/anthropic.rs +++ b/src/crates/core/src/infrastructure/ai/ai_stream_handlers/src/types/anthropic.rs @@ -8,7 +8,7 @@ pub struct MessageStart { #[derive(Debug, Deserialize)] pub struct Message { - pub usage: Usage, + pub usage: Option, } #[derive(Debug, Clone, Deserialize)] @@ -16,6 +16,7 @@ pub struct Usage { input_tokens: Option, output_tokens: Option, cache_read_input_tokens: Option, + cache_creation_input_tokens: Option, } impl Default for Usage { @@ -24,6 +25,7 @@ impl Default for Usage { input_tokens: None, output_tokens: None, cache_read_input_tokens: None, + cache_creation_input_tokens: None, } } } @@ -39,18 +41,36 @@ impl Usage { if other.cache_read_input_tokens.is_some() { self.cache_read_input_tokens = other.cache_read_input_tokens; } + if other.cache_creation_input_tokens.is_some() { + self.cache_creation_input_tokens = other.cache_creation_input_tokens; + } + } + + pub fn is_empty(&self) -> bool { + self.input_tokens.is_none() + && self.output_tokens.is_none() + && self.cache_read_input_tokens.is_none() + && self.cache_creation_input_tokens.is_none() } } impl From for UnifiedTokenUsage { fn from(value: Usage) -> Self { - let prompt_token_count = value.input_tokens.unwrap_or(0) + value.cache_read_input_tokens.unwrap_or(0); + let cache_read = value.cache_read_input_tokens.unwrap_or(0); + let cache_creation = value.cache_creation_input_tokens.unwrap_or(0); + let prompt_token_count = value.input_tokens.unwrap_or(0) + cache_read + cache_creation; let candidates_token_count = value.output_tokens.unwrap_or(0); Self { prompt_token_count, candidates_token_count, total_token_count: prompt_token_count + candidates_token_count, - cached_content_token_count: value.cache_read_input_tokens, + cached_content_token_count: match ( + value.cache_read_input_tokens, + value.cache_creation_input_tokens, + ) { + (None, None) => None, + (read, creation) => Some(read.unwrap_or(0) + creation.unwrap_or(0)), + }, } } } @@ -58,12 +78,13 @@ impl From for UnifiedTokenUsage { #[derive(Debug, Deserialize)] pub struct MessageDelta { pub delta: MessageDeltaDelta, - pub usage: Usage, + pub usage: Option, } #[derive(Debug, Deserialize)] pub struct MessageDeltaDelta { - pub stop_reason: String, + pub stop_reason: Option, + pub stop_sequence: Option, } impl From for UnifiedResponse { @@ -73,8 +94,8 @@ impl From for UnifiedResponse { reasoning_content: None, thinking_signature: None, tool_call: None, - usage: Some(UnifiedTokenUsage::from(value.usage)), - finish_reason: Some(value.delta.stop_reason), + usage: value.usage.map(UnifiedTokenUsage::from), + finish_reason: value.delta.stop_reason, } } } @@ -93,6 +114,8 @@ pub enum ContentBlock { Text, #[serde(rename = "tool_use")] ToolUse { id: String, name: String }, + #[serde(other)] + Unknown, } impl From for UnifiedResponse { @@ -129,6 +152,8 @@ pub enum Delta { InputJsonDelta { partial_json: String }, #[serde(rename = "signature_delta")] SignatureDelta { signature: String }, + #[serde(other)] + Unknown, } impl TryFrom for UnifiedResponse { @@ -153,6 +178,9 @@ impl TryFrom for UnifiedResponse { Delta::SignatureDelta { signature } => { result.thinking_signature = Some(signature); } + Delta::Unknown => { + return Err("Unsupported anthropic delta type".to_string()); + } } Ok(result) } From 8aa9769c673b70fc9e8e42aa39144c46d1d2263d Mon Sep 17 00:00:00 2001 From: wsp Date: Thu, 12 Feb 2026 21:19:11 +0800 Subject: [PATCH 4/6] feat: add logging page in config center --- src/apps/desktop/src/api/commands.rs | 52 +++-- src/apps/desktop/src/api/config_api.rs | 12 ++ src/apps/desktop/src/lib.rs | 80 +++++++- src/apps/desktop/src/logging.rs | 161 ++++++++++++--- src/crates/core/src/service/config/global.rs | 5 + src/crates/core/src/service/config/manager.rs | 18 ++ .../core/src/service/config/providers.rs | 15 +- src/crates/core/src/service/config/types.rs | 22 +- .../api/service-api/ConfigAPI.ts | 17 +- .../config/components/ConfigCenterPanel.tsx | 11 +- .../config/components/LoggingConfig.scss | 113 +++++++++++ .../config/components/LoggingConfig.tsx | 191 ++++++++++++++++++ .../infrastructure/config/components/index.ts | 1 + .../src/infrastructure/config/types/index.ts | 15 ++ .../infrastructure/i18n/core/I18nService.ts | 5 + src/web-ui/src/locales/en-US/settings.json | 1 + .../src/locales/en-US/settings/logging.json | 33 +++ src/web-ui/src/locales/zh-CN/settings.json | 1 + .../src/locales/zh-CN/settings/logging.json | 33 +++ 19 files changed, 728 insertions(+), 58 deletions(-) create mode 100644 src/web-ui/src/infrastructure/config/components/LoggingConfig.scss create mode 100644 src/web-ui/src/infrastructure/config/components/LoggingConfig.tsx create mode 100644 src/web-ui/src/locales/en-US/settings/logging.json create mode 100644 src/web-ui/src/locales/zh-CN/settings/logging.json diff --git a/src/apps/desktop/src/api/commands.rs b/src/apps/desktop/src/api/commands.rs index 31250a65..eedb6d57 100644 --- a/src/apps/desktop/src/api/commands.rs +++ b/src/apps/desktop/src/api/commands.rs @@ -910,33 +910,55 @@ pub async fn list_directory_files( #[tauri::command] pub async fn reveal_in_explorer(request: RevealInExplorerRequest) -> Result<(), String> { - let _path = std::path::Path::new(&request.path); + let path = std::path::Path::new(&request.path); + if !path.exists() { + return Err(format!("Path does not exist: {}", request.path)); + } + let is_directory = path.is_dir(); #[cfg(target_os = "windows")] { - let normalized_path = request.path.replace("/", "\\"); - bitfun_core::util::process_manager::create_command("explorer") - .args(&["/select,", &normalized_path]) - .spawn() - .map_err(|e| format!("Failed to open explorer: {}", e))?; + if is_directory { + let normalized_path = request.path.replace("/", "\\"); + bitfun_core::util::process_manager::create_command("explorer") + .arg(&normalized_path) + .spawn() + .map_err(|e| format!("Failed to open explorer: {}", e))?; + } else { + let normalized_path = request.path.replace("/", "\\"); + bitfun_core::util::process_manager::create_command("explorer") + .args(&["/select,", &normalized_path]) + .spawn() + .map_err(|e| format!("Failed to open explorer: {}", e))?; + } } #[cfg(target_os = "macos")] { - bitfun_core::util::process_manager::create_command("open") - .args(&["-R", &request.path]) - .spawn() - .map_err(|e| format!("Failed to open finder: {}", e))?; + if is_directory { + bitfun_core::util::process_manager::create_command("open") + .arg(&request.path) + .spawn() + .map_err(|e| format!("Failed to open finder: {}", e))?; + } else { + bitfun_core::util::process_manager::create_command("open") + .args(&["-R", &request.path]) + .spawn() + .map_err(|e| format!("Failed to open finder: {}", e))?; + } } #[cfg(target_os = "linux")] { - use std::process::Command; - let parent = path - .parent() - .ok_or_else(|| "Failed to get parent directory".to_string())?; + let target = if is_directory { + path.to_path_buf() + } else { + path.parent() + .ok_or_else(|| "Failed to get parent directory".to_string())? + .to_path_buf() + }; bitfun_core::util::process_manager::create_command("xdg-open") - .arg(parent) + .arg(target) .spawn() .map_err(|e| format!("Failed to open file manager: {}", e))?; } diff --git a/src/apps/desktop/src/api/config_api.rs b/src/apps/desktop/src/api/config_api.rs index d00d37d7..2dadcb59 100644 --- a/src/apps/desktop/src/api/config_api.rs +++ b/src/apps/desktop/src/api/config_api.rs @@ -22,6 +22,9 @@ pub struct ResetConfigRequest { pub path: Option, } +#[derive(Debug, Deserialize, Default)] +pub struct GetRuntimeLoggingInfoRequest {} + fn to_json_value(value: T, context: &str) -> Result { serde_json::to_value(value).map_err(|e| format!("Failed to serialize {}: {}", context, e)) } @@ -201,6 +204,15 @@ pub async fn get_global_config_health() -> Result { Ok(bitfun_core::service::config::GlobalConfigManager::is_initialized()) } +#[tauri::command] +pub async fn get_runtime_logging_info( + _state: State<'_, AppState>, + _request: GetRuntimeLoggingInfoRequest, +) -> Result { + let logging_info = crate::logging::get_runtime_logging_info(); + to_json_value(logging_info, "runtime logging info") +} + #[tauri::command] pub async fn get_mode_configs(state: State<'_, AppState>) -> Result { use bitfun_core::service::config::types::ModeConfig; diff --git a/src/apps/desktop/src/lib.rs b/src/apps/desktop/src/lib.rs index 914cfb8a..687f11cb 100644 --- a/src/apps/desktop/src/lib.rs +++ b/src/apps/desktop/src/lib.rs @@ -8,19 +8,17 @@ pub mod theme; use bitfun_core::infrastructure::ai::AIClientFactory; use bitfun_core::infrastructure::{ - get_path_manager_arc, - get_workspace_path, - try_get_path_manager_arc, + get_path_manager_arc, get_workspace_path, try_get_path_manager_arc, }; use bitfun_transport::{TauriTransportAdapter, TransportAdapter}; use std::sync::{ atomic::{AtomicBool, Ordering}, Arc, }; -use tauri::Manager; -use tauri_plugin_log::{RotationStrategy, TimezoneStrategy}; #[cfg(target_os = "macos")] use tauri::Emitter; +use tauri::Manager; +use tauri_plugin_log::{RotationStrategy, TimezoneStrategy}; // Re-export API pub use api::*; @@ -57,6 +55,7 @@ pub async fn run() { let in_debug = cfg!(debug_assertions) || std::env::var("DEBUG").unwrap_or_default() == "1"; let log_config = logging::LogConfig::new(in_debug); let log_targets = logging::build_log_targets(&log_config); + let session_log_dir = log_config.session_log_dir.clone(); eprintln!("=== BitFun Desktop Starting ==="); @@ -65,6 +64,8 @@ pub async fn run() { return; } + let startup_log_level = resolve_runtime_log_level(log_config.level).await; + if let Err(e) = AIClientFactory::initialize_global().await { log::error!("Failed to initialize global AIClientFactory: {}", e); return; @@ -105,7 +106,7 @@ pub async fn run() { let run_result = tauri::Builder::default() .plugin( tauri_plugin_log::Builder::new() - .level(log_config.level) + .level(log::LevelFilter::Trace) .level_for("ignore", log::LevelFilter::Off) .level_for("ignore::walk", log::LevelFilter::Off) .level_for("globset", log::LevelFilter::Off) @@ -153,6 +154,8 @@ pub async fn run() { .manage(coordinator) .manage(terminal_state) .setup(move |app| { + logging::register_runtime_log_state(startup_log_level, session_log_dir.clone()); + let app_handle = app.handle().clone(); theme::create_main_window(&app_handle); @@ -203,7 +206,7 @@ pub async fn run() { init_mcp_servers(app_handle.clone()); - init_services(app_handle.clone()); + init_services(app_handle.clone(), startup_log_level); logging::spawn_log_cleanup_task(); @@ -298,6 +301,7 @@ pub async fn run() { reload_config, sync_config_to_global, get_global_config_health, + get_runtime_logging_info, get_mode_configs, get_mode_config, set_mode_config, @@ -666,10 +670,11 @@ fn start_event_loop_with_transport( }); } -fn init_services(app_handle: tauri::AppHandle) { +fn init_services(app_handle: tauri::AppHandle, default_log_level: log::LevelFilter) { use bitfun_core::{infrastructure, service}; spawn_ingest_server_with_config_listener(); + spawn_runtime_log_level_listener(default_log_level); tokio::spawn(async move { let transport = Arc::new(TauriTransportAdapter::new(app_handle.clone())); @@ -688,6 +693,65 @@ fn init_services(app_handle: tauri::AppHandle) { }); } +async fn resolve_runtime_log_level(default_level: log::LevelFilter) -> log::LevelFilter { + use bitfun_core::service::config::get_global_config_service; + + if let Ok(config_service) = get_global_config_service().await { + if let Ok(config_level) = config_service + .get_config::(Some("app.logging.level")) + .await + { + if let Some(level) = logging::parse_log_level(&config_level) { + return level; + } + log::warn!( + "Invalid app.logging.level '{}', falling back to default={}", + config_level, + logging::level_to_str(default_level) + ); + } + } + + default_level +} + +fn spawn_runtime_log_level_listener(default_level: log::LevelFilter) { + use bitfun_core::service::config::{subscribe_config_updates, ConfigUpdateEvent}; + + tokio::spawn(async move { + if let Some(mut receiver) = subscribe_config_updates() { + loop { + match receiver.recv().await { + Ok(ConfigUpdateEvent::LogLevelUpdated { new_level }) => { + if let Some(level) = logging::parse_log_level(&new_level) { + logging::apply_runtime_log_level(level, "config_update_event"); + } else { + log::warn!( + "Received invalid log level from config update event: {}", + new_level + ); + } + } + Ok(ConfigUpdateEvent::ConfigReloaded) => { + let level = resolve_runtime_log_level(default_level).await; + logging::apply_runtime_log_level(level, "config_reloaded"); + } + Ok(_) => {} + Err(tokio::sync::broadcast::error::RecvError::Closed) => { + log::warn!("Log-level listener channel closed, stopping listener"); + break; + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(n)) => { + log::warn!("Log-level listener lagged by {} messages", n); + } + } + } + } else { + log::warn!("Config update subscription unavailable for log-level listener"); + } + }); +} + fn create_event_emitter( transport: Arc, ) -> Arc { diff --git a/src/apps/desktop/src/logging.rs b/src/apps/desktop/src/logging.rs index 418b272a..34f34048 100644 --- a/src/apps/desktop/src/logging.rs +++ b/src/apps/desktop/src/logging.rs @@ -2,13 +2,20 @@ use bitfun_core::infrastructure::get_path_manager_arc; use chrono::Local; +use serde::Serialize; use std::path::PathBuf; +use std::sync::{ + atomic::{AtomicU8, Ordering}, + OnceLock, +}; use std::thread; use tauri_plugin_log::{fern, Target, TargetKind}; const SESSION_DIR_PATTERN: &str = r"^\d{8}T\d{6}$"; const MAX_LOG_SESSIONS: usize = 50; const LOG_RETENTION_DAYS: i64 = 7; +static SESSION_LOG_DIR: OnceLock = OnceLock::new(); +static CURRENT_LOG_LEVEL: AtomicU8 = AtomicU8::new(level_filter_to_u8(log::LevelFilter::Info)); fn get_thread_id() -> u64 { let thread_id = thread::current().id(); @@ -29,34 +36,7 @@ pub struct LogConfig { impl LogConfig { pub fn new(is_debug: bool) -> Self { - let level = match std::env::var("BITFUN_LOG_LEVEL") { - Ok(val) => match val.to_lowercase().as_str() { - "trace" => log::LevelFilter::Trace, - "debug" => log::LevelFilter::Debug, - "info" => log::LevelFilter::Info, - "warn" => log::LevelFilter::Warn, - "error" => log::LevelFilter::Error, - "off" => log::LevelFilter::Off, - other => { - eprintln!( - "Warning: Invalid BITFUN_LOG_LEVEL '{}', falling back to default", - other - ); - if is_debug { - log::LevelFilter::Debug - } else { - log::LevelFilter::Info - } - } - }, - Err(_) => { - if is_debug { - log::LevelFilter::Debug - } else { - log::LevelFilter::Info - } - } - }; + let level = resolve_default_level(is_debug); let session_log_dir = create_session_log_dir(); @@ -68,6 +48,131 @@ impl LogConfig { } } +const fn level_filter_to_u8(level: log::LevelFilter) -> u8 { + match level { + log::LevelFilter::Off => 0, + log::LevelFilter::Error => 1, + log::LevelFilter::Warn => 2, + log::LevelFilter::Info => 3, + log::LevelFilter::Debug => 4, + log::LevelFilter::Trace => 5, + } +} + +const fn u8_to_level_filter(value: u8) -> log::LevelFilter { + match value { + 0 => log::LevelFilter::Off, + 1 => log::LevelFilter::Error, + 2 => log::LevelFilter::Warn, + 3 => log::LevelFilter::Info, + 4 => log::LevelFilter::Debug, + 5 => log::LevelFilter::Trace, + _ => log::LevelFilter::Info, + } +} + +fn resolve_default_level(is_debug: bool) -> log::LevelFilter { + match std::env::var("BITFUN_LOG_LEVEL") { + Ok(val) => parse_log_level(&val).unwrap_or_else(|| { + eprintln!( + "Warning: Invalid BITFUN_LOG_LEVEL '{}', falling back to default", + val + ); + if is_debug { + log::LevelFilter::Debug + } else { + log::LevelFilter::Info + } + }), + Err(_) => { + if is_debug { + log::LevelFilter::Debug + } else { + log::LevelFilter::Info + } + } + } +} + +pub fn parse_log_level(value: &str) -> Option { + match value.trim().to_lowercase().as_str() { + "trace" => Some(log::LevelFilter::Trace), + "debug" => Some(log::LevelFilter::Debug), + "info" => Some(log::LevelFilter::Info), + "warn" => Some(log::LevelFilter::Warn), + "error" => Some(log::LevelFilter::Error), + "off" => Some(log::LevelFilter::Off), + _ => None, + } +} + +pub fn level_to_str(level: log::LevelFilter) -> &'static str { + match level { + log::LevelFilter::Trace => "trace", + log::LevelFilter::Debug => "debug", + log::LevelFilter::Info => "info", + log::LevelFilter::Warn => "warn", + log::LevelFilter::Error => "error", + log::LevelFilter::Off => "off", + } +} + +pub fn register_runtime_log_state(initial_level: log::LevelFilter, session_log_dir: PathBuf) { + let _ = SESSION_LOG_DIR.set(session_log_dir); + CURRENT_LOG_LEVEL.store(level_filter_to_u8(initial_level), Ordering::Relaxed); + log::set_max_level(initial_level); +} + +pub fn current_runtime_log_level() -> log::LevelFilter { + u8_to_level_filter(CURRENT_LOG_LEVEL.load(Ordering::Relaxed)) +} + +pub fn apply_runtime_log_level(level: log::LevelFilter, source: &str) { + let old_level = current_runtime_log_level(); + if old_level == level { + return; + } + + log::set_max_level(level); + CURRENT_LOG_LEVEL.store(level_filter_to_u8(level), Ordering::Relaxed); + log::info!( + "Runtime log level updated: old_level={}, new_level={}, source={}", + level_to_str(old_level), + level_to_str(level), + source + ); +} + +pub fn session_log_dir() -> Option { + SESSION_LOG_DIR.get().cloned() +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct RuntimeLoggingInfo { + pub effective_level: String, + pub session_log_dir: String, + pub app_log_path: String, + pub ai_log_path: String, + pub webview_log_path: String, +} + +pub fn get_runtime_logging_info() -> RuntimeLoggingInfo { + let fallback_dir = get_path_manager_arc().logs_dir(); + let session_dir = session_log_dir().unwrap_or(fallback_dir); + + RuntimeLoggingInfo { + effective_level: level_to_str(current_runtime_log_level()).to_string(), + session_log_dir: session_dir.to_string_lossy().to_string(), + app_log_path: session_dir.join("app.log").to_string_lossy().to_string(), + ai_log_path: session_dir.join("ai.log").to_string_lossy().to_string(), + webview_log_path: session_dir + .join("webview.log") + .to_string_lossy() + .to_string(), + } +} + pub fn create_session_log_dir() -> PathBuf { let pm = get_path_manager_arc(); let logs_root = pm.logs_dir(); diff --git a/src/crates/core/src/service/config/global.rs b/src/crates/core/src/service/config/global.rs index 585d5f46..2a844d1a 100644 --- a/src/crates/core/src/service/config/global.rs +++ b/src/crates/core/src/service/config/global.rs @@ -48,6 +48,11 @@ pub enum ConfigUpdateEvent { /// The new log path. new_log_path: String, }, + /// Runtime log level updated. + LogLevelUpdated { + /// New runtime log level. + new_level: String, + }, } /// Global configuration service manager. diff --git a/src/crates/core/src/service/config/manager.rs b/src/crates/core/src/service/config/manager.rs index c5a94620..f6700317 100644 --- a/src/crates/core/src/service/config/manager.rs +++ b/src/crates/core/src/service/config/manager.rs @@ -500,6 +500,7 @@ impl ConfigManager { old_config: &GlobalConfig, ) -> BitFunResult<()> { self.check_and_broadcast_debug_mode_change(old_config).await; + self.check_and_broadcast_log_level_change(old_config).await; self.providers .notify_config_changed(path, old_config, &self.config) @@ -530,6 +531,23 @@ impl ConfigManager { .await; } } + + /// Detects and broadcasts runtime log-level changes. + async fn check_and_broadcast_log_level_change(&self, old_config: &GlobalConfig) { + let old_level = old_config.app.logging.level.trim().to_lowercase(); + let new_level = self.config.app.logging.level.trim().to_lowercase(); + + if old_level != new_level { + debug!( + "App logging level change detected: {} -> {}", + old_level, new_level + ); + + use super::global::{ConfigUpdateEvent, GlobalConfigManager}; + GlobalConfigManager::broadcast_update(ConfigUpdateEvent::LogLevelUpdated { new_level }) + .await; + } + } } /// Configuration statistics. diff --git a/src/crates/core/src/service/config/providers.rs b/src/crates/core/src/service/config/providers.rs index 402dd058..0bff49c0 100644 --- a/src/crates/core/src/service/config/providers.rs +++ b/src/crates/core/src/service/config/providers.rs @@ -469,6 +469,17 @@ impl ConfigProvider for AppConfigProvider { if app_config.sidebar.width < 200 || app_config.sidebar.width > 800 { warnings.push("Sidebar width should be between 200 and 800 pixels".to_string()); } + + let valid_log_level = matches!( + app_config.logging.level.to_lowercase().as_str(), + "trace" | "debug" | "info" | "warn" | "error" | "off" + ); + if !valid_log_level { + return Err(BitFunError::validation(format!( + "Invalid app.logging.level '{}': expected one of trace/debug/info/warn/error/off", + app_config.logging.level + ))); + } } else { return Err(BitFunError::validation( "Invalid app config format".to_string(), @@ -485,8 +496,8 @@ impl ConfigProvider for AppConfigProvider { ) -> BitFunResult<()> { if let Ok(app_config) = serde_json::from_value::(new_config.clone()) { info!( - "App config changed: language={}, zoom_level={}", - app_config.language, app_config.zoom_level + "App config changed: language={}, zoom_level={}, log_level={}", + app_config.language, app_config.zoom_level, app_config.logging.level ); } Ok(()) diff --git a/src/crates/core/src/service/config/types.rs b/src/crates/core/src/service/config/types.rs index f97e034e..3ac3555a 100644 --- a/src/crates/core/src/service/config/types.rs +++ b/src/crates/core/src/service/config/types.rs @@ -40,12 +40,23 @@ pub struct AppConfig { pub confirm_on_exit: bool, pub restore_windows: bool, pub zoom_level: f64, + #[serde(default)] + pub logging: AppLoggingConfig, pub sidebar: SidebarConfig, pub right_panel: RightPanelConfig, pub notifications: NotificationConfig, pub ai_experience: AIExperienceConfig, } +/// App logging configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(default)] +pub struct AppLoggingConfig { + /// Runtime backend log level. + /// Allowed values: trace, debug, info, warn, error, off. + pub level: String, +} + /// AI experience configuration. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default)] @@ -750,7 +761,6 @@ pub struct ProxyConfig { pub password: Option, } - /// Configuration provider interface. #[async_trait] pub trait ConfigProvider: Send + Sync { @@ -841,6 +851,7 @@ impl Default for AppConfig { confirm_on_exit: true, restore_windows: true, zoom_level: 1.0, + logging: AppLoggingConfig::default(), sidebar: SidebarConfig { width: 300, collapsed: false, @@ -859,6 +870,14 @@ impl Default for AppConfig { } } +impl Default for AppLoggingConfig { + fn default() -> Self { + Self { + level: "info".to_string(), + } + } +} + impl Default for AIExperienceConfig { fn default() -> Self { Self { @@ -1111,7 +1130,6 @@ impl Default for AIModelConfig { } } - impl Default for SidebarConfig { fn default() -> Self { Self { diff --git a/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts index 0a439f1b..ecfa0ad7 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts @@ -81,6 +81,16 @@ export class ConfigAPI { } } + async getRuntimeLoggingInfo(): Promise { + try { + return await api.invoke('get_runtime_logging_info', { + request: {}, + }); + } catch (error) { + throw createTauriCommandError('get_runtime_logging_info', error); + } + } + async getModelConfigs(): Promise { try { @@ -232,7 +242,12 @@ export class ConfigAPI { } -import type { SkillInfo, SkillLevel, SkillValidationResult } from '../../config/types'; +import type { + RuntimeLoggingInfo, + SkillInfo, + SkillLevel, + SkillValidationResult, +} from '../../config/types'; export const configAPI = new ConfigAPI(); \ No newline at end of file diff --git a/src/web-ui/src/infrastructure/config/components/ConfigCenterPanel.tsx b/src/web-ui/src/infrastructure/config/components/ConfigCenterPanel.tsx index 197b43cb..386a3fc2 100644 --- a/src/web-ui/src/infrastructure/config/components/ConfigCenterPanel.tsx +++ b/src/web-ui/src/infrastructure/config/components/ConfigCenterPanel.tsx @@ -12,6 +12,7 @@ import AgenticToolsConfig from './AgenticToolsConfig'; import AIMemoryConfig from './AIMemoryConfig'; import LspConfig from './LspConfig'; import DebugConfig from './DebugConfig'; +import LoggingConfig from './LoggingConfig'; import TerminalConfig from './TerminalConfig'; import EditorConfig from './EditorConfig'; import { ThemeConfig } from './ThemeConfig'; @@ -22,10 +23,10 @@ import './ConfigCenter.scss'; export interface ConfigCenterPanelProps { - initialTab?: 'models' | 'ai-rules' | 'agents' | 'mcp' | 'agentic-tools'; + initialTab?: 'models' | 'ai-rules' | 'agents' | 'mcp' | 'agentic-tools' | 'logging'; } -type ConfigTab = 'models' | 'super-agent' | 'ai-features' | 'modes' | 'ai-rules' | 'agents' | 'skills' | 'mcp' | 'agentic-tools' | 'ai-memory' | 'lsp' | 'debug' | 'terminal' | 'editor' | 'theme' | 'prompt-templates'; +type ConfigTab = 'models' | 'super-agent' | 'ai-features' | 'modes' | 'ai-rules' | 'agents' | 'skills' | 'mcp' | 'agentic-tools' | 'ai-memory' | 'lsp' | 'debug' | 'logging' | 'terminal' | 'editor' | 'theme' | 'prompt-templates'; interface TabCategory { name: string; @@ -144,6 +145,10 @@ const ConfigCenterPanel: React.FC = ({ { id: 'terminal' as ConfigTab, label: t('configCenter.tabs.terminal') + }, + { + id: 'logging' as ConfigTab, + label: t('configCenter.tabs.logging') } ] } @@ -194,6 +199,8 @@ const ConfigCenterPanel: React.FC = ({ return ; case 'debug': return ; + case 'logging': + return ; case 'terminal': return ; case 'editor': diff --git a/src/web-ui/src/infrastructure/config/components/LoggingConfig.scss b/src/web-ui/src/infrastructure/config/components/LoggingConfig.scss new file mode 100644 index 00000000..54847993 --- /dev/null +++ b/src/web-ui/src/infrastructure/config/components/LoggingConfig.scss @@ -0,0 +1,113 @@ +.bitfun-logging-config { + &__content { + display: flex; + flex-direction: column; + gap: 16px; + } + + &__message-container { + margin-bottom: 4px; + } + + &__section { + background: var(--color-surface); + border: 1px solid var(--color-border); + border-radius: 8px; + overflow: visible; + } + + &__section-header { + display: flex; + align-items: center; + justify-content: space-between; + padding: 12px 16px; + border-bottom: 1px solid var(--color-border); + background: var(--color-background); + } + + &__header-actions { + display: inline-flex; + align-items: center; + gap: 8px; + } + + &__section-title h3 { + margin: 0; + font-size: 14px; + font-weight: 600; + color: var(--color-text); + } + + &__refresh-btn { + width: 28px; + height: 28px; + border: 1px solid var(--color-border); + border-radius: 6px; + background: transparent; + color: var(--color-text-secondary); + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; + + &:hover:not(:disabled) { + color: var(--color-text); + border-color: var(--color-primary); + } + + &:disabled { + opacity: 0.6; + cursor: not-allowed; + } + } + + &__section-content { + padding: 16px; + display: flex; + flex-direction: column; + gap: 12px; + overflow: visible; + } + + &__description { + margin: 0; + font-size: 13px; + color: var(--color-text-secondary); + } + + &__select-wrapper { + max-width: 280px; + position: relative; + z-index: 5; + } + + &__path-box { + padding: 10px 12px; + border-radius: 6px; + background: var(--color-background); + border: 1px dashed var(--color-border); + color: var(--color-text); + font-family: var(--font-family-mono, Consolas, "Courier New", monospace); + font-size: 12px; + word-break: break-all; + } + + &__loading { + padding: 40px; + text-align: center; + color: var(--color-text-muted); + } +} + +.spinning { + animation: bitfun-logging-config-spin 1s linear infinite; +} + +@keyframes bitfun-logging-config-spin { + from { + transform: rotate(0deg); + } + to { + transform: rotate(360deg); + } +} diff --git a/src/web-ui/src/infrastructure/config/components/LoggingConfig.tsx b/src/web-ui/src/infrastructure/config/components/LoggingConfig.tsx new file mode 100644 index 00000000..00ff848f --- /dev/null +++ b/src/web-ui/src/infrastructure/config/components/LoggingConfig.tsx @@ -0,0 +1,191 @@ +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { FolderOpen, RefreshCw } from 'lucide-react'; +import { Alert, Button, Select, Tooltip } from '@/component-library'; +import { configAPI, workspaceAPI } from '@/infrastructure/api'; +import { ConfigPageContent, ConfigPageHeader, ConfigPageLayout } from './common'; +import { configManager } from '../services/ConfigManager'; +import { createLogger } from '@/shared/utils/logger'; +import type { BackendLogLevel, RuntimeLoggingInfo } from '../types'; +import './LoggingConfig.scss'; + +const log = createLogger('LoggingConfig'); + +const LoggingConfig: React.FC = () => { + const { t } = useTranslation('settings/logging'); + const [configLevel, setConfigLevel] = useState('info'); + const [runtimeInfo, setRuntimeInfo] = useState(null); + const [loading, setLoading] = useState(true); + const [saving, setSaving] = useState(false); + const [openingFolder, setOpeningFolder] = useState(false); + const [message, setMessage] = useState<{ type: 'success' | 'error' | 'info'; text: string } | null>(null); + + const levelOptions = useMemo( + () => [ + { value: 'trace', label: t('levels.trace') }, + { value: 'debug', label: t('levels.debug') }, + { value: 'info', label: t('levels.info') }, + { value: 'warn', label: t('levels.warn') }, + { value: 'error', label: t('levels.error') }, + { value: 'off', label: t('levels.off') }, + ], + [t] + ); + + const showMessage = useCallback((type: 'success' | 'error' | 'info', text: string) => { + setMessage({ type, text }); + setTimeout(() => setMessage(null), 3000); + }, []); + + const loadData = useCallback(async () => { + try { + setLoading(true); + + const [savedLevel, info] = await Promise.all([ + configManager.getConfig('app.logging.level'), + configAPI.getRuntimeLoggingInfo(), + ]); + + setConfigLevel(savedLevel || info.effectiveLevel || 'info'); + setRuntimeInfo(info); + } catch (error) { + log.error('Failed to load logging config', error); + showMessage('error', t('messages.loadFailed')); + } finally { + setLoading(false); + } + }, [showMessage, t]); + + useEffect(() => { + loadData(); + }, [loadData]); + + const handleLevelChange = useCallback(async (value: string) => { + const nextLevel = value as BackendLogLevel; + const previousLevel = configLevel; + setConfigLevel(nextLevel); + setSaving(true); + + try { + await configManager.setConfig('app.logging.level', nextLevel); + configManager.clearCache(); + + const info = await configAPI.getRuntimeLoggingInfo(); + setRuntimeInfo(info); + showMessage('success', t('messages.levelUpdated')); + } catch (error) { + setConfigLevel(previousLevel); + log.error('Failed to update logging level', { nextLevel, error }); + showMessage('error', t('messages.saveFailed')); + } finally { + setSaving(false); + } + }, [configLevel, showMessage, t]); + + const handleRefresh = useCallback(async () => { + await loadData(); + showMessage('info', t('messages.refreshed')); + }, [loadData, showMessage, t]); + + const handleOpenFolder = useCallback(async () => { + const folder = runtimeInfo?.sessionLogDir; + if (!folder) { + showMessage('error', t('messages.pathUnavailable')); + return; + } + + try { + setOpeningFolder(true); + await workspaceAPI.revealInExplorer(folder); + showMessage('success', t('messages.openedFolder')); + } catch (error) { + log.error('Failed to open log folder', { folder, error }); + showMessage('error', t('messages.openFailed')); + } finally { + setOpeningFolder(false); + } + }, [runtimeInfo?.sessionLogDir, showMessage, t]); + + if (loading) { + return ( + + + +
{t('messages.loading')}
+ + + ); + } + + return ( + + + + {message && ( +
+ +
+ )} + +
+
+
+

{t('sections.level')}

+
+ + + +
+ +
+

{t('level.description')}

+
+