diff --git a/apps/staged/src-tauri/src/actions/commands.rs b/apps/staged/src-tauri/src/actions/commands.rs index 27c037a5..14747c27 100644 --- a/apps/staged/src-tauri/src/actions/commands.rs +++ b/apps/staged/src-tauri/src/actions/commands.rs @@ -3,7 +3,7 @@ use anyhow::Result; use builderbot_actions::{ ActionDetector, ActionExecutor, ActionMetadata, ActionType, FileExplorationMode, - RunDetectionMode, SuggestedAction, + RunDetectionMode, StopOptions, SuggestedAction, }; use std::sync::{Arc, Mutex}; use tauri::{AppHandle, Emitter, State}; @@ -437,14 +437,24 @@ pub fn stop_actions_for_branches( } /// Stop all running actions across all branches (best-effort). -pub fn stop_all_actions(executor: &ActionExecutor, registry: &ActionRegistry) { +pub fn stop_all_actions( + executor: &ActionExecutor, + registry: &ActionRegistry, + stop_options: StopOptions, +) -> Vec { + let mut stopped_execution_ids = Vec::new(); + for info in registry.get_all_running() { if executor.is_running(&info.execution_id) { - if let Err(e) = executor.stop(&info.execution_id) { + if let Err(e) = executor.stop_with_options(&info.execution_id, stop_options) { log::warn!("Failed to stop action {}: {e}", info.execution_id); + } else { + stopped_execution_ids.push(info.execution_id); } } } + + stopped_execution_ids } /// Get all currently running actions for a branch diff --git a/apps/staged/src-tauri/src/actions/mod.rs b/apps/staged/src-tauri/src/actions/mod.rs index 24c1791b..1e2e6cb6 100644 --- a/apps/staged/src-tauri/src/actions/mod.rs +++ b/apps/staged/src-tauri/src/actions/mod.rs @@ -13,6 +13,6 @@ pub mod run_detector; // Re-export types for convenience pub use builderbot_actions::{ ActionDetector, ActionExecutor, ActionMetadata, ActionStatus, ActionType, OutputChunk, - SuggestedAction, + StopOptions, SuggestedAction, }; pub use registry::{ActionRegistry, RunPhase, RunningActionInfo}; diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index aef8cf45..7ee71a5f 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -27,7 +27,9 @@ pub mod util_commands; use serde::Serialize; use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; +use std::time::Duration; use store::Store; use tauri::{Emitter, Manager}; @@ -44,6 +46,11 @@ struct DbState { needs_reset: Mutex>, } +#[derive(Default)] +struct ShutdownState { + quit_in_progress: AtomicBool, +} + pub(crate) fn preferences_store_path_buf() -> Option { crate::paths::data_dir().map(|d| d.join("preferences.json")) } @@ -205,6 +212,29 @@ fn emit_setup_progress( ); } +fn stop_actions_for_app_shutdown(app_handle: &tauri::AppHandle) { + let executor = app_handle.state::>(); + let registry = app_handle.state::>(); + let stopped_execution_ids = actions::commands::stop_all_actions( + &executor, + ®istry, + actions::StopOptions { + force_kill_after: Some(Duration::from_secs(1)), + }, + ); + + if stopped_execution_ids.is_empty() { + return; + } + + if !executor.wait_for_executions(&stopped_execution_ids, Duration::from_secs(2)) { + log::warn!( + "Timed out waiting for {} action(s) to stop during app shutdown", + stopped_execution_ids.len() + ); + } +} + // ============================================================================= // Store status commands // ============================================================================= @@ -1624,6 +1654,7 @@ pub fn run() { app.manage(Arc::new(session_runner::SessionRegistry::new())); app.manage(Arc::new(actions::ActionExecutor::new())); app.manage(Arc::new(actions::ActionRegistry::new())); + app.manage(ShutdownState::default()); app.manage(DbState { db_path, needs_reset: Mutex::new(reset_info), @@ -1797,13 +1828,15 @@ pub fn run() { .build(tauri::generate_context!()) .expect("error while building tauri application") .run(|app_handle, event| { - if let tauri::RunEvent::Exit = event { - // Stop all running actions on quit (fire-and-forget). - let executor = app_handle.state::>(); - let registry = app_handle.state::>(); - actions::commands::stop_all_actions(&executor, ®istry); - // Brief grace period for processes to receive SIGTERM. - std::thread::sleep(std::time::Duration::from_secs(1)); + if let tauri::RunEvent::ExitRequested { api, .. } = event { + let shutdown = app_handle.state::(); + if shutdown.quit_in_progress.swap(true, Ordering::SeqCst) { + return; + } + + api.prevent_exit(); + stop_actions_for_app_shutdown(app_handle); + app_handle.exit(0); } }); } diff --git a/crates/builderbot-actions/src/executor.rs b/crates/builderbot-actions/src/executor.rs index 4facafd6..5934c779 100644 --- a/crates/builderbot-actions/src/executor.rs +++ b/crates/builderbot-actions/src/executor.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; use std::process::{Child, Command, Stdio}; use std::sync::{Arc, Mutex}; use std::thread; +use std::time::{Duration, Instant}; use tokio::runtime::Handle; use tokio::sync::oneshot; @@ -105,6 +106,19 @@ pub struct ActionMetadata { pub auto_commit: bool, } +#[derive(Clone, Copy, Debug)] +pub struct StopOptions { + pub force_kill_after: Option, +} + +impl Default for StopOptions { + fn default() -> Self { + Self { + force_kill_after: Some(Duration::from_secs(5)), + } + } +} + /// Internal state for a running action struct RunningActionState { child_pid: Option, @@ -626,6 +640,11 @@ impl ActionExecutor { /// group. The completion thread will see the stopped flag and emit a /// `Stopped` status event once the process actually exits. pub fn stop(&self, execution_id: &str) -> Result<()> { + self.stop_with_options(execution_id, StopOptions::default()) + } + + /// Stop a running action with configurable shutdown timing. + pub fn stop_with_options(&self, execution_id: &str, options: StopOptions) -> Result<()> { // Mark as stopped BEFORE sending the signal so the completion thread // knows this was an intentional stop (not a crash/failure). { @@ -660,20 +679,22 @@ impl ActionExecutor { libc::kill(-(pid as i32), libc::SIGTERM); } - // Escalate to SIGKILL after a short grace period in case the - // process group ignores SIGTERM (e.g. a process traps the signal). - thread::spawn(move || { - thread::sleep(std::time::Duration::from_secs(5)); - // SAFETY: Same considerations as above. If the process group - // already exited, kill() harmlessly returns ESRCH. - unsafe { - // Check if the process group still exists before sending SIGKILL - let ret = libc::kill(-(pid as i32), 0); - if ret == 0 { - libc::kill(-(pid as i32), libc::SIGKILL); + if let Some(force_kill_after) = options.force_kill_after { + // Escalate to SIGKILL after a short grace period in case the + // process group ignores SIGTERM (e.g. a process traps the signal). + thread::spawn(move || { + thread::sleep(force_kill_after); + // SAFETY: Same considerations as above. If the process group + // already exited, kill() harmlessly returns ESRCH. + unsafe { + // Check if the process group still exists before sending SIGKILL + let ret = libc::kill(-(pid as i32), 0); + if ret == 0 { + libc::kill(-(pid as i32), libc::SIGKILL); + } } - } - }); + }); + } } #[cfg(windows)] @@ -688,6 +709,32 @@ impl ActionExecutor { Ok(()) } + /// Wait until all listed executions are no longer running, or until the timeout elapses. + /// + /// Returns `true` if every execution completed before the timeout. + pub fn wait_for_executions(&self, execution_ids: &[String], timeout: Duration) -> bool { + let deadline = Instant::now() + timeout; + + loop { + let all_stopped = { + let running = self.running.lock().unwrap(); + execution_ids + .iter() + .all(|execution_id| !running.contains_key(execution_id)) + }; + + if all_stopped { + return true; + } + + if Instant::now() >= deadline { + return false; + } + + thread::sleep(Duration::from_millis(25)); + } + } + /// Get buffered output for an execution pub fn get_buffered_output(&self, execution_id: &str) -> Option> { // First check running actions diff --git a/crates/builderbot-actions/src/lib.rs b/crates/builderbot-actions/src/lib.rs index 9b993edf..e115aafd 100644 --- a/crates/builderbot-actions/src/lib.rs +++ b/crates/builderbot-actions/src/lib.rs @@ -67,5 +67,5 @@ pub mod models; // Re-export main types for convenience pub use acp_provider::AcpAiProvider; pub use detector::{ActionDetector, AiProvider, FileExplorationMode, SuggestedAction}; -pub use executor::{ActionExecutor, ActionMetadata, ExecutionListener}; +pub use executor::{ActionExecutor, ActionMetadata, ExecutionListener, StopOptions}; pub use models::{ActionStatus, ActionType, ExecutionEvent, OutputChunk, RunDetectionMode};