[stack 36/39] Reap review agents on shutdown

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<i32, String> {
+    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}");

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")
 }

apps/desktop/src-tauri/src/commands/review.rs

@@ -64,6 +64,23 @@ fn review_cancellation(repo_path: &str) -> Arc<AtomicBool> {
         .unwrap_or_else(|| Arc::new(AtomicBool::new(false)))
 }
 
+fn mark_review_cancellations(active: &std::collections::HashMap<String, Arc<AtomicBool>>) -> 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<Value, String> {
     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;