From cebdb732eacfeae9823be93aad273c242428b8a0 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 12 Sep 2026 00:04:04 +0000 Subject: [PATCH] Route TUI Windows sandbox setup through the app server (#44945) ## What changed Use `windowsSandbox/setupStart` for elevated and unelevated setup, and handle completion notifications while retaining the pending approval preset and permission profile selection. Verify the effective sandbox mode before enabling Agent mode. Keep input locked and setup pending when the start request times out or its response is lost. Block thread replacement during setup, ignore completion notifications for a different mode, and clear interrupted setup state with a restart message after reconnection. Preserve the fallback prompt when elevated setup fails and report unelevated setup failures. Remove the TUI's direct sandbox setup helpers and sandbox-mode configuration writes. ## Testing Add regression coverage for uncertain setup responses, matching completion modes, blocked thread replacement with retained input, and interrupted setup during reconnection. GitOrigin-RevId: 5ec1101a1a69b3f96f01fc2ba6a03112705f4918 --- codex-rs/tui/src/app/agents_overview.rs | 3 + .../tui/src/app/agents_overview_actions.rs | 3 + codex-rs/tui/src/app/app_server_events.rs | 46 ++++ codex-rs/tui/src/app/config_persistence.rs | 70 ++---- codex-rs/tui/src/app/event_dispatch.rs | 233 +++--------------- codex-rs/tui/src/app/platform_actions.rs | 130 ++++++++++ codex-rs/tui/src/app/reconnect.rs | 17 ++ codex-rs/tui/src/app/session_lifecycle.rs | 20 +- .../src/app/tests/agents_navigation_tests.rs | 51 ++++ .../app/tests/navigation_reconnect_tests.rs | 34 ++- codex-rs/tui/src/app/tests/startup.rs | 103 ++++++++ codex-rs/tui/src/config_update.rs | 18 -- codex-rs/tui/src/windows_sandbox.rs | 72 +----- 13 files changed, 452 insertions(+), 348 deletions(-) diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index ceeadb6a92..d13acf7b76 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -334,6 +334,9 @@ impl App { app_server: &mut AppServerSession, root_thread_id: ThreadId, ) -> color_eyre::Result { + if self.windows_sandbox_blocks_thread_switch() { + return Ok(AppRunControl::Continue); + } if self.current_displayed_thread_id() == Some(root_thread_id) && (!self.thread_unavailable(root_thread_id) || self.chat_widget.is_external_writer_view()) diff --git a/codex-rs/tui/src/app/agents_overview_actions.rs b/codex-rs/tui/src/app/agents_overview_actions.rs index f51b5e1ac2..7e3ddff863 100644 --- a/codex-rs/tui/src/app/agents_overview_actions.rs +++ b/codex-rs/tui/src/app/agents_overview_actions.rs @@ -169,6 +169,9 @@ impl App { thread_id: ThreadId, action: AgentsOverviewAction, ) -> color_eyre::Result<()> { + if self.windows_sandbox_blocks_thread_switch() { + return Ok(()); + } // The overview may lack intermediate ancestors, or even the primary's metadata. let mut removes_primary = self.primary_thread_id == Some(thread_id); let mut attempted = false; diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index a8317b3145..8844588457 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -8,6 +8,8 @@ use super::app_server_event_targets::server_request_thread_id; use crate::app_command::AppCommand; use crate::app_event::AppEvent; use crate::app_event::RateLimitRefreshOrigin; +#[cfg(any(target_os = "windows", test))] +use crate::app_event::WindowsSandboxEnableMode; use crate::app_info::app_info_from_api; use crate::app_server_session::AppServerSession; use crate::app_server_session::status_account_display_from_auth_mode; @@ -384,6 +386,50 @@ impl App { ServerNotificationThreadTarget::Global => {} } + #[cfg(any(target_os = "windows", test))] + if let ServerNotification::WindowsSandboxSetupCompleted(result) = notification { + let Some((mode, preset, profile_selection)) = self.windows_sandbox.pending_setup.take() + else { + return; + }; + let expected_mode = match mode { + WindowsSandboxEnableMode::Elevated => { + codex_app_server_protocol::WindowsSandboxSetupMode::Elevated + } + WindowsSandboxEnableMode::Legacy => { + codex_app_server_protocol::WindowsSandboxSetupMode::Unelevated + } + }; + if result.mode != expected_mode { + self.windows_sandbox.pending_setup = Some((mode, preset, profile_selection)); + return; + } + if result.success { + self.app_event_tx + .send(AppEvent::EnableWindowsSandboxForAgentMode { + preset, + mode, + profile_selection, + }); + } else if mode == WindowsSandboxEnableMode::Elevated { + self.app_event_tx + .send(AppEvent::OpenWindowsSandboxFallbackPrompt { + preset, + profile_selection, + }); + } else { + self.chat_widget.clear_windows_sandbox_setup_status(); + self.windows_sandbox.setup_started_at = None; + self.chat_widget + .retain_input_after_failed_permission_selection(); + self.chat_widget.add_error_message(format!( + "Windows sandbox setup failed: {}", + result.error.unwrap_or_else(|| "unknown error".to_string()) + )); + } + return; + } + self.chat_widget .handle_server_notification(notification, /*replay_kind*/ None); } diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index e2a1f8c220..7d3f0f25a5 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -6,14 +6,6 @@ use super::*; use codex_config::ConfigLayerSource; -#[cfg(target_os = "windows")] -use codex_utils_approval_presets::ApprovalPreset; - -#[cfg(target_os = "windows")] -pub(super) struct WindowsSetupPermissions { - pub(super) permission_profile: PermissionProfile, - pub(super) workspace_roots: Vec, -} async fn build_config_on_runtime_worker( builder: ConfigBuilder, @@ -127,29 +119,6 @@ impl App { .await } - #[cfg(target_os = "windows")] - pub(super) async fn windows_setup_permissions( - &self, - preset: &ApprovalPreset, - profile_selection: Option<&PermissionProfileSelection>, - ) -> Result { - match profile_selection { - Some(selection) => { - let selected_config = self - .rebuild_config_for_permission_profile(selection.profile_id.as_str()) - .await?; - Ok(WindowsSetupPermissions { - permission_profile: selected_config.permissions.permission_profile().clone(), - workspace_roots: selected_config.effective_workspace_roots(), - }) - } - None => Ok(WindowsSetupPermissions { - permission_profile: preset.permission_profile.clone(), - workspace_roots: self.config.effective_workspace_roots(), - }), - } - } - pub(super) async fn apply_permission_profile_selection( &mut self, selection: PermissionProfileSelection, @@ -1266,31 +1235,34 @@ impl App { } #[cfg(target_os = "windows")] - pub(super) async fn sync_windows_sandbox_after_overridden_write( + pub(super) async fn verify_windows_sandbox_mode_after_setup( &mut self, app_server: &mut AppServerSession, - write_response: &ConfigWriteResponse, - ) { - let message = overridden_write_message(write_response); - tracing::warn!( - message, - "Windows sandbox config write was overridden by effective config" - ); - self.chat_widget.add_error_message(format!( - "Windows sandbox changes were saved but not applied: {message}" - )); - let Some(effective_config) = self - .read_effective_config_after_overridden_write(app_server, "Windows sandbox changes") + requested_mode: codex_config::types::WindowsSandboxModeToml, + ) -> bool { + let cwd = self.chat_widget.config_ref().cwd.display().to_string(); + let mode = crate::config_update::read_effective_config(app_server.request_handle(), cwd) .await - else { - return; - }; - let Some(mode) = windows_sandbox_mode_from_effective_config(&effective_config) else { - return; + .ok() + .and_then(|config| windows_sandbox_mode_from_effective_config(&config)); + let Some(mode) = mode else { + self.chat_widget.add_error_message( + "Windows sandbox setup completed, but Codex could not verify the effective sandbox mode." + .to_string(), + ); + return false; }; self.config.permissions.windows_sandbox_mode = Some(mode); + if mode == requested_mode { + return true; + } self.chat_widget.set_windows_sandbox_mode(Some(mode)); self.propagate_windows_sandbox_turn_context(); + self.chat_widget.add_error_message( + "Windows sandbox setup completed, but its mode was overridden by the effective configuration." + .to_string(), + ); + false } fn propagate_windows_sandbox_turn_context(&self) { diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 5fd3cd2c28..8b5ad52bd9 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1988,186 +1988,31 @@ impl App { preset, profile_selection, } => { - #[cfg(any(target_os = "windows", test))] - if !self.chat_widget.windows_sandbox_mode_allowed( - codex_config::types::WindowsSandboxModeToml::Elevated, - ) { - tracing::warn!( - "refusing to set up elevated Windows sandbox mode disallowed by requirements" - ); - self.chat_widget.add_info_message( - "That Windows sandbox option is disallowed by requirements.".to_string(), - /*hint*/ None, - ); - return Ok(AppRunControl::Continue); - } #[cfg(target_os = "windows")] - { - let setup_permissions = match self - .windows_setup_permissions(&preset, profile_selection.as_ref()) - .await - { - Ok(setup_permissions) => setup_permissions, - Err(err) => { - tracing::warn!( - error = %err, - "failed to resolve permission profile for elevated Windows sandbox setup" - ); - self.chat_widget.add_error_message(format!( - "Failed to prepare Windows sandbox for the selected permission profile: {err}" - )); - return Ok(AppRunControl::Continue); - } - }; - let permission_profile = setup_permissions.permission_profile; - let workspace_roots = setup_permissions.workspace_roots; - let command_cwd = self.config.cwd.clone(); - let env_map: std::collections::HashMap = - std::env::vars().collect(); - let codex_home = self.config.codex_home.clone(); - let tx = self.app_event_tx.clone(); - - self.chat_widget.show_windows_sandbox_setup_status(); - self.windows_sandbox.setup_started_at = Some(Instant::now()); - let session_telemetry = self.session_telemetry.clone(); - tokio::task::spawn_blocking(move || { - let result = crate::windows_sandbox::prepare_elevated_sandbox( - &permission_profile, - workspace_roots.as_slice(), - command_cwd.as_path(), - &env_map, - codex_home.as_path(), - ); - let event = match result { - Ok(()) => { - session_telemetry.counter( - "codex.windows_sandbox.elevated_setup_success", - /*inc*/ 1, - &[], - ); - AppEvent::EnableWindowsSandboxForAgentMode { - preset: preset.clone(), - mode: WindowsSandboxEnableMode::Elevated, - profile_selection: profile_selection.clone(), - } - } - Err(err) => { - let mut code_tag: Option = None; - let mut message_tag: Option = None; - if let Some((code, message)) = - crate::windows_sandbox::elevated_setup_failure_details(&err) - { - code_tag = Some(code); - message_tag = Some(message); - } - let mut tags: Vec<(&str, &str)> = Vec::new(); - if let Some(code) = code_tag.as_deref() { - tags.push(("code", code)); - } - if let Some(message) = message_tag.as_deref() { - tags.push(("message", message)); - } - session_telemetry.counter( - crate::windows_sandbox::elevated_setup_failure_metric_name( - &err, - ), - /*inc*/ 1, - &tags, - ); - tracing::error!( - error = %err, - "failed to run elevated Windows sandbox setup" - ); - AppEvent::OpenWindowsSandboxFallbackPrompt { - preset, - profile_selection, - } - } - }; - tx.send(event); - }); - } + self.begin_windows_sandbox_setup( + app_server, + preset, + profile_selection, + WindowsSandboxEnableMode::Elevated, + ) + .await; #[cfg(not(target_os = "windows"))] - { - let _ = (preset, profile_selection); - } + let _ = (preset, profile_selection); } AppEvent::BeginWindowsSandboxLegacySetup { preset, profile_selection, } => { - #[cfg(any(target_os = "windows", test))] - if !self.chat_widget.windows_sandbox_mode_allowed( - codex_config::types::WindowsSandboxModeToml::Unelevated, - ) { - tracing::warn!( - "refusing to set up unelevated Windows sandbox mode disallowed by requirements" - ); - self.chat_widget.add_info_message( - "That Windows sandbox option is disallowed by requirements.".to_string(), - /*hint*/ None, - ); - return Ok(AppRunControl::Continue); - } #[cfg(target_os = "windows")] - { - let setup_permissions = match self - .windows_setup_permissions(&preset, profile_selection.as_ref()) - .await - { - Ok(setup_permissions) => setup_permissions, - Err(err) => { - tracing::warn!( - error = %err, - "failed to resolve permission profile for legacy Windows sandbox setup" - ); - self.chat_widget.add_error_message(format!( - "Failed to prepare Windows sandbox for the selected permission profile: {err}" - )); - return Ok(AppRunControl::Continue); - } - }; - let permission_profile = setup_permissions.permission_profile; - let workspace_roots = setup_permissions.workspace_roots; - let command_cwd = self.config.cwd.clone(); - let env_map: std::collections::HashMap = - std::env::vars().collect(); - let codex_home = self.config.codex_home.clone(); - let tx = self.app_event_tx.clone(); - let session_telemetry = self.session_telemetry.clone(); - - self.chat_widget.show_windows_sandbox_setup_status(); - tokio::task::spawn_blocking(move || { - if let Err(err) = - codex_windows_sandbox::run_windows_sandbox_legacy_preflight( - &permission_profile, - workspace_roots.as_slice(), - codex_home.as_path(), - command_cwd.as_path(), - &env_map, - ) - { - session_telemetry.counter( - "codex.windows_sandbox.legacy_setup_preflight_failed", - /*inc*/ 1, - &[], - ); - tracing::warn!( - error = %err, - "failed to preflight non-admin Windows sandbox setup" - ); - } - tx.send(AppEvent::EnableWindowsSandboxForAgentMode { - preset, - mode: WindowsSandboxEnableMode::Legacy, - profile_selection, - }); - }); - } + self.begin_windows_sandbox_setup( + app_server, + preset, + profile_selection, + WindowsSandboxEnableMode::Legacy, + ) + .await; #[cfg(not(target_os = "windows"))] - { - let _ = (preset, profile_selection); - } + let _ = (preset, profile_selection); } AppEvent::EnableWindowsSandboxForAgentMode { preset, @@ -2177,7 +2022,9 @@ impl App { #[cfg(target_os = "windows")] { self.chat_widget.clear_windows_sandbox_setup_status(); - if let Some(started_at) = self.windows_sandbox.setup_started_at.take() { + if let Some(started_at) = self.windows_sandbox.setup_started_at.take() + && mode == WindowsSandboxEnableMode::Elevated + { self.session_telemetry.record_duration( "codex.windows_sandbox.elevated_setup_duration_ms", started_at.elapsed(), @@ -2192,7 +2039,7 @@ impl App { if !self.chat_widget.windows_sandbox_mode_allowed(selected_mode) { tracing::warn!( ?selected_mode, - "refusing to persist Windows sandbox mode disallowed by requirements" + "refusing to enable Windows sandbox mode disallowed by requirements" ); self.chat_widget.add_info_message( "That Windows sandbox option is disallowed by requirements." @@ -2201,20 +2048,12 @@ impl App { ); return Ok(AppRunControl::Continue); } - self.chat_widget.windows_sandbox_elevated_setup_complete = elevated_enabled; - let edits = - crate::config_update::build_windows_sandbox_mode_edits(elevated_enabled); - match crate::config_update::write_config_batch( - app_server.request_handle(), - edits, - ) - .await + if self + .verify_windows_sandbox_mode_after_setup(app_server, selected_mode) + .await { - Ok(response) if response.status == WriteStatus::OkOverridden => { - self.sync_windows_sandbox_after_overridden_write(app_server, &response) - .await; - } - Ok(_) => { + self.chat_widget.windows_sandbox_elevated_setup_complete = + elevated_enabled; if elevated_enabled { self.config.set_windows_sandbox_enabled(/*value*/ false); self.config @@ -2250,14 +2089,6 @@ impl App { if self.apply_permission_profile_selection(selection).await { self.chat_widget.submit_initial_user_message_if_pending(); } - self.chat_widget.add_plain_history_lines(vec![ - Line::from(vec!["• ".dim(), "Sandbox ready".into()]), - Line::from(vec![ - " ".into(), - "Codex can now safely edit files and execute commands in your computer" - .dark_gray(), - ]), - ]); } else { self.app_event_tx.send(AppEvent::CodexOp( AppCommand::override_turn_context( @@ -2283,6 +2114,7 @@ impl App { .send(AppEvent::UpdateActivePermissionProfile( preset.active_permission_profile.clone(), )); + } self.chat_widget.add_plain_history_lines(vec![ Line::from(vec!["• ".dim(), "Sandbox ready".into()]), Line::from(vec![ @@ -2291,17 +2123,8 @@ impl App { .dark_gray(), ]), ]); - } - } - Err(err) => { - tracing::error!( - error = %err, - "failed to enable Windows sandbox feature" - ); - self.chat_widget.add_error_message(format!( - "Failed to enable the Windows sandbox feature: {err}" - )); - } + } else { + self.chat_widget.retain_input_after_failed_permission_selection(); } } #[cfg(not(target_os = "windows"))] diff --git a/codex-rs/tui/src/app/platform_actions.rs b/codex-rs/tui/src/app/platform_actions.rs index 6ca93bc2d7..69d42904d8 100644 --- a/codex-rs/tui/src/app/platform_actions.rs +++ b/codex-rs/tui/src/app/platform_actions.rs @@ -4,6 +4,12 @@ //! and Windows sandbox helper actions that are compiled only on Windows. use super::*; +#[cfg(all(test, not(target_os = "windows")))] +use crate::app_event::WindowsSandboxEnableMode; +#[cfg(target_os = "windows")] +use codex_config::types::WindowsSandboxModeToml; +#[cfg(any(target_os = "windows", test))] +use codex_utils_approval_presets::ApprovalPreset; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum WindowsSandboxHost { @@ -15,6 +21,12 @@ pub(crate) enum WindowsSandboxHost { #[derive(Default)] pub(super) struct WindowsSandboxState { pub(super) setup_started_at: Option, + #[cfg(any(target_os = "windows", test))] + pub(super) pending_setup: Option<( + WindowsSandboxEnableMode, + ApprovalPreset, + Option, + )>, } pub(super) fn windows_sandbox_host( @@ -70,6 +82,124 @@ impl App { pub(super) fn windows_sandbox_setup_is_local(&self) -> bool { self.windows_sandbox_host() == WindowsSandboxHost::Local } + + pub(super) fn windows_sandbox_blocks_thread_switch(&self) -> bool { + #[cfg(any(target_os = "windows", test))] + { + self.windows_sandbox_setup_is_local() + && (self.windows_sandbox.pending_setup.is_some() + || self.windows_sandbox.setup_started_at.is_some() + || self.chat_widget.initial_user_message.is_some() + && self.chat_widget.required_elevated_windows_sandbox() + && !self.chat_widget.windows_sandbox_elevated_setup_complete) + } + #[cfg(not(any(target_os = "windows", test)))] + { + false + } + } + + #[cfg(target_os = "windows")] + pub(super) async fn begin_windows_sandbox_setup( + &mut self, + app_server: &mut AppServerSession, + preset: ApprovalPreset, + profile_selection: Option, + mode: WindowsSandboxEnableMode, + ) { + let (setup_mode, config_mode) = match mode { + WindowsSandboxEnableMode::Elevated => ( + codex_app_server_protocol::WindowsSandboxSetupMode::Elevated, + WindowsSandboxModeToml::Elevated, + ), + WindowsSandboxEnableMode::Legacy => ( + codex_app_server_protocol::WindowsSandboxSetupMode::Unelevated, + WindowsSandboxModeToml::Unelevated, + ), + }; + if self.windows_sandbox.pending_setup.is_some() { + if self.windows_sandbox.setup_started_at.is_none() { + self.chat_widget.add_info_message( + "Windows sandbox setup is still running. Restart Codex to retry.".to_string(), + /*hint*/ None, + ); + } + return; + } + if !self.chat_widget.windows_sandbox_mode_allowed(config_mode) { + self.chat_widget.add_info_message( + "That Windows sandbox option is disallowed by requirements.".to_string(), + /*hint*/ None, + ); + return; + } + + self.windows_sandbox.pending_setup = Some((mode, preset, profile_selection)); + self.chat_widget.show_windows_sandbox_setup_status(); + self.windows_sandbox.setup_started_at = Some(Instant::now()); + let request_id = app_server.next_request_id(); + let response = tokio::time::timeout( + std::time::Duration::from_secs(15), + app_server + .request_handle() + .request_typed(ClientRequest::WindowsSandboxSetupStart { + request_id, + params: codex_app_server_protocol::WindowsSandboxSetupStartParams { + mode: setup_mode, + cwd: Some(self.config.cwd.clone()), + }, + }), + ) + .await; + self.finish_windows_sandbox_setup_start(response); + } + + #[cfg(any(target_os = "windows", test))] + pub(super) fn finish_windows_sandbox_setup_start( + &mut self, + response: std::result::Result< + std::result::Result< + codex_app_server_protocol::WindowsSandboxSetupStartResponse, + TypedRequestError, + >, + tokio::time::error::Elapsed, + >, + ) { + match response { + Ok(Ok(codex_app_server_protocol::WindowsSandboxSetupStartResponse { + started: true, + })) => {} + Err(_) => { + self.chat_widget.add_error_message( + "Windows sandbox setup request timed out. Waiting for completion; restart Codex if it does not finish." + .to_string(), + ); + } + Ok(Err( + TypedRequestError::Transport { .. } | TypedRequestError::Deserialize { .. }, + )) => { + self.chat_widget.add_error_message( + "Windows sandbox setup response was lost. Waiting for completion or reconnection." + .to_string(), + ); + } + Ok(result) => { + self.windows_sandbox.pending_setup = None; + self.chat_widget + .retain_input_after_failed_permission_selection(); + self.chat_widget.clear_windows_sandbox_setup_status(); + self.windows_sandbox.setup_started_at = None; + let message = match result { + Err(TypedRequestError::Server { source, .. }) if source.code == -32601 => { + "Update the local app server to set up the Windows sandbox.".to_string() + } + Err(_) => "Windows sandbox setup failed.".to_string(), + Ok(_) => "Windows sandbox setup did not start.".to_string(), + }; + self.chat_widget.add_error_message(message); + } + } + } } pub(super) fn side_return_shortcut_matches(key_event: KeyEvent) -> bool { diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index e816683d58..8f5e5295f2 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -259,6 +259,16 @@ impl App { self.rate_limit_refresh_state.invalidate_recovery(); session.inherit_task_tool_capabilities(app_server); *app_server = session; + #[cfg(any(target_os = "windows", test))] + let interrupted_windows_setup = self.windows_sandbox.pending_setup.take().is_some(); + #[cfg(any(target_os = "windows", test))] + { + if interrupted_windows_setup { + self.windows_sandbox.setup_started_at = None; + self.chat_widget.clear_windows_sandbox_setup_status(); + self.chat_widget.windows_sandbox_elevated_setup_complete = false; + } + } self.chat_widget.set_local_worktree_operations( !crate::uses_remote_workspace_or_environment( &self.app_server_target, @@ -419,6 +429,13 @@ impl App { self.chat_widget.show_bottom_pane_view(Box::new(view)); self.refresh_agents_overview_threads(app_server); } + #[cfg(any(target_os = "windows", test))] + if interrupted_windows_setup { + self.chat_widget.add_error_message( + "Windows sandbox setup was interrupted. Restart Codex before using Agent mode." + .to_string(), + ); + } // Only accept fresh task-tool calls once this connection and its event queue are adopted. if let ThreadToolTransport::Mcp(server) = app_server.thread_tool_transport() { server.reconnect(app_server.request_handle(), self.app_event_tx.clone()); diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index f1451569c0..8c99bdd84e 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -538,10 +538,17 @@ impl App { } else { None }; + if self.active_thread_id == Some(thread_id) && !self.thread_unavailable(thread_id) { + return Ok(()); + } + if self.windows_sandbox_blocks_thread_switch() { + self.chat_widget.add_info_message( + "Finish Windows sandbox setup before switching threads.".to_string(), + /*hint*/ None, + ); + return Ok(()); + } if self.active_thread_id == Some(thread_id) { - if !self.thread_unavailable(thread_id) { - return Ok(()); - } // Detach the cached receiver before a successful attachment replaces its channel. self.store_active_thread_receiver().await; } @@ -1207,6 +1214,13 @@ impl App { tui.frame_requester().schedule_frame(); return Ok(AppRunControl::Continue); } + if self.windows_sandbox_blocks_thread_switch() { + self.chat_widget.add_info_message( + "Finish Windows sandbox setup before switching threads.".to_string(), + /*hint*/ None, + ); + return Ok(AppRunControl::Continue); + } let (mut resume_config, local_settings) = match self .resume_config_for_target(tui, app_server, &target_session) diff --git a/codex-rs/tui/src/app/tests/agents_navigation_tests.rs b/codex-rs/tui/src/app/tests/agents_navigation_tests.rs index 33b8638645..93393e3b61 100644 --- a/codex-rs/tui/src/app/tests/agents_navigation_tests.rs +++ b/codex-rs/tui/src/app/tests/agents_navigation_tests.rs @@ -1,4 +1,5 @@ use super::*; +use pretty_assertions::assert_eq; #[tokio::test] async fn agents_navigation_requires_local_daemon() -> Result<()> { @@ -48,3 +49,53 @@ async fn agents_navigation_requires_local_daemon() -> Result<()> { app_server.shutdown().await?; Ok(()) } + +#[tokio::test] +async fn pending_windows_sandbox_setup_blocks_thread_replacement() -> Result<()> { + use crate::app_event::WindowsSandboxEnableMode; + + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + while events.try_recv().is_ok() {} + let mut app_server = start_config_write_test_app_server(&app).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let current = ThreadId::new(); + let other = ThreadId::new(); + app.active_thread_id = Some(current); + app.primary_thread_id = Some(current); + let preset = codex_utils_approval_presets::builtin_approval_presets() + .into_iter() + .find(|preset| preset.id == "auto") + .expect("auto preset"); + app.windows_sandbox.pending_setup = Some((WindowsSandboxEnableMode::Elevated, preset, None)); + let prompt = crate::chatwidget::create_initial_user_message( + Some("review this".to_string()), + Vec::new(), + Vec::new(), + ); + app.chat_widget.initial_user_message = prompt.clone(); + + app.select_agent_thread(&mut tui, &mut app_server, other) + .await?; + assert_eq!(app.active_thread_id, Some(current)); + assert_eq!(app.chat_widget.initial_user_message, prompt); + let cell = match events.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected setup navigation message, got {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 100)); + insta::assert_snapshot!(rendered, @"• Finish Windows sandbox setup before switching threads."); + + app.chat_widget.initial_user_message = None; + app.run_agents_overview_action( + &mut tui, + &mut app_server, + current, + crate::app_event::AgentsOverviewAction::Archive, + ) + .await?; + assert_eq!(app.active_thread_id, Some(current)); + assert!(events.try_recv().is_err()); + + app_server.shutdown().await?; + Ok(()) +} diff --git a/codex-rs/tui/src/app/tests/navigation_reconnect_tests.rs b/codex-rs/tui/src/app/tests/navigation_reconnect_tests.rs index b04091cb1b..1103300666 100644 --- a/codex-rs/tui/src/app/tests/navigation_reconnect_tests.rs +++ b/codex-rs/tui/src/app/tests/navigation_reconnect_tests.rs @@ -6,6 +6,7 @@ use super::*; use crate::app::reconnect::ReconnectPresentation; use crate::app::reconnect::reconnect; use crate::app_event::AgentsOverviewThreadRefresh; +use crate::app_event::WindowsSandboxEnableMode; use crate::app_server_session::ThreadParamsMode; use codex_app_server_client::AppServerEvent; use pretty_assertions::assert_eq; @@ -170,7 +171,16 @@ async fn reconnect_daemon_command_center_after_socket_replacement_without_a_conv app.app_server_target = AppServerTarget::LocalDaemon { endpoint: endpoint.clone(), }; - if previous_thread.is_none() { + let interrupted_setup = previous_thread.is_none() && !overview_initialized; + if interrupted_setup { + let preset = codex_utils_approval_presets::builtin_approval_presets() + .into_iter() + .find(|preset| preset.id == "auto") + .expect("auto preset"); + app.windows_sandbox.pending_setup = + Some((WindowsSandboxEnableMode::Elevated, preset, None)); + app.windows_sandbox.setup_started_at = Some(Instant::now()); + } else if previous_thread.is_none() { app.chat_widget.windows_sandbox_elevated_setup_complete = true; } let available = Arc::new(std::sync::atomic::AtomicBool::new(false)); @@ -317,7 +327,27 @@ async fn reconnect_daemon_command_center_after_socket_replacement_without_a_conv CODEX_CLI_VERSION, ) .await?; - if previous_thread.is_none() { + if interrupted_setup { + assert!(app.windows_sandbox.pending_setup.is_none()); + assert!(app.windows_sandbox.setup_started_at.is_none()); + let mut retained = Vec::new(); + let mut saw_warning = false; + while let Ok(event) = events.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = &event { + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 100)); + if rendered.contains("Windows sandbox setup was interrupted") { + insta::assert_snapshot!(rendered, @"■ Windows sandbox setup was interrupted. Restart Codex before using Agent mode."); + saw_warning = true; + continue; + } + } + retained.push(event); + } + assert!(saw_warning); + for event in retained { + app.app_event_tx.send(event); + } + } else if previous_thread.is_none() { assert!(app.chat_widget.windows_sandbox_elevated_setup_complete); } assert!(!app.reconnect.offline); diff --git a/codex-rs/tui/src/app/tests/startup.rs b/codex-rs/tui/src/app/tests/startup.rs index 455128cf43..a3cb1093aa 100644 --- a/codex-rs/tui/src/app/tests/startup.rs +++ b/codex-rs/tui/src/app/tests/startup.rs @@ -63,6 +63,109 @@ async fn windows_sandbox_setup_skips_remote_default_executor() -> Result<()> { Ok(()) } +#[tokio::test] +async fn uncertain_windows_sandbox_setup_keeps_intent_and_input_locked() { + use crate::app_event::WindowsSandboxEnableMode; + use codex_app_server_protocol::WindowsSandboxSetupStartResponse; + let preset = builtin_approval_presets() + .into_iter() + .find(|preset| preset.id == "auto") + .expect("auto preset"); + + for response in [ + Ok(Err(TypedRequestError::Transport { + method: "windowsSandbox/setupStart".to_string(), + source: std::io::Error::new(std::io::ErrorKind::BrokenPipe, "connection closed"), + })), + tokio::time::timeout( + Duration::ZERO, + std::future::pending::< + std::result::Result, + >(), + ) + .await, + ] { + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + while events.try_recv().is_ok() {} + app.windows_sandbox.pending_setup = + Some((WindowsSandboxEnableMode::Elevated, preset.clone(), None)); + app.windows_sandbox.setup_started_at = Some(Instant::now()); + app.chat_widget.show_windows_sandbox_setup_status(); + let transport_error = matches!(&response, Ok(Err(TypedRequestError::Transport { .. }))); + + app.finish_windows_sandbox_setup_start(response); + + assert!(app.windows_sandbox.pending_setup.is_some()); + app.chat_widget + .handle_key_event(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + assert!(app.chat_widget.composer_text_with_pending().is_empty()); + let cell = match events.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected setup error message, got {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 160)); + if transport_error { + insta::assert_snapshot!(rendered, @"■ Windows sandbox setup response was lost. Waiting for completion or reconnection."); + } else { + insta::assert_snapshot!(rendered, @"■ Windows sandbox setup request timed out. Waiting for completion; restart Codex if it does not finish."); + } + } +} + +#[tokio::test] +async fn windows_sandbox_setup_completion_requires_matching_pending_mode() -> Result<()> { + use crate::app_event::WindowsSandboxEnableMode; + use codex_app_server_protocol::WindowsSandboxSetupCompletedNotification; + use codex_app_server_protocol::WindowsSandboxSetupMode; + + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + while events.try_recv().is_ok() {} + let app_server = + crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?; + let preset = builtin_approval_presets() + .into_iter() + .find(|preset| preset.id == "auto") + .expect("auto preset"); + app.windows_sandbox.pending_setup = Some((WindowsSandboxEnableMode::Elevated, preset, None)); + app.windows_sandbox.setup_started_at = Some(Instant::now()); + + for mode in [ + WindowsSandboxSetupMode::Unelevated, + WindowsSandboxSetupMode::Elevated, + ] { + app.handle_app_server_event( + &app_server, + codex_app_server_client::AppServerEvent::ServerNotification(Box::new( + ServerNotification::WindowsSandboxSetupCompleted( + WindowsSandboxSetupCompletedNotification { + mode, + success: true, + error: None, + }, + ), + )), + ) + .await; + assert_eq!( + app.windows_sandbox.pending_setup.is_some(), + mode == WindowsSandboxSetupMode::Unelevated + ); + } + assert!(!app.chat_widget.windows_sandbox_elevated_setup_complete); + assert!(app.windows_sandbox_blocks_thread_switch()); + assert!(matches!( + events.try_recv(), + Ok(AppEvent::EnableWindowsSandboxForAgentMode { + mode: WindowsSandboxEnableMode::Elevated, + .. + }) + )); + assert!(events.try_recv().is_err()); + + app_server.shutdown().await?; + Ok(()) +} + fn startup_bottom_pane() -> (BottomPane, UnboundedReceiver) { let (app_event_tx, app_event_rx) = unbounded_channel(); ( diff --git a/codex-rs/tui/src/config_update.rs b/codex-rs/tui/src/config_update.rs index 96b16a4564..7a50e6dd04 100644 --- a/codex-rs/tui/src/config_update.rs +++ b/codex-rs/tui/src/config_update.rs @@ -121,24 +121,6 @@ pub(crate) fn build_service_tier_selection_edits(service_tier: Option<&str>) -> vec![service_tier_edit] } -#[cfg(target_os = "windows")] -pub(crate) fn build_windows_sandbox_mode_edits(elevated_enabled: bool) -> Vec { - let feature_key_path = |feature: &str| format!("features.{feature}"); - vec![ - replace_config_value( - "windows.sandbox", - serde_json::json!(if elevated_enabled { - "elevated" - } else { - "unelevated" - }), - ), - clear_config_value(feature_key_path("experimental_windows_sandbox")), - clear_config_value(feature_key_path("elevated_windows_sandbox")), - clear_config_value(feature_key_path("enable_experimental_windows_sandbox")), - ] -} - pub(crate) fn build_feature_enabled_edit(feature_key: &str, enabled: bool) -> ConfigEdit { let key_path = format!("features.{feature_key}"); let is_default_false_feature = FEATURES diff --git a/codex-rs/tui/src/windows_sandbox.rs b/codex-rs/tui/src/windows_sandbox.rs index 153023a450..d8976efbfb 100644 --- a/codex-rs/tui/src/windows_sandbox.rs +++ b/codex-rs/tui/src/windows_sandbox.rs @@ -1,22 +1,9 @@ -//! TUI-owned Windows sandbox helpers retained while setup still runs in the local client process. -//! -//! TODO: These helpers inspect and modify the TUI host, so they do not support -//! cross-platform remote app servers. Move readiness and setup to the existing -//! `windowsSandbox/*` RPCs while preserving the pending permission profile and -//! using the server platform reported during initialization. +//! Windows sandbox display state derived from configuration. use crate::legacy_core::config::Config; use codex_config::types::WindowsSandboxModeToml; use codex_features::Feature; use codex_protocol::config_types::WindowsSandboxLevel; -#[cfg(target_os = "windows")] -use codex_protocol::models::PermissionProfile; -#[cfg(target_os = "windows")] -use codex_utils_absolute_path::AbsolutePathBuf; -#[cfg(target_os = "windows")] -use std::collections::HashMap; -#[cfg(target_os = "windows")] -use std::path::Path; pub(crate) fn level_from_config(config: &Config) -> WindowsSandboxLevel { match config.permissions.windows_sandbox_mode { @@ -31,60 +18,3 @@ pub(crate) fn level_from_config(config: &Config) -> WindowsSandboxLevel { None => WindowsSandboxLevel::Disabled, } } - -#[cfg(target_os = "windows")] -pub(crate) use codex_windows_sandbox::sandbox_setup_is_complete; - -#[cfg(target_os = "windows")] -pub(crate) fn prepare_elevated_sandbox( - permission_profile: &PermissionProfile, - workspace_roots: &[AbsolutePathBuf], - command_cwd: &Path, - env_map: &HashMap, - codex_home: &Path, -) -> anyhow::Result<()> { - if !sandbox_setup_is_complete(codex_home) { - let permissions = codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots( - permission_profile, - workspace_roots, - )?; - codex_windows_sandbox::run_elevated_setup(codex_windows_sandbox::SandboxSetupRequest { - permissions: &permissions, - command_cwd, - env_map, - codex_home, - proxy_enforced: false, - })?; - } - codex_windows_sandbox::run_setup_refresh( - permission_profile, - workspace_roots, - command_cwd, - env_map, - codex_home, - /*proxy_enforced*/ false, - ) -} - -#[cfg(target_os = "windows")] -pub(crate) fn elevated_setup_failure_details(err: &anyhow::Error) -> Option<(String, String)> { - let failure = codex_windows_sandbox::extract_setup_failure(err)?; - Some(( - failure.code.as_str().to_string(), - codex_windows_sandbox::sanitize_setup_metric_tag_value(&failure.message), - )) -} - -#[cfg(target_os = "windows")] -pub(crate) fn elevated_setup_failure_metric_name(err: &anyhow::Error) -> &'static str { - if codex_windows_sandbox::extract_setup_failure(err).is_some_and(|failure| { - matches!( - failure.code, - codex_windows_sandbox::SetupErrorCode::OrchestratorHelperLaunchCanceled - ) - }) { - "codex.windows_sandbox.elevated_setup_canceled" - } else { - "codex.windows_sandbox.elevated_setup_failure" - } -}