fix(tui): close Windows job before foreground joins

(cherry picked from commit 96adffb243801dcef6c6332611728930e438f1a1)

crates/tui/src/tools/shell.rs

@@ -311,6 +311,29 @@ fn terminate_windows_job(job: Option<&WindowsJob>, child: &mut Child) -> std::io
     child.kill()
 }
 
+#[cfg(windows)]
+fn terminate_and_close_windows_job(windows_job: Option<WindowsJob>) {
+    if let Some(job) = windows_job.as_ref()
+        && let Err(err) = job.terminate()
+    {
+        tracing::warn!(
+            ?err,
+            "failed to terminate Windows shell job before closing job handle"
+        );
+    }
+    drop(windows_job);
+}
+
+#[cfg(windows)]
+fn terminate_child_and_close_windows_job(
+    windows_job: Option<WindowsJob>,
+    child: &mut Child,
+) -> std::io::Result<()> {
+    let result = terminate_windows_job(windows_job.as_ref(), child);
+    drop(windows_job);
+    result
+}
+
 #[cfg(windows)]
 fn attach_windows_job(child: &Child, command: &str) -> Option<WindowsJob> {
     match WindowsJob::attach_to_child(child) {
@@ -485,16 +508,7 @@ impl BackgroundShell {
             let _ = kill_child_process_group(proc);
         }
         #[cfg(windows)]
-        if let Some(job) = self.windows_job.as_ref() {
-            let _ = job.terminate();
-        }
-        #[cfg(windows)]
-        {
-            // Close the job handle before joining reader threads so
-            // JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE can still release inherited
-            // pipe handles if explicit termination failed.
-            self.windows_job = None;
-        }
+        terminate_and_close_windows_job(self.windows_job.take());
         if let Some(handle) = self.stdout_thread.take() {
             let _ = handle.join();
         }
@@ -1039,9 +1053,7 @@ impl ShellManager {
         // Wait with timeout
         if let Some(status) = child.wait_timeout(timeout)? {
             #[cfg(windows)]
-            if let Some(job) = windows_job.as_ref() {
-                let _ = job.terminate();
-            }
+            terminate_and_close_windows_job(windows_job);
             let stdout = stdout_thread.join().unwrap_or_default();
             let stderr = stderr_thread.join().unwrap_or_default();
             let stdout_str = String::from_utf8_lossy(&stdout).to_string();
@@ -1083,7 +1095,7 @@ impl ShellManager {
             #[cfg(unix)]
             let _ = kill_child_process_group(&mut child);
             #[cfg(windows)]
-            let _ = terminate_windows_job(windows_job.as_ref(), &mut child);
+            let _ = terminate_child_and_close_windows_job(windows_job, &mut child);
             #[cfg(all(not(unix), not(windows)))]
             let _ = child.kill();
             let status = child.wait().ok();
@@ -1175,9 +1187,7 @@ impl ShellManager {
 
         if let Some(status) = child.wait_timeout(timeout)? {
             #[cfg(windows)]
-            if let Some(job) = windows_job.as_ref() {
-                let _ = job.terminate();
-            }
+            terminate_and_close_windows_job(windows_job);
             Ok(ShellResult {
                 task_id: None,
                 status: if status.success() {
@@ -1207,7 +1217,7 @@ impl ShellManager {
             #[cfg(unix)]
             let _ = kill_child_process_group(&mut child);
             #[cfg(windows)]
-            let _ = terminate_windows_job(windows_job.as_ref(), &mut child);
+            let _ = terminate_child_and_close_windows_job(windows_job, &mut child);
             #[cfg(all(not(unix), not(windows)))]
             let _ = child.kill();
             let status = child.wait().ok();

crates/tui/src/tools/shell/tests.rs

@@ -1005,7 +1005,8 @@ fn windows_job_terminate_denied_falls_back_to_child_kill() {
         "limited job handle should not allow TerminateJobObject"
     );
 
-    terminate_windows_job(Some(&limited_job), &mut child).expect("fallback child kill");
+    terminate_child_and_close_windows_job(Some(limited_job), &mut child)
+        .expect("fallback child kill");
 
     let status = child
         .wait_timeout(std::time::Duration::from_secs(3))
@@ -1016,6 +1017,54 @@ fn windows_job_terminate_denied_falls_back_to_child_kill() {
     );
 }
 
+#[cfg(windows)]
+#[test]
+fn windows_job_close_releases_foreground_reader_threads_when_terminate_denied() {
+    let mut child = Command::new("ping")
+        .args(["127.0.0.1", "-n", "8"])
+        .stdin(Stdio::null())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()
+        .expect("spawn ping");
+
+    let job = WindowsJob::attach_to_child(&child).expect("attach job");
+    let limited_job = duplicate_job_without_terminate_access(job);
+    assert!(
+        limited_job.terminate().is_err(),
+        "limited job handle should not allow TerminateJobObject"
+    );
+
+    let stdout_handle = child.stdout.take().expect("stdout pipe");
+    let stderr_handle = child.stderr.take().expect("stderr pipe");
+    let stdout_thread = std::thread::spawn(move || {
+        let mut reader = stdout_handle;
+        let mut buf = Vec::new();
+        let _ = reader.read_to_end(&mut buf);
+        buf
+    });
+    let stderr_thread = std::thread::spawn(move || {
+        let mut reader = stderr_handle;
+        let mut buf = Vec::new();
+        let _ = reader.read_to_end(&mut buf);
+        buf
+    });
+
+    let started = std::time::Instant::now();
+    terminate_and_close_windows_job(Some(limited_job));
+    let _ = stdout_thread.join().unwrap_or_default();
+    let _ = stderr_thread.join().unwrap_or_default();
+    let status = child
+        .wait_timeout(std::time::Duration::from_secs(3))
+        .expect("wait after kill-on-close");
+
+    assert!(
+        started.elapsed() < std::time::Duration::from_secs(4),
+        "reader joins waited for natural descendant exit instead of kill-on-close"
+    );
+    assert!(status.is_some(), "kill-on-close should terminate child");
+}
+
 #[cfg(windows)]
 #[test]
 fn windows_job_kill_on_close_releases_reader_threads_when_terminate_denied() {