diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index 7f46d2da..f21b42ad 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -298,9 +298,31 @@ fn windows_io_error(error: windows::core::Error) -> std::io::Error { #[cfg(windows)] fn terminate_windows_job(job: Option<&WindowsJob>, child: &mut Child) -> std::io::Result<()> { if let Some(job) = job { - job.terminate() - } else { - child.kill() + match job.terminate() { + Ok(()) => return Ok(()), + Err(error) => { + tracing::warn!( + ?error, + "failed to terminate Windows job object; falling back to immediate child kill" + ); + } + } + } + child.kill() +} + +#[cfg(windows)] +fn attach_windows_job(child: &Child, command: &str) -> Option { + match WindowsJob::attach_to_child(child) { + Ok(job) => Some(job), + Err(error) => { + tracing::warn!( + ?error, + command, + "failed to attach Windows shell process to job object; descendant cleanup degraded" + ); + None + } } } @@ -982,7 +1004,7 @@ impl ShellManager { .spawn() .with_context(|| format!("Failed to execute: {original_command}"))?; #[cfg(windows)] - let windows_job = WindowsJob::attach_to_child(&child).ok(); + let windows_job = attach_windows_job(&child, original_command); if let Some(input) = stdin_data && let Some(mut stdin) = child.stdin.take() @@ -1146,7 +1168,7 @@ impl ShellManager { .spawn() .with_context(|| format!("Failed to execute: {original_command}"))?; #[cfg(windows)] - let windows_job = WindowsJob::attach_to_child(&child).ok(); + let windows_job = attach_windows_job(&child, original_command); if let Some(status) = child.wait_timeout(timeout)? { #[cfg(windows)] @@ -1298,7 +1320,7 @@ impl ShellManager { .with_context(|| format!("Failed to spawn background: {original_command}"))?; #[cfg(windows)] { - windows_job = WindowsJob::attach_to_child(&child).ok(); + windows_job = attach_windows_job(&child, original_command); } let stdout_handle = child.stdout.take().context("Failed to capture stdout")?; diff --git a/crates/tui/src/tools/shell/tests.rs b/crates/tui/src/tools/shell/tests.rs index 4894df9b..5a3ed92c 100644 --- a/crates/tui/src/tools/shell/tests.rs +++ b/crates/tui/src/tools/shell/tests.rs @@ -934,7 +934,7 @@ fn background_collection_does_not_block_on_detached_descendant_pipe() { let result = manager .execute( - r#"cmd /c start "" /b ping 127.0.0.1 -n 8"#, + r#"cmd /c start "" /b ping 127.0.0.1 -n 4"#, None, 5000, true, @@ -948,7 +948,7 @@ fn background_collection_does_not_block_on_detached_descendant_pipe() { .expect("get_output must complete, not hang"); assert!( - started.elapsed() < std::time::Duration::from_secs(3), + started.elapsed() < std::time::Duration::from_secs(6), "get_output blocked on descendant pipe handles" ); assert_eq!(done.status, ShellStatus::Completed);