From 578f194279a528064ab216ac791938f68e0d446f Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Mon, 9 Mar 2026 23:45:47 -0400 Subject: [PATCH 1/8] fix(exec): detach background helpers from the tty --- code-rs/core/src/codex/session.rs | 2 +- code-rs/core/src/spawn.rs | 98 +++++++++++++++++++++++++++ code-rs/core/src/user_notification.rs | 2 +- code-rs/tui/src/chatwidget.rs | 14 ++-- 4 files changed, 107 insertions(+), 9 deletions(-) diff --git a/code-rs/core/src/codex/session.rs b/code-rs/core/src/codex/session.rs index 2561dfffcf0..4f4a0bbfa49 100644 --- a/code-rs/core/src/codex/session.rs +++ b/code-rs/core/src/codex/session.rs @@ -2210,7 +2210,7 @@ impl Session { command.arg(json); // Fire-and-forget – we do not wait for completion. - if let Err(e) = crate::spawn::spawn_std_command_with_retry(&mut command) { + if let Err(e) = crate::spawn::spawn_background_command_with_retry(&mut command) { warn!("failed to spawn notifier '{}': {e}", notify_command[0]); } } diff --git a/code-rs/core/src/spawn.rs b/code-rs/core/src/spawn.rs index f2a744cd92c..dc129f5e738 100644 --- a/code-rs/core/src/spawn.rs +++ b/code-rs/core/src/spawn.rs @@ -80,6 +80,34 @@ pub fn spawn_std_command_with_retry( spawn_with_retry_blocking(|| cmd.spawn()) } +/// Spawn a fire-and-forget helper without sharing this process's controlling +/// terminal. This avoids job-control collisions with the TUI when background +/// helpers are launched from interactive sessions. +pub fn spawn_background_command_with_retry( + cmd: &mut std::process::Command, +) -> io::Result { + cmd.stdin(Stdio::null()); + + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + + unsafe { + cmd.pre_exec(|| { + if libc::setsid() == -1 { + let err = io::Error::last_os_error(); + if err.raw_os_error() != Some(libc::EPERM) { + return Err(err); + } + } + Ok(()) + }); + } + } + + spawn_std_command_with_retry(cmd) +} + pub async fn spawn_tokio_command_with_retry(cmd: &mut Command) -> io::Result { spawn_with_retry_async(|| cmd.spawn()).await } @@ -180,3 +208,73 @@ pub(crate) async fn spawn_child_async( spawn_tokio_command_with_retry(&mut cmd).await } + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(unix)] + static STDIN_GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + #[cfg(unix)] + struct StdinRedirectGuard { + saved_stdin_fd: i32, + read_fd: i32, + write_fd: i32, + } + + #[cfg(unix)] + impl StdinRedirectGuard { + fn install_pipe_as_stdin() -> Self { + let mut fds = [0; 2]; + assert_eq!(unsafe { libc::pipe(fds.as_mut_ptr()) }, 0, "pipe"); + let saved_stdin_fd = unsafe { libc::dup(libc::STDIN_FILENO) }; + assert!(saved_stdin_fd >= 0, "dup stdin"); + assert_eq!(unsafe { libc::dup2(fds[0], libc::STDIN_FILENO) }, libc::STDIN_FILENO, "dup2 stdin"); + Self { + saved_stdin_fd, + read_fd: fds[0], + write_fd: fds[1], + } + } + } + + #[cfg(unix)] + impl Drop for StdinRedirectGuard { + fn drop(&mut self) { + unsafe { + let _ = libc::dup2(self.saved_stdin_fd, libc::STDIN_FILENO); + let _ = libc::close(self.saved_stdin_fd); + let _ = libc::close(self.read_fd); + let _ = libc::close(self.write_fd); + } + } + } + + #[cfg(unix)] + #[test] + fn background_spawn_redirects_stdin_away_from_parent_terminal() { + let _guard = STDIN_GUARD.lock().expect("stdin test mutex"); + let _stdin_guard = StdinRedirectGuard::install_pipe_as_stdin(); + + let mut cmd = std::process::Command::new("python3"); + cmd.arg("-c") + .arg("import sys; data = sys.stdin.read(); print('eof' if data == '' else 'data')") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let mut child = spawn_background_command_with_retry(&mut cmd).expect("spawn background helper"); + let deadline = std::time::Instant::now() + Duration::from_secs(2); + loop { + if let Some(_status) = child.try_wait().expect("poll child") { + break; + } + assert!(std::time::Instant::now() < deadline, "background helper should not block on inherited stdin"); + std::thread::sleep(Duration::from_millis(10)); + } + + let output = child.wait_with_output().expect("wait with output"); + assert!(output.status.success(), "child should exit successfully: {output:?}"); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "eof"); + } +} diff --git a/code-rs/core/src/user_notification.rs b/code-rs/core/src/user_notification.rs index f3c4f93dcc5..c79d405c93e 100644 --- a/code-rs/core/src/user_notification.rs +++ b/code-rs/core/src/user_notification.rs @@ -31,7 +31,7 @@ impl UserNotifier { command.arg(json); // Fire-and-forget – we do not wait for completion. - if let Err(e) = crate::spawn::spawn_std_command_with_retry(&mut command) { + if let Err(e) = crate::spawn::spawn_background_command_with_retry(&mut command) { warn!("failed to spawn notifier '{}': {e}", notify_command[0]); } } diff --git a/code-rs/tui/src/chatwidget.rs b/code-rs/tui/src/chatwidget.rs index 99db08be7b0..e8c3c8b6b6a 100644 --- a/code-rs/tui/src/chatwidget.rs +++ b/code-rs/tui/src/chatwidget.rs @@ -43,7 +43,7 @@ use code_core::config_types::Notifications; use code_core::config_types::ReasoningEffort; use code_core::config_types::ServiceTier; use code_core::config_types::TextVerbosity; -use code_core::spawn::spawn_std_command_with_retry; +use code_core::spawn::spawn_background_command_with_retry; use code_core::plan_tool::{PlanItemArg, StepStatus, UpdatePlanArgs}; use code_core::model_family::derive_default_model_family; use code_core::model_family::find_family_for_model; @@ -27106,7 +27106,7 @@ Have we met every part of this goal and is there no further work to do?"# .stderr(Stdio::null()) .stdin(Stdio::null()); self.apply_chrome_logging(&mut cmd, log_path.as_deref()); - if let Err(err) = spawn_std_command_with_retry(&mut cmd) { + if let Err(err) = spawn_background_command_with_retry(&mut cmd) { tracing::warn!("failed to launch Chrome with profile: {err}"); } } @@ -27128,7 +27128,7 @@ Have we met every part of this goal and is there no further work to do?"# .stderr(Stdio::null()) .stdin(Stdio::null()); self.apply_chrome_logging(&mut cmd, log_path.as_deref()); - if let Err(err) = spawn_std_command_with_retry(&mut cmd) { + if let Err(err) = spawn_background_command_with_retry(&mut cmd) { tracing::warn!("failed to launch Chrome with profile: {err}"); } } @@ -27161,7 +27161,7 @@ Have we met every part of this goal and is there no further work to do?"# .stderr(Stdio::null()) .stdin(Stdio::null()); self.apply_chrome_logging(&mut cmd, log_path.as_deref()); - if let Err(err) = spawn_std_command_with_retry(&mut cmd) { + if let Err(err) = spawn_background_command_with_retry(&mut cmd) { tracing::warn!("failed to launch Chrome with profile: {err}"); } break; @@ -27761,7 +27761,7 @@ Have we met every part of this goal and is there no further work to do?"# .stderr(Stdio::null()) .stdin(Stdio::null()); self.apply_chrome_logging(&mut cmd, log_path.as_deref()); - if let Err(err) = spawn_std_command_with_retry(&mut cmd) { + if let Err(err) = spawn_background_command_with_retry(&mut cmd) { tracing::warn!("failed to launch Chrome with temp profile: {err}"); } } @@ -27784,7 +27784,7 @@ Have we met every part of this goal and is there no further work to do?"# .stderr(Stdio::null()) .stdin(Stdio::null()); self.apply_chrome_logging(&mut cmd, log_path.as_deref()); - if let Err(err) = spawn_std_command_with_retry(&mut cmd) { + if let Err(err) = spawn_background_command_with_retry(&mut cmd) { tracing::warn!("failed to launch Chrome with temp profile: {err}"); } } @@ -27818,7 +27818,7 @@ Have we met every part of this goal and is there no further work to do?"# .stderr(Stdio::null()) .stdin(Stdio::null()); self.apply_chrome_logging(&mut cmd, log_path.as_deref()); - if let Err(err) = spawn_std_command_with_retry(&mut cmd) { + if let Err(err) = spawn_background_command_with_retry(&mut cmd) { tracing::warn!("failed to launch Chrome with temp profile: {err}"); } break; From 349a8a6d124a44ef47966b6a05c579da91e49d36 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Tue, 10 Mar 2026 00:29:44 -0400 Subject: [PATCH 2/8] fix(agent): detach read-only agent stdin --- code-rs/core/src/agent_tool.rs | 98 ++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/code-rs/core/src/agent_tool.rs b/code-rs/core/src/agent_tool.rs index 7c3f02365da..6d4a1de6dba 100644 --- a/code-rs/core/src/agent_tool.rs +++ b/code-rs/core/src/agent_tool.rs @@ -1895,6 +1895,7 @@ async fn execute_model_with_permissions( } cmd.args(final_args.clone()); + cmd.stdin(Stdio::null()); cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); for (k, v) in &env { @@ -2829,6 +2830,44 @@ mod tests { use std::path::PathBuf; use std::sync::{Mutex, OnceLock}; + #[cfg(unix)] + static STDIN_LOCK: Mutex<()> = Mutex::new(()); + + #[cfg(unix)] + struct StdinRedirectGuard { + saved_stdin_fd: i32, + read_fd: i32, + write_fd: i32, + } + + #[cfg(unix)] + impl StdinRedirectGuard { + fn install_pipe_as_stdin() -> Self { + let mut fds = [0; 2]; + assert_eq!(unsafe { libc::pipe(fds.as_mut_ptr()) }, 0, "pipe"); + let saved_stdin_fd = unsafe { libc::dup(libc::STDIN_FILENO) }; + assert!(saved_stdin_fd >= 0, "dup stdin"); + assert_eq!(unsafe { libc::dup2(fds[0], libc::STDIN_FILENO) }, libc::STDIN_FILENO, "dup2 stdin"); + Self { + saved_stdin_fd, + read_fd: fds[0], + write_fd: fds[1], + } + } + } + + #[cfg(unix)] + impl Drop for StdinRedirectGuard { + fn drop(&mut self) { + unsafe { + assert_eq!(libc::dup2(self.saved_stdin_fd, libc::STDIN_FILENO), libc::STDIN_FILENO, "restore stdin"); + libc::close(self.saved_stdin_fd); + libc::close(self.read_fd); + libc::close(self.write_fd); + } + } + } + #[test] fn drops_empty_names() { assert_eq!(normalize_agent_name(None), None); @@ -2956,6 +2995,41 @@ mod tests { assert_eq!(output.trim(), "current"); } + #[cfg(unix)] + #[tokio::test] + async fn read_only_agents_redirect_stdin_away_from_parent_pipe() { + let _env_lock = env_lock().lock().expect("env lock"); + let _stdin_lock = STDIN_LOCK.lock().expect("stdin lock"); + let _reset_binary = EnvReset::capture("CODE_BINARY_PATH"); + + let dir = tempdir().expect("tempdir"); + let current = script_path(dir.path(), "current"); + write_stdin_mode_script(¤t); + + let _stdin_guard = StdinRedirectGuard::install_pipe_as_stdin(); + + unsafe { + std::env::set_var("CODE_BINARY_PATH", ¤t); + } + + let output = execute_model_with_permissions( + "agent-test", + "code-gpt-5.3-codex", + "ok", + true, + None, + None, + ReasoningEffort::Low, + None, + None, + None, + ) + .await + .expect("execute read-only agent"); + + assert_eq!(output.trim(), "detached"); + } + #[cfg(not(target_os = "windows"))] #[tokio::test] async fn claude_agent_uses_local_install_when_not_on_path() { @@ -3071,6 +3145,30 @@ mod tests { std::fs::set_permissions(path, perms).expect("chmod script"); } + #[cfg(unix)] + fn write_stdin_mode_script(path: &Path) { + let script = r#"#!/bin/sh +python3 - <<'PY' +import os +import stat + +mode = os.fstat(0).st_mode +if stat.S_ISFIFO(mode): + print("fifo") +else: + print("detached") +PY +exit 0 +"#; + std::fs::write(path, script).expect("write stdin mode script"); + let mut perms = std::fs::metadata(path) + .expect("script metadata") + .permissions(); + use std::os::unix::fs::PermissionsExt; + perms.set_mode(0o755); + std::fs::set_permissions(path, perms).expect("chmod script"); + } + #[test] fn gemini_config_dir_is_injected_when_missing_api_key() { let tmp = tempfile::tempdir().expect("tempdir"); From 60373fd538b9c5f8757705c9ae352f419e4848ae Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Tue, 10 Mar 2026 09:12:22 -0400 Subject: [PATCH 3/8] fix(tui): avoid tty reads outside foreground --- code-rs/tui/src/app/init.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/code-rs/tui/src/app/init.rs b/code-rs/tui/src/app/init.rs index 37a02999ecd..60d86147f72 100644 --- a/code-rs/tui/src/app/init.rs +++ b/code-rs/tui/src/app/init.rs @@ -28,6 +28,20 @@ use crate::tui::TerminalInfo; use super::state::{App, AppState, ChatWidgetArgs, FrameTimer}; +#[cfg(unix)] +fn stdin_is_foreground_process_group() -> bool { + // Avoid SIGTTIN from the input loop if some earlier terminal handoff left + // the TUI out of the foreground process group. In that case we should wait + // for foreground ownership to be restored instead of touching stdin. + unsafe { + let stdin_pgrp = libc::tcgetpgrp(libc::STDIN_FILENO); + if stdin_pgrp == -1 { + return true; + } + stdin_pgrp == libc::getpgrp() + } +} + impl App<'_> { pub(crate) fn new( config: Config, @@ -138,6 +152,11 @@ impl App<'_> { std::thread::sleep(Duration::from_millis(10)); continue; } + #[cfg(unix)] + if !stdin_is_foreground_process_group() { + std::thread::sleep(Duration::from_millis(10)); + continue; + } // This timeout is necessary to avoid holding the event lock // that crossterm::event::read() acquires. In particular, // reading the cursor position (crossterm::cursor::position()) From 672368e32f58f2427ff8f3b0953cb5d92cddd162 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Tue, 10 Mar 2026 16:41:41 -0400 Subject: [PATCH 4/8] fix(tui): prefer auto-drive summary before next prompt --- code-rs/tui/src/chatwidget.rs | 55 ++++++++++++++++++++++++++++--- code-rs/tui/src/chatwidget/esc.rs | 8 ++--- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/code-rs/tui/src/chatwidget.rs b/code-rs/tui/src/chatwidget.rs index e8c3c8b6b6a..7f80f87f226 100644 --- a/code-rs/tui/src/chatwidget.rs +++ b/code-rs/tui/src/chatwidget.rs @@ -20515,10 +20515,6 @@ Have we met every part of this goal and is there no further work to do?"# fn auto_rebuild_live_ring(&mut self) { if !self.auto_state.is_active() { - if self.auto_state.should_show_goal_entry() { - self.auto_show_goal_entry_panel(); - return; - } if let Some(summary) = self.auto_state.last_run_summary.clone() { self.bottom_pane.clear_live_ring(); self.auto_reset_intro_timing(); @@ -20570,6 +20566,11 @@ Have we met every part of this goal and is there no further work to do?"# return; } + if self.auto_state.should_show_goal_entry() { + self.auto_show_goal_entry_panel(); + return; + } + self.bottom_pane.clear_auto_coordinator_view(true); self.bottom_pane.clear_live_ring(); self.bottom_pane.set_standard_terminal_hint(None); @@ -33399,6 +33400,52 @@ use code_core::protocol::OrderMeta; assert_eq!(route.intent, EscIntent::AutoDismissSummary); } + #[test] + fn completed_auto_drive_prefers_summary_before_next_goal_prompt() { + let mut harness = ChatWidgetHarness::new(); + let chat = harness.chat(); + + chat.auto_state.last_run_summary = Some(AutoRunSummary { + duration: Duration::from_secs(42), + turns_completed: 3, + message: Some("All tasks done.".to_string()), + goal: Some("Finish feature".to_string()), + }); + chat.auto_state.set_phase(AutoRunPhase::AwaitingGoalEntry); + + chat.auto_rebuild_live_ring(); + + let model = chat + .bottom_pane + .auto_view_model() + .expect("auto coordinator view should be active"); + let AutoCoordinatorViewModel::Active(active) = model; + assert_eq!(active.goal.as_deref(), Some("Finish feature")); + assert_eq!(active.status_lines, vec!["All tasks done.".to_string()]); + assert!(active.ctrl_switch_hint.contains("Esc")); + + assert!(chat.auto_state.should_show_goal_entry()); + assert!(chat.auto_state.last_run_summary.is_some()); + + let route = chat.describe_esc_context(); + assert_eq!(route.intent, EscIntent::AutoDismissSummary); + assert!(chat.execute_esc_intent( + route.intent, + KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE), + )); + + let model = chat + .bottom_pane + .auto_view_model() + .expect("auto coordinator view should show goal entry after dismissing summary"); + let AutoCoordinatorViewModel::Active(active) = model; + assert!(active.goal.is_none()); + assert_eq!( + active.status_lines, + vec!["Let's do this! What's your goal?".to_string()] + ); + } + #[test] fn goal_entry_typing_arms_escape_state() { let mut harness = ChatWidgetHarness::new(); diff --git a/code-rs/tui/src/chatwidget/esc.rs b/code-rs/tui/src/chatwidget/esc.rs index 67b5c660ea9..ab215c8cf05 100644 --- a/code-rs/tui/src/chatwidget/esc.rs +++ b/code-rs/tui/src/chatwidget/esc.rs @@ -152,6 +152,10 @@ impl ChatWidget<'_> { return EscRoute::new(EscIntent::CancelAgents, true, false); } + if self.auto_state.last_run_summary.is_some() { + return EscRoute::new(EscIntent::AutoDismissSummary, true, false); + } + if self.auto_state.should_show_goal_entry() { return EscRoute::new( match self.auto_goal_escape_state { @@ -164,10 +168,6 @@ impl ChatWidget<'_> { ); } - if self.auto_state.last_run_summary.is_some() { - return EscRoute::new(EscIntent::AutoDismissSummary, true, false); - } - if self.auto_manual_entry_active() && !self.composer_is_empty() { return EscRoute::new(EscIntent::ClearComposer, true, false); } From b5bd6c293f929aaba1be3e6588951bf3170ba9a5 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Tue, 10 Mar 2026 18:51:08 -0400 Subject: [PATCH 5/8] fix(tui): preserve drafted goal when dismissing summary --- code-rs/tui/src/chatwidget.rs | 65 ++++++++++++++++++++++++++++++- code-rs/tui/src/chatwidget/esc.rs | 7 ++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/code-rs/tui/src/chatwidget.rs b/code-rs/tui/src/chatwidget.rs index 7f80f87f226..2c4a948b055 100644 --- a/code-rs/tui/src/chatwidget.rs +++ b/code-rs/tui/src/chatwidget.rs @@ -18570,6 +18570,10 @@ fi\n\ } fn auto_show_goal_entry_panel(&mut self) { + self.auto_show_goal_entry_panel_with_draft(false); + } + + fn auto_show_goal_entry_panel_with_draft(&mut self, preserve_draft: bool) { self.auto_state.set_phase(AutoRunPhase::AwaitingGoalEntry); self.auto_state.goal = None; self.auto_pending_goal_request = false; @@ -18579,7 +18583,9 @@ fi\n\ self.auto_reset_intro_timing(); self.auto_ensure_intro_timing(); } - self.auto_goal_escape_state = AutoGoalEscState::Inactive; + if !preserve_draft { + self.auto_goal_escape_state = AutoGoalEscState::Inactive; + } let hint = "Let's do this! What's your goal?".to_string(); let status_lines = vec![hint]; let model = AutoCoordinatorViewModel::Active(AutoActiveViewModel { @@ -18613,7 +18619,11 @@ fi\n\ self.bottom_pane.update_status_text("Auto Drive".to_string()); self.auto_update_terminal_hint(); self.bottom_pane.ensure_input_focus(); - self.clear_composer(); + if preserve_draft { + self.auto_sync_goal_escape_state_from_composer(); + } else { + self.clear_composer(); + } self.request_redraw(); } @@ -33446,6 +33456,57 @@ use code_core::protocol::OrderMeta; ); } + #[test] + fn dismissing_completed_summary_preserves_typed_next_goal() { + let mut harness = ChatWidgetHarness::new(); + let chat = harness.chat(); + + chat.auto_state.last_run_summary = Some(AutoRunSummary { + duration: Duration::from_secs(42), + turns_completed: 3, + message: Some("All tasks done.".to_string()), + goal: Some("Finish feature".to_string()), + }); + chat.auto_state.set_phase(AutoRunPhase::AwaitingGoalEntry); + chat.auto_rebuild_live_ring(); + chat.handle_paste("Suggested goal".to_string()); + + assert_eq!(chat.bottom_pane.composer_text(), "Suggested goal"); + assert!(matches!( + chat.auto_goal_escape_state, + AutoGoalEscState::NeedsEnableEditing + )); + + let route = chat.describe_esc_context(); + assert_eq!(route.intent, EscIntent::AutoDismissSummary); + assert!(chat.execute_esc_intent( + route.intent, + KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE), + )); + + assert!(chat.auto_state.last_run_summary.is_none()); + assert!(chat.auto_state.should_show_goal_entry()); + assert_eq!(chat.bottom_pane.composer_text(), "Suggested goal"); + assert!(matches!( + chat.auto_goal_escape_state, + AutoGoalEscState::NeedsEnableEditing + )); + + let model = chat + .bottom_pane + .auto_view_model() + .expect("auto coordinator view should show preserved goal draft"); + let AutoCoordinatorViewModel::Active(active) = model; + assert!(active.goal.is_none()); + assert_eq!( + active.status_lines, + vec!["Let's do this! What's your goal?".to_string()] + ); + + let route = chat.describe_esc_context(); + assert_eq!(route.intent, EscIntent::AutoGoalEnableEdit); + } + #[test] fn goal_entry_typing_arms_escape_state() { let mut harness = ChatWidgetHarness::new(); diff --git a/code-rs/tui/src/chatwidget/esc.rs b/code-rs/tui/src/chatwidget/esc.rs index ab215c8cf05..0346fadb1af 100644 --- a/code-rs/tui/src/chatwidget/esc.rs +++ b/code-rs/tui/src/chatwidget/esc.rs @@ -89,11 +89,18 @@ impl ChatWidget<'_> { self.request_redraw(); return; } + let should_restore_goal_entry = self.auto_state.should_show_goal_entry(); + let preserve_goal_draft = should_restore_goal_entry + && !self.bottom_pane.composer_text().trim().is_empty(); self.auto_state.last_run_summary = None; self.bottom_pane.clear_auto_coordinator_view(true); self.bottom_pane.clear_live_ring(); self.bottom_pane.set_standard_terminal_hint(None); self.bottom_pane.ensure_input_focus(); + if should_restore_goal_entry { + self.auto_show_goal_entry_panel_with_draft(preserve_goal_draft); + return; + } self.auto_rebuild_live_ring(); self.request_redraw(); } From 05302c9211208bc4adbc61031c3f606438b3e502 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Wed, 11 Mar 2026 12:47:33 -0400 Subject: [PATCH 6/8] fix(exec): start redirected shell tools in a new session --- code-rs/core/src/spawn.rs | 89 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/code-rs/core/src/spawn.rs b/code-rs/core/src/spawn.rs index dc129f5e738..90967f11f31 100644 --- a/code-rs/core/src/spawn.rs +++ b/code-rs/core/src/spawn.rs @@ -27,6 +27,27 @@ pub const CODEX_SANDBOX_ENV_VAR: &str = "CODEX_SANDBOX"; const SPAWN_RETRY_DELAYS_MS: [u64; 3] = [0, 10, 50]; +#[cfg(unix)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum UnixChildSessionStrategy { + NewSession, + NewProcessGroup, +} + +#[cfg(unix)] +fn unix_child_session_strategy(stdio_policy: StdioPolicy) -> UnixChildSessionStrategy { + match stdio_policy { + // Shell tool commands are non-interactive, but they may launch their own + // long-lived descendants (for example via `nohup ... &`). Starting the + // shell tool in a new session ensures those descendants cannot retain + // the TUI's controlling terminal and steal foreground ownership. + StdioPolicy::RedirectForShellTool => UnixChildSessionStrategy::NewSession, + // Interactive children should keep terminal semantics while still being + // isolated in their own process group for targeted signal handling. + StdioPolicy::Inherit => UnixChildSessionStrategy::NewProcessGroup, + } +} + fn is_temporary_resource_error(err: &io::Error) -> bool { err.kind() == io::ErrorKind::WouldBlock || matches!(err.raw_os_error(), Some(35) | Some(libc::ENOMEM)) @@ -163,9 +184,24 @@ pub(crate) async fn spawn_child_async( StdioPolicy::RedirectForShellTool => crate::cgroup::default_exec_memory_max_bytes(), StdioPolicy::Inherit => None, }; + #[cfg(unix)] + let session_strategy = unix_child_session_strategy(stdio_policy); cmd.pre_exec(move || { - // Start a new process group - let _ = libc::setpgid(0, 0); + #[cfg(unix)] + match session_strategy { + UnixChildSessionStrategy::NewSession => { + if libc::setsid() == -1 { + let err = std::io::Error::last_os_error(); + if err.raw_os_error() != Some(libc::EPERM) { + return Err(err); + } + } + } + UnixChildSessionStrategy::NewProcessGroup => { + // Start a new process group. + let _ = libc::setpgid(0, 0); + } + } #[cfg(target_os = "linux")] { if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { @@ -212,6 +248,7 @@ pub(crate) async fn spawn_child_async( #[cfg(test)] mod tests { use super::*; + use crate::protocol::SandboxPolicy; #[cfg(unix)] static STDIN_GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(()); @@ -277,4 +314,52 @@ mod tests { assert!(output.status.success(), "child should exit successfully: {output:?}"); assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "eof"); } + + #[cfg(unix)] + #[test] + fn redirect_for_shell_tool_uses_new_session_strategy() { + assert_eq!( + unix_child_session_strategy(StdioPolicy::RedirectForShellTool), + UnixChildSessionStrategy::NewSession, + ); + assert_eq!( + unix_child_session_strategy(StdioPolicy::Inherit), + UnixChildSessionStrategy::NewProcessGroup, + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn redirect_for_shell_tool_detaches_from_controlling_tty() { + let parent_tty = match std::fs::OpenOptions::new().read(true).open("/dev/tty") { + Ok(tty) => tty, + Err(_) => return, + }; + + drop(parent_tty); + + let child = spawn_child_async( + PathBuf::from("python3"), + vec![ + "-c".to_string(), + "import os\ntry:\n os.open('/dev/tty', os.O_RDONLY)\n print('tty-present')\nexcept OSError as e:\n print(f'tty-missing:{e.errno}')".to_string(), + ], + None, + std::env::current_dir().expect("cwd"), + &SandboxPolicy::DangerFullAccess, + StdioPolicy::RedirectForShellTool, + HashMap::new(), + ) + .await + .expect("spawn shell tool child"); + + let output = child.wait_with_output().await.expect("wait for child"); + assert!(output.status.success(), "child should exit successfully: {output:?}"); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.trim_start().starts_with("tty-missing:"), + "shell tool child should not retain a controlling tty: {stdout:?}" + ); + } } From 4a791d540bead241e95ced6b716183ad1b7b8d62 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Wed, 11 Mar 2026 13:44:50 -0400 Subject: [PATCH 7/8] fix(tui): reclaim stdin from dead foreground groups --- code-rs/tui/src/app/init.rs | 71 ++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/code-rs/tui/src/app/init.rs b/code-rs/tui/src/app/init.rs index 60d86147f72..69f1bc20efa 100644 --- a/code-rs/tui/src/app/init.rs +++ b/code-rs/tui/src/app/init.rs @@ -28,6 +28,46 @@ use crate::tui::TerminalInfo; use super::state::{App, AppState, ChatWidgetArgs, FrameTimer}; +#[cfg(unix)] +fn process_group_exists(pgid: libc::pid_t) -> bool { + if pgid <= 0 { + return false; + } + + // `kill(-pgid, 0)` checks whether any process in the process group still + // exists without sending a signal. `EPERM` still means the group exists. + let result = unsafe { libc::kill(-pgid, 0) }; + if result == 0 { + return true; + } + + let err = std::io::Error::last_os_error(); + matches!(err.raw_os_error(), Some(libc::EPERM)) +} + +#[cfg(unix)] +fn reclaim_stdin_foreground_process_group(target_pgrp: libc::pid_t) -> bool { + if target_pgrp <= 0 { + return false; + } + + unsafe { + let mut block_set = std::mem::zeroed(); + let mut previous_set = std::mem::zeroed(); + libc::sigemptyset(&mut block_set); + libc::sigaddset(&mut block_set, libc::SIGTTOU); + + if libc::pthread_sigmask(libc::SIG_BLOCK, &block_set, &mut previous_set) != 0 { + return false; + } + + let reclaimed = libc::tcsetpgrp(libc::STDIN_FILENO, target_pgrp) == 0; + let _ = libc::pthread_sigmask(libc::SIG_SETMASK, &previous_set, std::ptr::null_mut()); + + reclaimed && libc::tcgetpgrp(libc::STDIN_FILENO) == target_pgrp + } +} + #[cfg(unix)] fn stdin_is_foreground_process_group() -> bool { // Avoid SIGTTIN from the input loop if some earlier terminal handoff left @@ -38,7 +78,36 @@ fn stdin_is_foreground_process_group() -> bool { if stdin_pgrp == -1 { return true; } - stdin_pgrp == libc::getpgrp() + let self_pgrp = libc::getpgrp(); + if stdin_pgrp == self_pgrp { + return true; + } + + // Some earlier noninteractive descendant can steal the tty foreground + // process group and then exit, leaving stdin pointed at a dead process + // group forever. In that case, reclaim the tty instead of sleeping in + // the input loop indefinitely. + if !process_group_exists(stdin_pgrp) { + return reclaim_stdin_foreground_process_group(self_pgrp); + } + + false + } +} + +#[cfg(all(test, unix))] +mod unix_tty_tests { + use super::*; + + #[test] + fn current_process_group_exists() { + let current_pgrp = unsafe { libc::getpgrp() }; + assert!(process_group_exists(current_pgrp)); + } + + #[test] + fn absurd_process_group_is_missing() { + assert!(!process_group_exists(999_999_999)); } } From 600375408543b10fe8fc2ab852c7ce9241dd2379 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Wed, 11 Mar 2026 13:57:12 -0400 Subject: [PATCH 8/8] fix(tui): route startup background preludes through background tagging --- code-rs/tui/src/chatwidget.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/code-rs/tui/src/chatwidget.rs b/code-rs/tui/src/chatwidget.rs index 2c4a948b055..1ecef1e8296 100644 --- a/code-rs/tui/src/chatwidget.rs +++ b/code-rs/tui/src/chatwidget.rs @@ -10865,7 +10865,17 @@ impl ChatWidget<'_> { /// Push a cell using a synthetic key at the TOP of the NEXT request. fn history_push_top_next_req(&mut self, cell: impl HistoryCell + 'static) { let key = self.next_req_key_top(); - let _ = self.history_insert_with_key_global_tagged(Box::new(cell), key, "prelude", None); + let cell = Box::new(cell); + if matches!(cell.kind(), HistoryCellType::BackgroundEvent) { + let record = cell + .as_any() + .downcast_ref::() + .map(|background| HistoryDomainRecord::BackgroundEvent(background.state().clone())); + let _ = self.history_insert_with_key_global_tagged(cell, key, "background", record); + return; + } + + let _ = self.history_insert_with_key_global_tagged(cell, key, "prelude", None); } fn history_replace_with_record( &mut self, @@ -35248,6 +35258,18 @@ use code_core::protocol::OrderMeta; ); } + #[test] + fn startup_background_prelude_uses_background_tagging() { + let mut harness = ChatWidgetHarness::new(); + let chat = harness.chat(); + reset_history(chat); + + chat.history_push_top_next_req(history_cell::new_connecting_mcp_status()); + + assert_eq!(chat.history_cells.len(), 1); + assert_eq!(chat.history_cells[0].kind(), HistoryCellType::BackgroundEvent); + } + #[test] fn ordering_tool_reasoning_explore_should_preserve_arrival_sequence() { let mut harness = ChatWidgetHarness::new();