Include installer stderr in daemon update failures (#50499)

## Why

Daemon updates discard installer stderr, leaving failed installs with only an exit status and no installer diagnostics.

## What changed

Capture the last 2 KiB of installer stderr and append it to errors when the installer exits unsuccessfully. Drain stderr concurrently and wait at most one second after failure so subprocesses holding the pipe open cannot delay error reporting indefinitely. Keep captured details within the manual update response limit, and configure PowerShell output as UTF-8 to preserve non-ASCII diagnostics.

## Testing

Add coverage for stderr tail truncation, bounded draining when a subprocess keeps stderr open, and manual update responses containing heavily escaped diagnostics. Extend the Windows installer test to check non-ASCII error text.

GitOrigin-RevId: d0da19b233006f97770d3f7f5f057e5e98fd2913
This commit is contained in:
Eric Traut
2026-10-03 02:39:17 +00:00
committed by copyberry
parent 8f7a0f7a87
commit 55922b7984
3 changed files with 162 additions and 17 deletions
@@ -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<UpdateOutput> {
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<UpdateOutput> {
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>(())
+70 -7
View File
@@ -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::<Vec<_>>())
.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}")
}
}
@@ -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::<UpdateOutput, _>(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)]