From 7b92b9f12504bd36c688244b9f158e31e5cb5d69 Mon Sep 17 00:00:00 2001 From: Danylo Date: Tue, 23 Jun 2026 22:20:34 +0300 Subject: [PATCH 01/10] Hotfix/tool update resilience (#1966) --- clients/openframe-client/src/lib.rs | 2 + .../src/models/installed_tool.rs | 10 ++ clients/openframe-client/src/models/mod.rs | 2 +- .../src/platform/directories.rs | 7 ++ .../src/services/installed_tools_service.rs | 42 ++++++- .../src/services/mesh_self_heal_service.rs | 10 ++ .../openframe_client_update_service.rs | 18 +++ .../src/services/tool_agent_update_service.rs | 56 ++++++++- .../tool_connection_processing_manager.rs | 10 ++ .../src/services/tool_installation_service.rs | 117 ++++++++++++++---- .../src/services/tool_run_manager.rs | 81 ++++++++++-- 11 files changed, 312 insertions(+), 43 deletions(-) diff --git a/clients/openframe-client/src/lib.rs b/clients/openframe-client/src/lib.rs index 8b06f073e..78ae619ab 100644 --- a/clients/openframe-client/src/lib.rs +++ b/clients/openframe-client/src/lib.rs @@ -321,6 +321,7 @@ impl Client { tool_kill_service.clone(), initial_configuration_service.clone(), config_service.clone(), + tool_run_manager.clone(), ); // Initialize tool connection service @@ -334,6 +335,7 @@ impl Client { tool_connection_message_publisher.clone(), config_service.clone(), tool_connection_service.clone(), + tool_run_manager.clone(), ); // Initialize OpenFrame client info service diff --git a/clients/openframe-client/src/models/installed_tool.rs b/clients/openframe-client/src/models/installed_tool.rs index 3fa44f59d..fe0b9f312 100644 --- a/clients/openframe-client/src/models/installed_tool.rs +++ b/clients/openframe-client/src/models/installed_tool.rs @@ -61,6 +61,14 @@ pub struct InstalledAsset { pub executable: bool, } +#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ToolRecordState { + Installing, + #[default] + Installed, +} + #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct InstalledTool { pub tool_agent_id: String, @@ -75,4 +83,6 @@ pub struct InstalledTool { pub installation: Installation, #[serde(default)] pub assets: Vec, + #[serde(default)] + pub state: ToolRecordState, } diff --git a/clients/openframe-client/src/models/mod.rs b/clients/openframe-client/src/models/mod.rs index c6d5fcec8..62c3c95bc 100644 --- a/clients/openframe-client/src/models/mod.rs +++ b/clients/openframe-client/src/models/mod.rs @@ -32,7 +32,7 @@ pub use device_tag::DeviceTag; pub use download_configuration::{DownloadConfiguration, InstallationType}; pub use initial_configuration::InitialConfiguration; pub use installed_agent_message::InstalledAgentMessage; -pub use installed_tool::{Installation, InstalledAsset, InstalledTool}; +pub use installed_tool::{Installation, InstalledAsset, InstalledTool, ToolRecordState}; pub use machine_heartbeat_message::MachineHeartbeatMessage; pub use openframe_client_info::OpenFrameClientInfo; pub use openframe_client_update_message::OpenFrameClientUpdateMessage; diff --git a/clients/openframe-client/src/platform/directories.rs b/clients/openframe-client/src/platform/directories.rs index aa297838f..1dac01f33 100644 --- a/clients/openframe-client/src/platform/directories.rs +++ b/clients/openframe-client/src/platform/directories.rs @@ -828,6 +828,13 @@ impl DirectoryManager { pub fn is_app_bundle_path(path: &Path) -> bool { path.to_string_lossy().contains(".app/") } + + pub async fn tool_artifact_present(&self, path: &Path, is_gui_app: bool) -> bool { + match tokio::fs::metadata(path).await { + Ok(m) => (m.is_file() && m.len() > 0) || (is_gui_app && m.is_dir()), + Err(_) => false, + } + } } #[cfg(target_os = "macos")] diff --git a/clients/openframe-client/src/services/installed_tools_service.rs b/clients/openframe-client/src/services/installed_tools_service.rs index 1ddb8a46f..ae0b4e6d1 100644 --- a/clients/openframe-client/src/services/installed_tools_service.rs +++ b/clients/openframe-client/src/services/installed_tools_service.rs @@ -1,12 +1,16 @@ -use crate::models::InstalledTool; +use crate::models::{InstalledTool, ToolRecordState}; use crate::platform::directories::DirectoryManager; use anyhow::{Context, Result}; use std::fs; +use std::io::Write; use std::path::PathBuf; +use std::sync::Arc; +use tokio::sync::Mutex; #[derive(Clone)] pub struct InstalledToolsService { file_path: PathBuf, + writer: Arc>, } impl InstalledToolsService { @@ -15,10 +19,14 @@ impl InstalledToolsService { directory_manager .ensure_directories() .with_context(|| "Failed to ensure secured directory exists")?; - Ok(Self { file_path: path }) + Ok(Self { + file_path: path, + writer: Arc::new(Mutex::new(())), + }) } pub async fn save(&self, tool: InstalledTool) -> Result<()> { + let _guard = self.writer.lock().await; let mut tools = self.get_all().await?; if let Some(existing) = tools @@ -33,6 +41,18 @@ impl InstalledToolsService { self.persist(&tools).await } + pub async fn set_state(&self, tool_agent_id: &str, state: ToolRecordState) -> Result { + let _guard = self.writer.lock().await; + let mut tools = self.get_all().await?; + if let Some(existing) = tools.iter_mut().find(|t| t.tool_agent_id == tool_agent_id) { + existing.state = state; + self.persist(&tools).await?; + Ok(true) + } else { + Ok(false) + } + } + pub async fn get_by_tool_agent_id(&self, tool_id: &str) -> Result> { let tools = self.get_all().await?; Ok(tools.into_iter().find(|t| t.tool_agent_id == tool_id)) @@ -53,6 +73,7 @@ impl InstalledToolsService { /// Delete an installed tool by its tool_agent_id pub async fn delete_by_tool_agent_id(&self, tool_agent_id: &str) -> Result { + let _guard = self.writer.lock().await; let mut tools = self.get_all().await?; let initial_len = tools.len(); tools.retain(|t| t.tool_agent_id != tool_agent_id); @@ -68,8 +89,21 @@ impl InstalledToolsService { async fn persist(&self, tools: &[InstalledTool]) -> Result<()> { let json = serde_json::to_string_pretty(tools) .context("Failed to serialize installed tools to JSON")?; - fs::write(&self.file_path, json).with_context(|| { - format!("Failed to write installed tools file: {:?}", self.file_path) + + let tmp_path = self.file_path.with_extension("json.tmp"); + { + let mut file = fs::File::create(&tmp_path) + .with_context(|| format!("Failed to create temp tools file: {:?}", tmp_path))?; + file.write_all(json.as_bytes()) + .with_context(|| format!("Failed to write temp tools file: {:?}", tmp_path))?; + file.sync_all() + .with_context(|| format!("Failed to fsync temp tools file: {:?}", tmp_path))?; + } + fs::rename(&tmp_path, &self.file_path).with_context(|| { + format!( + "Failed to atomically replace tools file: {:?}", + self.file_path + ) })?; Ok(()) } diff --git a/clients/openframe-client/src/services/mesh_self_heal_service.rs b/clients/openframe-client/src/services/mesh_self_heal_service.rs index a8862119b..427c26e96 100644 --- a/clients/openframe-client/src/services/mesh_self_heal_service.rs +++ b/clients/openframe-client/src/services/mesh_self_heal_service.rs @@ -10,6 +10,7 @@ use tracing::{debug, error, info, warn}; use crate::models::Installation; use crate::platform::{system_service, DirectoryManager}; use crate::services::tool_kill_service::ToolKillService; +use crate::services::tool_run_manager::ToolRunManager; use crate::services::{ AgentConfigurationService, InitialConfigurationService, InstalledToolsService, }; @@ -35,6 +36,7 @@ pub struct MeshSelfHealService { tool_kill: ToolKillService, initial_config: InitialConfigurationService, agent_config: AgentConfigurationService, + tool_run_manager: ToolRunManager, http: reqwest::Client, } @@ -45,6 +47,7 @@ impl MeshSelfHealService { tool_kill: ToolKillService, initial_config: InitialConfigurationService, agent_config: AgentConfigurationService, + tool_run_manager: ToolRunManager, ) -> Self { Self { directory_manager, @@ -52,6 +55,7 @@ impl MeshSelfHealService { tool_kill, initial_config, agent_config, + tool_run_manager, http: reqwest::Client::builder() .timeout(HTTP_TIMEOUT) .build() @@ -112,6 +116,12 @@ impl MeshSelfHealService { continue; } + if self.tool_run_manager.is_updating(MESH_TOOL_ID).await { + info!("meshcentral-agent is updating — skipping MeshID self-heal this cycle"); + stuck_since = None; + continue; + } + warn!( "meshcentral-agent stuck for {}s with no successful connect — attempting MeshID self-heal", stuck_for.as_secs() diff --git a/clients/openframe-client/src/services/openframe_client_update_service.rs b/clients/openframe-client/src/services/openframe_client_update_service.rs index 399564175..740bd3fad 100644 --- a/clients/openframe-client/src/services/openframe_client_update_service.rs +++ b/clients/openframe-client/src/services/openframe_client_update_service.rs @@ -45,6 +45,13 @@ impl OpenFrameClientUpdateService { let requested_version = message.version.trim(); info!("Received update request for version: {}", requested_version); + if self.tool_run_manager.any_tool_op_in_progress().await { + warn!("Tool operation in progress, deferring client update to version {} (will redeliver)", requested_version); + return Err(anyhow!( + "Tool operation in progress, deferring client update" + )); + } + // 1. Check if update is already in progress (race condition protection) { let mut update_lock = self.update_in_progress.lock().await; @@ -227,6 +234,17 @@ impl OpenFrameClientUpdateService { update_state_path: self.update_state_service.get_state_file_path(), }; + if self.tool_run_manager.any_tool_op_in_progress().await { + warn!("Tool operation started during client download, deferring client update (will redeliver)"); + if let Err(cleanup_err) = std::fs::remove_file(&archive_path) { + warn!("Failed to remove archive after deferring: {}", cleanup_err); + } + self.update_state_service.clear().await?; + return Err(anyhow!( + "Tool operation started during download, deferring client update" + )); + } + let launch_result = updater_launcher::launch_updater(params).await; // If launch failed, cleanup archive and state diff --git a/clients/openframe-client/src/services/tool_agent_update_service.rs b/clients/openframe-client/src/services/tool_agent_update_service.rs index 4b52baf59..b672ace32 100644 --- a/clients/openframe-client/src/services/tool_agent_update_service.rs +++ b/clients/openframe-client/src/services/tool_agent_update_service.rs @@ -1,6 +1,6 @@ use crate::clients::tool_agent_file_client::ToolAgentFileClient; use crate::models::tool_agent_update_message::{AssetUpdate, ToolAgentUpdateMessage}; -use crate::models::{Installation, InstalledAsset}; +use crate::models::{Installation, InstalledAsset, ToolRecordState}; use crate::platform::{ binary_writer, detect_actual_installation, needs_migration, run_migration, run_update, DirectoryManager, ToolUpdaterDeps, @@ -77,12 +77,49 @@ impl ToolAgentUpdateService { { Some(tool) => tool, None => { - warn!("Tool {} is not installed, skipping update", tool_agent_id); + warn!("Tool {} has no registry record (orphaned) — cannot self-heal from a lean update message; awaiting reinstall (TOOL_INSTALLATION). Skipping update", tool_agent_id); return Ok(()); } }; - let needs_tool_update = installed_tool.version != *new_version; + let was_installing = installed_tool.state == ToolRecordState::Installing; + let agent_path = self + .directory_manager + .get_tool_executable_path(tool_agent_id, installed_tool.installation.executable_path()); + let binary_missing = !self + .directory_manager + .tool_artifact_present(&agent_path, installed_tool.installation.is_gui_app()) + .await; + let needs_repair = was_installing || binary_missing; + if was_installing { + warn!( + "Tool {} record is in Installing state (interrupted (re)install) — forcing repair", + tool_agent_id + ); + } else if binary_missing { + warn!( + "Tool {} has a registry record but its binary is missing at {} — forcing repair", + tool_agent_id, + agent_path.display() + ); + } + if needs_repair { + installed_tool.state = ToolRecordState::Installing; + if binary_missing && !was_installing { + if let Err(e) = self + .installed_tools_service + .set_state(tool_agent_id, ToolRecordState::Installing) + .await + { + warn!( + "Failed to mark tool {} as installing before repair: {:#}", + tool_agent_id, e + ); + } + } + } + + let needs_tool_update = needs_repair || installed_tool.version != *new_version; let assets_to_update: Vec<_> = message .assets .as_ref() @@ -134,6 +171,19 @@ impl ToolAgentUpdateService { // Clear updating flag - for Standard tools the run manager relaunches them via this flag. self.tool_run_manager.clear_updating(tool_agent_id).await; + if result.is_ok() && needs_repair { + if let Err(e) = self + .installed_tools_service + .set_state(tool_agent_id, ToolRecordState::Installed) + .await + { + warn!( + "Failed to finalize tool {} record to Installed after repair: {:#}", + tool_agent_id, e + ); + } + } + // Windows GUI apps aren't run-manager-supervised; relaunch once after a successful update (no-op otherwise). if result.is_ok() && was_gui_before_update { self.relaunch_windows_gui_app(&installed_tool); diff --git a/clients/openframe-client/src/services/tool_connection_processing_manager.rs b/clients/openframe-client/src/services/tool_connection_processing_manager.rs index b4a51e276..3e0c66f78 100644 --- a/clients/openframe-client/src/services/tool_connection_processing_manager.rs +++ b/clients/openframe-client/src/services/tool_connection_processing_manager.rs @@ -14,6 +14,7 @@ use crate::services::installed_tools_service::InstalledToolsService; use crate::services::tool_command_params_resolver::ToolCommandParamsResolver; use crate::services::tool_connection_message_publisher::ToolConnectionMessagePublisher; use crate::services::tool_connection_service::ToolConnectionService; +use crate::services::tool_run_manager::ToolRunManager; const RETRY_DELAY_SECONDS: u64 = 15; @@ -25,6 +26,7 @@ pub struct ToolConnectionProcessingManager { tool_connection_publisher: ToolConnectionMessagePublisher, config_service: AgentConfigurationService, tool_connection_service: ToolConnectionService, + tool_run_manager: ToolRunManager, running_tools: Arc>>, } @@ -35,6 +37,7 @@ impl ToolConnectionProcessingManager { tool_connection_publisher: ToolConnectionMessagePublisher, config_service: AgentConfigurationService, tool_connection_service: ToolConnectionService, + tool_run_manager: ToolRunManager, ) -> Self { Self { installed_tools_service, @@ -42,6 +45,7 @@ impl ToolConnectionProcessingManager { tool_connection_publisher, config_service, tool_connection_service, + tool_run_manager, running_tools: Arc::new(RwLock::new(HashSet::new())), } } @@ -135,9 +139,15 @@ impl ToolConnectionProcessingManager { let config_service = self.config_service.clone(); let tool_connection_publisher = self.tool_connection_publisher.clone(); let tool_connection_service = self.tool_connection_service.clone(); + let tool_run_manager = self.tool_run_manager.clone(); tokio::spawn(async move { loop { + while tool_run_manager.is_updating(&tool.tool_agent_id).await { + info!(tool_id = %tool.tool_id, "Tool is being updated, deferring node-id resolution..."); + sleep(Duration::from_secs(RETRY_DELAY_SECONDS)).await; + } + // If tool_agent_id_command_args is empty, use empty string as agent_tool_id let agent_tool_id = if tool.tool_agent_id_command_args.is_empty() { info!( diff --git a/clients/openframe-client/src/services/tool_installation_service.rs b/clients/openframe-client/src/services/tool_installation_service.rs index 0f0357abd..7c43770db 100644 --- a/clients/openframe-client/src/services/tool_installation_service.rs +++ b/clients/openframe-client/src/services/tool_installation_service.rs @@ -3,7 +3,7 @@ use crate::clients::tool_api_client::ToolApiClient; use crate::models::download_configuration::{DownloadConfiguration, InstallationType}; use crate::models::tool_installation_message::AssetSource; use crate::models::ToolInstallationMessage; -use crate::models::{Installation, InstalledTool}; +use crate::models::{Installation, InstalledTool, ToolRecordState}; #[cfg(target_os = "windows")] use crate::platform::file_lock::log_file_lock_info; use crate::platform::DirectoryManager; @@ -27,6 +27,13 @@ use tokio::io::AsyncWriteExt; use tokio::process::Command; use tracing::{debug, info, warn}; +/// Hard cap on how long an external install/uninstall command may run before we abort +/// it. Prevents a hung installer from pinning the tool-op marker (and therefore the +/// client self-update defer) indefinitely: on timeout the op fails, clears its marker, +/// and the message redelivers. `kill_on_drop` ensures the spawned process is actually +/// terminated when the timeout fires. +const TOOL_COMMAND_TIMEOUT_SECS: u64 = 300; + #[derive(Clone)] pub struct ToolInstallationService { github_download_service: GithubDownloadService, @@ -85,6 +92,17 @@ impl ToolInstallationService { #[tracing::instrument(skip_all, fields(tool_id = %tool_installation_message.tool_agent_id))] pub async fn install(&self, tool_installation_message: ToolInstallationMessage) -> Result<()> { + let tool_agent_id = tool_installation_message.tool_agent_id.clone(); + self.tool_run_manager.mark_updating(&tool_agent_id).await; + let result = self.install_inner(tool_installation_message).await; + self.tool_run_manager.clear_updating(&tool_agent_id).await; + result + } + + async fn install_inner( + &self, + tool_installation_message: ToolInstallationMessage, + ) -> Result<()> { let tool_agent_id = &tool_installation_message.tool_agent_id; info!( "Installing tool {} with version {}", @@ -111,6 +129,17 @@ impl ToolInstallationService { tool_agent_id, version_clone ); + if let Err(e) = self + .installed_tools_service + .set_state(tool_agent_id, ToolRecordState::Installing) + .await + { + warn!( + "Failed to mark tool {} as installing before reinstall: {:#}", + tool_agent_id, e + ); + } + // Stop the tool process if it's running info!("Stopping existing tool process for {}", tool_agent_id); if let Err(e) = self @@ -145,14 +174,20 @@ impl ToolInstallationService { ); let mut cmd = Command::new(&agent_path); cmd.args(&processed_args); - match cmd.output().await { - Ok(output) if output.status.success() => { + cmd.kill_on_drop(true); + let uninstall_result = tokio::time::timeout( + tokio::time::Duration::from_secs(TOOL_COMMAND_TIMEOUT_SECS), + cmd.output(), + ) + .await; + match uninstall_result { + Ok(Ok(output)) if output.status.success() => { info!( "Uninstall command completed for {} before reinstall", tool_agent_id ); } - Ok(output) => { + Ok(Ok(output)) => { warn!( "Uninstall command for {} exited with status {}: {}", tool_agent_id, @@ -160,7 +195,7 @@ impl ToolInstallationService { String::from_utf8_lossy(&output.stderr) ); } - Err(e) => { + Ok(Err(e)) => { #[cfg(target_os = "windows")] log_file_lock_info( &e, @@ -172,6 +207,9 @@ impl ToolInstallationService { tool_agent_id, e ); } + Err(_) => { + warn!("Uninstall command for {} timed out after {}s; continuing with reinstall", tool_agent_id, TOOL_COMMAND_TIMEOUT_SECS); + } } if let Err(e) = self @@ -227,8 +265,6 @@ impl ToolInstallationService { tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; - // Delete from both services - info!("Removing tool {} from services", tool_agent_id); if let Err(e) = self .tool_connection_service .delete_by_tool_agent_id(tool_agent_id) @@ -236,13 +272,6 @@ impl ToolInstallationService { { warn!("Failed to remove tool connection: {:#}", e); } - if let Err(e) = self - .installed_tools_service - .delete_by_tool_agent_id(tool_agent_id) - .await - { - warn!("Failed to remove from installed tools: {:#}", e); - } // Clear from both manager tracking sets to allow tool restart after reinstall self.tool_connection_processing_manager @@ -257,11 +286,32 @@ impl ToolInstallationService { tool_agent_id ); } else { - info!( - "Tool {} is already installed with version {}, skipping installation", - tool_agent_id, installed_tool.version + let agent_path = self.directory_manager.get_tool_executable_path( + tool_agent_id, + installed_tool.installation.executable_path(), ); - return Ok(()); + let binary_present = self + .directory_manager + .tool_artifact_present(&agent_path, installed_tool.installation.is_gui_app()) + .await; + if binary_present { + info!( + "Tool {} is already installed with version {}, skipping installation", + tool_agent_id, installed_tool.version + ); + return Ok(()); + } + warn!("Tool {} has a registry record (version {}) but its binary is missing at {} — repairing via install", tool_agent_id, installed_tool.version, agent_path.display()); + if let Err(e) = self + .installed_tools_service + .set_state(tool_agent_id, ToolRecordState::Installing) + .await + { + warn!( + "Failed to mark tool {} as installing before repair: {:#}", + tool_agent_id, e + ); + } } } @@ -599,11 +649,33 @@ impl ToolInstallationService { let mut cmd = Command::new(&file_path); cmd.args(&installation_command_args); + cmd.kill_on_drop(true); - let output = cmd - .output() - .await - .context("Failed to execute installation command for tool")?; + let output = match tokio::time::timeout( + tokio::time::Duration::from_secs(TOOL_COMMAND_TIMEOUT_SECS), + cmd.output(), + ) + .await + { + Ok(Ok(output)) => output, + Ok(Err(e)) => { + #[cfg(target_os = "windows")] + log_file_lock_info( + &e, + &file_path.to_string_lossy(), + "execute installation command", + ); + return Err(anyhow::Error::new(e) + .context("Failed to execute installation command for tool")); + } + Err(_) => { + return Err(anyhow::anyhow!( + "Installation command for {} timed out after {}s", + tool_agent_id, + TOOL_COMMAND_TIMEOUT_SECS + )); + } + }; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -641,6 +713,7 @@ impl ToolInstallationService { uninstallation_command_args: tool_installation_message.uninstallation_command_args, installation, assets: Vec::new(), + state: ToolRecordState::Installed, }; self.installed_tools_service diff --git a/clients/openframe-client/src/services/tool_run_manager.rs b/clients/openframe-client/src/services/tool_run_manager.rs index fd1124454..755bfa786 100644 --- a/clients/openframe-client/src/services/tool_run_manager.rs +++ b/clients/openframe-client/src/services/tool_run_manager.rs @@ -1,9 +1,9 @@ -use crate::models::installed_tool::{Installation, InstalledTool}; +use crate::models::installed_tool::{Installation, InstalledTool, ToolRecordState}; use crate::services::installed_tools_service::InstalledToolsService; use crate::services::tool_command_params_resolver::ToolCommandParamsResolver; use crate::services::tool_kill_service::ToolKillService; use anyhow::{Context, Result}; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::process::Stdio; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -397,7 +397,7 @@ pub struct ToolRunManager { params_processor: ToolCommandParamsResolver, tool_kill_service: ToolKillService, running_tools: Arc>>, - updating_tools: Arc>>, + updating_tools: Arc>>, shutting_down: Arc, } @@ -412,7 +412,7 @@ impl ToolRunManager { params_processor, tool_kill_service, running_tools: Arc::new(RwLock::new(HashSet::new())), - updating_tools: Arc::new(RwLock::new(HashSet::new())), + updating_tools: Arc::new(RwLock::new(HashMap::new())), shutting_down: Arc::new(AtomicBool::new(false)), } } @@ -425,20 +425,37 @@ impl ToolRunManager { } pub async fn mark_updating(&self, tool_id: &str) { - self.updating_tools - .write() - .await - .insert(tool_id.to_string()); - info!("Tool {} marked as updating", tool_id); + let mut map = self.updating_tools.write().await; + let count = map.entry(tool_id.to_string()).or_insert(0); + *count += 1; + info!( + "Tool {} marked as updating (in-flight ops: {})", + tool_id, *count + ); } pub async fn clear_updating(&self, tool_id: &str) { - self.updating_tools.write().await.remove(tool_id); - info!("Tool {} update flag cleared", tool_id); + let mut map = self.updating_tools.write().await; + if let Some(count) = map.get_mut(tool_id) { + *count -= 1; + if *count == 0 { + map.remove(tool_id); + info!("Tool {} update flag cleared", tool_id); + } else { + info!( + "Tool {} op finished (in-flight ops remaining: {})", + tool_id, *count + ); + } + } } pub async fn is_updating(&self, tool_id: &str) -> bool { - self.updating_tools.read().await.contains(tool_id) + self.updating_tools.read().await.contains_key(tool_id) + } + + pub async fn any_tool_op_in_progress(&self) -> bool { + !self.updating_tools.read().await.is_empty() } pub async fn run(&self) -> Result<()> { @@ -455,6 +472,40 @@ impl ToolRunManager { return Ok(()); } + for tool in &tools { + if tool.state != ToolRecordState::Installing { + continue; + } + let path = self + .params_processor + .directory_manager + .get_tool_executable_path(&tool.tool_agent_id, tool.installation.executable_path()); + let binary_present = self + .params_processor + .directory_manager + .tool_artifact_present(&path, tool.installation.is_gui_app()) + .await; + + if !binary_present { + warn!(tool_id = %tool.tool_agent_id, "Record left Installing and binary missing/empty at {} — awaiting reinstall", path.display()); + continue; + } + + if tool.installation.is_service() { + warn!(tool_id = %tool.tool_agent_id, "Record left Installing; binary present at {} but service registration can't be verified from disk — leaving Installing for a verified repair", path.display()); + continue; + } + + warn!(tool_id = %tool.tool_agent_id, "Record left Installing but binary is present at {} — marking Installed", path.display()); + if let Err(e) = self + .installed_tools_service + .set_state(&tool.tool_agent_id, ToolRecordState::Installed) + .await + { + warn!(tool_id = %tool.tool_agent_id, "Failed to mark Installed during startup recheck: {:#}", e); + } + } + for tool in tools { if self.try_mark_running(&tool.tool_agent_id).await { info!("Running tool {}", tool.tool_agent_id); @@ -535,7 +586,11 @@ impl ToolRunManager { } let mut was_updating = false; - while updating_tools.read().await.contains(&tool.tool_agent_id) { + while updating_tools + .read() + .await + .contains_key(&tool.tool_agent_id) + { was_updating = true; info!(tool_id = %tool.tool_agent_id, "Tool is being updated, waiting..."); sleep(Duration::from_secs(1)).await; From d5b2afb3457b0824e9e6b56580034d57e4f297c7 Mon Sep 17 00:00:00 2001 From: mikhailm-coder Date: Wed, 24 Jun 2026 18:51:52 +0300 Subject: [PATCH 02/10] fix(client): back off mesh self-heal to hourly after a no-op (+ log .msh target) (#1981) Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/services/mesh_self_heal_service.rs | 57 +++++++++++++++---- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/clients/openframe-client/src/services/mesh_self_heal_service.rs b/clients/openframe-client/src/services/mesh_self_heal_service.rs index 427c26e96..7e9209b04 100644 --- a/clients/openframe-client/src/services/mesh_self_heal_service.rs +++ b/clients/openframe-client/src/services/mesh_self_heal_service.rs @@ -24,8 +24,10 @@ const HEALTHY_MARKER: &str = "Received CoreOk from server"; /// How often we scan the agent log. const POLL_INTERVAL: Duration = Duration::from_secs(30); -/// How long continuously stuck before we act; also the sole rate-limiter (streak resets after each attempt). Cooldown may return after dev testing. +/// How long continuously stuck before we act the first time. const STUCK_DURATION: Duration = Duration::from_secs(10 * 60); +/// After a no-op/failed heal (MeshID unchanged / server-side outage we can't fix) back off to this before retrying, so a persistently-down server doesn't spam the log every STUCK_DURATION. +const NOOP_COOLDOWN: Duration = Duration::from_secs(60 * 60); /// Timeout for the /generate-msh fetch so an unresponsive server can't block the heal loop. const HTTP_TIMEOUT: Duration = Duration::from_secs(30); @@ -87,6 +89,8 @@ impl MeshSelfHealService { let mut offset: u64 = 0; let mut stuck_since: Option = None; + // Set after a no-op/failed heal so further attempts (and their logs) are throttled to NOOP_COOLDOWN. + let mut last_attempt: Option = None; loop { sleep(POLL_INTERVAL).await; @@ -96,6 +100,7 @@ impl MeshSelfHealService { for line in &lines { if line.contains(HEALTHY_MARKER) { stuck_since = None; + last_attempt = None; } else if line.contains(FAILURE_MARKER) { stuck_since.get_or_insert_with(Instant::now); } @@ -115,6 +120,12 @@ impl MeshSelfHealService { if stuck_for < STUCK_DURATION { continue; } + // Throttle: after a no-op/failed heal don't retry (or log) until NOOP_COOLDOWN has passed. + if let Some(t) = last_attempt { + if t.elapsed() < NOOP_COOLDOWN { + continue; + } + } if self.tool_run_manager.is_updating(MESH_TOOL_ID).await { info!("meshcentral-agent is updating — skipping MeshID self-heal this cycle"); @@ -127,16 +138,24 @@ impl MeshSelfHealService { stuck_for.as_secs() ); match self.try_heal().await { - Ok(true) => info!("mesh self-heal: adopted a new MeshID and restarted the agent"), + Ok(true) => { + info!("mesh self-heal: adopted a new MeshID and restarted the agent"); + last_attempt = None; + } Ok(false) => { - debug!( - "mesh self-heal: MeshID unchanged or server unreachable — no action taken" - ) + last_attempt = Some(Instant::now()); + info!("mesh self-heal: MeshID unchanged — likely a server-side mesh outage, not a client problem; backing off for {}s", NOOP_COOLDOWN.as_secs()); + } + Err(e) => { + last_attempt = Some(Instant::now()); + error!( + "mesh self-heal failed: {e:#} — backing off for {}s", + NOOP_COOLDOWN.as_secs() + ); } - Err(e) => error!("mesh self-heal failed: {e:#}"), } - // Sole rate-limiter: reset after every attempt; a real heal is cleared by the CoreOk marker. + // Reset the stuck streak after each attempt; a real heal is cleared by the CoreOk marker. stuck_since = None; } } @@ -161,12 +180,16 @@ impl MeshSelfHealService { parse_mesh_id(&body).ok_or_else(|| anyhow!("no MeshID in /generate-msh response"))?; let msh_path = self.mesh_msh_path().await?; - let current_id = tokio::fs::read_to_string(&msh_path) - .await - .ok() - .and_then(|s| parse_mesh_id(&s)); + let current_msh = tokio::fs::read_to_string(&msh_path).await.ok(); + let current_id = current_msh.as_deref().and_then(parse_mesh_id); if current_id.as_deref() == Some(new_id.as_str()) { + // No MeshID drift to fix. Log the target the agent is dialing so a server/gateway-side outage is distinguishable from a stale .msh target. + let server = current_msh + .as_deref() + .and_then(|s| parse_msh_field(s, "MeshServer")) + .unwrap_or_else(|| "".to_string()); + info!("mesh self-heal: MeshID unchanged ({new_id}); agent .msh MeshServer={server}"); return Ok(false); } @@ -232,6 +255,18 @@ fn parse_mesh_id(msh: &str) -> Option { .filter(|v| !v.is_empty()) } +/// Extract the value of a `Key=` line from an `.msh` body. +fn parse_msh_field(msh: &str, key: &str) -> Option { + let prefix = format!("{key}="); + msh.lines() + .find_map(|l| { + l.trim() + .strip_prefix(prefix.as_str()) + .map(|v| v.trim().to_string()) + }) + .filter(|v| !v.is_empty()) +} + /// Read whole new lines since *offset, advancing only to the last newline; resets offset if the file shrank. async fn read_new_lines(path: &Path, offset: &mut u64) -> Result> { use tokio::io::{AsyncReadExt, AsyncSeekExt, SeekFrom}; From 1ea2c8af3217a2c038962bcf6bb7746c98909bf5 Mon Sep 17 00:00:00 2001 From: Danylo Date: Wed, 24 Jun 2026 22:46:18 +0300 Subject: [PATCH 03/10] Hotfix/mesh agent resilience (#1986) --- .../src/platform/system_service.rs | 260 ++++++++++++++++-- .../src/platform/tool_updater/gui_app.rs | 2 +- .../src/platform/tool_updater/service.rs | 7 +- .../src/services/mesh_self_heal_service.rs | 141 +++++----- .../tool_connection_processing_manager.rs | 54 +++- .../src/services/tool_installation_service.rs | 141 +++++++++- .../src/services/tool_kill_service.rs | 11 +- .../src/services/tool_uninstall_service.rs | 2 +- 8 files changed, 498 insertions(+), 120 deletions(-) diff --git a/clients/openframe-client/src/platform/system_service.rs b/clients/openframe-client/src/platform/system_service.rs index fbbdd0832..92f6af5a5 100644 --- a/clients/openframe-client/src/platform/system_service.rs +++ b/clients/openframe-client/src/platform/system_service.rs @@ -135,28 +135,74 @@ pub async fn start_service(service_name: &str) -> Result<()> { Ok(()) } -/// Stop an OS service via the platform service manager. -pub async fn stop_service(service_name: &str) -> Result<()> { +/// Stop an OS service via the platform service manager. `allow_delete` permits the last-resort +/// SCM delete of a wedged service; pass it only from install/reinstall/uninstall, which recreate +/// the service. The update/restore paths must pass false (they don't re-register, so a delete bricks the tool). +pub async fn stop_service(service_name: &str, allow_delete: bool) -> Result<()> { info!("Stopping service: {}", service_name); #[cfg(target_os = "windows")] { - stop_service_windows(service_name).await + stop_service_windows(service_name, allow_delete).await } #[cfg(target_os = "macos")] { + let _ = allow_delete; stop_service_macos(service_name).await } #[cfg(target_os = "linux")] { + let _ = allow_delete; stop_service_linux(service_name).await } } +/// Confirm a freshly (re)installed service actually reached RUNNING, starting it if the +/// installer left it stopped; errors if it cannot be confirmed RUNNING. On non-Windows this is +/// a no-op — the `StopPending` wedge that motivates it is Windows-specific, and blindly +/// re-issuing a start on launchd/systemd risks failing an already-loaded unit. +pub async fn verify_service_running(service_name: &str) -> Result<()> { + #[cfg(target_os = "windows")] + { + use windows_service::service::ServiceState; + if let Ok(status) = query_service_status_windows(service_name) { + if status.current_state == ServiceState::Running { + return Ok(()); + } + } + start_service(service_name).await.with_context(|| { + format!( + "service {} did not reach RUNNING after install", + service_name + ) + }) + } + #[cfg(not(target_os = "windows"))] + { + let _ = service_name; + Ok(()) + } +} + +/// True if the service is absent or Stopped — i.e. safe to (re)install over. On non-Windows +/// always true. Used to abort a reinstall rather than overwrite/register on top of a service +/// we could not stop or clear (e.g. a wedged `StopPending` or a still-live old agent). +pub async fn service_clear_for_install(service_name: &str) -> bool { + #[cfg(target_os = "windows")] + { + service_stopped_or_missing(&query_service_status_windows(service_name)) + } + #[cfg(not(target_os = "windows"))] + { + let _ = service_name; + true + } +} + #[cfg(target_os = "windows")] -async fn stop_service_windows(service_name: &str) -> Result<()> { +async fn stop_service_windows(service_name: &str, allow_delete: bool) -> Result<()> { use winapi::shared::winerror::{ ERROR_SERVICE_CANNOT_ACCEPT_CTRL, ERROR_SERVICE_DOES_NOT_EXIST, ERROR_SERVICE_NOT_ACTIVE, }; @@ -182,14 +228,14 @@ async fn stop_service_windows(service_name: &str) -> Result<()> { "service.stop() for {} timed out after {}s; force-killing service process", service_name, SERVICE_STOP_CALL_TIMEOUT_SECS ); - return force_stop_service_windows(service_name).await; + return force_stop_service_windows(service_name, allow_delete).await; } Ok(Err(join_err)) => { error!( "service.stop() task for {} failed: {}; force-killing service process", service_name, join_err ); - return force_stop_service_windows(service_name).await; + return force_stop_service_windows(service_name, allow_delete).await; } Ok(Ok(result)) => result, }; @@ -204,7 +250,7 @@ async fn stop_service_windows(service_name: &str) -> Result<()> { "Service {} did not reach STOPPED after stop request; force-killing", service_name ); - force_stop_service_windows(service_name).await + force_stop_service_windows(service_name, allow_delete).await } Err(windows_service::Error::Winapi(e)) if e.raw_os_error() == Some(ERROR_SERVICE_DOES_NOT_EXIST as i32) => @@ -231,20 +277,20 @@ async fn stop_service_windows(service_name: &str) -> Result<()> { "Service {} cannot accept stop control (error {}); force-killing service process", service_name, ERROR_SERVICE_CANNOT_ACCEPT_CTRL ); - force_stop_service_windows(service_name).await + force_stop_service_windows(service_name, allow_delete).await } Err(e) => { error!( "Failed to stop service {} via SCM: {}; force-killing service process", service_name, e ); - force_stop_service_windows(service_name).await + force_stop_service_windows(service_name, allow_delete).await } } } #[cfg(target_os = "windows")] -async fn force_stop_service_windows(service_name: &str) -> Result<()> { +async fn force_stop_service_windows(service_name: &str, allow_delete: bool) -> Result<()> { for attempt in 1..=SERVICE_FORCE_KILL_MAX_ATTEMPTS { let status = query_service_status_windows(service_name); if service_stopped_or_missing(&status) { @@ -286,14 +332,30 @@ async fn force_stop_service_windows(service_name: &str) -> Result<()> { } } None => { - let state = status - .as_ref() - .map(|s| format!("{:?}", s.current_state)) - .unwrap_or_else(|_| "unqueryable".to_string()); - info!( - "Service {} has no reportable PID (state {}, attempt {}/{}); waiting", - service_name, state, attempt, SERVICE_FORCE_KILL_MAX_ATTEMPTS - ); + // SCM only reports a PID while the service is Running; in StopPending (and + // other transitional states) the PID is hidden, so the SCM-PID path can never + // act on a wedged service. Fall back to the service's configured image path + // and kill any live process running from it directly. + match service_image_exe_path_windows(service_name) { + Some(exe) => { + let killed = kill_processes_by_exe_path_windows(&exe).await; + if killed > 0 { + info!("Force-killed {} process(es) for service {} by image path {} (attempt {}/{})", + killed, service_name, exe.display(), attempt, SERVICE_FORCE_KILL_MAX_ATTEMPTS); + } else { + info!("Service {} has no reportable PID and no live process at {} (attempt {}/{}); waiting for SCM to settle", + service_name, exe.display(), attempt, SERVICE_FORCE_KILL_MAX_ATTEMPTS); + } + } + None => { + let state = status + .as_ref() + .map(|s| format!("{:?}", s.current_state)) + .unwrap_or_else(|_| "unqueryable".to_string()); + info!("Service {} has no reportable PID and no resolvable image path (state {}, attempt {}/{}); waiting", + service_name, state, attempt, SERVICE_FORCE_KILL_MAX_ATTEMPTS); + } + } } } @@ -303,22 +365,52 @@ async fn force_stop_service_windows(service_name: &str) -> Result<()> { let status = query_service_status_windows(service_name); if service_stopped_or_missing(&status) { info!("Service {} force-stopped successfully", service_name); - Ok(()) - } else { - let state = status - .as_ref() - .map(|s| format!("{:?}", s.current_state)) - .unwrap_or_else(|_| "unqueryable".to_string()); - error!( - "Service {} still not stopped (state {}) after {} force-kill attempts", - service_name, state, SERVICE_FORCE_KILL_MAX_ATTEMPTS - ); - Err(anyhow::anyhow!( + return Ok(()); + } + + let state = status + .as_ref() + .map(|s| format!("{:?}", s.current_state)) + .unwrap_or_else(|_| "unqueryable".to_string()); + + // Only delete when the caller will recreate the service (install/reinstall/uninstall). The + // update/restore paths pass allow_delete=false: deleting there would brick the tool because + // nothing re-registers the service, so report failure and let the caller retry/repair. + if !allow_delete { + return Err(anyhow::anyhow!( "Failed to force-stop service {} (state {} after {} attempts)", service_name, state, SERVICE_FORCE_KILL_MAX_ATTEMPTS - )) + )); + } + + // Last resort: the service is wedged (typically StopPending that SCM won't reap, with no + // killable process). Mark it for deletion via the SCM so the follow-up reinstall recreates + // it cleanly. This is what lets a reinstall recover the agent without a machine reboot. + warn!("Service {} still not stopped (state {}) after {} force-kill attempts; deleting it via SCM to clear the wedged state", + service_name, state, SERVICE_FORCE_KILL_MAX_ATTEMPTS); + match delete_service_windows(service_name).await { + Ok(()) => { + info!( + "Service {} deleted; a fresh install will recreate it", + service_name + ); + Ok(()) + } + Err(e) => { + error!( + "Service {} could not be stopped or deleted: {:#}", + service_name, e + ); + Err(anyhow::anyhow!( + "Failed to force-stop or delete service {} (state {} after {} attempts): {:#}", + service_name, + state, + SERVICE_FORCE_KILL_MAX_ATTEMPTS, + e + )) + } } } @@ -461,3 +553,111 @@ fn service_stopped_or_missing( Err(_) => false, } } + +/// True only if SCM reports the service does not exist. +#[cfg(target_os = "windows")] +fn service_missing_windows(service_name: &str) -> bool { + use winapi::shared::winerror::ERROR_SERVICE_DOES_NOT_EXIST; + match query_service_status_windows(service_name) { + Err(windows_service::Error::Winapi(e)) => { + e.raw_os_error() == Some(ERROR_SERVICE_DOES_NOT_EXIST as i32) + } + _ => false, + } +} + +/// The on-disk executable path from the service's SCM image path (`lpBinaryPathName`), +/// stripped of surrounding quotes and any trailing arguments. +#[cfg(target_os = "windows")] +fn service_image_exe_path_windows(service_name: &str) -> Option { + use windows_service::service::ServiceAccess; + let service = open_service_windows(service_name, ServiceAccess::QUERY_CONFIG).ok()?; + let config = service.query_config().ok()?; + parse_exe_from_image_path(&config.executable_path.to_string_lossy()) +} + +/// Extract the executable path from a raw SCM image-path string, e.g. +/// `"C:\\path\\agent.exe" -arg` or `C:\\path\\agent.exe -arg`. +#[cfg(target_os = "windows")] +fn parse_exe_from_image_path(image_path: &str) -> Option { + let trimmed = image_path.trim(); + if trimmed.is_empty() { + return None; + } + // Quoted form: take the contents of the first quoted span. + if let Some(rest) = trimmed.strip_prefix('"') { + if let Some(end) = rest.find('"') { + return Some(std::path::PathBuf::from(&rest[..end])); + } + } + // Unquoted: cut after the first ".exe" (case-insensitive) to drop trailing args. + let lower = trimmed.to_lowercase(); + if let Some(idx) = lower.find(".exe") { + return Some(std::path::PathBuf::from(&trimmed[..idx + 4])); + } + Some(std::path::PathBuf::from(trimmed)) +} + +/// Force-kill every running process whose executable is exactly `exe_path`. Matching on the +/// full path (not the image name) avoids killing sibling tools that share an `agent.exe` name. +/// Returns the number of processes a kill was issued for. +#[cfg(target_os = "windows")] +async fn kill_processes_by_exe_path_windows(exe_path: &std::path::Path) -> usize { + use sysinfo::System; + let target = exe_path.to_string_lossy().to_lowercase(); + let mut sys = System::new_all(); + sys.refresh_all(); + + let mut killed = 0usize; + for (pid, process) in sys.processes() { + let proc_exe = process + .exe() + .map(|p| p.to_string_lossy().to_lowercase()) + .unwrap_or_default(); + if !proc_exe.is_empty() && proc_exe == target { + let _ = Command::new("taskkill") + .args(["/F", "/T", "/PID", &pid.to_string()]) + .output() + .await; + killed += 1; + } + } + killed +} + +/// Mark a wedged service for deletion via the SCM and wait for it to disappear, so a follow-up +/// reinstall can recreate it under the same name without hitting `ERROR_SERVICE_MARKED_FOR_DELETE`. +#[cfg(target_os = "windows")] +async fn delete_service_windows(service_name: &str) -> Result<()> { + use windows_service::service::ServiceAccess; + + if service_missing_windows(service_name) { + return Ok(()); + } + + // Scope the handle so it is closed before we poll — SCM only finalizes removal once the + // last open handle is released. + { + let service = open_service_windows(service_name, ServiceAccess::DELETE) + .with_context(|| format!("open service {} for deletion", service_name))?; + service + .delete() + .with_context(|| format!("DeleteService failed for {}", service_name))?; + } + + for _ in 1..=SERVICE_STOP_MAX_ATTEMPTS { + if service_missing_windows(service_name) { + return Ok(()); + } + sleep(Duration::from_millis(PROCESS_CHECK_INTERVAL_MS)).await; + } + + if service_missing_windows(service_name) { + Ok(()) + } else { + Err(anyhow::anyhow!( + "service {} still present after delete request", + service_name + )) + } +} diff --git a/clients/openframe-client/src/platform/tool_updater/gui_app.rs b/clients/openframe-client/src/platform/tool_updater/gui_app.rs index 83bd4ebb3..0ec138526 100644 --- a/clients/openframe-client/src/platform/tool_updater/gui_app.rs +++ b/clients/openframe-client/src/platform/tool_updater/gui_app.rs @@ -28,7 +28,7 @@ impl ToolUpdater for GuiAppToolUpdater { info!(tool_id = %tool_agent_id, "Stopping GUI app process"); self.deps .tool_kill_service - .stop_installed_tool(tool) + .stop_installed_tool(tool, false) .await .with_context(|| format!("Failed to stop GUI app: {}", tool_agent_id))?; diff --git a/clients/openframe-client/src/platform/tool_updater/service.rs b/clients/openframe-client/src/platform/tool_updater/service.rs index 2e2fbce31..d91853241 100644 --- a/clients/openframe-client/src/platform/tool_updater/service.rs +++ b/clients/openframe-client/src/platform/tool_updater/service.rs @@ -57,7 +57,12 @@ impl ToolUpdater for ServiceToolUpdater { info!(tool_id = %tool_agent_id, "Preparing Service tool for update"); info!(tool_id = %tool_agent_id, "Stopping service"); - if let Err(e) = self.deps.tool_kill_service.stop_installed_tool(tool).await { + if let Err(e) = self + .deps + .tool_kill_service + .stop_installed_tool(tool, false) + .await + { warn!(tool_id = %tool_agent_id, "Failed to stop service (non-fatal): {:#}", e); } diff --git a/clients/openframe-client/src/services/mesh_self_heal_service.rs b/clients/openframe-client/src/services/mesh_self_heal_service.rs index 7e9209b04..771fcd359 100644 --- a/clients/openframe-client/src/services/mesh_self_heal_service.rs +++ b/clients/openframe-client/src/services/mesh_self_heal_service.rs @@ -1,5 +1,3 @@ -//! Mesh self-heal: when the agent is held/orphaned on a stale MeshID, re-fetch the current .msh and bounce the agent so it re-enrolls. - use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; @@ -17,17 +15,17 @@ use crate::services::{ const MESH_TOOL_ID: &str = "meshcentral-agent"; -/// Agent log line for a control channel that can't connect (orphaned/dead-upstream); its authState= field is ignored (unreliable, ill machines show 0). +/// Agent log line for a control channel that can't connect. const FAILURE_MARKER: &str = "Connection FAILED: No HTTP response"; -/// Sent only after the mesh check passes, so an orphaned/dead-upstream agent never prints it (unlike "Server fully authenticated", printed before the hold). +/// Printed only after a successful server connect. const HEALTHY_MARKER: &str = "Received CoreOk from server"; /// How often we scan the agent log. const POLL_INTERVAL: Duration = Duration::from_secs(30); -/// How long continuously stuck before we act the first time. +/// How long continuously stuck before we act. const STUCK_DURATION: Duration = Duration::from_secs(10 * 60); -/// After a no-op/failed heal (MeshID unchanged / server-side outage we can't fix) back off to this before retrying, so a persistently-down server doesn't spam the log every STUCK_DURATION. -const NOOP_COOLDOWN: Duration = Duration::from_secs(60 * 60); +/// Minimum wait between heal attempts (success or no-op), so a server-side outage can't spin. +const NOOP_HEAL_COOLDOWN: Duration = Duration::from_secs(60 * 60); /// Timeout for the /generate-msh fetch so an unresponsive server can't block the heal loop. const HTTP_TIMEOUT: Duration = Duration::from_secs(30); @@ -65,7 +63,6 @@ impl MeshSelfHealService { } } - /// Spawn the self-heal watcher in the background (matches tool_run_manager). pub async fn run(&self) -> Result<()> { let this = self.clone(); tokio::spawn(async move { @@ -89,78 +86,64 @@ impl MeshSelfHealService { let mut offset: u64 = 0; let mut stuck_since: Option = None; - // Set after a no-op/failed heal so further attempts (and their logs) are throttled to NOOP_COOLDOWN. - let mut last_attempt: Option = None; + let mut last_heal_attempt: Option = None; loop { sleep(POLL_INTERVAL).await; + let msh_missing_serverid = self.current_msh_missing_serverid().await; + match read_new_lines(&log_path, &mut offset).await { Ok(lines) => { for line in &lines { if line.contains(HEALTHY_MARKER) { stuck_since = None; - last_attempt = None; + last_heal_attempt = None; } else if line.contains(FAILURE_MARKER) { stuck_since.get_or_insert_with(Instant::now); } } } Err(e) => { - // Log not present yet (agent not installed/started) — just wait. debug!("mesh self-heal: cannot read {}: {e}", log_path.display()); - continue; } } - let stuck_for = match stuck_since { - Some(t) => t.elapsed(), - None => continue, - }; - if stuck_for < STUCK_DURATION { + let stuck = stuck_since.map_or(false, |t| t.elapsed() >= STUCK_DURATION); + if !msh_missing_serverid && !stuck { continue; } - // Throttle: after a no-op/failed heal don't retry (or log) until NOOP_COOLDOWN has passed. - if let Some(t) = last_attempt { - if t.elapsed() < NOOP_COOLDOWN { + + if let Some(t) = last_heal_attempt { + if t.elapsed() < NOOP_HEAL_COOLDOWN { continue; } } if self.tool_run_manager.is_updating(MESH_TOOL_ID).await { - info!("meshcentral-agent is updating — skipping MeshID self-heal this cycle"); + info!("meshcentral-agent is updating — skipping .msh self-heal this cycle"); stuck_since = None; continue; } - warn!( - "meshcentral-agent stuck for {}s with no successful connect — attempting MeshID self-heal", - stuck_for.as_secs() - ); + let reason = if msh_missing_serverid { + "current .msh has no ServerID (agent cannot authenticate the server)".to_string() + } else { + format!("no successful connect within {}s", STUCK_DURATION.as_secs()) + }; + warn!("meshcentral-agent unhealthy: {reason} — attempting .msh self-heal"); + match self.try_heal().await { - Ok(true) => { - info!("mesh self-heal: adopted a new MeshID and restarted the agent"); - last_attempt = None; - } - Ok(false) => { - last_attempt = Some(Instant::now()); - info!("mesh self-heal: MeshID unchanged — likely a server-side mesh outage, not a client problem; backing off for {}s", NOOP_COOLDOWN.as_secs()); - } - Err(e) => { - last_attempt = Some(Instant::now()); - error!( - "mesh self-heal failed: {e:#} — backing off for {}s", - NOOP_COOLDOWN.as_secs() - ); - } + Ok(true) => info!("mesh self-heal: refreshed .msh and restarted the agent (NodeID preserved)"), + Ok(false) => debug!("mesh self-heal: .msh already current — likely a server-side issue, no action taken"), + Err(e) => error!("mesh self-heal failed: {e:#}"), } - // Reset the stuck streak after each attempt; a real heal is cleared by the CoreOk marker. stuck_since = None; + last_heal_attempt = Some(Instant::now()); } } - /// Fetch the current .msh; if its MeshID differs from the agent's, rewrite it and bounce. Ok(true) only when applied. async fn try_heal(&self) -> Result { let host = self.initial_config.get_server_url()?; let url = format!("https://{host}/tools/agent/meshcentral-server/generate-msh?host={host}"); @@ -176,35 +159,54 @@ impl MeshSelfHealService { return Err(anyhow!("/generate-msh returned HTTP {}", resp.status())); } let body = resp.text().await?; - let new_id = - parse_mesh_id(&body).ok_or_else(|| anyhow!("no MeshID in /generate-msh response"))?; + let new_mesh = parse_msh_field(&body, "MeshID"); + let new_server = parse_msh_field(&body, "ServerID"); + if new_mesh.is_none() && new_server.is_none() { + return Err(anyhow!( + "/generate-msh response has neither MeshID nor ServerID" + )); + } let msh_path = self.mesh_msh_path().await?; - let current_msh = tokio::fs::read_to_string(&msh_path).await.ok(); - let current_id = current_msh.as_deref().and_then(parse_mesh_id); - - if current_id.as_deref() == Some(new_id.as_str()) { - // No MeshID drift to fix. Log the target the agent is dialing so a server/gateway-side outage is distinguishable from a stale .msh target. - let server = current_msh + let current = tokio::fs::read_to_string(&msh_path).await.ok(); + let cur_mesh = current + .as_deref() + .and_then(|s| parse_msh_field(s, "MeshID")); + let cur_server = current + .as_deref() + .and_then(|s| parse_msh_field(s, "ServerID")); + + let mesh_changed = new_mesh.is_some() && cur_mesh != new_mesh; + let server_changed = new_server.is_some() && cur_server != new_server; + if !mesh_changed && !server_changed { + let server = current .as_deref() .and_then(|s| parse_msh_field(s, "MeshServer")) .unwrap_or_else(|| "".to_string()); - info!("mesh self-heal: MeshID unchanged ({new_id}); agent .msh MeshServer={server}"); + info!("mesh self-heal: .msh already current (MeshServer={server}) — no action"); return Ok(false); } info!( - "mesh self-heal: MeshID change {} -> {} (writing {})", - current_id.as_deref().unwrap_or(""), - new_id, - msh_path.display() + "mesh self-heal: rewriting {} (mesh_changed={}, serverid {} -> {})", + msh_path.display(), + mesh_changed, + if cur_server.is_some() { + "present" + } else { + "missing" + }, + if new_server.is_some() { + "present" + } else { + "missing" + } ); let tmp_path = msh_path.with_extension("msh.tmp"); tokio::fs::write(&tmp_path, body.as_bytes()).await?; tokio::fs::rename(&tmp_path, &msh_path).await?; - // Restart the service to re-import the .msh: kill, then start (redundant start is a no-op under mac KeepAlive). self.tool_kill.stop_tool(MESH_TOOL_ID).await?; if let Some(service_name) = self.mesh_service_name().await? { if let Err(e) = system_service::start_service(&service_name).await { @@ -216,7 +218,17 @@ impl MeshSelfHealService { Ok(true) } - /// Find the .msh the client saved in the tool dir (agent.msh on Windows, meshagent.msh on macOS). + async fn current_msh_missing_serverid(&self) -> bool { + let msh_path = match self.mesh_msh_path().await { + Ok(p) => p, + Err(_) => return false, + }; + match tokio::fs::read_to_string(&msh_path).await { + Ok(s) => parse_msh_field(&s, "ServerID").is_none(), + Err(_) => false, + } + } + async fn mesh_msh_path(&self) -> Result { self.installed_tools .get_by_tool_agent_id(MESH_TOOL_ID) @@ -233,7 +245,6 @@ impl MeshSelfHealService { Err(anyhow!("no .msh found in {}", dir.display())) } - /// The service name (launchd/SCM/systemd) if the agent installs as a service. async fn mesh_service_name(&self) -> Result> { let tool = self .installed_tools @@ -247,15 +258,6 @@ impl MeshSelfHealService { } } -/// Extract the value of the `MeshID=` line from an `.msh` body. -fn parse_mesh_id(msh: &str) -> Option { - msh.lines() - .find_map(|l| l.trim().strip_prefix("MeshID=")) - .map(|v| v.trim().to_string()) - .filter(|v| !v.is_empty()) -} - -/// Extract the value of a `Key=` line from an `.msh` body. fn parse_msh_field(msh: &str, key: &str) -> Option { let prefix = format!("{key}="); msh.lines() @@ -267,7 +269,6 @@ fn parse_msh_field(msh: &str, key: &str) -> Option { .filter(|v| !v.is_empty()) } -/// Read whole new lines since *offset, advancing only to the last newline; resets offset if the file shrank. async fn read_new_lines(path: &Path, offset: &mut u64) -> Result> { use tokio::io::{AsyncReadExt, AsyncSeekExt, SeekFrom}; @@ -286,7 +287,7 @@ async fn read_new_lines(path: &Path, offset: &mut u64) -> Result> { let consume = match buf.iter().rposition(|&b| b == b'\n') { Some(i) => i + 1, - None => return Ok(Vec::new()), // no complete line yet + None => return Ok(Vec::new()), }; *offset += consume as u64; diff --git a/clients/openframe-client/src/services/tool_connection_processing_manager.rs b/clients/openframe-client/src/services/tool_connection_processing_manager.rs index 3e0c66f78..51ed04fa0 100644 --- a/clients/openframe-client/src/services/tool_connection_processing_manager.rs +++ b/clients/openframe-client/src/services/tool_connection_processing_manager.rs @@ -17,6 +17,12 @@ use crate::services::tool_connection_service::ToolConnectionService; use crate::services::tool_run_manager::ToolRunManager; const RETRY_DELAY_SECONDS: u64 = 15; +/// Consecutive agentId-resolution failures tolerated at the normal cadence before the tool +/// connection is treated as degraded and the retry loop backs off (e.g. a hung `-nodeid-base64`). +const AGENT_ID_MAX_FAST_RETRIES: u32 = 5; +/// Back-off delay between agentId attempts once resolution is degraded, so a persistently +/// unhealthy agent can't spin a tight 15s loop forever while staying invisible. +const AGENT_ID_DEGRADED_BACKOFF_SECONDS: u64 = 300; // TODO: refactor class #[derive(Clone)] @@ -142,6 +148,9 @@ impl ToolConnectionProcessingManager { let tool_run_manager = self.tool_run_manager.clone(); tokio::spawn(async move { + // Counts consecutive agentId-resolution failures so a hung agent backs off + // (and is reported as degraded) instead of spinning a tight retry loop forever. + let mut agent_id_failures: u32 = 0; loop { while tool_run_manager.is_updating(&tool.tool_agent_id).await { info!(tool_id = %tool.tool_id, "Tool is being updated, deferring node-id resolution..."); @@ -166,7 +175,7 @@ impl ToolConnectionProcessingManager { "Failed to resolve tool {} agent_tool_id_command args: {:#}", tool.tool_id, e ); - sleep(Duration::from_secs(RETRY_DELAY_SECONDS)).await; + backoff_agent_id_failure(&tool.tool_id, &mut agent_id_failures).await; continue; } }; @@ -202,13 +211,13 @@ impl ToolConnectionProcessingManager { // Command returned an error before timeout Ok(Err(e)) => { error!("Failed to execute agentId command: {:#} – retrying", e); - sleep(Duration::from_secs(RETRY_DELAY_SECONDS)).await; + backoff_agent_id_failure(&tool.tool_id, &mut agent_id_failures).await; continue; } // Timeout expired Err(_) => { error!("agentId command timed out after 15 seconds – retrying"); - sleep(Duration::from_secs(RETRY_DELAY_SECONDS)).await; + backoff_agent_id_failure(&tool.tool_id, &mut agent_id_failures).await; continue; } }; @@ -222,14 +231,14 @@ impl ToolConnectionProcessingManager { // Parse agent_tool_id from command output if !stdout.is_empty() { // TODO: add mechanism to verify that it's correct agent id + agent_id_failures = 0; stdout // Use the command output as agent_tool_id } else { info!( tool_id = %tool.tool_id, - "agentId command returned empty output - retrying in {} seconds", - RETRY_DELAY_SECONDS + "agentId command returned empty output - retrying" ); - sleep(Duration::from_secs(RETRY_DELAY_SECONDS)).await; + backoff_agent_id_failure(&tool.tool_id, &mut agent_id_failures).await; continue; } } else { @@ -238,12 +247,11 @@ impl ToolConnectionProcessingManager { error!( tool_id = %tool.tool_id, exit_status = %output.status, - "agentId command failed - stdout: {} stderr: {}. Retrying in {} seconds", + "agentId command failed - stdout: {} stderr: {}. Retrying", stdout, - stderr, - RETRY_DELAY_SECONDS + stderr ); - sleep(Duration::from_secs(RETRY_DELAY_SECONDS)).await; + backoff_agent_id_failure(&tool.tool_id, &mut agent_id_failures).await; continue; } }; @@ -290,3 +298,29 @@ impl ToolConnectionProcessingManager { Ok(()) } } + +/// Sleep between agentId-resolution attempts, escalating to a longer back-off once failures +/// are sustained. A persistently failing `-nodeid-base64` (hung agent / missing node identity) +/// would otherwise spin a tight 15s loop indefinitely and stay invisible; after +/// `AGENT_ID_MAX_FAST_RETRIES` it logs a one-time degraded error and slows to +/// `AGENT_ID_DEGRADED_BACKOFF_SECONDS`, while still retrying so it self-recovers if the agent +/// becomes healthy (e.g. after a server-side reinstall). +async fn backoff_agent_id_failure(tool_id: &str, failures: &mut u32) { + *failures += 1; + if *failures == AGENT_ID_MAX_FAST_RETRIES + 1 { + error!( + tool_id = %tool_id, + consecutive_failures = *failures, + "agentId resolution is failing repeatedly — tool connection is DEGRADED (agent likely \ + unhealthy: hung -nodeid-base64 or missing node identity). Backing off to {}s; will keep \ + retrying. A server-side reinstall may be required to recover.", + AGENT_ID_DEGRADED_BACKOFF_SECONDS + ); + } + let delay = if *failures > AGENT_ID_MAX_FAST_RETRIES { + AGENT_ID_DEGRADED_BACKOFF_SECONDS + } else { + RETRY_DELAY_SECONDS + }; + sleep(Duration::from_secs(delay)).await; +} diff --git a/clients/openframe-client/src/services/tool_installation_service.rs b/clients/openframe-client/src/services/tool_installation_service.rs index 7c43770db..7a076b1a4 100644 --- a/clients/openframe-client/src/services/tool_installation_service.rs +++ b/clients/openframe-client/src/services/tool_installation_service.rs @@ -34,6 +34,16 @@ use tracing::{debug, info, warn}; /// terminated when the timeout fires. const TOOL_COMMAND_TIMEOUT_SECS: u64 = 300; +/// TEMP (remove with the agent.db backup/restore): deletes the mesh reinstall backup on every exit path. +struct ReinstallBackupGuard(Option); +impl Drop for ReinstallBackupGuard { + fn drop(&mut self) { + if let Some(path) = self.0.take() { + let _ = std::fs::remove_file(path); + } + } +} + #[derive(Clone)] pub struct ToolInstallationService { github_download_service: GithubDownloadService, @@ -113,10 +123,37 @@ impl ToolInstallationService { let effective_version = tool_installation_message.effective_version().to_string(); let run_args_clone = tool_installation_message.run_command_args.clone(); let reinstall = tool_installation_message.reinstall; + let mut reinstall_dir_cleared = false; // Create tool-specific directory let base_folder_path = self.directory_manager.app_support_dir(); let tool_folder_path = base_folder_path.join(tool_agent_id); + // TEMP (remove when the agent persists its identity across a db wipe): back up the mesh + // agent.db outside the tool dir so it survives the reinstall wipe; restored after install to + // preserve the NodeID. The guard deletes the backup on every exit path. + let mesh_db_backup = ReinstallBackupGuard( + if reinstall && tool_agent_id == "meshcentral-agent" { + let db = tool_folder_path.join("agent.db"); + let backup = base_folder_path.join("meshcentral-agent.db.reinstall-backup"); + if db.exists() { + match fs::copy(&db, &backup).await { + Ok(_) => { + info!("TEMP: backed up mesh agent.db to {} to preserve NodeID across reinstall", backup.display()); + Some(backup) + } + Err(e) => { + warn!("TEMP: failed to back up mesh agent.db: {:#}; NodeID may rotate on reinstall", e); + None + } + } + } else { + None + } + } else { + None + }, + ); + // Check if tool is already installed if let Some(installed_tool) = self .installed_tools_service @@ -144,7 +181,7 @@ impl ToolInstallationService { info!("Stopping existing tool process for {}", tool_agent_id); if let Err(e) = self .tool_kill_service - .stop_installed_tool(&installed_tool) + .stop_installed_tool(&installed_tool, true) .await { warn!("Failed to stop tool process: {:#}", e); @@ -262,6 +299,7 @@ impl ToolInstallationService { tool_folder_path.display() ) })?; + reinstall_dir_cleared = true; tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; @@ -391,7 +429,7 @@ impl ToolInstallationService { if let Some(stop_installation) = &stop_installation { if let Err(e) = self .tool_kill_service - .stop_for_installation(tool_agent_id, stop_installation) + .stop_for_installation(tool_agent_id, stop_installation, true) .await { warn!("Failed to stop leftover holder before download: {:#}", e); @@ -407,6 +445,48 @@ impl ToolInstallationService { } tokio::time::sleep(tokio::time::Duration::from_secs(2)).await; + // Don't install on top of a service we couldn't clear: if the existing service is still + // active (e.g. a wedged StopPending that even delete couldn't remove, or a live process + // we couldn't kill), abort so the record stays `Installing` and the install is retried, + // rather than overwriting/registering over a live agent. + if let Some(Installation::Service { + service_name: svc, .. + }) = &stop_installation + { + if !crate::platform::system_service::service_clear_for_install(svc).await { + return Err(anyhow::anyhow!( + "Aborting reinstall of {}: existing service '{}' is still active and could not be cleared; will retry", + tool_agent_id, svc + )); + } + } + + // Reinstall with no registry record: cleanup above was skipped, so wipe the dir now (holder + // is stopped and the service confirmed clear) to drop stale on-disk state. + if reinstall && !reinstall_dir_cleared && tool_folder_path.exists() { + info!( + "Reinstall without registry record: removing stale tool directory {}", + tool_folder_path.display() + ); + crate::platform::remove_directory_with_retry(&tool_folder_path, 5) + .await + .with_context(|| { + format!( + "Failed to remove existing tool directory: {}", + tool_folder_path.display() + ) + })?; + fs::create_dir_all(&tool_folder_path) + .await + .with_context(|| { + format!( + "Failed to recreate tool directory: {}", + tool_folder_path.display() + ) + })?; + reinstall_dir_cleared = true; + } + // Download and install the tool let (executable_path, installation_type, bundle_id, config_service_name) = match resolved_config { @@ -472,8 +552,10 @@ impl ToolInstallationService { let asset_original_version = asset.original_version(); let asset_effective_version = asset.effective_version(); - // Download and save asset if it doesn't already exist - if !asset_path.exists() { + // On reinstall, always refresh server-generated config assets (e.g. the mesh .msh). + let refresh_config_asset = + reinstall && matches!(asset.source, AssetSource::ToolApi); + if !asset_path.exists() || refresh_config_asset { if is_executable { if let Err(e) = self .tool_kill_service @@ -700,6 +782,57 @@ impl ToolInstallationService { ); } + // For Service tools, confirm the service actually came up before recording it as + // Installed. A `-install` that exits 0 but leaves the service stopped/wedged would + // otherwise be marked healthy; failing here keeps the record `Installing` for retry. + if let Installation::Service { + service_name: svc, .. + } = &installation + { + crate::platform::system_service::verify_service_running(svc) + .await + .with_context(|| { + format!( + "Post-install service verification failed for {}", + tool_agent_id + ) + })?; + info!( + "Verified service {} is running after install of {}", + svc, tool_agent_id + ); + } + + // TEMP (remove when the agent persists its identity across a db wipe): restore the preserved + // agent.db so the agent keeps its previous NodeID; the fresh .msh is re-imported on restart. + if let Some(backup) = mesh_db_backup.0.as_ref() { + if let Installation::Service { + service_name: svc, .. + } = &installation + { + let db_path = tool_folder_path.join("agent.db"); + info!("TEMP: restoring preserved mesh agent.db to keep NodeID across reinstall"); + if let Err(e) = crate::platform::system_service::stop_service(svc, false).await { + warn!( + "TEMP: failed to stop {} before agent.db restore: {:#}", + svc, e + ); + } + tokio::time::sleep(tokio::time::Duration::from_secs(2)).await; + match fs::copy(backup, &db_path).await { + Ok(_) => info!("TEMP: restored mesh agent.db from {}", backup.display()), + Err(e) => warn!( + "TEMP: failed to restore mesh agent.db from {}: {:#}; NodeID will rotate", + backup.display(), + e + ), + } + crate::platform::system_service::start_service(svc) + .await + .with_context(|| format!("Failed to restart {} after agent.db restore", svc))?; + } + } + // Persist installed tool information let installed_tool = InstalledTool { tool_agent_id: tool_agent_id.clone(), diff --git a/clients/openframe-client/src/services/tool_kill_service.rs b/clients/openframe-client/src/services/tool_kill_service.rs index fe1191d4e..13aa46dc2 100644 --- a/clients/openframe-client/src/services/tool_kill_service.rs +++ b/clients/openframe-client/src/services/tool_kill_service.rs @@ -270,8 +270,12 @@ impl ToolKillService { .await } - pub async fn stop_installed_tool(&self, tool: &InstalledTool) -> Result<()> { - self.stop_for_installation(&tool.tool_agent_id, &tool.installation) + pub async fn stop_installed_tool( + &self, + tool: &InstalledTool, + allow_delete: bool, + ) -> Result<()> { + self.stop_for_installation(&tool.tool_agent_id, &tool.installation, allow_delete) .await } @@ -279,6 +283,7 @@ impl ToolKillService { &self, tool_agent_id: &str, installation: &Installation, + allow_delete: bool, ) -> Result<()> { match installation { Installation::GuiApp { @@ -299,7 +304,7 @@ impl ToolKillService { } => { info!(service_name = %service_name, "Stopping Service type tool via system service manager"); - if let Err(e) = system_service::stop_service(service_name).await { + if let Err(e) = system_service::stop_service(service_name, allow_delete).await { warn!( "Failed to stop service {} (continuing with process kill by path): {:#}", service_name, e diff --git a/clients/openframe-client/src/services/tool_uninstall_service.rs b/clients/openframe-client/src/services/tool_uninstall_service.rs index 654f2f48e..704919547 100644 --- a/clients/openframe-client/src/services/tool_uninstall_service.rs +++ b/clients/openframe-client/src/services/tool_uninstall_service.rs @@ -185,7 +185,7 @@ impl ToolUninstallService { } async fn stop_tool_process(&self, tool: &InstalledTool) -> Result<()> { - self.tool_kill_service.stop_installed_tool(tool).await + self.tool_kill_service.stop_installed_tool(tool, true).await } async fn cleanup_tool_processes(&self, tool: &InstalledTool) { From 96bd72d765a61969b84b515b78664375d9674f51 Mon Sep 17 00:00:00 2001 From: Danylo Date: Thu, 25 Jun 2026 13:25:06 +0300 Subject: [PATCH 04/10] =?UTF-8?q?fix(client):=20cleaned=20up=20the=20force?= =?UTF-8?q?=20resinstall=20functionality=20as=20we=20don'=E2=80=A6=20(#199?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/services/tool_installation_service.rs | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/clients/openframe-client/src/services/tool_installation_service.rs b/clients/openframe-client/src/services/tool_installation_service.rs index 7a076b1a4..984b6b58c 100644 --- a/clients/openframe-client/src/services/tool_installation_service.rs +++ b/clients/openframe-client/src/services/tool_installation_service.rs @@ -34,16 +34,6 @@ use tracing::{debug, info, warn}; /// terminated when the timeout fires. const TOOL_COMMAND_TIMEOUT_SECS: u64 = 300; -/// TEMP (remove with the agent.db backup/restore): deletes the mesh reinstall backup on every exit path. -struct ReinstallBackupGuard(Option); -impl Drop for ReinstallBackupGuard { - fn drop(&mut self) { - if let Some(path) = self.0.take() { - let _ = std::fs::remove_file(path); - } - } -} - #[derive(Clone)] pub struct ToolInstallationService { github_download_service: GithubDownloadService, @@ -128,32 +118,6 @@ impl ToolInstallationService { let base_folder_path = self.directory_manager.app_support_dir(); let tool_folder_path = base_folder_path.join(tool_agent_id); - // TEMP (remove when the agent persists its identity across a db wipe): back up the mesh - // agent.db outside the tool dir so it survives the reinstall wipe; restored after install to - // preserve the NodeID. The guard deletes the backup on every exit path. - let mesh_db_backup = ReinstallBackupGuard( - if reinstall && tool_agent_id == "meshcentral-agent" { - let db = tool_folder_path.join("agent.db"); - let backup = base_folder_path.join("meshcentral-agent.db.reinstall-backup"); - if db.exists() { - match fs::copy(&db, &backup).await { - Ok(_) => { - info!("TEMP: backed up mesh agent.db to {} to preserve NodeID across reinstall", backup.display()); - Some(backup) - } - Err(e) => { - warn!("TEMP: failed to back up mesh agent.db: {:#}; NodeID may rotate on reinstall", e); - None - } - } - } else { - None - } - } else { - None - }, - ); - // Check if tool is already installed if let Some(installed_tool) = self .installed_tools_service @@ -803,36 +767,6 @@ impl ToolInstallationService { ); } - // TEMP (remove when the agent persists its identity across a db wipe): restore the preserved - // agent.db so the agent keeps its previous NodeID; the fresh .msh is re-imported on restart. - if let Some(backup) = mesh_db_backup.0.as_ref() { - if let Installation::Service { - service_name: svc, .. - } = &installation - { - let db_path = tool_folder_path.join("agent.db"); - info!("TEMP: restoring preserved mesh agent.db to keep NodeID across reinstall"); - if let Err(e) = crate::platform::system_service::stop_service(svc, false).await { - warn!( - "TEMP: failed to stop {} before agent.db restore: {:#}", - svc, e - ); - } - tokio::time::sleep(tokio::time::Duration::from_secs(2)).await; - match fs::copy(backup, &db_path).await { - Ok(_) => info!("TEMP: restored mesh agent.db from {}", backup.display()), - Err(e) => warn!( - "TEMP: failed to restore mesh agent.db from {}: {:#}; NodeID will rotate", - backup.display(), - e - ), - } - crate::platform::system_service::start_service(svc) - .await - .with_context(|| format!("Failed to restart {} after agent.db restore", svc))?; - } - } - // Persist installed tool information let installed_tool = InstalledTool { tool_agent_id: tool_agent_id.clone(), From 82e13fa936676df83274172197462041d5d6635f Mon Sep 17 00:00:00 2001 From: denys-gif Date: Thu, 25 Jun 2026 15:38:19 +0100 Subject: [PATCH 05/10] chore: reduce logs --- clients/openframe-client/src/logging/log_source.rs | 4 ++-- clients/openframe-client/src/logging/mod.rs | 4 ++-- clients/openframe-client/src/logging/nats_streaming.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clients/openframe-client/src/logging/log_source.rs b/clients/openframe-client/src/logging/log_source.rs index 83db3a328..91be9fe7f 100644 --- a/clients/openframe-client/src/logging/log_source.rs +++ b/clients/openframe-client/src/logging/log_source.rs @@ -5,7 +5,7 @@ use std::io::{BufRead, BufReader, Seek, SeekFrom}; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; -use tracing::{error, info}; +use tracing::{debug, error, info}; use super::log_parser::{parse_log_line, LogDeduplicator, LogEntry}; @@ -160,7 +160,7 @@ impl LogSourceRegistry { if count == 0 { active[i] = false; } else { - info!("Read {} logs from '{}'", count, source.name()); + debug!("Read {} logs from '{}'", count, source.name()); remaining = remaining.saturating_sub(count); all_logs.extend(entries); diff --git a/clients/openframe-client/src/logging/mod.rs b/clients/openframe-client/src/logging/mod.rs index 0fc713259..58036d646 100644 --- a/clients/openframe-client/src/logging/mod.rs +++ b/clients/openframe-client/src/logging/mod.rs @@ -296,8 +296,8 @@ fn init_inner( let _ = METRICS_STORE.set(Arc::clone(&metrics_store)); // Set up the full tracing subscriber - let env_filter = - EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info")); + let env_filter = EnvFilter::try_from_default_env() + .unwrap_or_else(|_| EnvFilter::new("info,async_nats=warn")); // Decide output format: default to text (one-liners). Set OPENFRAME_LOG_FORMAT=json to use JSON let format = std::env::var("OPENFRAME_LOG_FORMAT").unwrap_or_else(|_| "text".into()); diff --git a/clients/openframe-client/src/logging/nats_streaming.rs b/clients/openframe-client/src/logging/nats_streaming.rs index 27f361e99..326776fd1 100644 --- a/clients/openframe-client/src/logging/nats_streaming.rs +++ b/clients/openframe-client/src/logging/nats_streaming.rs @@ -101,7 +101,7 @@ impl NatsLogConnection { .await .context("Failed to receive publish acknowledgment")?; - info!( + debug!( "Published {} logs to NATS (ack received)", payload.logs.len() ); From 2add28f60e967b7c5593a4f07bb89573ec5a7be1 Mon Sep 17 00:00:00 2001 From: Danylo Date: Tue, 30 Jun 2026 22:08:13 +0300 Subject: [PATCH 06/10] Fix Fleet/tool force-reinstall (Windows + macOS/Linux Orbit cleanup) (#2030) Co-authored-by: Danylo Babenko --- .../src/platform/uninstall.rs | 15 +++++++--- .../src/services/tool_installation_service.rs | 28 +++++++++++++------ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/clients/openframe-client/src/platform/uninstall.rs b/clients/openframe-client/src/platform/uninstall.rs index fa467bee9..4ae76cd54 100644 --- a/clients/openframe-client/src/platform/uninstall.rs +++ b/clients/openframe-client/src/platform/uninstall.rs @@ -14,10 +14,17 @@ const DISPLAY_NAME: &str = "OpenFrame Client Service"; const DESCRIPTION: &str = "OpenFrame client service for remote management and monitoring"; pub fn orbit_dir() -> std::path::PathBuf { - std::path::PathBuf::from( - std::env::var("ProgramFiles").unwrap_or_else(|_| "C:\\Program Files".to_string()), - ) - .join("Orbit") + #[cfg(target_os = "windows")] + { + std::path::PathBuf::from( + std::env::var("ProgramFiles").unwrap_or_else(|_| "C:\\Program Files".to_string()), + ) + .join("Orbit") + } + #[cfg(any(target_os = "macos", target_os = "linux"))] + { + std::path::PathBuf::from("/opt/orbit") + } } /// Remove a directory with retry logic for locked files diff --git a/clients/openframe-client/src/services/tool_installation_service.rs b/clients/openframe-client/src/services/tool_installation_service.rs index 984b6b58c..6df88fa1a 100644 --- a/clients/openframe-client/src/services/tool_installation_service.rs +++ b/clients/openframe-client/src/services/tool_installation_service.rs @@ -236,13 +236,12 @@ impl ToolInstallationService { tokio::time::sleep(tokio::time::Duration::from_secs(2)).await; } - let installed_agent_path = self.directory_manager.get_tool_executable_path( - tool_agent_id, - installed_tool.installation.executable_path(), - ); + // Match the tool folder path (not just agent.exe) so child processes like the + // orbit-spawned osqueryd are killed too; on Windows their locked binaries would + // otherwise block the directory removal below with "Access is denied". if let Err(e) = self .tool_kill_service - .stop_tool_by_path(&installed_agent_path.to_string_lossy()) + .stop_tool_by_path(&tool_folder_path.to_string_lossy()) .await { warn!( @@ -275,13 +274,13 @@ impl ToolInstallationService { warn!("Failed to remove tool connection: {:#}", e); } - // Clear from both manager tracking sets to allow tool restart after reinstall self.tool_connection_processing_manager .clear_running_tool(&installed_tool.tool_id) .await; - self.tool_run_manager - .clear_running_tool(&installed_tool.tool_agent_id) - .await; + // Do NOT clear tool_run_manager's tracking entry: the existing supervisor loop + // resumes with the new binary on its own. Clearing it makes the post-install + // run_new_tool spawn a second supervisor, causing two osqueryd to fight over the + // osquery.db lock (permanent crash loop). info!( "Previous installation of tool {} was uninstalled", @@ -328,6 +327,17 @@ impl ToolInstallationService { } let orbit_dir = crate::platform::orbit_dir(); if orbit_dir.exists() { + if let Err(e) = self + .tool_kill_service + .stop_tool_by_path(&orbit_dir.to_string_lossy()) + .await + { + warn!( + "Failed to stop processes under Orbit directory {}: {:#}", + orbit_dir.display(), + e + ); + } info!("Removing leftover Orbit directory: {}", orbit_dir.display()); if let Err(e) = crate::platform::remove_directory_with_retry(&orbit_dir, 5).await { warn!( From aa9e2be28ae2c8ee114ce9a11e5951abb6df4967 Mon Sep 17 00:00:00 2001 From: denys-gif Date: Thu, 2 Jul 2026 17:05:21 +0100 Subject: [PATCH 07/10] Feat/tool remote uninstall --- .../src/config/update_config.rs | 1 + clients/openframe-client/src/lib.rs | 23 ++ clients/openframe-client/src/listener/mod.rs | 2 + .../tool_uninstall_message_listener.rs | 290 ++++++++++++++++++ clients/openframe-client/src/models/mod.rs | 2 + .../src/models/tool_uninstall_message.rs | 22 ++ clients/openframe-client/src/services/mod.rs | 4 +- .../src/services/tool_installation_service.rs | 2 + .../src/services/tool_run_manager.rs | 16 +- .../tool_uninstall_result_publisher.rs | 36 +++ .../src/services/tool_uninstall_service.rs | 40 +++ 11 files changed, 436 insertions(+), 2 deletions(-) create mode 100644 clients/openframe-client/src/listener/tool_uninstall_message_listener.rs create mode 100644 clients/openframe-client/src/models/tool_uninstall_message.rs create mode 100644 clients/openframe-client/src/services/tool_uninstall_result_publisher.rs diff --git a/clients/openframe-client/src/config/update_config.rs b/clients/openframe-client/src/config/update_config.rs index 56c45dd57..f1dcc1657 100644 --- a/clients/openframe-client/src/config/update_config.rs +++ b/clients/openframe-client/src/config/update_config.rs @@ -19,3 +19,4 @@ pub const RECONNECTION_DELAY_MS: u64 = 5000; // 5 seconds // NATS message settings pub const CONSUMER_ACK_WAIT_SECS: u64 = 120; pub const CONSUMER_MAX_DELIVER: i64 = 10; // Maximum delivery attempts +pub const UNINSTALL_CONSUMER_MAX_DELIVER: i64 = 20; // Larger budget: uninstall may defer behind a long install holding the tool lock diff --git a/clients/openframe-client/src/lib.rs b/clients/openframe-client/src/lib.rs index 78ae619ab..46326b201 100644 --- a/clients/openframe-client/src/lib.rs +++ b/clients/openframe-client/src/lib.rs @@ -43,6 +43,7 @@ use crate::listener::execution_listener::ExecutionListener; use crate::listener::openframe_client_update_listener::OpenFrameClientUpdateListener; use crate::listener::tool_agent_update_listener::ToolAgentUpdateListener; use crate::listener::tool_installation_message_listener::ToolInstallationMessageListener; +use crate::listener::tool_uninstall_message_listener::ToolUninstallMessageListener; use crate::logging::nats_streaming::LogStreamingRunManager; use crate::models::{CommandMessage, ScriptMessage}; use crate::platform::DirectoryManager; @@ -68,6 +69,8 @@ use crate::services::tool_agent_update_service::ToolAgentUpdateService; use crate::services::tool_connection_message_publisher::ToolConnectionMessagePublisher; use crate::services::tool_connection_service::ToolConnectionService; use crate::services::tool_installation_service::ToolInstallationService; +use crate::services::tool_uninstall_result_publisher::ToolUninstallResultPublisher; +use crate::services::tool_uninstall_service::ToolUninstallService; use crate::services::InstalledToolsService; use crate::services::{ AgentAuthService, AgentRegistrationService, InitialConfigurationService, @@ -144,6 +147,7 @@ pub struct Client { auth_processor: InitialAuthenticationProcessor, nats_connection_manager: NatsConnectionManager, tool_installation_message_listener: ToolInstallationMessageListener, + tool_uninstall_message_listener: ToolUninstallMessageListener, openframe_client_update_listener: OpenFrameClientUpdateListener, tool_agent_update_listener: ToolAgentUpdateListener, command_execution_listener: ExecutionListener, @@ -397,6 +401,22 @@ impl Client { config_service.clone(), ); + let tool_uninstall_service = ToolUninstallService::new( + installed_tools_service.clone(), + tool_command_params_resolver.clone(), + tool_kill_service.clone(), + directory_manager.clone(), + ); + let tool_uninstall_result_publisher = + ToolUninstallResultPublisher::new(nats_message_publisher.clone()); + let tool_uninstall_message_listener = ToolUninstallMessageListener::new( + nats_connection_manager.clone(), + tool_run_manager.clone(), + tool_uninstall_service, + tool_uninstall_result_publisher, + config_service.clone(), + ); + // Initialize OpenFrame client update listener let openframe_client_update_listener = OpenFrameClientUpdateListener::new( nats_connection_manager.clone(), @@ -447,6 +467,7 @@ impl Client { auth_processor, nats_connection_manager, tool_installation_message_listener, + tool_uninstall_message_listener, openframe_client_update_listener, tool_agent_update_listener, command_execution_listener, @@ -496,6 +517,8 @@ impl Client { //Start tool installation message listener in background self.tool_installation_message_listener.start().await?; + self.tool_uninstall_message_listener.start().await?; + // Start OpenFrame client update listener in background self.openframe_client_update_listener.start().await?; diff --git a/clients/openframe-client/src/listener/mod.rs b/clients/openframe-client/src/listener/mod.rs index cdcbe50da..f6ad2c663 100644 --- a/clients/openframe-client/src/listener/mod.rs +++ b/clients/openframe-client/src/listener/mod.rs @@ -2,8 +2,10 @@ pub mod execution_listener; pub mod openframe_client_update_listener; pub mod tool_agent_update_listener; pub mod tool_installation_message_listener; +pub mod tool_uninstall_message_listener; pub use execution_listener::ExecutionListener; pub use openframe_client_update_listener::OpenFrameClientUpdateListener; pub use tool_agent_update_listener::ToolAgentUpdateListener; pub use tool_installation_message_listener::ToolInstallationMessageListener; +pub use tool_uninstall_message_listener::ToolUninstallMessageListener; diff --git a/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs b/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs new file mode 100644 index 000000000..be60f7b99 --- /dev/null +++ b/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs @@ -0,0 +1,290 @@ +use crate::config::update_config::{ + CONSUMER_ACK_WAIT_SECS, CONSUMER_CYCLE_PAUSE_MS, CONSUMER_RETRY_ATTEMPTS_PER_CYCLE, + INITIAL_RETRY_DELAY_MS, MAX_RETRY_DELAY_MS, RECONNECTION_DELAY_MS, + UNINSTALL_CONSUMER_MAX_DELIVER, +}; +use crate::models::{ToolUninstallMessage, UninstallStatus}; +use crate::services::nats_connection_manager::NatsConnectionManager; +use crate::services::tool_run_manager::ToolRunManager; +use crate::services::tool_uninstall_result_publisher::ToolUninstallResultPublisher; +use crate::services::tool_uninstall_service::ToolUninstallService; +use crate::services::tool_uninstall_service::UninstallOutcome; +use crate::services::AgentConfigurationService; +use anyhow::Result; +use async_nats::jetstream; +use async_nats::jetstream::consumer::push; +use async_nats::jetstream::consumer::PushConsumer; +use async_nats::jetstream::Message; +use futures::{FutureExt, StreamExt}; +use tokio::time::Duration; +use tracing::{error, info, warn}; + +#[derive(Clone)] +pub struct ToolUninstallMessageListener { + nats_connection_manager: NatsConnectionManager, + tool_run_manager: ToolRunManager, + tool_uninstall_service: ToolUninstallService, + result_publisher: ToolUninstallResultPublisher, + config_service: AgentConfigurationService, +} + +impl ToolUninstallMessageListener { + const STREAM_NAME: &'static str = "TOOL_INSTALLATION"; + + pub fn new( + nats_connection_manager: NatsConnectionManager, + tool_run_manager: ToolRunManager, + tool_uninstall_service: ToolUninstallService, + result_publisher: ToolUninstallResultPublisher, + config_service: AgentConfigurationService, + ) -> Self { + Self { + nats_connection_manager, + tool_run_manager, + tool_uninstall_service, + result_publisher, + config_service, + } + } + + pub async fn start(&self) -> Result> { + let listener = self.clone(); + let handle = tokio::spawn(async move { + loop { + info!("Starting tool uninstall message listener..."); + match listener.listen().await { + Ok(_) => { + warn!("Tool uninstall message listener exited normally (unexpected)"); + } + Err(e) => { + error!("Tool uninstall message listener error: {:#}", e); + } + } + + info!( + "Reconnecting tool uninstall message listener in {} seconds...", + RECONNECTION_DELAY_MS / 1000 + ); + tokio::time::sleep(Duration::from_millis(RECONNECTION_DELAY_MS)).await; + } + }); + Ok(handle) + } + + async fn listen(&self) -> Result<()> { + info!("Run tool uninstall message listener"); + let client = self.nats_connection_manager.get_client().await?; + let js = jetstream::new((*client).clone()); + + let machine_id = self.config_service.get_machine_id()?; + + let consumer = self.create_consumer(&js, &machine_id).await; + + info!("Start listening for tool uninstall messages"); + let mut messages = consumer.messages().await?; + + while let Some(msg_result) = messages.next().await { + let message = match msg_result { + Ok(msg) => msg, + Err(e) => { + error!("Failed to receive message: {:#}", e); + continue; + } + }; + + if let Err(e) = self.handle_message(message, &machine_id).await { + error!("Failed to handle message: {:#}", e); + } + } + + Ok(()) + } + + async fn handle_message(&self, message: Message, machine_id: &str) -> Result<()> { + let payload = String::from_utf8_lossy(&message.payload); + info!("Received tool uninstall message: {:?}", payload); + + let uninstall_message: ToolUninstallMessage = match serde_json::from_str(&payload) { + Ok(msg) => msg, + Err(e) => { + error!("Failed to parse tool uninstall message: {:#}", e); + if let Err(ack_err) = message.ack().await { + warn!("Failed to ack malformed message: {}", ack_err); + } + return Ok(()); + } + }; + + let tool_agent_id = uninstall_message.tool_agent_id.clone(); + + let tool_lock = self.tool_run_manager.tool_lock(&tool_agent_id).await; + let _guard = match tool_lock.try_lock() { + Ok(guard) => guard, + Err(_) => { + info!( + "Tool {} busy with another operation, deferring uninstall for redelivery", + tool_agent_id + ); + return Ok(()); + } + }; + + self.tool_run_manager.mark_updating(&tool_agent_id).await; + + let outcome = std::panic::AssertUnwindSafe( + self.tool_uninstall_service + .uninstall_by_tool_agent_id(&tool_agent_id), + ) + .catch_unwind() + .await; + + let (status, ack_message, remove_supervision) = match outcome { + Ok(Ok(UninstallOutcome::Removed)) => (UninstallStatus::Removed, true, true), + Ok(Ok(UninstallOutcome::NotInstalled)) => (UninstallStatus::NotInstalled, true, true), + Ok(Err(e)) => { + error!("Failed to uninstall tool {}: {:#}", tool_agent_id, e); + (UninstallStatus::Failed, false, false) + } + Err(_) => { + error!("Uninstall panicked for tool {}", tool_agent_id); + (UninstallStatus::Failed, false, false) + } + }; + + if remove_supervision { + self.tool_run_manager + .clear_running_tool(&tool_agent_id) + .await; + } + self.tool_run_manager.clear_updating(&tool_agent_id).await; + + if let Err(e) = self + .result_publisher + .publish(machine_id, &tool_agent_id, status) + .await + { + warn!( + "Failed to publish uninstall result for {}: {:#}", + tool_agent_id, e + ); + } + + if ack_message { + message + .ack() + .await + .map_err(|e| anyhow::anyhow!("Failed to ack message: {}", e))?; + info!("Uninstall message acknowledged for tool: {}", tool_agent_id); + } else { + info!( + "Leaving uninstall message unacked for potential redelivery: tool {}", + tool_agent_id + ); + } + + Ok(()) + } + + async fn create_consumer(&self, js: &jetstream::Context, machine_id: &str) -> PushConsumer { + let consumer_configuration = Self::build_consumer_configuration(machine_id); + let mut cycle = 0u32; + + loop { + cycle += 1; + let mut delay_ms = INITIAL_RETRY_DELAY_MS; + + for attempt in 1..=CONSUMER_RETRY_ATTEMPTS_PER_CYCLE { + info!( + "Creating uninstall consumer for stream {} (cycle {}, attempt {}/{})", + Self::STREAM_NAME, + cycle, + attempt, + CONSUMER_RETRY_ATTEMPTS_PER_CYCLE + ); + + match js + .create_consumer_on_stream(consumer_configuration.clone(), Self::STREAM_NAME) + .await + { + Ok(consumer) => { + info!( + "Uninstall consumer created for stream: {}", + Self::STREAM_NAME + ); + return consumer; + } + Err(e) => { + let error_msg = format!("{:?}", e); + if error_msg.contains("consumer name already in use") + || error_msg.contains("10013") + { + warn!("Uninstall consumer already exists, attempting to get existing consumer"); + let durable_name = Self::build_durable_name(machine_id); + if let Ok(existing_consumer) = js + .get_consumer_from_stream(Self::STREAM_NAME, &durable_name) + .await + { + info!( + "Retrieved existing uninstall consumer for stream: {}", + Self::STREAM_NAME + ); + return existing_consumer; + } + } + + if attempt < CONSUMER_RETRY_ATTEMPTS_PER_CYCLE { + warn!( + "Failed to create uninstall consumer (cycle {}, attempt {}/{}): {:#}. Retrying in {} ms...", + cycle, attempt, CONSUMER_RETRY_ATTEMPTS_PER_CYCLE, e, delay_ms + ); + tokio::time::sleep(Duration::from_millis(delay_ms)).await; + delay_ms = (delay_ms * 2).min(MAX_RETRY_DELAY_MS); + } else { + warn!( + "Failed to create uninstall consumer (cycle {}, attempt {}/{}): {:#}", + cycle, attempt, CONSUMER_RETRY_ATTEMPTS_PER_CYCLE, e + ); + } + } + } + } + + info!( + "All {} attempts in cycle {} failed. Pausing {} seconds before next cycle...", + CONSUMER_RETRY_ATTEMPTS_PER_CYCLE, + cycle, + CONSUMER_CYCLE_PAUSE_MS / 1000 + ); + tokio::time::sleep(Duration::from_millis(CONSUMER_CYCLE_PAUSE_MS)).await; + } + } + + fn build_consumer_configuration(machine_id: &str) -> push::Config { + let filter_subject = Self::build_filter_subject(machine_id); + let deliver_subject = Self::build_deliver_subject(machine_id); + let durable_name = Self::build_durable_name(machine_id); + + info!("Uninstall consumer configuration - filter subject: {}, deliver subject: {}, durable name: {}", filter_subject, deliver_subject, durable_name); + + push::Config { + filter_subject, + deliver_subject, + durable_name: Some(durable_name), + ack_wait: Duration::from_secs(CONSUMER_ACK_WAIT_SECS), + max_deliver: UNINSTALL_CONSUMER_MAX_DELIVER, + ..Default::default() + } + } + + fn build_filter_subject(machine_id: &str) -> String { + format!("machine.{}.tool-uninstall", machine_id) + } + + fn build_deliver_subject(machine_id: &str) -> String { + format!("machine.{}.tool-uninstall.inbox", machine_id) + } + + fn build_durable_name(machine_id: &str) -> String { + format!("machine_{}_tool-uninstall_consumer", machine_id) + } +} diff --git a/clients/openframe-client/src/models/mod.rs b/clients/openframe-client/src/models/mod.rs index 62c3c95bc..9d4b689d3 100644 --- a/clients/openframe-client/src/models/mod.rs +++ b/clients/openframe-client/src/models/mod.rs @@ -16,6 +16,7 @@ pub mod tool_connection; pub mod tool_connection_message; pub mod tool_installation_message; pub mod tool_installation_result; +pub mod tool_uninstall_message; pub mod tool_version_overrides; pub mod update_state; @@ -41,4 +42,5 @@ pub use tool_connection::ToolConnection; pub use tool_connection_message::ToolConnectionMessage; pub use tool_installation_message::ToolInstallationMessage; pub use tool_installation_result::ToolInstallationResult; +pub use tool_uninstall_message::{ToolUninstallMessage, ToolUninstallResult, UninstallStatus}; pub use update_state::{UpdatePhase, UpdateState}; diff --git a/clients/openframe-client/src/models/tool_uninstall_message.rs b/clients/openframe-client/src/models/tool_uninstall_message.rs new file mode 100644 index 000000000..cce6b48e0 --- /dev/null +++ b/clients/openframe-client/src/models/tool_uninstall_message.rs @@ -0,0 +1,22 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ToolUninstallMessage { + pub tool_agent_id: String, +} + +#[derive(Debug, Clone, Copy, Serialize)] +#[serde(rename_all = "SCREAMING_SNAKE_CASE")] +pub enum UninstallStatus { + Removed, + NotInstalled, + Failed, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ToolUninstallResult { + pub tool_agent_id: String, + pub status: UninstallStatus, +} diff --git a/clients/openframe-client/src/services/mod.rs b/clients/openframe-client/src/services/mod.rs index 17d0fcc47..4fa872207 100644 --- a/clients/openframe-client/src/services/mod.rs +++ b/clients/openframe-client/src/services/mod.rs @@ -28,6 +28,7 @@ pub mod tool_connection_service; pub mod tool_installation_service; pub mod tool_kill_service; pub mod tool_run_manager; +pub mod tool_uninstall_result_publisher; pub mod tool_uninstall_service; pub mod tool_url_params_resolver; pub mod update_cleanup_service; @@ -59,7 +60,8 @@ pub use tool_connection_service::ToolConnectionService; pub use tool_installation_service::ToolInstallationService; pub use tool_kill_service::ToolKillService; pub use tool_run_manager::ToolRunManager; -pub use tool_uninstall_service::ToolUninstallService; +pub use tool_uninstall_result_publisher::ToolUninstallResultPublisher; +pub use tool_uninstall_service::{ToolUninstallService, UninstallOutcome}; pub use tool_url_params_resolver::ToolUrlParamsResolver; pub use update_cleanup_service::UpdateCleanupService; pub use update_handler_service::UpdateHandlerService; diff --git a/clients/openframe-client/src/services/tool_installation_service.rs b/clients/openframe-client/src/services/tool_installation_service.rs index 6df88fa1a..8c77e4b13 100644 --- a/clients/openframe-client/src/services/tool_installation_service.rs +++ b/clients/openframe-client/src/services/tool_installation_service.rs @@ -93,6 +93,8 @@ impl ToolInstallationService { #[tracing::instrument(skip_all, fields(tool_id = %tool_installation_message.tool_agent_id))] pub async fn install(&self, tool_installation_message: ToolInstallationMessage) -> Result<()> { let tool_agent_id = tool_installation_message.tool_agent_id.clone(); + let tool_lock = self.tool_run_manager.tool_lock(&tool_agent_id).await; + let _guard = tool_lock.lock().await; self.tool_run_manager.mark_updating(&tool_agent_id).await; let result = self.install_inner(tool_installation_message).await; self.tool_run_manager.clear_updating(&tool_agent_id).await; diff --git a/clients/openframe-client/src/services/tool_run_manager.rs b/clients/openframe-client/src/services/tool_run_manager.rs index 755bfa786..0790e6912 100644 --- a/clients/openframe-client/src/services/tool_run_manager.rs +++ b/clients/openframe-client/src/services/tool_run_manager.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use std::time::Duration; use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; -use tokio::sync::RwLock; +use tokio::sync::{Mutex, RwLock}; use tokio::time::sleep; use tracing::{debug, error, info, warn}; @@ -398,6 +398,7 @@ pub struct ToolRunManager { tool_kill_service: ToolKillService, running_tools: Arc>>, updating_tools: Arc>>, + tool_locks: Arc>>>>, shutting_down: Arc, } @@ -413,10 +414,18 @@ impl ToolRunManager { tool_kill_service, running_tools: Arc::new(RwLock::new(HashSet::new())), updating_tools: Arc::new(RwLock::new(HashMap::new())), + tool_locks: Arc::new(RwLock::new(HashMap::new())), shutting_down: Arc::new(AtomicBool::new(false)), } } + pub async fn tool_lock(&self, tool_id: &str) -> Arc> { + let mut map = self.tool_locks.write().await; + map.entry(tool_id.to_string()) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .clone() + } + /// Signal all run loops to stop launching new processes. /// Called by self-update before the updater kills the service. pub fn signal_shutdown(&self) { @@ -605,6 +614,11 @@ impl ToolRunManager { } } + if !running_tools.read().await.contains(&tool.tool_agent_id) { + info!(tool_id = %tool.tool_agent_id, "Tool no longer supervised, stopping run loop"); + break; + } + let processed_args = match params_processor .process(&tool.tool_agent_id, tool.run_command_args.clone()) { diff --git a/clients/openframe-client/src/services/tool_uninstall_result_publisher.rs b/clients/openframe-client/src/services/tool_uninstall_result_publisher.rs new file mode 100644 index 000000000..b6779a47c --- /dev/null +++ b/clients/openframe-client/src/services/tool_uninstall_result_publisher.rs @@ -0,0 +1,36 @@ +use crate::models::{ToolUninstallResult, UninstallStatus}; +use crate::services::nats_message_publisher::NatsMessagePublisher; +use anyhow::Context; + +#[derive(Clone)] +pub struct ToolUninstallResultPublisher { + nats_message_publisher: NatsMessagePublisher, +} + +impl ToolUninstallResultPublisher { + pub fn new(nats_message_publisher: NatsMessagePublisher) -> Self { + Self { + nats_message_publisher, + } + } + + pub async fn publish( + &self, + machine_id: &str, + tool_agent_id: &str, + status: UninstallStatus, + ) -> anyhow::Result<()> { + let topic = format!("machine.{}.tool-uninstall.result", machine_id); + let result = ToolUninstallResult { + tool_agent_id: tool_agent_id.to_string(), + status, + }; + self.nats_message_publisher + .publish(&topic, result) + .await + .context(format!( + "Failed to publish tool uninstall result to topic: {}", + topic + )) + } +} diff --git a/clients/openframe-client/src/services/tool_uninstall_service.rs b/clients/openframe-client/src/services/tool_uninstall_service.rs index 704919547..aad30df63 100644 --- a/clients/openframe-client/src/services/tool_uninstall_service.rs +++ b/clients/openframe-client/src/services/tool_uninstall_service.rs @@ -12,6 +12,11 @@ use anyhow::{Context, Result}; use tokio::process::Command; use tracing::{debug, info, warn}; +pub enum UninstallOutcome { + Removed, + NotInstalled, +} + #[derive(Clone)] pub struct ToolUninstallService { installed_tools_service: InstalledToolsService, @@ -35,6 +40,41 @@ impl ToolUninstallService { } } + pub async fn uninstall_by_tool_agent_id( + &self, + tool_agent_id: &str, + ) -> Result { + match self + .installed_tools_service + .get_by_tool_agent_id(tool_agent_id) + .await? + { + None => { + info!( + "Tool {} not present in registry, nothing to uninstall", + tool_agent_id + ); + Ok(UninstallOutcome::NotInstalled) + } + Some(tool) => { + self.uninstall_tool(&tool) + .await + .with_context(|| format!("Failed to uninstall tool: {}", tool_agent_id))?; + self.installed_tools_service + .delete_by_tool_agent_id(tool_agent_id) + .await + .with_context(|| { + format!("Failed to remove registry record for: {}", tool_agent_id) + })?; + info!( + "Tool {} uninstalled and removed from registry", + tool_agent_id + ); + Ok(UninstallOutcome::Removed) + } + } + } + /// Uninstall all installed tools by running their uninstallation commands pub async fn uninstall_all(&self) -> Result<()> { info!("Starting uninstallation of all installed tools"); From eb3168872955591d2ba0a8766f4d4393cb458ed5 Mon Sep 17 00:00:00 2001 From: denys-gif Date: Thu, 2 Jul 2026 20:31:28 +0100 Subject: [PATCH 08/10] feat: delete tool --- clients/openframe-client/src/lib.rs | 4 --- .../tool_uninstall_message_listener.rs | 31 +++++----------- clients/openframe-client/src/models/mod.rs | 2 +- .../src/models/tool_uninstall_message.rs | 17 +-------- clients/openframe-client/src/services/mod.rs | 2 -- .../tool_uninstall_result_publisher.rs | 36 ------------------- .../src/services/tool_uninstall_service.rs | 9 +++++ 7 files changed, 19 insertions(+), 82 deletions(-) delete mode 100644 clients/openframe-client/src/services/tool_uninstall_result_publisher.rs diff --git a/clients/openframe-client/src/lib.rs b/clients/openframe-client/src/lib.rs index 46326b201..f8d453859 100644 --- a/clients/openframe-client/src/lib.rs +++ b/clients/openframe-client/src/lib.rs @@ -69,7 +69,6 @@ use crate::services::tool_agent_update_service::ToolAgentUpdateService; use crate::services::tool_connection_message_publisher::ToolConnectionMessagePublisher; use crate::services::tool_connection_service::ToolConnectionService; use crate::services::tool_installation_service::ToolInstallationService; -use crate::services::tool_uninstall_result_publisher::ToolUninstallResultPublisher; use crate::services::tool_uninstall_service::ToolUninstallService; use crate::services::InstalledToolsService; use crate::services::{ @@ -407,13 +406,10 @@ impl Client { tool_kill_service.clone(), directory_manager.clone(), ); - let tool_uninstall_result_publisher = - ToolUninstallResultPublisher::new(nats_message_publisher.clone()); let tool_uninstall_message_listener = ToolUninstallMessageListener::new( nats_connection_manager.clone(), tool_run_manager.clone(), tool_uninstall_service, - tool_uninstall_result_publisher, config_service.clone(), ); diff --git a/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs b/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs index be60f7b99..edbf95280 100644 --- a/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs +++ b/clients/openframe-client/src/listener/tool_uninstall_message_listener.rs @@ -3,10 +3,9 @@ use crate::config::update_config::{ INITIAL_RETRY_DELAY_MS, MAX_RETRY_DELAY_MS, RECONNECTION_DELAY_MS, UNINSTALL_CONSUMER_MAX_DELIVER, }; -use crate::models::{ToolUninstallMessage, UninstallStatus}; +use crate::models::ToolUninstallMessage; use crate::services::nats_connection_manager::NatsConnectionManager; use crate::services::tool_run_manager::ToolRunManager; -use crate::services::tool_uninstall_result_publisher::ToolUninstallResultPublisher; use crate::services::tool_uninstall_service::ToolUninstallService; use crate::services::tool_uninstall_service::UninstallOutcome; use crate::services::AgentConfigurationService; @@ -24,7 +23,6 @@ pub struct ToolUninstallMessageListener { nats_connection_manager: NatsConnectionManager, tool_run_manager: ToolRunManager, tool_uninstall_service: ToolUninstallService, - result_publisher: ToolUninstallResultPublisher, config_service: AgentConfigurationService, } @@ -35,14 +33,12 @@ impl ToolUninstallMessageListener { nats_connection_manager: NatsConnectionManager, tool_run_manager: ToolRunManager, tool_uninstall_service: ToolUninstallService, - result_publisher: ToolUninstallResultPublisher, config_service: AgentConfigurationService, ) -> Self { Self { nats_connection_manager, tool_run_manager, tool_uninstall_service, - result_publisher, config_service, } } @@ -92,7 +88,7 @@ impl ToolUninstallMessageListener { } }; - if let Err(e) = self.handle_message(message, &machine_id).await { + if let Err(e) = self.handle_message(message).await { error!("Failed to handle message: {:#}", e); } } @@ -100,7 +96,7 @@ impl ToolUninstallMessageListener { Ok(()) } - async fn handle_message(&self, message: Message, machine_id: &str) -> Result<()> { + async fn handle_message(&self, message: Message) -> Result<()> { let payload = String::from_utf8_lossy(&message.payload); info!("Received tool uninstall message: {:?}", payload); @@ -138,16 +134,16 @@ impl ToolUninstallMessageListener { .catch_unwind() .await; - let (status, ack_message, remove_supervision) = match outcome { - Ok(Ok(UninstallOutcome::Removed)) => (UninstallStatus::Removed, true, true), - Ok(Ok(UninstallOutcome::NotInstalled)) => (UninstallStatus::NotInstalled, true, true), + let (ack_message, remove_supervision) = match outcome { + Ok(Ok(UninstallOutcome::Removed)) => (true, true), + Ok(Ok(UninstallOutcome::NotInstalled)) => (true, true), Ok(Err(e)) => { error!("Failed to uninstall tool {}: {:#}", tool_agent_id, e); - (UninstallStatus::Failed, false, false) + (false, false) } Err(_) => { error!("Uninstall panicked for tool {}", tool_agent_id); - (UninstallStatus::Failed, false, false) + (false, false) } }; @@ -158,17 +154,6 @@ impl ToolUninstallMessageListener { } self.tool_run_manager.clear_updating(&tool_agent_id).await; - if let Err(e) = self - .result_publisher - .publish(machine_id, &tool_agent_id, status) - .await - { - warn!( - "Failed to publish uninstall result for {}: {:#}", - tool_agent_id, e - ); - } - if ack_message { message .ack() diff --git a/clients/openframe-client/src/models/mod.rs b/clients/openframe-client/src/models/mod.rs index 9d4b689d3..3aaedcb66 100644 --- a/clients/openframe-client/src/models/mod.rs +++ b/clients/openframe-client/src/models/mod.rs @@ -42,5 +42,5 @@ pub use tool_connection::ToolConnection; pub use tool_connection_message::ToolConnectionMessage; pub use tool_installation_message::ToolInstallationMessage; pub use tool_installation_result::ToolInstallationResult; -pub use tool_uninstall_message::{ToolUninstallMessage, ToolUninstallResult, UninstallStatus}; +pub use tool_uninstall_message::ToolUninstallMessage; pub use update_state::{UpdatePhase, UpdateState}; diff --git a/clients/openframe-client/src/models/tool_uninstall_message.rs b/clients/openframe-client/src/models/tool_uninstall_message.rs index cce6b48e0..4147cf813 100644 --- a/clients/openframe-client/src/models/tool_uninstall_message.rs +++ b/clients/openframe-client/src/models/tool_uninstall_message.rs @@ -1,22 +1,7 @@ -use serde::{Deserialize, Serialize}; +use serde::Deserialize; #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ToolUninstallMessage { pub tool_agent_id: String, } - -#[derive(Debug, Clone, Copy, Serialize)] -#[serde(rename_all = "SCREAMING_SNAKE_CASE")] -pub enum UninstallStatus { - Removed, - NotInstalled, - Failed, -} - -#[derive(Debug, Clone, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct ToolUninstallResult { - pub tool_agent_id: String, - pub status: UninstallStatus, -} diff --git a/clients/openframe-client/src/services/mod.rs b/clients/openframe-client/src/services/mod.rs index 4fa872207..0bf1eac11 100644 --- a/clients/openframe-client/src/services/mod.rs +++ b/clients/openframe-client/src/services/mod.rs @@ -28,7 +28,6 @@ pub mod tool_connection_service; pub mod tool_installation_service; pub mod tool_kill_service; pub mod tool_run_manager; -pub mod tool_uninstall_result_publisher; pub mod tool_uninstall_service; pub mod tool_url_params_resolver; pub mod update_cleanup_service; @@ -60,7 +59,6 @@ pub use tool_connection_service::ToolConnectionService; pub use tool_installation_service::ToolInstallationService; pub use tool_kill_service::ToolKillService; pub use tool_run_manager::ToolRunManager; -pub use tool_uninstall_result_publisher::ToolUninstallResultPublisher; pub use tool_uninstall_service::{ToolUninstallService, UninstallOutcome}; pub use tool_url_params_resolver::ToolUrlParamsResolver; pub use update_cleanup_service::UpdateCleanupService; diff --git a/clients/openframe-client/src/services/tool_uninstall_result_publisher.rs b/clients/openframe-client/src/services/tool_uninstall_result_publisher.rs deleted file mode 100644 index b6779a47c..000000000 --- a/clients/openframe-client/src/services/tool_uninstall_result_publisher.rs +++ /dev/null @@ -1,36 +0,0 @@ -use crate::models::{ToolUninstallResult, UninstallStatus}; -use crate::services::nats_message_publisher::NatsMessagePublisher; -use anyhow::Context; - -#[derive(Clone)] -pub struct ToolUninstallResultPublisher { - nats_message_publisher: NatsMessagePublisher, -} - -impl ToolUninstallResultPublisher { - pub fn new(nats_message_publisher: NatsMessagePublisher) -> Self { - Self { - nats_message_publisher, - } - } - - pub async fn publish( - &self, - machine_id: &str, - tool_agent_id: &str, - status: UninstallStatus, - ) -> anyhow::Result<()> { - let topic = format!("machine.{}.tool-uninstall.result", machine_id); - let result = ToolUninstallResult { - tool_agent_id: tool_agent_id.to_string(), - status, - }; - self.nats_message_publisher - .publish(&topic, result) - .await - .context(format!( - "Failed to publish tool uninstall result to topic: {}", - topic - )) - } -} diff --git a/clients/openframe-client/src/services/tool_uninstall_service.rs b/clients/openframe-client/src/services/tool_uninstall_service.rs index aad30df63..c5551ea2a 100644 --- a/clients/openframe-client/src/services/tool_uninstall_service.rs +++ b/clients/openframe-client/src/services/tool_uninstall_service.rs @@ -60,6 +60,15 @@ impl ToolUninstallService { self.uninstall_tool(&tool) .await .with_context(|| format!("Failed to uninstall tool: {}", tool_agent_id))?; + + let tool_dir = self.directory_manager.app_support_dir().join(tool_agent_id); + if tool_dir.exists() { + std::fs::remove_dir_all(&tool_dir).with_context(|| { + format!("Failed to remove tool directory: {}", tool_dir.display()) + })?; + info!("Removed tool directory: {}", tool_dir.display()); + } + self.installed_tools_service .delete_by_tool_agent_id(tool_agent_id) .await From a4675035384214ad11637cd612343debf6a82151 Mon Sep 17 00:00:00 2001 From: mikhailm-coder Date: Sat, 4 Jul 2026 09:24:37 +0200 Subject: [PATCH 09/10] fix(client): resolve clippy warnings in ported tenant changes - drop dead trailing assignment to reinstall_dir_cleared (unused_assignments; the flag is never read after the no-registry-record repair block) - mesh self-heal: map_or(false, ..) -> is_some_and(..) (clippy::unnecessary_map_or) Both warnings originate in code ported verbatim from openframe-oss-tenant; this keeps the repo clippy gate (-D warnings) green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011z7MGH2qYeKt2ZQV88EaD8 --- clients/openframe-client/src/services/mesh_self_heal_service.rs | 2 +- .../openframe-client/src/services/tool_installation_service.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clients/openframe-client/src/services/mesh_self_heal_service.rs b/clients/openframe-client/src/services/mesh_self_heal_service.rs index 771fcd359..833520b28 100644 --- a/clients/openframe-client/src/services/mesh_self_heal_service.rs +++ b/clients/openframe-client/src/services/mesh_self_heal_service.rs @@ -109,7 +109,7 @@ impl MeshSelfHealService { } } - let stuck = stuck_since.map_or(false, |t| t.elapsed() >= STUCK_DURATION); + let stuck = stuck_since.is_some_and(|t| t.elapsed() >= STUCK_DURATION); if !msh_missing_serverid && !stuck { continue; } diff --git a/clients/openframe-client/src/services/tool_installation_service.rs b/clients/openframe-client/src/services/tool_installation_service.rs index 8c77e4b13..03eec4949 100644 --- a/clients/openframe-client/src/services/tool_installation_service.rs +++ b/clients/openframe-client/src/services/tool_installation_service.rs @@ -460,7 +460,6 @@ impl ToolInstallationService { tool_folder_path.display() ) })?; - reinstall_dir_cleared = true; } // Download and install the tool From d947e8703f119d89b1a34a33a6ecf790040deddb Mon Sep 17 00:00:00 2001 From: mikhailm-coder Date: Sat, 4 Jul 2026 10:25:51 +0200 Subject: [PATCH 10/10] fix(client): address CodeRabbit findings from the tenant port (#1359) 1. tool_agent_update_listener: serialize updates with the per-tool lock. The per-tool lock introduced with remote uninstall was held by install (blocking) and uninstall (try_lock -> defer via redelivery), but the update path never acquired it, so an uninstall arriving mid-update could remove the tool while binaries/state were being written. The listener now try_locks before dispatching (same pattern as the uninstall listener) and leaves the message unacked for JetStream redelivery when the tool is busy. 2. system_service::parse_exe_from_image_path: use to_ascii_lowercase(). to_lowercase() can change byte lengths for non-ASCII characters, so slicing the original string with an index found in the lowered string could return a wrong path or panic on a char boundary. Both issues exist identically in openframe-oss-tenant and should be fixed there too (or picked up when tenant consumes this lib). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011z7MGH2qYeKt2ZQV88EaD8 --- clients/openframe-client/src/lib.rs | 1 + .../listener/tool_agent_update_listener.rs | 19 +++++++++++++++++++ .../src/platform/system_service.rs | 4 +++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/clients/openframe-client/src/lib.rs b/clients/openframe-client/src/lib.rs index f8d453859..2744c776e 100644 --- a/clients/openframe-client/src/lib.rs +++ b/clients/openframe-client/src/lib.rs @@ -425,6 +425,7 @@ impl Client { nats_connection_manager.clone(), tool_agent_update_service, config_service.clone(), + tool_run_manager.clone(), ); let execution_service = ExecutionService::new(); diff --git a/clients/openframe-client/src/listener/tool_agent_update_listener.rs b/clients/openframe-client/src/listener/tool_agent_update_listener.rs index 2047b97e6..0bae2c264 100644 --- a/clients/openframe-client/src/listener/tool_agent_update_listener.rs +++ b/clients/openframe-client/src/listener/tool_agent_update_listener.rs @@ -6,6 +6,7 @@ use crate::config::update_config::{ use crate::models::tool_agent_update_message::ToolAgentUpdateMessage; use crate::services::nats_connection_manager::NatsConnectionManager; use crate::services::tool_agent_update_service::ToolAgentUpdateService; +use crate::services::tool_run_manager::ToolRunManager; use crate::services::AgentConfigurationService; use anyhow::Result; use async_nats::jetstream; @@ -22,6 +23,7 @@ pub struct ToolAgentUpdateListener { pub nats_connection_manager: NatsConnectionManager, pub tool_agent_update_service: ToolAgentUpdateService, pub config_service: AgentConfigurationService, + pub tool_run_manager: ToolRunManager, } impl ToolAgentUpdateListener { @@ -31,11 +33,13 @@ impl ToolAgentUpdateListener { nats_connection_manager: NatsConnectionManager, tool_agent_update_service: ToolAgentUpdateService, config_service: AgentConfigurationService, + tool_run_manager: ToolRunManager, ) -> Self { Self { nats_connection_manager, tool_agent_update_service, config_service, + tool_run_manager, } } @@ -111,6 +115,21 @@ impl ToolAgentUpdateListener { let tool_agent_id = tool_agent_update_message.tool_agent_id.clone(); + // Serialize with install/uninstall via the per-tool lock (same pattern as the + // uninstall listener): if another operation holds it, leave the message unacked + // so JetStream redelivers the update once the tool is free. + let tool_lock = self.tool_run_manager.tool_lock(&tool_agent_id).await; + let _guard = match tool_lock.try_lock() { + Ok(guard) => guard, + Err(_) => { + info!( + "Tool {} busy with another operation, deferring update for redelivery", + tool_agent_id + ); + return Ok(()); + } + }; + match self .tool_agent_update_service .process_update(tool_agent_update_message) diff --git a/clients/openframe-client/src/platform/system_service.rs b/clients/openframe-client/src/platform/system_service.rs index 92f6af5a5..9787d0839 100644 --- a/clients/openframe-client/src/platform/system_service.rs +++ b/clients/openframe-client/src/platform/system_service.rs @@ -591,7 +591,9 @@ fn parse_exe_from_image_path(image_path: &str) -> Option { } } // Unquoted: cut after the first ".exe" (case-insensitive) to drop trailing args. - let lower = trimmed.to_lowercase(); + // to_ascii_lowercase keeps byte indices aligned with `trimmed` (to_lowercase can + // change byte lengths for non-ASCII chars, misaligning the slice below). + let lower = trimmed.to_ascii_lowercase(); if let Some(idx) = lower.find(".exe") { return Some(std::path::PathBuf::from(&trimmed[..idx + 4])); }