diff --git a/codex-rs/terminal-browser/src/process.rs b/codex-rs/terminal-browser/src/process.rs index 4cc1898d9c..3c7154d4e4 100644 --- a/codex-rs/terminal-browser/src/process.rs +++ b/codex-rs/terminal-browser/src/process.rs @@ -137,6 +137,10 @@ impl Drop for StartupGuard<'_> { impl Inner { pub(crate) async fn ensure_session(self: &Arc, config: SessionConfig) -> Result<()> { + anyhow::ensure!( + !self.terminated.load(Ordering::SeqCst), + "terminal browser has been terminated" + ); let expected_network_policy = config.network_policy.clone(); let process_exited = self .process @@ -190,6 +194,10 @@ impl Inner { async fn start_session(self: &Arc, config: SessionConfig) -> Result<()> { let binary = self.validated_binary().await?; + anyhow::ensure!( + !self.terminated.load(Ordering::SeqCst), + "terminal browser has been terminated" + ); self.closing.store(/*val*/ false, Ordering::SeqCst); let persistent_profile = self.selected_profile_resources()?; let runtime = @@ -278,6 +286,10 @@ impl Inner { }); startup.set_exit_task(exit_task); let mut session_slot = self.session.lock().await; + anyhow::ensure!( + !self.terminated.load(Ordering::SeqCst), + "terminal browser has been terminated" + ); let CommittedStartup { profile_lock, output_task, @@ -296,11 +308,17 @@ impl Inner { }); drop(session_slot); self.update_view(|view| { - view.status = BrowserStatus::Running; - if !target.title.is_empty() { - view.title = Some(target.title); + if !self.terminated.load(Ordering::SeqCst) { + view.status = BrowserStatus::Running; + if !target.title.is_empty() { + view.title = Some(target.title); + } } }); + if self.terminated.load(Ordering::SeqCst) { + self.close_session().await; + anyhow::bail!("terminal browser has been terminated"); + } Ok(()) } @@ -377,6 +395,52 @@ impl Inner { view.human_control = false; }); } + + pub(crate) fn terminate_now(&self) { + self.terminated.store(/*val*/ true, Ordering::SeqCst); + self.closing.store(/*val*/ true, Ordering::SeqCst); + self.human_control.store(/*val*/ false, Ordering::SeqCst); + self.human_control_generation + .fetch_add(/*val*/ 1, Ordering::SeqCst); + + if let Some(process) = self + .process + .lock() + .unwrap_or_else(PoisonError::into_inner) + .take() + { + process.terminate(); + } + if let Ok(mut session) = self.session.try_lock() + && let Some(session) = session.take() + { + session.output_task.abort(); + session.exit_task.abort(); + session.navigation_policy_task.abort(); + } + + let status = if self + .binary + .read() + .unwrap_or_else(PoisonError::into_inner) + .is_some() + { + BrowserStatus::Idle + } else { + self.view + .read() + .unwrap_or_else(PoisonError::into_inner) + .status + .clone() + }; + self.update_view(|view| { + view.status = status; + view.visible = false; + view.title = None; + view.url = None; + view.human_control = false; + }); + } } fn spawn_screen_task( diff --git a/codex-rs/terminal-browser/src/session.rs b/codex-rs/terminal-browser/src/session.rs index a98ed75776..008e36270f 100644 --- a/codex-rs/terminal-browser/src/session.rs +++ b/codex-rs/terminal-browser/src/session.rs @@ -51,6 +51,7 @@ pub(crate) struct Inner { pub(crate) process: Mutex>>, pub(crate) resize_tx: watch::Sender, pub(crate) closing: AtomicBool, + pub(crate) terminated: AtomicBool, pub(crate) human_control: AtomicBool, pub(crate) human_control_transition: AtomicBool, pub(crate) human_control_generation: AtomicU64, @@ -112,6 +113,7 @@ impl TerminalBrowser { process: Mutex::new(/*t*/ None), resize_tx, closing: AtomicBool::new(/*v*/ false), + terminated: AtomicBool::new(/*v*/ false), human_control: AtomicBool::new(/*v*/ false), human_control_transition: AtomicBool::new(/*v*/ false), human_control_generation: AtomicU64::new(/*v*/ 0), @@ -235,6 +237,13 @@ impl TerminalBrowser { let _operation = self.inner.operation.lock().await; self.inner.close_session().await; } + + /// Immediately terminates the browser process during application teardown. + /// + /// Normal lifecycle paths should prefer [`Self::close`] so Carbonyl can exit gracefully. + pub fn terminate(&self) { + self.inner.terminate_now(); + } } impl Inner { diff --git a/codex-rs/terminal-browser/src/terminal_browser_tests.rs b/codex-rs/terminal-browser/src/terminal_browser_tests.rs index eee0d44e17..b5870abd53 100644 --- a/codex-rs/terminal-browser/src/terminal_browser_tests.rs +++ b/codex-rs/terminal-browser/src/terminal_browser_tests.rs @@ -33,6 +33,26 @@ async fn open_requires_network_enabled_by_the_active_permission_profile() { ); } +#[tokio::test] +async fn emergency_termination_prevents_future_browser_startup() { + let browser = TerminalBrowser::discover(); + browser + .set_network_policy(BrowserNetworkPolicy::Direct) + .await; + + browser.terminate(); + + let error = browser + .execute( + "test-session", + "open", + serde_json::json!({ "url": "https://example.com" }), + ) + .await + .expect_err("terminated browser should reject startup"); + assert_eq!(error.to_string(), "terminal browser has been terminated"); +} + #[tokio::test] async fn model_actions_are_rejected_during_human_control() { let browser = TerminalBrowser::discover();