fix(shell): kill process group in collect_output to prevent UI freeze

Cherry-picked from PR #1475 by CrepuscularIRIS (autoghclaw/issue-828).

When a shell command spawns background subprocesses (nohup, sleep &, etc.),
those subprocesses inherit the pipe write-ends. After the shell exits, the
subprocesses keep those write-ends open, causing handle.join() on reader
threads to block indefinitely in read(). Since list_jobs() calls poll() →
collect_output() on every TUI render tick, the entire UI event loop blocks.

Fix: kill the process group (PGID = child PID) before joining reader threads,
so orphaned subprocesses release their pipe write-ends. Also wires the
previously dead-coded cleanup() into list_jobs() with a 1-hour eviction
window to bound process table growth.

Fixes #828.

crates/tui/src/tools/shell.rs

@@ -329,6 +329,15 @@ impl BackgroundShell {
 
     /// Collect output from the background threads
     fn collect_output(&mut self) {
+        // Kill the whole process group before joining reader threads.
+        // When the shell spawned persistent background jobs (e.g. `nohup curl`),
+        // those subprocesses keep the pipe write-ends open after the shell exits.
+        // Without this kill, handle.join() blocks indefinitely, freezing the UI
+        // event loop that calls list_jobs() → poll() → collect_output().
+        #[cfg(unix)]
+        if let Some(ShellChild::Process(ref mut proc)) = self.child {
+            let _ = kill_child_process_group(proc);
+        }
         if let Some(handle) = self.stdout_thread.take() {
             let _ = handle.join();
         }
@@ -1299,6 +1308,8 @@ impl ShellManager {
         for shell in self.processes.values_mut() {
             shell.poll();
         }
+        // Evict completed processes older than 1 hour to bound memory growth.
+        self.cleanup(Duration::from_secs(3600));
 
         let mut jobs = self
             .processes
@@ -1346,7 +1357,6 @@ impl ShellManager {
     }
 
     /// Clean up completed processes older than the given duration
-    #[allow(dead_code)]
     pub fn cleanup(&mut self, max_age: Duration) {
         let _now = Instant::now();
         self.processes.retain(|_, shell| {

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

@@ -743,3 +743,49 @@ fn test_macos_provenance_not_triggered_on_unrelated_eperm() {
     let result = make_failed_result("open /some/other/path: operation not permitted");
     assert!(!looks_like_macos_provenance_failure(&result));
 }
+
+// Regression test for #828: shell spawns an orphaned background subprocess
+// (simulating `nohup curl`) that keeps the pipe write-end open after the shell
+// exits. collect_output() must not block indefinitely — it kills the whole
+// process group first, allowing reader threads to get EOF and exit.
+#[cfg(unix)]
+#[test]
+fn test_orphaned_subprocess_does_not_block_collect_output() {
+    let tmp = tempdir().expect("tempdir");
+    let mut manager = ShellManager::new(tmp.path().to_path_buf());
+
+    // sh spawns `sleep 100 &` and exits; the sleep subprocess inherits the
+    // pipe write-ends and would keep reader threads blocked without the fix.
+    let result = manager
+        .execute("sh -c 'sleep 100 &'", None, 5000, true)
+        .expect("execute");
+    let task_id = result.task_id.expect("task id");
+
+    // Drive to completion with a tight timeout — must not hang.
+    let done = manager
+        .get_output(&task_id, true, 3000)
+        .expect("get_output must complete, not hang");
+    assert_eq!(done.status, ShellStatus::Completed);
+}
+
+#[test]
+fn test_list_jobs_cleans_up_completed_old_processes() {
+    let tmp = tempdir().expect("tempdir");
+    let mut manager = ShellManager::new(tmp.path().to_path_buf());
+
+    let bg = manager
+        .execute(&echo_command("bg"), None, 5000, true)
+        .expect("execute bg");
+    let bg_id = bg.task_id.expect("bg task id");
+    manager.get_output(&bg_id, true, 3000).expect("bg done");
+
+    // Both the completed job and any tracking state should be present.
+    assert!(!manager.processes.is_empty());
+
+    // cleanup(ZERO) removes all completed processes immediately.
+    manager.cleanup(Duration::ZERO);
+    assert!(
+        manager.processes.is_empty(),
+        "completed processes should be evicted by cleanup"
+    );
+}