diff --git a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs index 682dc0619..bc8c4cadd 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs @@ -14,6 +14,7 @@ mod cwd_junction; use anyhow::Context; use anyhow::Result; use codex_windows_sandbox::ErrorPayload; +use codex_windows_sandbox::ErrorStage; use codex_windows_sandbox::ExitPayload; use codex_windows_sandbox::FramedMessage; use codex_windows_sandbox::IPC_PROTOCOL_VERSION; @@ -166,13 +167,19 @@ fn open_pipe(name: &str, access: u32) -> Result { } /// Send an error frame back to the parent process. -fn send_error(writer: &Arc>, code: &str, message: String) -> Result<()> { +fn send_error( + writer: &Arc>, + stage: ErrorStage, + windows_error_code: Option, + message: String, +) -> Result<()> { let msg = FramedMessage { version: IPC_PROTOCOL_VERSION, message: Message::Error { payload: ErrorPayload { message, - code: code.to_string(), + stage, + windows_error_code, }, }, }; @@ -182,6 +189,15 @@ fn send_error(writer: &Arc>, code: &str, message: String) -> Resu Ok(()) } +fn windows_error_code(err: &anyhow::Error) -> Option { + err.chain().find_map(|cause| { + cause + .downcast_ref::() + .and_then(std::io::Error::raw_os_error) + .and_then(|code| u32::try_from(code).ok()) + }) +} + /// Read and validate the initial spawn request frame. fn read_spawn_request(reader: &mut File) -> Result { let Some(msg) = read_frame(reader)? else { @@ -528,7 +544,12 @@ pub fn main() -> Result<()> { let req = match read_spawn_request(&mut pipe_read) { Ok(v) => v, Err(err) => { - let _ = send_error(&pipe_write, "spawn_failed", err.to_string()); + let _ = send_error( + &pipe_write, + ErrorStage::ReadSpawnRequest, + /*windows_error_code*/ None, + err.to_string(), + ); return Err(err); } }; @@ -536,7 +557,12 @@ pub fn main() -> Result<()> { let ipc_spawn = match spawn_ipc_process(&req) { Ok(value) => value, Err(err) => { - let _ = send_error(&pipe_write, "spawn_failed", err.to_string()); + let _ = send_error( + &pipe_write, + ErrorStage::SpawnChild, + windows_error_code(&err), + err.to_string(), + ); return Err(err); } }; @@ -570,7 +596,12 @@ pub fn main() -> Result<()> { } else { anyhow::bail!("runner spawn_ready write failed: pipe_write lock poisoned"); } { - let _ = send_error(&pipe_write, "spawn_failed", err.to_string()); + let _ = send_error( + &pipe_write, + ErrorStage::WriteSpawnReady, + /*windows_error_code*/ None, + err.to_string(), + ); return Err(err); } let log_dir_owned = log_dir.map(Path::to_path_buf); diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index cd839952b..09dc52b57 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -11,6 +11,7 @@ use crate::proc_thread_attr::ProcThreadAttributeList; use crate::winutil::format_last_error; use crate::winutil::quote_windows_arg; use crate::winutil::to_wide; +use anyhow::Context; use anyhow::Result; use codex_utils_pty::PsuedoCon; use codex_utils_pty::RawConPty; @@ -145,14 +146,15 @@ pub fn spawn_conpty_process_as_user( }; if ok == 0 { let err = unsafe { GetLastError() } as i32; - return Err(anyhow::anyhow!( + let message = format!( "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={}", err, format_last_error(err), cwd.display(), cmdline_str, env_block.len() - )); + ); + return Err(std::io::Error::from_raw_os_error(err)).context(message); } Ok((pi, conpty)) } diff --git a/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs b/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs index 7df6a1f4f..fb99529f0 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs @@ -26,7 +26,7 @@ use std::path::PathBuf; const MAX_FRAME_LEN: usize = 8 * 1024 * 1024; /// Protocol version shared by the parent process and elevated command runner. -pub const IPC_PROTOCOL_VERSION: u8 = 3; +pub const IPC_PROTOCOL_VERSION: u8 = 4; /// Length-prefixed, JSON-encoded frame. #[derive(Debug, Serialize, Deserialize, Clone)] @@ -118,7 +118,17 @@ pub struct ExitPayload { #[derive(Debug, Serialize, Deserialize, Clone)] pub struct ErrorPayload { pub message: String, - pub code: String, + pub stage: ErrorStage, + pub windows_error_code: Option, +} + +/// Runner startup stage that produced an error. +#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ErrorStage { + ReadSpawnRequest, + SpawnChild, + WriteSpawnReady, } /// Empty payload for control messages. @@ -236,4 +246,32 @@ mod tests { assert_eq!(PermissionProfile::read_only(), payload.permission_profile); assert_eq!(workspace_roots, payload.workspace_roots); } + + #[test] + fn error_payload_serializes_stage_and_windows_error_code() { + let msg = FramedMessage { + version: IPC_PROTOCOL_VERSION, + message: Message::Error { + payload: ErrorPayload { + message: "CreateProcessAsUserW failed".to_string(), + stage: ErrorStage::SpawnChild, + windows_error_code: Some(1312), + }, + }, + }; + + let encoded = serde_json::to_value(&msg).expect("serialize"); + assert_eq!( + serde_json::json!({ + "version": IPC_PROTOCOL_VERSION, + "type": "error", + "payload": { + "message": "CreateProcessAsUserW failed", + "stage": "spawn_child", + "windows_error_code": 1312, + } + }), + encoded + ); + } } diff --git a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs index 6919face4..1d5ea9297 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs @@ -1,4 +1,6 @@ use crate::identity::SandboxCreds; +use crate::ipc_framed::ErrorPayload; +use crate::ipc_framed::ErrorStage; use crate::ipc_framed::FramedMessage; use crate::ipc_framed::IPC_PROTOCOL_VERSION; use crate::ipc_framed::Message; @@ -29,6 +31,7 @@ use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS; use windows_sys::Win32::Foundation::DuplicateHandle; use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE; +use windows_sys::Win32::Foundation::ERROR_NO_SUCH_LOGON_SESSION; use windows_sys::Win32::Foundation::ERROR_NOT_FOUND; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; @@ -63,14 +66,69 @@ impl std::fmt::Display for RunnerLogonError { impl std::error::Error for RunnerLogonError {} +#[derive(Debug)] +pub(crate) struct RunnerStartupError { + payload: ErrorPayload, +} + +impl RunnerStartupError { + pub(crate) fn new(payload: ErrorPayload) -> Self { + Self { payload } + } +} + +impl std::fmt::Display for RunnerStartupError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "runner failed during {:?}: {}", + self.payload.stage, self.payload.message + )?; + if let Some(code) = self.payload.windows_error_code { + write!(f, " (Windows error {code})")?; + } + Ok(()) + } +} + +impl std::error::Error for RunnerStartupError {} + pub(crate) struct RunnerTransport { pipe_write: File, pipe_read: File, } -pub(crate) fn is_stale_sandbox_creds_error(err: &anyhow::Error) -> bool { - err.downcast_ref::() - .is_some_and(|err| err.code == ERROR_LOGON_FAILURE) +fn is_refreshable_windows_error(code: u32) -> bool { + matches!(code, ERROR_LOGON_FAILURE | ERROR_NO_SUCH_LOGON_SESSION) +} + +pub(crate) fn is_refreshable_sandbox_creds_error(err: &anyhow::Error) -> bool { + if err + .downcast_ref::() + .is_some_and(|err| is_refreshable_windows_error(err.code)) + { + return true; + } + + err.downcast_ref::().is_some_and(|err| { + err.payload.stage == ErrorStage::SpawnChild + && err + .payload + .windows_error_code + .is_some_and(is_refreshable_windows_error) + }) +} + +pub(crate) fn retry_runner_spawn_once( + sandbox_creds: SandboxCreds, + mut spawn: impl FnMut(SandboxCreds) -> Result, + refresh: impl FnOnce() -> Result, +) -> Result { + match spawn(sandbox_creds) { + Ok(result) => Ok(result), + Err(err) if is_refreshable_sandbox_creds_error(&err) => spawn(refresh()?), + Err(err) => Err(err), + } } impl RunnerTransport { @@ -90,7 +148,7 @@ impl RunnerTransport { .ok_or_else(|| anyhow::anyhow!("runner pipe closed before spawn_ready"))?; match msg.message { Message::SpawnReady { .. } => Ok(()), - Message::Error { payload } => Err(anyhow::anyhow!("runner error: {}", payload.message)), + Message::Error { payload } => Err(RunnerStartupError::new(payload).into()), other => Err(anyhow::anyhow!( "expected spawn_ready from runner, got {other:?}" )), @@ -416,20 +474,46 @@ fn wait_for_complete_frame(pipe_read: &File, timeout: Duration) -> Result<()> { #[cfg(test)] mod tests { use super::RunnerLogonError; - use super::is_stale_sandbox_creds_error; + use super::RunnerStartupError; + use super::is_refreshable_sandbox_creds_error; + use crate::ipc_framed::ErrorPayload; + use crate::ipc_framed::ErrorStage; use pretty_assertions::assert_eq; use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE; + use windows_sys::Win32::Foundation::ERROR_NO_SUCH_LOGON_SESSION; use windows_sys::Win32::Foundation::ERROR_NOT_FOUND; #[test] - fn stale_sandbox_creds_error_recognizes_logon_failures() { + fn refreshable_sandbox_creds_error_recognizes_credential_and_child_start_failures() { assert_eq!( - [ERROR_LOGON_FAILURE, ERROR_NOT_FOUND].map(|code| { + [ + ERROR_LOGON_FAILURE, + ERROR_NO_SUCH_LOGON_SESSION, + ERROR_NOT_FOUND, + ] + .map(|code| { let err = anyhow::Error::new(RunnerLogonError { code }).context("runner launch failed"); - is_stale_sandbox_creds_error(&err) + is_refreshable_sandbox_creds_error(&err) }), - [true, false] + [true, true, false] + ); + + assert_eq!( + [ + (ErrorStage::SpawnChild, ERROR_NO_SUCH_LOGON_SESSION), + (ErrorStage::SpawnChild, ERROR_NOT_FOUND), + (ErrorStage::ReadSpawnRequest, ERROR_NO_SUCH_LOGON_SESSION), + ] + .map(|(stage, windows_error_code)| { + let err = anyhow::Error::new(RunnerStartupError::new(ErrorPayload { + message: "runner startup failed".to_string(), + stage, + windows_error_code: Some(windows_error_code), + })); + is_refreshable_sandbox_creds_error(&err) + }), + [true, false, false] ); } } diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index 1da6f3294..ccda7429f 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -44,7 +44,7 @@ mod windows_impl { use crate::logging::log_start; use crate::logging::log_success; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; - use crate::runner_client::is_stale_sandbox_creds_error; + use crate::runner_client::retry_runner_spawn_once; use crate::runner_client::spawn_runner_transport; use crate::sandbox_utils::ensure_codex_home_exists; use crate::sandbox_utils::inject_git_safe_directory; @@ -139,7 +139,7 @@ mod windows_impl { let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path()); log_start(&command, logs_base_dir); - let mut sandbox_creds = require_logon_sandbox_creds( + let sandbox_creds = require_logon_sandbox_creds( &permissions, cwd, &env_map, @@ -195,16 +195,19 @@ mod windows_impl { stdin_open: false, use_private_desktop, }; - let transport = match spawn_runner_transport( - codex_home, - cwd, - &sandbox_creds, - logs_base_dir, - spawn_request.clone(), - ) { - Ok(transport) => transport, - Err(err) if is_stale_sandbox_creds_error(&err) => { - sandbox_creds = refresh_logon_sandbox_creds( + let transport = retry_runner_spawn_once( + sandbox_creds, + |sandbox_creds| { + spawn_runner_transport( + codex_home, + cwd, + &sandbox_creds, + logs_base_dir, + spawn_request.clone(), + ) + }, + || { + refresh_logon_sandbox_creds( &permissions, cwd, &env_map, @@ -216,17 +219,9 @@ mod windows_impl { &deny_write_paths_override, proxy_enforced, crate::WindowsSandboxProxySettingsMode::Reconcile, - )?; - spawn_runner_transport( - codex_home, - cwd, - &sandbox_creds, - logs_base_dir, - spawn_request, - )? - } - Err(err) => return Err(err), - }; + ) + }, + )?; let (pipe_write, mut pipe_read) = transport.into_files(); let cancel_writer = spawn_cancel_writer(&pipe_write, cancellation)?; diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 766faf1a0..2a394f11f 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -195,6 +195,8 @@ pub use identity::sandbox_setup_is_complete; #[cfg(target_os = "windows")] pub use ipc_framed::ErrorPayload; #[cfg(target_os = "windows")] +pub use ipc_framed::ErrorStage; +#[cfg(target_os = "windows")] pub use ipc_framed::ExitPayload; #[cfg(target_os = "windows")] pub use ipc_framed::FramedMessage; diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index 0899d34c2..86859baf1 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -4,6 +4,7 @@ use crate::proc_thread_attr::ProcThreadAttributeList; use crate::winutil::argv_to_command_line; use crate::winutil::format_last_error; use crate::winutil::to_wide; +use anyhow::Context; use anyhow::Result; use anyhow::anyhow; use std::collections::HashMap; @@ -146,7 +147,7 @@ pub unsafe fn create_process_as_user( creation_flags, ); logging::debug_log(&msg, logs_base_dir); - return Err(anyhow!("CreateProcessAsUserW failed: {err}")); + return Err(std::io::Error::from_raw_os_error(err)).context(msg); } Ok(CreatedProcess { process_info: pi, @@ -187,7 +188,7 @@ pub unsafe fn create_process_as_user( creation_flags, ); logging::debug_log(&msg, logs_base_dir); - return Err(anyhow!("CreateProcessAsUserW failed: {err}")); + return Err(std::io::Error::from_raw_os_error(err)).context(msg); } Ok(CreatedProcess { process_info: pi, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs index efe4a3f42..4fc07d9c4 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs @@ -12,7 +12,7 @@ use crate::ipc_framed::Message; use crate::ipc_framed::SpawnRequest; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use crate::runner_client::RunnerTransport; -use crate::runner_client::is_stale_sandbox_creds_error; +use crate::runner_client::retry_runner_spawn_once; use crate::runner_client::spawn_runner_transport; use crate::spawn_prep::prepare_elevated_spawn_context_for_permissions; use anyhow::Result; @@ -27,20 +27,79 @@ use tokio::sync::broadcast; use tokio::sync::mpsc; use tokio::sync::oneshot; -async fn spawn_runner_transport_task( +struct RunnerTransportRequest { + permissions: ResolvedWindowsSandboxPermissions, codex_home: PathBuf, cwd: PathBuf, - sandbox_creds: SandboxCreds, + env_map: HashMap, logs_base_dir: Option, spawn_request: SpawnRequest, + read_roots_override: Option>, + read_roots_include_platform_defaults: bool, + write_roots_override: Option>, + deny_read_paths_override: Vec, + deny_write_paths_override: Vec, + proxy_enforced: bool, + proxy_settings_mode: crate::WindowsSandboxProxySettingsMode, +} + +fn spawn_runner_transport_with_retry( + sandbox_creds: SandboxCreds, + request: &RunnerTransportRequest, + mut spawn: impl FnMut(&Path, &Path, &SandboxCreds, Option<&Path>, SpawnRequest) -> Result, + refresh: impl FnOnce( + &ResolvedWindowsSandboxPermissions, + &Path, + &HashMap, + &Path, + Option<&[PathBuf]>, + bool, + Option<&[PathBuf]>, + &[PathBuf], + &[PathBuf], + bool, + crate::WindowsSandboxProxySettingsMode, + ) -> Result, +) -> Result { + retry_runner_spawn_once( + sandbox_creds, + |sandbox_creds| { + spawn( + &request.codex_home, + &request.cwd, + &sandbox_creds, + request.logs_base_dir.as_deref(), + request.spawn_request.clone(), + ) + }, + || { + refresh( + &request.permissions, + &request.cwd, + &request.env_map, + &request.codex_home, + request.read_roots_override.as_deref(), + request.read_roots_include_platform_defaults, + request.write_roots_override.as_deref(), + &request.deny_read_paths_override, + &request.deny_write_paths_override, + request.proxy_enforced, + request.proxy_settings_mode, + ) + }, + ) +} + +async fn spawn_runner_transport_task( + sandbox_creds: SandboxCreds, + request: RunnerTransportRequest, ) -> Result { tokio::task::spawn_blocking(move || -> Result<_> { - spawn_runner_transport( - &codex_home, - &cwd, - &sandbox_creds, - logs_base_dir.as_deref(), - spawn_request, + spawn_runner_transport_with_retry( + sandbox_creds, + &request, + spawn_runner_transport, + refresh_logon_sandbox_creds, ) }) .await @@ -95,59 +154,36 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil proxy_settings_mode, )?; - let spawn_request = SpawnRequest { - command: command.clone(), - cwd: cwd.to_path_buf(), - env: env_map.clone(), - permission_profile: permission_profile.clone(), - workspace_roots: workspace_roots.to_vec(), - codex_home: elevated.sandbox_base.clone(), - real_codex_home: codex_home.to_path_buf(), - cap_sids: elevated.cap_sids.clone(), - timeout_ms, - tty, - stdin_open, - use_private_desktop, - }; - let codex_home = codex_home.to_path_buf(); - let cwd = cwd.to_path_buf(); let sandbox_creds = elevated.sandbox_creds; - let logs_base_dir = elevated.logs_base_dir.clone(); - let transport = match spawn_runner_transport_task( - codex_home.clone(), - cwd.clone(), - sandbox_creds, - logs_base_dir.clone(), - spawn_request.clone(), - ) - .await - { - Ok(transport) => transport, - Err(err) if is_stale_sandbox_creds_error(&err) => { - let sandbox_creds = refresh_logon_sandbox_creds( - &permissions, - &cwd, - &env_map, - &codex_home, - read_roots_override, - read_roots_include_platform_defaults, - write_roots_override, - &deny_read_paths_override, - &deny_write_paths_override, - /*proxy_enforced*/ false, - proxy_settings_mode, - )?; - spawn_runner_transport_task( - codex_home, - cwd, - sandbox_creds, - logs_base_dir, - spawn_request, - ) - .await? - } - Err(err) => return Err(err), + let request = RunnerTransportRequest { + permissions, + codex_home: codex_home.to_path_buf(), + cwd: cwd.to_path_buf(), + env_map: env_map.clone(), + logs_base_dir: elevated.logs_base_dir, + spawn_request: SpawnRequest { + command, + cwd: cwd.to_path_buf(), + env: env_map, + permission_profile: permission_profile.clone(), + workspace_roots: workspace_roots.to_vec(), + codex_home: elevated.sandbox_base, + real_codex_home: codex_home.to_path_buf(), + cap_sids: elevated.cap_sids, + timeout_ms, + tty, + stdin_open, + use_private_desktop, + }, + read_roots_override: read_roots_override.map(<[PathBuf]>::to_vec), + read_roots_include_platform_defaults, + write_roots_override: write_roots_override.map(<[PathBuf]>::to_vec), + deny_read_paths_override, + deny_write_paths_override, + proxy_enforced, + proxy_settings_mode, }; + let transport = spawn_runner_transport_task(sandbox_creds, request).await?; let (pipe_write, pipe_read) = transport.into_files(); let (writer_tx, writer_rx) = mpsc::channel::>(128); @@ -197,3 +233,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil stdin_open, )) } + +#[cfg(test)] +#[path = "elevated_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs new file mode 100644 index 000000000..952727a21 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs @@ -0,0 +1,192 @@ +use super::RunnerTransportRequest; +use super::spawn_runner_transport_with_retry; +use crate::WindowsSandboxProxySettingsMode; +use crate::identity::SandboxCreds; +use crate::ipc_framed::ErrorPayload; +use crate::ipc_framed::ErrorStage; +use crate::ipc_framed::SpawnRequest; +use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; +use crate::runner_client::RunnerStartupError; +use codex_protocol::models::PermissionProfile; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use serde_json::Value; +use std::cell::Cell; +use std::cell::RefCell; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; +use windows_sys::Win32::Foundation::ERROR_NO_SUCH_LOGON_SESSION; + +#[derive(Debug, Clone, PartialEq, Eq)] +struct SpawnObservation { + codex_home: PathBuf, + cwd: PathBuf, + username: String, + password: String, + logs_base_dir: Option, + spawn_request: Value, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct RefreshObservation { + permissions: ResolvedWindowsSandboxPermissions, + cwd: PathBuf, + env_map: HashMap, + codex_home: PathBuf, + read_roots_override: Option>, + read_roots_include_platform_defaults: bool, + write_roots_override: Option>, + deny_read_paths_override: Vec, + deny_write_paths_override: Vec, + proxy_enforced: bool, + proxy_settings_mode: WindowsSandboxProxySettingsMode, +} + +#[test] +fn retry_uses_original_unified_exec_request_and_stops_after_second_failure() { + let workspace_root = AbsolutePathBuf::from_absolute_path(PathBuf::from(r"C:\workspace")) + .expect("absolute workspace root"); + let permission_profile = PermissionProfile::workspace_write(); + let permissions = + ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots( + &permission_profile, + std::slice::from_ref(&workspace_root), + ) + .expect("resolved permissions"); + let env_map = HashMap::from([ + ("Path".to_string(), r"C:\tools".to_string()), + ( + "HTTPS_PROXY".to_string(), + "http://localhost:1234".to_string(), + ), + ]); + let request = RunnerTransportRequest { + permissions: permissions.clone(), + codex_home: PathBuf::from(r"C:\Users\codex"), + cwd: PathBuf::from(r"C:\workspace"), + env_map: env_map.clone(), + logs_base_dir: Some(PathBuf::from(r"C:\Users\codex\.sandbox")), + spawn_request: SpawnRequest { + command: vec!["pwsh.exe".to_string(), "-NoProfile".to_string()], + cwd: PathBuf::from(r"C:\workspace"), + env: env_map.clone(), + permission_profile, + workspace_roots: vec![workspace_root], + codex_home: PathBuf::from(r"C:\Users\codex\.sandbox"), + real_codex_home: PathBuf::from(r"C:\Users\codex"), + cap_sids: vec!["S-1-15-3-1024-1".to_string()], + timeout_ms: Some(5_000), + tty: true, + stdin_open: true, + use_private_desktop: true, + }, + read_roots_override: Some(vec![PathBuf::from(r"C:\workspace\read")]), + read_roots_include_platform_defaults: true, + write_roots_override: Some(vec![PathBuf::from(r"C:\workspace\write")]), + deny_read_paths_override: vec![PathBuf::from(r"C:\secrets")], + deny_write_paths_override: vec![PathBuf::from(r"C:\workspace\.codex")], + proxy_enforced: true, + proxy_settings_mode: WindowsSandboxProxySettingsMode::Preserve, + }; + let expected_spawn_request = + serde_json::to_value(&request.spawn_request).expect("serialize spawn request"); + let expected_refresh = RefreshObservation { + permissions, + cwd: request.cwd.clone(), + env_map, + codex_home: request.codex_home.clone(), + read_roots_override: request.read_roots_override.clone(), + read_roots_include_platform_defaults: true, + write_roots_override: request.write_roots_override.clone(), + deny_read_paths_override: request.deny_read_paths_override.clone(), + deny_write_paths_override: request.deny_write_paths_override.clone(), + proxy_enforced: true, + proxy_settings_mode: WindowsSandboxProxySettingsMode::Preserve, + }; + let spawn_observations = RefCell::new(Vec::new()); + let refresh_observations = RefCell::new(Vec::new()); + let spawn_attempts = Cell::new(0); + + let result = spawn_runner_transport_with_retry( + SandboxCreds { + username: "stale".to_string(), + password: "old".to_string(), + }, + &request, + |codex_home, cwd, sandbox_creds, logs_base_dir, spawn_request| { + spawn_observations.borrow_mut().push(SpawnObservation { + codex_home: codex_home.to_path_buf(), + cwd: cwd.to_path_buf(), + username: sandbox_creds.username.clone(), + password: sandbox_creds.password.clone(), + logs_base_dir: logs_base_dir.map(Path::to_path_buf), + spawn_request: serde_json::to_value(spawn_request) + .expect("serialize spawn request"), + }); + spawn_attempts.set(spawn_attempts.get() + 1); + Err::<(), _>(anyhow::Error::new(RunnerStartupError::new(ErrorPayload { + message: format!("spawn attempt {} failed", spawn_attempts.get()), + stage: ErrorStage::SpawnChild, + windows_error_code: Some(ERROR_NO_SUCH_LOGON_SESSION), + }))) + }, + |permissions, + cwd, + env_map, + codex_home, + read_roots_override, + read_roots_include_platform_defaults, + write_roots_override, + deny_read_paths_override, + deny_write_paths_override, + proxy_enforced, + proxy_settings_mode| { + refresh_observations.borrow_mut().push(RefreshObservation { + permissions: permissions.clone(), + cwd: cwd.to_path_buf(), + env_map: env_map.clone(), + codex_home: codex_home.to_path_buf(), + read_roots_override: read_roots_override.map(<[PathBuf]>::to_vec), + read_roots_include_platform_defaults, + write_roots_override: write_roots_override.map(<[PathBuf]>::to_vec), + deny_read_paths_override: deny_read_paths_override.to_vec(), + deny_write_paths_override: deny_write_paths_override.to_vec(), + proxy_enforced, + proxy_settings_mode, + }); + Ok(SandboxCreds { + username: "refreshed".to_string(), + password: "new".to_string(), + }) + }, + ); + + let err = result.expect_err("second spawn should fail"); + assert_eq!( + "runner failed during SpawnChild: spawn attempt 2 failed (Windows error 1312)", + err.to_string() + ); + assert_eq!( + vec![ + SpawnObservation { + codex_home: request.codex_home.clone(), + cwd: request.cwd.clone(), + username: "stale".to_string(), + password: "old".to_string(), + logs_base_dir: request.logs_base_dir.clone(), + spawn_request: expected_spawn_request.clone(), + }, + SpawnObservation { + codex_home: request.codex_home.clone(), + cwd: request.cwd.clone(), + username: "refreshed".to_string(), + password: "new".to_string(), + logs_base_dir: request.logs_base_dir, + spawn_request: expected_spawn_request, + }, + ], + *spawn_observations.borrow() + ); + assert_eq!(vec![expected_refresh], *refresh_observations.borrow()); +}