diff --git a/crates/app-tauri/src/lib.rs b/crates/app-tauri/src/lib.rs index 98df2bb..0787119 100644 --- a/crates/app-tauri/src/lib.rs +++ b/crates/app-tauri/src/lib.rs @@ -47,6 +47,7 @@ use uuid::Uuid; use state::AppState; static EXIT_GUARD_CONFIRMED: AtomicBool = AtomicBool::new(false); +static PANIC_HOOK_INSTALLED: AtomicBool = AtomicBool::new(false); #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum MainCloseAction { @@ -69,6 +70,33 @@ fn should_install_exit_guard(window_label: &str) -> bool { window_label == "main" } +fn install_panic_diagnostics_hook() { + if PANIC_HOOK_INSTALLED.swap(true, Ordering::SeqCst) { + return; + } + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + let message = info + .payload() + .downcast_ref::<&str>() + .copied() + .or_else(|| info.payload().downcast_ref::().map(String::as_str)) + .unwrap_or(""); + let location = info + .location() + .map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column())) + .unwrap_or_else(|| "".to_owned()); + let thread = std::thread::current(); + let thread_name = thread.name().unwrap_or(""); + application::diag!("[panic] thread={thread_name} location={location} message={message}"); + application::diag!( + "[panic] backtrace:\n{}", + std::backtrace::Backtrace::force_capture() + ); + previous(info); + })); +} + fn apply_main_close_decision( guard: application::AppExitWorkGuardState, already_confirmed: bool, @@ -151,6 +179,7 @@ pub fn run() { // click-launched AppImage (whose stderr is otherwise discarded). Best-effort: // if the file can't be opened the beacons simply stay on stderr. application::diag::set_log_path(app_data_dir.join("logs").join("idea.log")); + install_panic_diagnostics_hook(); application::diag!("[startup] IdeA launched; diagnostics log armed"); let app_state = AppState::build_with_resource_dir(app_data_dir, resource_dir); diff --git a/crates/app-tauri/src/plugins.rs b/crates/app-tauri/src/plugins.rs index c748655..0759d6e 100644 --- a/crates/app-tauri/src/plugins.rs +++ b/crates/app-tauri/src/plugins.rs @@ -57,12 +57,28 @@ pub async fn plugin_install_from_archive( path: String, state: State<'_, AppState>, ) -> Result { - state + application::diag!("[plugins] install archive start path={path}"); + let result = state .install_plugin_from_archive .execute(path) .await .map(PluginInstallResultDto::from) - .map_err(ErrorDto::from) + .map_err(ErrorDto::from); + match &result { + Ok(result) => application::diag!( + "[plugins] install archive ok plugin={} version={} hash={} lifecycle={:?}", + result.plugin.id, + result.plugin.version, + result.review.content_hash, + result.plugin.lifecycle_state + ), + Err(err) => application::diag!( + "[plugins] install archive failed code={} message={}", + err.code, + err.message + ), + } + result } /// Installs a plugin from a local directory snapshot. @@ -71,12 +87,28 @@ pub async fn plugin_install_from_directory( path: String, state: State<'_, AppState>, ) -> Result { - state + application::diag!("[plugins] install directory start path={path}"); + let result = state .install_plugin_from_directory .execute(path) .await .map(PluginInstallResultDto::from) - .map_err(ErrorDto::from) + .map_err(ErrorDto::from); + match &result { + Ok(result) => application::diag!( + "[plugins] install directory ok plugin={} version={} hash={} lifecycle={:?}", + result.plugin.id, + result.plugin.version, + result.review.content_hash, + result.plugin.lifecycle_state + ), + Err(err) => application::diag!( + "[plugins] install directory failed code={} message={}", + err.code, + err.message + ), + } + result } /// Enables or disables a plugin. @@ -145,13 +177,30 @@ pub fn plugin_asset_protocol( app: &AppHandle, request: http::Request>, ) -> Response> { + let uri = request.uri().to_string(); match plugin_asset_response(app, request) { - Ok(response) => response, - Err((status, message)) => Response::builder() - .status(status) - .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") - .body(message.into_bytes()) - .expect("valid protocol error response"), + Ok(response) => { + application::diag!( + "[plugins] asset ok uri={} status={} bytes={}", + uri, + response.status(), + response.body().len() + ); + response + } + Err((status, message)) => { + application::diag!( + "[plugins] asset failed uri={uri} status={status} message={message}" + ); + Response::builder() + .status(status) + .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") + .body(message.into_bytes()) + .unwrap_or_else(|e| { + application::diag!("[plugins] asset error response build failed: {e}"); + Response::new(Vec::new()) + }) + } } } diff --git a/crates/application/src/plugin/mod.rs b/crates/application/src/plugin/mod.rs index 0a5d314..220106c 100644 --- a/crates/application/src/plugin/mod.rs +++ b/crates/application/src/plugin/mod.rs @@ -502,10 +502,18 @@ async fn install_from_staged( ) -> Result { let review = review_staged(packages, validator, &staged).await?; let plugin_id = review.manifest.id.clone(); + crate::diag!( + "[plugins] install staged reviewed plugin={} version={} hash={} source={}", + plugin_id.as_str(), + review.manifest.version.as_str(), + review.content_hash, + review.source.kind() + ); packages .commit_install(staged, &plugin_id) .await .map_err(map_store)?; + crate::diag!("[plugins] install committed plugin={}", plugin_id.as_str()); let mut registry = registry_store.load_registry().await.map_err(map_registry)?; let mut entry = PluginRegistryEntry { id: plugin_id.clone(), @@ -530,6 +538,11 @@ async fn install_from_staged( .save_registry(®istry) .await .map_err(map_registry)?; + crate::diag!( + "[plugins] install registry saved plugin={} lifecycle={:?}", + plugin_id.as_str(), + entry.lifecycle_state + ); events.publish(DomainEvent::PluginInstalled { plugin_id: plugin_id.clone(), version: review.manifest.version.clone(), @@ -537,7 +550,21 @@ async fn install_from_staged( let (active_servers, invalid_plugins) = active_mcp_specs(packages, validator, ®istry).await?; persist_invalid_runtime_plugins(registry_store, &mut registry, invalid_plugins).await; - let _ = mcp.reconcile(active_servers).await; + let active_server_count = active_servers.len(); + match mcp.reconcile(active_servers).await { + Ok(statuses) => crate::diag!( + "[plugins] install MCP reconcile ok plugin={} requested={} statuses={}", + plugin_id.as_str(), + active_server_count, + statuses.servers.len() + ), + Err(err) => crate::diag!( + "[plugins] install MCP reconcile failed plugin={} requested={} error={}", + plugin_id.as_str(), + active_server_count, + err + ), + } let admin = admin_from_descriptor( PluginDescriptor { manifest: review.manifest.clone(), @@ -846,7 +873,19 @@ impl ReconcilePluginMcpServers { active_mcp_specs(self.packages.as_ref(), self.validator.as_ref(), ®istry).await?; persist_invalid_runtime_plugins(self.registry.as_ref(), &mut registry, invalid_plugins) .await; - self.mcp.reconcile(specs).await.map_err(map_mcp) + let requested = specs.len(); + let result = self.mcp.reconcile(specs).await.map_err(map_mcp); + match &result { + Ok(statuses) => crate::diag!( + "[plugins] MCP reconcile ok requested={} statuses={}", + requested, + statuses.servers.len() + ), + Err(err) => { + crate::diag!("[plugins] MCP reconcile failed requested={requested} error={err}") + } + } + result } } diff --git a/crates/infrastructure/src/plugin/mod.rs b/crates/infrastructure/src/plugin/mod.rs index 43732b0..6277db8 100644 --- a/crates/infrastructure/src/plugin/mod.rs +++ b/crates/infrastructure/src/plugin/mod.rs @@ -5,7 +5,7 @@ use std::fs; use std::io::Read; use std::path::{Path, PathBuf}; use std::process::Stdio; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, MutexGuard}; use std::time::{SystemTime, UNIX_EPOCH}; use async_trait::async_trait; @@ -386,6 +386,8 @@ trait ExternalMcpServerBridge: Send + Sync { ) -> Result, PluginMcpError>; } +type ExternalMcpChildren = HashMap>; + struct ProcessMcpServerHandle { child: Child, } @@ -433,7 +435,7 @@ impl ExternalMcpServerBridge for StdioExternalMcpServerBridge { /// External process supervisor for plugin MCP servers. pub struct ExternalMcpPluginSupervisor { bridge: Arc, - children: Mutex>>, + children: Mutex, } impl ExternalMcpPluginSupervisor { @@ -453,6 +455,12 @@ impl ExternalMcpPluginSupervisor { children: Mutex::new(HashMap::new()), } } + + fn children(&self) -> Result, PluginMcpError> { + self.children + .lock() + .map_err(|_| PluginMcpError::Process("plugin mcp supervisor mutex poisoned".to_owned())) + } } impl Default for ExternalMcpPluginSupervisor { @@ -469,10 +477,7 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor { ) -> Result { let desired: HashSet = active_servers.iter().map(|s| s.identity.clone()).collect(); let to_stop = { - let children = self - .children - .lock() - .expect("plugin mcp supervisor poisoned"); + let children = self.children()?; children .keys() .filter(|id| !desired.contains(*id)) @@ -480,22 +485,14 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor { .collect::>() }; for id in to_stop { - let child = self - .children - .lock() - .expect("plugin mcp supervisor poisoned") - .remove(&id); + let child = self.children()?.remove(&id); if let Some(mut child) = child { let _ = child.stop().await; } } let mut statuses = Vec::new(); for spec in active_servers { - let already = self - .children - .lock() - .expect("plugin mcp supervisor poisoned") - .contains_key(&spec.identity); + let already = self.children()?.contains_key(&spec.identity); if already { statuses.push(PluginMcpStatus { identity: spec.identity, @@ -506,10 +503,12 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor { } match self.bridge.start(&spec).await { Ok(handle) => { - self.children - .lock() - .expect("plugin mcp supervisor poisoned") - .insert(spec.identity.clone(), handle); + match self.children() { + Ok(mut children) => { + children.insert(spec.identity.clone(), handle); + } + Err(err) => return Err(err), + } statuses.push(PluginMcpStatus { identity: spec.identity, running: true, @@ -529,10 +528,7 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor { async fn stop_plugin(&self, plugin_id: &PluginId) -> Result<(), PluginMcpError> { let prefix = format!("plugin:{}:", plugin_id.as_str()); let ids = { - let children = self - .children - .lock() - .expect("plugin mcp supervisor poisoned"); + let children = self.children()?; children .keys() .filter(|id| id.starts_with(&prefix)) @@ -540,11 +536,7 @@ impl PluginMcpSupervisor for ExternalMcpPluginSupervisor { .collect::>() }; for id in ids { - let child = self - .children - .lock() - .expect("plugin mcp supervisor poisoned") - .remove(&id); + let child = self.children()?.remove(&id); if let Some(mut child) = child { child.stop().await?; } @@ -751,4 +743,20 @@ mod tests { assert_eq!(statuses.servers[0].identity, other.identity); assert_eq!(bridge.started.lock().unwrap().len(), 2); } + + #[tokio::test] + async fn supervisor_poisoned_registry_returns_typed_error() { + let supervisor = ExternalMcpPluginSupervisor::new(); + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let _guard = supervisor.children.lock().unwrap(); + panic!("poison plugin mcp supervisor registry"); + })); + + let err = supervisor.reconcile(Vec::new()).await.unwrap_err(); + + assert_eq!( + err, + PluginMcpError::Process("plugin mcp supervisor mutex poisoned".to_owned()) + ); + } }