fix: harden sidebar, approval, and updater lifecycle

.github/workflows/ci.yml

@@ -372,6 +372,14 @@ jobs:
       - name: Run file watch contract tests
         run: cargo test --locked -p openbitfun-services-integrations --no-default-features --features file-watch --test file_watch_contracts
 
+      # Exercise native child cleanup outside the Tauri test harness. In
+      # particular, Windows update/restart cleanup must leave the host alive.
+      - name: Run process lifecycle contract tests
+        shell: bash
+        run: |
+          cargo test --locked -p openbitfun-services-core --no-default-features --features process-runtime --test process_runtime_contracts
+          cargo test --locked -p openbitfun-services-core --no-default-features --features process-runtime --lib process_tree::tests::
+
       # Search tools share matching but retain provider-specific path and
       # symlink handling. Keep the search module in the full OS matrix.
       - name: Run search tool tests

src/crates/services/services-core/AGENTS.md

@@ -68,6 +68,11 @@ crate.
   resource-limit safety. Unix descendants that deliberately create a new
   session/process group are outside this boundary and must be treated as a
   disclosed residual risk until a platform supervisor is introduced.
+- Windows final cleanup closes only registered `process_tree` child Jobs and
+  rejects new managed spawns after shutdown begins. It must not initialize or
+  close the Job used by `contain_current_process_tree`: that explicit CLI/SDK
+  host-lifetime guard includes the host itself and stays alive until process
+  exit. Keep updater/restart handoff processes outside managed child trees.
 
 ## Verification
 
@@ -88,6 +93,7 @@ cargo test -p openbitfun-services-core --no-default-features --features local-st
 cargo test -p openbitfun-services-core --no-default-features --features local-storage --test session_write_lock_contracts
 cargo test -p openbitfun-services-core --no-default-features --features token-usage-statistics --lib token_usage::
 cargo test -p openbitfun-services-core --no-default-features --features process-runtime --test process_runtime_contracts
+cargo test -p openbitfun-services-core --no-default-features --features process-runtime --lib process_tree::tests::
 cargo test --locked -p openbitfun-services-core --no-default-features --features tls-provider --lib tls_provider::tests
 pnpm run check:core-boundaries
 ```

src/crates/services/services-core/src/process_manager.rs

@@ -1,6 +1,11 @@
-//! Unified process management to avoid Windows child process leaks
+//! Hidden process creation and explicit host-lifetime containment.
+//!
+//! Host containment belongs to long-lived CLI/SDK services. Graceful cleanup
+//! only closes managed child trees; it must never close a Job containing the
+//! calling host before an updater, restart, or shutdown can finish.
 
 use std::process::Command;
+#[cfg(windows)]
 use std::sync::LazyLock;
 #[cfg(target_os = "macos")]
 use std::sync::OnceLock;
@@ -21,6 +26,7 @@ use win32job::Job;
 #[cfg(windows)]
 const CREATE_NO_WINDOW: u32 = 0x08000000;
 
+#[cfg(windows)]
 static GLOBAL_PROCESS_MANAGER: LazyLock<ProcessManager> = LazyLock::new(ProcessManager::new);
 
 pub struct ProcessManager {
@@ -29,6 +35,7 @@ pub struct ProcessManager {
 }
 
 impl ProcessManager {
+    #[cfg(windows)]
     fn new() -> Self {
         let manager = Self {
             #[cfg(windows)]
@@ -68,17 +75,7 @@ impl ProcessManager {
     }
 
     pub fn cleanup_all(&self) {
-        #[cfg(windows)]
-        {
-            let mut job_guard = match self.job.lock() {
-                Ok(guard) => guard,
-                Err(poisoned) => {
-                    warn!("Process manager job mutex was poisoned during cleanup, recovering lock");
-                    poisoned.into_inner() as std::sync::MutexGuard<'_, Option<Job>>
-                }
-            };
-            job_guard.take();
-        }
+        crate::process_tree::cleanup_all_process_trees();
     }
 }
 
@@ -160,11 +157,17 @@ fn build_macos_path_env() -> Option<std::ffi::OsString> {
     std::env::join_paths(merged).ok()
 }
 
+/// Stop managed child trees without creating or closing host containment.
+/// Safe to call repeatedly, including when no child process was ever started.
 pub fn cleanup_all_processes() {
-    GLOBAL_PROCESS_MANAGER.cleanup_all();
+    // Accessing the lazy host manager here would assign this process to a Job
+    // for the first time during Desktop exit. Keep that initialization exclusive
+    // to contain_current_process_tree(), and keep its handle alive until exit.
+    crate::process_tree::cleanup_all_process_trees();
 }
 
-/// Keep descendants of a long-lived service in the process-wide Job.
+/// Keep descendants of a long-lived service in the process-wide Job until the
+/// host exits. This lifetime guard is independent of managed-child cleanup.
 pub fn contain_current_process_tree() -> std::io::Result<()> {
     #[cfg(windows)]
     if GLOBAL_PROCESS_MANAGER

src/crates/services/services-core/src/process_tree.rs

@@ -7,6 +7,8 @@
 
 use std::fmt;
 use std::io;
+#[cfg(windows)]
+use std::sync::{Arc, LazyLock, Mutex, Weak};
 use std::time::Duration;
 #[cfg(unix)]
 use std::time::Instant;
@@ -94,7 +96,7 @@ impl ProcessTreeChild {
         #[cfg(windows)]
         {
             let _ = grace;
-            let had_job = self.platform.job.take().is_some();
+            let had_job = self.platform.job.close();
             if !parent_exited {
                 self.child.wait().await?;
             }
@@ -135,7 +137,7 @@ impl Drop for ProcessTreeChild {
         }
         #[cfg(windows)]
         {
-            self.platform.job.take();
+            self.platform.job.close();
         }
         if !parent_exited {
             let _ = self.child.start_kill();
@@ -200,7 +202,48 @@ fn process_group_is_alive(process_group_id: i32) -> bool {
 
 #[cfg(windows)]
 struct PlatformProcessTree {
-    job: Option<win32job::Job>,
+    job: Arc<ManagedWindowsJob>,
+}
+
+#[cfg(windows)]
+struct ManagedWindowsJob(Mutex<Option<win32job::Job>>);
+
+#[cfg(windows)]
+impl ManagedWindowsJob {
+    fn close(&self) -> bool {
+        self.0
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner())
+            .take()
+            .is_some()
+    }
+}
+
+#[cfg(windows)]
+#[derive(Default)]
+struct ManagedWindowsJobs {
+    shutting_down: bool,
+    jobs: Vec<Weak<ManagedWindowsJob>>,
+}
+
+#[cfg(windows)]
+static MANAGED_WINDOWS_JOBS: LazyLock<Mutex<ManagedWindowsJobs>> =
+    LazyLock::new(|| Mutex::new(ManagedWindowsJobs::default()));
+
+/// Emergency/final cleanup for managed children. Protocol shutdown belongs to
+/// callers and runs first. Host-lifetime Jobs are deliberately not registered.
+pub(crate) fn cleanup_all_process_trees() {
+    #[cfg(windows)]
+    {
+        let mut registry = MANAGED_WINDOWS_JOBS
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner());
+        registry.shutting_down = true;
+        // Also serialize repeated callers until every registered handle closes.
+        for job in registry.jobs.drain(..).filter_map(|job| job.upgrade()) {
+            job.close();
+        }
+    }
 }
 
 #[cfg(windows)]
@@ -217,6 +260,17 @@ async fn spawn_windows_process_tree(command: &mut Command) -> io::Result<Process
     command.creation_flags(CREATE_SUSPENDED.0 | CREATE_NO_WINDOW);
     let mut child = command.spawn()?;
     let attach_result = (|| {
+        // Serialize registration/resume with final cleanup. Once shutdown has
+        // begun, a newly spawned child remains suspended and is killed below.
+        let mut registry = MANAGED_WINDOWS_JOBS
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner());
+        if registry.shutting_down {
+            return Err(io::Error::new(
+                io::ErrorKind::BrokenPipe,
+                "managed process trees are shutting down",
+            ));
+        }
         let process_id = child
             .id()
             .ok_or_else(|| io::Error::other("spawned process has no process id"))?;
@@ -225,18 +279,24 @@ async fn spawn_windows_process_tree(command: &mut Command) -> io::Result<Process
             .ok_or_else(|| io::Error::other("spawned process has no process handle"))?;
         job.assign_process(process_handle as isize)
             .map_err(job_error)?;
-        resume_primary_thread(process_id)
+        resume_primary_thread(process_id)?;
+        let job = Arc::new(ManagedWindowsJob(Mutex::new(Some(job))));
+        registry.jobs.retain(|job| job.strong_count() > 0);
+        registry.jobs.push(Arc::downgrade(&job));
+        Ok(job)
     })();
 
-    if let Err(error) = attach_result {
-        drop(job);
-        let _ = child.kill().await;
-        return Err(error);
-    }
+    let job = match attach_result {
+        Ok(job) => job,
+        Err(error) => {
+            let _ = child.kill().await;
+            return Err(error);
+        }
+    };
 
     Ok(ProcessTreeChild {
         child,
-        platform: PlatformProcessTree { job: Some(job) },
+        platform: PlatformProcessTree { job },
     })
 }
 
@@ -360,7 +420,11 @@ mod tests {
         let mut command = Command::new("sh");
         command
             .arg("-c")
-            .arg("\"$OPENBITFUN_PROCESS_TREE_TEST_EXE\" --exact process_tree::tests::unix_detached_fixture_process --nocapture")
+            // Keep the shell as the managed process-group leader. Some shells
+            // replace themselves with their final foreground command, which
+            // would make the fixture a process-group leader and cause setsid()
+            // to fail on macOS before it can publish its PID.
+            .arg("\"$OPENBITFUN_PROCESS_TREE_TEST_EXE\" --exact process_tree::tests::unix_detached_fixture_process --nocapture & wait")
             .env("OPENBITFUN_PROCESS_TREE_TEST_EXE", executable)
             .env("OPENBITFUN_DETACHED_FIXTURE", "1")
             .env("OPENBITFUN_DESCENDANT_PID_FILE", &pid_file)