diff --git a/codex-rs/app-server-daemon/src/manual_update.rs b/codex-rs/app-server-daemon/src/manual_update.rs index 640c5bc1f6..6bd0b01db0 100644 --- a/codex-rs/app-server-daemon/src/manual_update.rs +++ b/codex-rs/app-server-daemon/src/manual_update.rs @@ -23,6 +23,8 @@ use crate::client; use crate::managed_install::executable_identity; use crate::managed_install::managed_codex_version; +pub(super) const MAX_RESPONSE_BYTES: u64 = 16 * 1024; + pub(crate) async fn request(daemon: &Daemon) -> Result { let socket_path = daemon.manual_update_socket_path(); let mut replacement_deadline = None; @@ -103,7 +105,7 @@ pub(crate) async fn request(daemon: &Daemon) -> Result { connection.ensure_non_elevated_peer()?; connection.write_all(b"update\n").await?; connection - .take(16 * 1024) + .take(MAX_RESPONSE_BYTES) .read_to_end(&mut response) .await?; Ok::<_, std::io::Error>(()) diff --git a/codex-rs/app-server-daemon/src/update_loop.rs b/codex-rs/app-server-daemon/src/update_loop.rs index 499acd113e..4dc4adf85f 100644 --- a/codex-rs/app-server-daemon/src/update_loop.rs +++ b/codex-rs/app-server-daemon/src/update_loop.rs @@ -1,9 +1,12 @@ //! Owns scheduled and manual installs, daemon restarts, and updater replacement. +use std::collections::VecDeque; use std::path::Path; #[cfg(unix)] use std::process::Command as StdCommand; use std::process::Stdio; +use std::sync::Arc; +use std::sync::Mutex; use std::time::Duration; use anyhow::Context; @@ -14,6 +17,7 @@ use codex_http_client::RouteAwareClientPool; use futures::FutureExt; #[cfg(unix)] use std::os::unix::process::CommandExt; +use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; use tokio::process::Command; #[cfg(unix)] @@ -62,6 +66,10 @@ pub(crate) async fn request_manual_update( const INITIAL_UPDATE_DELAY: Duration = Duration::from_secs(5 * 60); const RESTART_RETRY_INTERVAL: Duration = Duration::from_millis(50); +// Manual update responses are capped after JSON serialization. This leaves room for the status, +// envelope, and the worst-case six-byte JSON escape for every captured byte. +const INSTALLER_STDERR_TAIL_BYTES: usize = 2 * 1024; +const INSTALLER_STDERR_DRAIN_TIMEOUT: Duration = Duration::from_secs(1); #[cfg(unix)] const INSTALL_URL: &str = "https://chatgpt.com/codex/install.sh"; #[cfg(windows)] @@ -571,7 +579,7 @@ async fn run_installer_script( let mut command = { let mut command = Command::new("powershell.exe"); command.creation_flags(windows_sys::Win32::System::Threading::CREATE_NO_WINDOW); - command.args(["-NoLogo", "-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", "try { Invoke-Expression ([Console]::In.ReadToEnd()) } catch { Write-Error $_; exit 1 }"]) + command.args(["-NoLogo", "-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", "[Console]::OutputEncoding = [System.Text.UTF8Encoding]::new($false); try { Invoke-Expression ([Console]::In.ReadToEnd()) } catch { Write-Error $_; exit 1 }"]) .kill_on_drop(true); command }; @@ -605,9 +613,44 @@ async fn run_installer_script( .kill_on_drop(true) .stdin(Stdio::piped()) .stdout(Stdio::null()) - .stderr(Stdio::null()) + .stderr(Stdio::piped()) .spawn() .context("failed to invoke standalone Codex updater")?; + let mut stderr = child + .stderr + .take() + .context("standalone Codex updater stderr was unavailable")?; + let stderr_tail = Arc::new(Mutex::new(VecDeque::with_capacity( + INSTALLER_STDERR_TAIL_BYTES, + ))); + let captured_tail = Arc::clone(&stderr_tail); + let mut stderr_task = tokio::spawn(async move { + let mut buf = [0; 4096]; + loop { + let read = stderr.read(&mut buf).await?; + if read == 0 { + return Ok::<_, std::io::Error>(()); + } + let Ok(mut tail) = captured_tail.lock() else { + return Ok(()); + }; + if read >= INSTALLER_STDERR_TAIL_BYTES { + tail.clear(); + tail.extend( + buf[read - INSTALLER_STDERR_TAIL_BYTES..read] + .iter() + .copied(), + ); + } else { + let retained = INSTALLER_STDERR_TAIL_BYTES - read; + if tail.len() > retained { + let remove = tail.len() - retained; + tail.drain(..remove); + } + tail.extend(buf[..read].iter().copied()); + } + } + }); #[cfg(windows)] let _installer_job = crate::backend::windows::installer_job(&child)?; let mut stdin = child @@ -627,11 +670,13 @@ async fn run_installer_script( #[cfg(unix)] if write_result.is_none() { cancel_installer(&mut child, package_root).await; + stderr_task.abort(); return Ok(UpdateLoopControl::Stop); } - write_result - .context("installer write was cancelled")? - .context("failed to pass standalone Codex updater to shell")?; + if let Some(Err(err)) = write_result { + stderr_task.abort(); + return Err(err).context("failed to pass standalone Codex updater to shell"); + } #[cfg(unix)] let status = tokio::select! { result = child.wait() => result, @@ -642,12 +687,30 @@ async fn run_installer_script( }; #[cfg(windows)] let status = child.wait().await; - let status = status.context("failed to wait for standalone Codex updater")?; + let status = match status { + Ok(status) => status, + Err(err) => { + stderr_task.abort(); + return Err(err).context("failed to wait for standalone Codex updater"); + } + }; if status.success() { + stderr_task.abort(); Ok(UpdateLoopControl::Continue) } else { - anyhow::bail!("standalone Codex updater exited with status {status}") + let _ = tokio::time::timeout(INSTALLER_STDERR_DRAIN_TIMEOUT, &mut stderr_task).await; + stderr_task.abort(); + let bytes = stderr_tail + .lock() + .map(|tail| tail.iter().copied().collect::>()) + .unwrap_or_default(); + let detail = String::from_utf8_lossy(&bytes); + let detail = detail.trim(); + if detail.is_empty() { + anyhow::bail!("standalone Codex updater exited with status {status}") + } + anyhow::bail!("standalone Codex updater exited with status {status}:\n{detail}") } } diff --git a/codex-rs/app-server-daemon/src/update_loop_tests.rs b/codex-rs/app-server-daemon/src/update_loop_tests.rs index af89c36be3..2f7cd1cfad 100644 --- a/codex-rs/app-server-daemon/src/update_loop_tests.rs +++ b/codex-rs/app-server-daemon/src/update_loop_tests.rs @@ -511,6 +511,46 @@ async fn manual_request_retries_after_updater_replacement() { server.await.expect("replacement task"); } +#[cfg(unix)] +#[tokio::test] +async fn manual_request_accepts_maximum_installer_stderr_detail() { + use tokio::io::AsyncReadExt; + use tokio::io::AsyncWriteExt; + + let home = TempDir::new().expect("home"); + let (daemon, _) = manual_update_daemon(&home); + let socket_path = daemon.manual_update_socket_path(); + codex_uds::prepare_private_socket_directory(socket_path.parent().expect("socket parent")) + .await + .expect("socket directory"); + let mut listener = codex_uds::UnixListener::bind(&socket_path) + .await + .expect("updater socket"); + let expected_suffix = "installer failed"; + let message = format!( + "standalone updater failed:\n{}{}", + "\0".repeat(super::INSTALLER_STDERR_TAIL_BYTES), + expected_suffix, + ); + let response = serde_json::to_vec(&Err::(message)).expect("serialize error"); + assert!(response.len() < super::manual_update::MAX_RESPONSE_BYTES as usize); + let server = tokio::spawn(async move { + let mut connection = listener.accept().await.expect("request connection"); + let mut request = [0; 7]; + connection.read_exact(&mut request).await.expect("request"); + connection + .write_all(&response) + .await + .expect("send response"); + }); + + let error = super::manual_update::request(&daemon) + .await + .expect_err("manual update should fail"); + assert!(error.to_string().ends_with(expected_suffix)); + server.await.expect("server task"); +} + #[cfg(unix)] #[tokio::test] async fn manual_request_recovers_when_one_shot_updater_exits() { @@ -1023,20 +1063,60 @@ Test-Installer .await .expect("installer succeeds"); let failing = FakeInstallerHttp::new(InstallerResponse::Success( - b"throw 'installer failed'".to_vec(), + b"throw ([string][char]0x00E9 + 'chec installation')".to_vec(), )); let script = super::fetch_installer_script(&failing) .await .expect("fetch failing installer"); - assert!( - super::run_installer_script( - &script, - super::InstallerMode::RestoreProduction("0.150.0-x86_64-pc-windows-msvc"), - std::path::Path::new("packages/app-server-daemon") - ) - .await - .is_err() + let error = super::run_installer_script( + &script, + super::InstallerMode::RestoreProduction("0.150.0-x86_64-pc-windows-msvc"), + std::path::Path::new("packages/app-server-daemon"), + ) + .await + .err() + .expect("installer should fail"); + assert!(error.to_string().contains("échec installation")); +} + +#[cfg(unix)] +#[tokio::test] +async fn installer_failure_includes_stderr() { + let script = format!( + "printf 'discard me{}' >&2\nprintf 'installer failed' >&2\nexit 1\n", + "x".repeat(super::INSTALLER_STDERR_TAIL_BYTES), ); + let error = super::run_installer_script( + script.as_bytes(), + super::InstallerMode::Update("0.150.0-x86_64-unknown-linux-gnu"), + std::path::Path::new("packages/app-server-daemon"), + futures::future::pending(), + ) + .await + .err() + .expect("installer should fail"); + let error = error.to_string(); + assert!(error.contains("installer failed")); + assert!(!error.contains("discard me")); +} + +#[cfg(unix)] +#[tokio::test] +async fn installer_failure_preserves_stderr_before_drain_timeout() { + let error = tokio::time::timeout( + Duration::from_secs(3), + super::run_installer_script( + b"printf 'installer failed' >&2\nsleep 5 &\nexit 1\n", + super::InstallerMode::Update("0.150.0-x86_64-unknown-linux-gnu"), + std::path::Path::new("packages/app-server-daemon"), + futures::future::pending(), + ), + ) + .await + .expect("stderr drain should time out") + .err() + .expect("installer should fail"); + assert!(error.to_string().contains("installer failed")); } #[cfg(unix)]