diff --git a/apps/desktop/src-tauri/src/bin/codevetter.rs b/apps/desktop/src-tauri/src/bin/codevetter.rs index a3a736e3..5afeef15 100644 --- a/apps/desktop/src-tauri/src/bin/codevetter.rs +++ b/apps/desktop/src-tauri/src/bin/codevetter.rs @@ -52,6 +52,8 @@ use codevetter_desktop::commands::repo_query::{ RepositoryHistorySelectorKind, RepositoryQueryDomain, RepositoryQueryInput, RepositoryQueryMode, RepositoryQueryReceipt, }; +#[cfg(unix)] +use codevetter_desktop::commands::review::cancel_all_cli_reviews; use codevetter_desktop::commands::rubric_settings::{ active_rubric_prompt, read_rubric_settings, select_rubric_pack, upsert_rubric_pack, RubricPackInput, RubricSettingsReceipt, @@ -542,9 +544,60 @@ enum CliCommand { Version, } +#[cfg(unix)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CliShutdownSignal { + Interrupt, + Terminate, +} + +#[cfg(unix)] +impl CliShutdownSignal { + const fn exit_code(self) -> i32 { + match self { + Self::Interrupt => 130, + Self::Terminate => 143, + } + } +} + +#[cfg(unix)] +async fn run_until_shutdown() -> Result { + use tokio::signal::unix::{signal, SignalKind}; + + let mut interrupt = signal(SignalKind::interrupt()) + .map_err(|error| format!("register SIGINT handler: {error}"))?; + let mut terminate = signal(SignalKind::terminate()) + .map_err(|error| format!("register SIGTERM handler: {error}"))?; + let mut command = Box::pin(run()); + let shutdown = async { + tokio::select! { + _ = interrupt.recv() => CliShutdownSignal::Interrupt, + _ = terminate.recv() => CliShutdownSignal::Terminate, + } + }; + + tokio::select! { + result = &mut command => result, + signal = shutdown => { + if cancel_all_cli_reviews() > 0 { + // The review future owns the agent process group. Wait for its + // normal cancellation branch to kill and reap that group before + // exiting the CLI parent, otherwise the executor is orphaned. + let _ = command.await; + } + Ok(signal.exit_code()) + } + } +} + #[tokio::main] async fn main() { - let code = match run().await { + #[cfg(unix)] + let result = run_until_shutdown().await; + #[cfg(not(unix))] + let result = run().await; + let code = match result { Ok(code) => code, Err(error) => { eprintln!("codevetter: {error}"); diff --git a/apps/desktop/src-tauri/src/codevetter_cli_tests.rs b/apps/desktop/src-tauri/src/codevetter_cli_tests.rs index 05ab681e..c380439a 100644 --- a/apps/desktop/src-tauri/src/codevetter_cli_tests.rs +++ b/apps/desktop/src-tauri/src/codevetter_cli_tests.rs @@ -10,6 +10,13 @@ const SURFACE_PARITY_FIXTURE: &str = const LOCAL_CHECK_PARITY_FIXTURE: &str = include_str!("../tests/fixtures/surface-parity/local-check-v1.json"); +#[cfg(unix)] +#[test] +fn cli_shutdown_uses_conventional_signal_exit_codes() { + assert_eq!(CliShutdownSignal::Interrupt.exit_code(), 130); + assert_eq!(CliShutdownSignal::Terminate.exit_code(), 143); +} + fn surface_parity_fixture() -> serde_json::Value { serde_json::from_str(SURFACE_PARITY_FIXTURE).expect("surface parity fixture") } diff --git a/apps/desktop/src-tauri/src/commands/review.rs b/apps/desktop/src-tauri/src/commands/review.rs index 0f6c2da8..84c7de79 100644 --- a/apps/desktop/src-tauri/src/commands/review.rs +++ b/apps/desktop/src-tauri/src/commands/review.rs @@ -64,6 +64,23 @@ fn review_cancellation(repo_path: &str) -> Arc { .unwrap_or_else(|| Arc::new(AtomicBool::new(false))) } +fn mark_review_cancellations(active: &std::collections::HashMap>) -> usize { + for cancellation in active.values() { + cancellation.store(true, Ordering::SeqCst); + } + active.len() +} + +/// Signal every review owned by this process so CLI shutdown can wait for the +/// normal process-group cleanup path instead of orphaning an agent executor. +pub fn cancel_all_cli_reviews() -> usize { + let active = ACTIVE_REVIEW_CANCELLATIONS + .get_or_init(|| Mutex::new(std::collections::HashMap::new())) + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + mark_review_cancellations(&active) +} + #[tauri::command] pub async fn cancel_cli_review(repo_path: String) -> Result { let key = std::fs::canonicalize(&repo_path) @@ -4169,6 +4186,20 @@ mod tests { ); } + #[test] + fn process_shutdown_marks_every_owned_review_for_cancellation() { + let first = Arc::new(AtomicBool::new(false)); + let second = Arc::new(AtomicBool::new(false)); + let active = std::collections::HashMap::from([ + ("first".to_string(), Arc::clone(&first)), + ("second".to_string(), Arc::clone(&second)), + ]); + + assert_eq!(mark_review_cancellations(&active), 2); + assert!(first.load(Ordering::SeqCst)); + assert!(second.load(Ordering::SeqCst)); + } + #[cfg(unix)] fn executable_script(temp: &tempfile::TempDir, name: &str, body: &str) -> String { use std::os::unix::fs::PermissionsExt;