mirror of
https://github.com/openai/codex.git
synced 2026-10-19 08:16:48 +00:00
Preserve daemon diagnostics and include updater logs in reports (#49843)
## Why Detached launches truncate stderr logs, losing details from previous failures. Updater handoffs can also leave existing writers with stale offsets after truncation. ## What changed - Record daemon lifecycle and update stages with timings, outcomes, and typed OS errors, omitting error messages and context. - Preserve the last 256 KiB of nonempty stderr logs before truncation and use append handles to prevent gaps during handoff. Preservation failures do not block launch. - Attach current and previous updater log tails when log inclusion is enabled for a report, with a 512 KiB total limit and legacy filename fallback. Skip missing, empty, and nonregular files. ## Testing Add regression coverage for diagnostic error redaction, bounded log preservation, append behavior during handoff, attachment selection, and updater logs in captured report uploads. GitOrigin-RevId: 918317022ab9dcfd1b13d676872706c77e9ffbb2
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
mod pid;
|
||||
mod stderr_log;
|
||||
#[cfg(windows)]
|
||||
pub(crate) mod windows;
|
||||
|
||||
|
||||
@@ -172,6 +172,7 @@ impl PidBackend {
|
||||
}
|
||||
|
||||
let pid = record.pid;
|
||||
crate::diagnostics::event("shutdown_requested", serde_json::json!({ "pid": pid }));
|
||||
let started_at = tokio::time::Instant::now();
|
||||
let force_after = Duration::from_secs(grace_seconds.into());
|
||||
let deadline = started_at + force_after + STOP_FORCE_TIMEOUT;
|
||||
@@ -227,6 +228,7 @@ impl PidBackend {
|
||||
break;
|
||||
}
|
||||
if !forced && started_at.elapsed() >= force_after {
|
||||
crate::diagnostics::event("shutdown_forced", serde_json::json!({ "pid": pid }));
|
||||
#[cfg(unix)]
|
||||
self.force_terminate_process(pid)?;
|
||||
#[cfg(windows)]
|
||||
@@ -353,18 +355,28 @@ impl PidBackend {
|
||||
#[cfg(any(unix, windows))]
|
||||
async fn open_stderr_log(&self) -> Result<fs::File> {
|
||||
let stderr_log_file = stderr_log_file_for_pid_file(&self.pid_file);
|
||||
fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.truncate(true)
|
||||
.write(true)
|
||||
.open(&stderr_log_file)
|
||||
.await
|
||||
.with_context(|| {
|
||||
format!(
|
||||
"failed to open stderr log for pid-managed app server {}",
|
||||
stderr_log_file.display()
|
||||
)
|
||||
})
|
||||
super::stderr_log::preserve(&stderr_log_file).await;
|
||||
let result = async {
|
||||
fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.truncate(true)
|
||||
.write(true)
|
||||
.open(&stderr_log_file)
|
||||
.await?;
|
||||
// The updater predecessor can still write after a Windows handoff
|
||||
// truncates this file. Append handles prevent stale offsets and gaps.
|
||||
fs::OpenOptions::new()
|
||||
.append(true)
|
||||
.open(&stderr_log_file)
|
||||
.await
|
||||
}
|
||||
.await;
|
||||
result.with_context(|| {
|
||||
format!(
|
||||
"failed to open stderr log for pid-managed app server {}",
|
||||
stderr_log_file.display()
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(any(unix, windows))]
|
||||
|
||||
@@ -188,10 +188,12 @@ impl PidBackend {
|
||||
}
|
||||
}
|
||||
|
||||
let started = std::time::Instant::now();
|
||||
#[cfg(windows)]
|
||||
let child = super::super::windows::spawn_without_inheriting_stdio(&mut command);
|
||||
#[cfg(not(windows))]
|
||||
let child = command.spawn().map_err(anyhow::Error::from);
|
||||
let child = crate::diagnostics::result("process_spawn", started, child);
|
||||
let child = match child {
|
||||
Ok(child) => child,
|
||||
Err(err) => {
|
||||
@@ -214,7 +216,9 @@ impl PidBackend {
|
||||
let pid = child
|
||||
.id()
|
||||
.context("spawned app-server process has no pid")?;
|
||||
let record = match async {
|
||||
crate::diagnostics::event("process_spawned", serde_json::json!({ "pid": pid }));
|
||||
let started = std::time::Instant::now();
|
||||
let record = async {
|
||||
let process_start_time = read_process_start_time(pid).await?;
|
||||
#[cfg(any(target_os = "linux", target_os = "macos"))]
|
||||
let process_identity = super::identity::read_process_details(pid)
|
||||
@@ -230,8 +234,8 @@ impl PidBackend {
|
||||
executable_identity: launched_identity,
|
||||
})
|
||||
}
|
||||
.await
|
||||
{
|
||||
.await;
|
||||
let record = match crate::diagnostics::result("process_identity", started, record) {
|
||||
Ok(record) => record,
|
||||
Err(err) => {
|
||||
let _ = self.terminate_process(pid);
|
||||
@@ -246,7 +250,14 @@ impl PidBackend {
|
||||
};
|
||||
let contents = serde_json::to_vec(&record).context("failed to serialize pid record")?;
|
||||
let temp_pid_file = self.pid_file.with_extension("pid.tmp");
|
||||
if let Err(err) = fs::write(&temp_pid_file, &contents).await {
|
||||
let started = std::time::Instant::now();
|
||||
if let Err(err) = crate::diagnostics::result(
|
||||
"pid_write",
|
||||
started,
|
||||
fs::write(&temp_pid_file, &contents)
|
||||
.await
|
||||
.map_err(anyhow::Error::from),
|
||||
) {
|
||||
let _ = self.terminate_process(pid);
|
||||
if replacement.is_none() {
|
||||
let _ = fs::remove_file(&self.pid_file).await;
|
||||
@@ -255,7 +266,14 @@ impl PidBackend {
|
||||
format!("failed to write pid temp file {}", temp_pid_file.display())
|
||||
});
|
||||
}
|
||||
if let Err(err) = fs::rename(&temp_pid_file, &self.pid_file).await {
|
||||
let started = std::time::Instant::now();
|
||||
if let Err(err) = crate::diagnostics::result(
|
||||
"pid_publish",
|
||||
started,
|
||||
fs::rename(&temp_pid_file, &self.pid_file)
|
||||
.await
|
||||
.map_err(anyhow::Error::from),
|
||||
) {
|
||||
let _ = self.terminate_process(pid);
|
||||
let _ = fs::remove_file(&temp_pid_file).await;
|
||||
if replacement.is_none() {
|
||||
|
||||
@@ -756,6 +756,7 @@ async fn failed_updater_handoff_preserves_predecessor_record() {
|
||||
#[tokio::test]
|
||||
async fn updater_readiness_and_post_publication_failure_preserve_ownership() {
|
||||
use futures::FutureExt;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
use windows_sys::Win32::Security::ImpersonateAnonymousToken;
|
||||
use windows_sys::Win32::Security::RevertToSelf;
|
||||
use windows_sys::Win32::System::Threading::GetCurrentThread;
|
||||
@@ -770,13 +771,23 @@ async fn updater_readiness_and_post_publication_failure_preserve_ownership() {
|
||||
.acquire_reservation_lock()
|
||||
.await
|
||||
.expect("reservation lock");
|
||||
let mut predecessor_stderr = backend.open_stderr_log().await.expect("predecessor log");
|
||||
predecessor_stderr
|
||||
.write_all(&vec![b'x'; 512 * 1024])
|
||||
.await
|
||||
.unwrap();
|
||||
predecessor_stderr.flush().await.unwrap();
|
||||
let successor_stderr = backend.open_stderr_log().await.expect("successor log");
|
||||
let ready = backend.pid_file.with_extension("ready");
|
||||
let mut child = tokio::process::Command::new("powershell.exe")
|
||||
.args([
|
||||
"-NoProfile",
|
||||
"-NonInteractive",
|
||||
"-Command",
|
||||
"Start-Sleep 60",
|
||||
"[Console]::Error.WriteLine('successor startup'); [IO.File]::WriteAllText($env:CODEX_TEST_UPDATER_READY, ''); Start-Sleep 60",
|
||||
])
|
||||
.env("CODEX_TEST_UPDATER_READY", &ready)
|
||||
.stderr(Stdio::from(successor_stderr.into_std().await))
|
||||
.creation_flags(windows_sys::Win32::System::Threading::CREATE_NO_WINDOW)
|
||||
.kill_on_drop(true)
|
||||
.spawn()
|
||||
@@ -801,12 +812,21 @@ async fn updater_readiness_and_post_publication_failure_preserve_ownership() {
|
||||
tokio::fs::write(&backend.pid_file, serde_json::to_vec(&successor).unwrap())
|
||||
.await
|
||||
.unwrap();
|
||||
let ready = backend.pid_file.with_extension("ready");
|
||||
tokio::fs::write(&ready, b"").await.unwrap();
|
||||
backend
|
||||
.finish_updater_start(&successor, Some(&predecessor))
|
||||
.await
|
||||
.expect("ready successor");
|
||||
predecessor_stderr
|
||||
.write_all(b"predecessor handoff complete\n")
|
||||
.await
|
||||
.unwrap();
|
||||
predecessor_stderr.flush().await.unwrap();
|
||||
let log_path = stderr_log_file_for_pid_file(&backend.pid_file);
|
||||
let log = tokio::fs::read(&log_path).await.unwrap();
|
||||
assert!(log.len() < 256 * 1024);
|
||||
let log = String::from_utf8(log).expect("no stale-offset gap");
|
||||
assert!(log.contains("successor startup"));
|
||||
assert!(log.contains("predecessor handoff complete"));
|
||||
assert!(!ready.exists());
|
||||
assert_eq!(
|
||||
backend.read_pid_file_state().await.unwrap(),
|
||||
@@ -934,3 +954,47 @@ fn inaccessible_pid_preserves_identity_check_error() {
|
||||
.join()
|
||||
.expect("anonymous identity check");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stderr_preservation_failure_does_not_prevent_opening_new_log() -> anyhow::Result<()> {
|
||||
let home = TempDir::new()?;
|
||||
let pid_file = home.path().join("daemon.pid");
|
||||
let path = stderr_log_file_for_pid_file(&pid_file);
|
||||
let backend = PidBackend::new(
|
||||
home.path().join("codex"),
|
||||
pid_file,
|
||||
/*remote_control_enabled*/ false,
|
||||
);
|
||||
tokio::fs::write(&path, b"previous failure").await?;
|
||||
tokio::fs::create_dir(path.with_extension("log.previous")).await?;
|
||||
let _log = backend.open_stderr_log().await?;
|
||||
assert_eq!(tokio::fs::read(&path).await?, b"");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stderr_handoff_appends_after_truncation_without_gaps() -> anyhow::Result<()> {
|
||||
use tokio::io::AsyncWriteExt;
|
||||
|
||||
let home = TempDir::new()?;
|
||||
let backend = PidBackend::new_update_loop(
|
||||
home.path().join("codex"),
|
||||
home.path().join("updater.pid"),
|
||||
/*restore_release*/ None,
|
||||
);
|
||||
let mut predecessor = backend.open_stderr_log().await?;
|
||||
predecessor.write_all(&vec![b'x'; 512 * 1024]).await?;
|
||||
predecessor.flush().await?;
|
||||
let mut successor = backend.open_stderr_log().await?;
|
||||
successor.write_all(b"successor startup failure\n").await?;
|
||||
successor.flush().await?;
|
||||
predecessor
|
||||
.write_all(b"predecessor handoff complete\n")
|
||||
.await?;
|
||||
predecessor.flush().await?;
|
||||
assert_eq!(
|
||||
tokio::fs::read(stderr_log_file_for_pid_file(&backend.pid_file)).await?,
|
||||
b"successor startup failure\npredecessor handoff complete\n"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
//! Saves a bounded previous-launch stderr tail before a detached launch truncates its log.
|
||||
//! Preservation is best effort; missing or empty logs leave the last useful tail intact.
|
||||
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::time::Instant;
|
||||
|
||||
use tokio::io::AsyncReadExt;
|
||||
use tokio::io::AsyncSeekExt;
|
||||
|
||||
const MAX_PREVIOUS_LOG_BYTES: u64 = 256 * 1024;
|
||||
|
||||
pub(super) async fn preserve(path: &Path) {
|
||||
let started = Instant::now();
|
||||
let result = async {
|
||||
match tokio::fs::symlink_metadata(path).await {
|
||||
Ok(metadata) if !metadata.is_file() => return Ok(()),
|
||||
Ok(_) => {}
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(error) => return Err(error.into()),
|
||||
}
|
||||
let mut file = match tokio::fs::File::open(path).await {
|
||||
Ok(file) => file,
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(error) => return Err(error.into()),
|
||||
};
|
||||
let metadata = file.metadata().await?;
|
||||
let len = metadata.len();
|
||||
if !metadata.is_file() || len == 0 {
|
||||
return Ok(());
|
||||
}
|
||||
file.seek(io::SeekFrom::Start(
|
||||
len.saturating_sub(MAX_PREVIOUS_LOG_BYTES),
|
||||
))
|
||||
.await?;
|
||||
let mut bytes = Vec::new();
|
||||
file.take(MAX_PREVIOUS_LOG_BYTES)
|
||||
.read_to_end(&mut bytes)
|
||||
.await?;
|
||||
let previous = path.with_extension("log.previous");
|
||||
let temporary = path.with_extension("log.previous.tmp");
|
||||
let result = async {
|
||||
tokio::fs::write(&temporary, bytes).await?;
|
||||
tokio::fs::rename(&temporary, previous).await
|
||||
}
|
||||
.await;
|
||||
if result.is_err() {
|
||||
let _ = tokio::fs::remove_file(&temporary).await;
|
||||
}
|
||||
result?;
|
||||
anyhow::Ok(())
|
||||
}
|
||||
.await;
|
||||
if let Err(error) = result {
|
||||
let _ = crate::diagnostics::result::<()>("stderr_preservation", started, Err(error));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "stderr_log_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,30 @@
|
||||
use super::MAX_PREVIOUS_LOG_BYTES;
|
||||
use super::preserve;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn preserves_bounded_tail_and_keeps_it_after_empty_launch() -> anyhow::Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
let path = home.path().join("daemon.stderr.log");
|
||||
let previous = path.with_extension("log.previous");
|
||||
preserve(&path).await;
|
||||
assert!(!previous.exists());
|
||||
let mut bytes = vec![b'x'; MAX_PREVIOUS_LOG_BYTES as usize + 100];
|
||||
bytes.extend_from_slice(b"latest failure");
|
||||
tokio::fs::write(&path, &bytes).await?;
|
||||
preserve(&path).await;
|
||||
assert_eq!(
|
||||
tokio::fs::read(&previous).await?,
|
||||
bytes[bytes.len() - MAX_PREVIOUS_LOG_BYTES as usize..]
|
||||
);
|
||||
tokio::fs::write(&path, b"").await?;
|
||||
preserve(&path).await;
|
||||
assert_eq!(
|
||||
tokio::fs::read(&previous).await?,
|
||||
bytes[bytes.len() - MAX_PREVIOUS_LOG_BYTES as usize..]
|
||||
);
|
||||
tokio::fs::write(&path, b"next launch").await?;
|
||||
preserve(&path).await;
|
||||
assert_eq!(tokio::fs::read(&previous).await?, b"next launch");
|
||||
Ok(())
|
||||
}
|
||||
@@ -0,0 +1,123 @@
|
||||
//! Default-on local lifecycle records contain only fixed events and typed OS errors.
|
||||
//! Error messages and chains can contain raw child stderr or protocol data; never print them.
|
||||
|
||||
use std::io::IsTerminal;
|
||||
use std::io::Write;
|
||||
use std::time::Instant;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
use anyhow::Result;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Debug, Serialize, PartialEq, Eq)]
|
||||
struct Failure {
|
||||
kind: String,
|
||||
os_code: Option<i32>,
|
||||
}
|
||||
|
||||
impl From<&anyhow::Error> for Failure {
|
||||
fn from(error: &anyhow::Error) -> Self {
|
||||
match error.downcast_ref::<std::io::Error>() {
|
||||
Some(error) => Self {
|
||||
kind: format!("{:?}", error.kind()),
|
||||
os_code: error.raw_os_error(),
|
||||
},
|
||||
None => Self {
|
||||
kind: "other".to_string(),
|
||||
os_code: None,
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct Record<D> {
|
||||
timestamp_ms: u128,
|
||||
process_pid: u32,
|
||||
#[serde(flatten)]
|
||||
payload: Payload<D>,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
#[serde(untagged)]
|
||||
enum Payload<D> {
|
||||
Event {
|
||||
event: &'static str,
|
||||
details: D,
|
||||
},
|
||||
Result {
|
||||
stage: &'static str,
|
||||
outcome: Outcome,
|
||||
#[serde(rename = "elapsedMs")]
|
||||
elapsed_ms: u128,
|
||||
failure: Option<Failure>,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
enum Outcome {
|
||||
Succeeded,
|
||||
Failed,
|
||||
}
|
||||
|
||||
pub(crate) fn event(name: &'static str, details: impl Serialize) {
|
||||
emit(Payload::Event {
|
||||
event: name,
|
||||
details,
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn result<T>(stage: &'static str, started: Instant, result: Result<T>) -> Result<T> {
|
||||
emit(Payload::<()>::Result {
|
||||
stage,
|
||||
outcome: if result.is_ok() {
|
||||
Outcome::Succeeded
|
||||
} else {
|
||||
Outcome::Failed
|
||||
},
|
||||
elapsed_ms: started.elapsed().as_millis(),
|
||||
failure: result.as_ref().err().map(Failure::from),
|
||||
});
|
||||
result
|
||||
}
|
||||
|
||||
fn emit(payload: Payload<impl Serialize>) {
|
||||
let failed = matches!(
|
||||
&payload,
|
||||
Payload::Result {
|
||||
outcome: Outcome::Failed,
|
||||
..
|
||||
}
|
||||
);
|
||||
let record = Record {
|
||||
timestamp_ms: SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_millis(),
|
||||
process_pid: std::process::id(),
|
||||
payload,
|
||||
};
|
||||
let Ok(record) = serde_json::to_string(&record) else {
|
||||
return;
|
||||
};
|
||||
if tracing::dispatcher::has_been_set() {
|
||||
// Use the configured logger whenever one is available.
|
||||
if failed {
|
||||
tracing::warn!(target: "codex_app_server_daemon", "{record}");
|
||||
} else {
|
||||
tracing::info!(target: "codex_app_server_daemon", "{record}");
|
||||
}
|
||||
} else if !std::io::stderr().is_terminal() {
|
||||
// The detached updater has redirected stderr and no tracing subscriber.
|
||||
// A TUI can own a live terminal before installing its subscriber.
|
||||
// Never let a log write failure interrupt the lifecycle operation.
|
||||
let _ = writeln!(std::io::stderr().lock(), "{record}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "diagnostics_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,55 @@
|
||||
use super::Failure;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn failure_omits_error_messages_and_context() {
|
||||
let error = anyhow::Error::new(std::io::Error::new(
|
||||
std::io::ErrorKind::PermissionDenied,
|
||||
"secret child output",
|
||||
))
|
||||
.context("secret protocol data");
|
||||
assert_eq!(
|
||||
Failure::from(&error),
|
||||
Failure {
|
||||
kind: "PermissionDenied".to_string(),
|
||||
os_code: None
|
||||
},
|
||||
);
|
||||
let error = anyhow::anyhow!("secret child output");
|
||||
assert_eq!(
|
||||
Failure::from(&error),
|
||||
Failure {
|
||||
kind: "other".to_string(),
|
||||
os_code: None
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_record_preserves_json_shape_and_redacts_error_text() {
|
||||
let error = anyhow::Error::new(std::io::Error::new(
|
||||
std::io::ErrorKind::PermissionDenied,
|
||||
"secret child output",
|
||||
));
|
||||
let record = super::Record {
|
||||
timestamp_ms: 123,
|
||||
process_pid: 456,
|
||||
payload: super::Payload::<()>::Result {
|
||||
stage: "process_spawn",
|
||||
outcome: super::Outcome::Failed,
|
||||
elapsed_ms: 789,
|
||||
failure: Some(Failure::from(&error)),
|
||||
},
|
||||
};
|
||||
assert_eq!(
|
||||
serde_json::to_value(record).unwrap(),
|
||||
serde_json::json!({
|
||||
"timestampMs": 123,
|
||||
"processPid": 456,
|
||||
"stage": "process_spawn",
|
||||
"outcome": "failed",
|
||||
"elapsedMs": 789,
|
||||
"failure": { "kind": "PermissionDenied", "os_code": null },
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -7,6 +7,7 @@ pub use backend::windows::DetachedLaunchRestricted;
|
||||
#[cfg(windows)]
|
||||
use backend::windows::try_lock_file;
|
||||
mod client;
|
||||
mod diagnostics;
|
||||
mod install_lock;
|
||||
mod launch;
|
||||
pub use launch::restart_with_features;
|
||||
@@ -549,7 +550,10 @@ impl Daemon {
|
||||
mode
|
||||
};
|
||||
match restart_decision(mode, info.as_ref(), managed_version.as_deref()) {
|
||||
RestartDecision::NotReady => return Ok(RestartIfRunningOutcome::NotReady),
|
||||
RestartDecision::NotReady => {
|
||||
diagnostics::event("daemon_not_ready", ());
|
||||
return Ok(RestartIfRunningOutcome::NotReady);
|
||||
}
|
||||
RestartDecision::AlreadyCurrent => RestartIfRunningOutcome::AlreadyCurrent,
|
||||
RestartDecision::Restart => {
|
||||
#[cfg(windows)]
|
||||
@@ -559,13 +563,29 @@ impl Daemon {
|
||||
"warning: failed to clear stale daemon recovery before update: {err}"
|
||||
);
|
||||
}
|
||||
backend
|
||||
.stop_with_grace(settings.shutdown_grace_seconds)
|
||||
.await?;
|
||||
let _ = self
|
||||
.start_managed_backend_with_bin(&settings, managed_codex_bin)
|
||||
.await?;
|
||||
self.wait_until_ready().await?;
|
||||
diagnostics::event(
|
||||
"restart_requested",
|
||||
serde_json::json!({
|
||||
"shutdownGraceSeconds": settings.shutdown_grace_seconds,
|
||||
}),
|
||||
);
|
||||
let started = std::time::Instant::now();
|
||||
diagnostics::result(
|
||||
"shutdown",
|
||||
started,
|
||||
backend
|
||||
.stop_with_grace(settings.shutdown_grace_seconds)
|
||||
.await,
|
||||
)?;
|
||||
let started = std::time::Instant::now();
|
||||
diagnostics::result(
|
||||
"replacement_launch",
|
||||
started,
|
||||
self.start_managed_backend_with_bin(&settings, managed_codex_bin)
|
||||
.await,
|
||||
)?;
|
||||
let started = std::time::Instant::now();
|
||||
diagnostics::result("readiness", started, self.wait_until_ready().await)?;
|
||||
RestartIfRunningOutcome::Restarted
|
||||
}
|
||||
}
|
||||
@@ -574,6 +594,7 @@ impl Daemon {
|
||||
"app server is running but is not managed by codex app-server daemon"
|
||||
));
|
||||
} else {
|
||||
diagnostics::event("daemon_not_running", ());
|
||||
RestartIfRunningOutcome::NotRunning
|
||||
};
|
||||
|
||||
|
||||
@@ -220,6 +220,7 @@ async fn run_with_http(
|
||||
}
|
||||
continue;
|
||||
}
|
||||
let started = std::time::Instant::now();
|
||||
match update_once(http, daemon, running_updater_identity, &mut terminate, UpdateTrigger::Scheduled).await {
|
||||
Ok((UpdateLoopControl::Continue, Some(_))) => {
|
||||
manual_handoff_pending = true;
|
||||
@@ -227,7 +228,9 @@ async fn run_with_http(
|
||||
continue;
|
||||
}
|
||||
Ok((UpdateLoopControl::Continue, None)) => {}
|
||||
Err(err) => eprintln!("warning: scheduled daemon update failed: {err:#}"),
|
||||
Err(err) => {
|
||||
let _ = crate::diagnostics::result::<()>("scheduled_update", started, Err(err));
|
||||
}
|
||||
Ok((UpdateLoopControl::Stop, _)) => return Ok(()),
|
||||
}
|
||||
let Some(delay) = next_update_delay(daemon).await else {
|
||||
@@ -351,8 +354,9 @@ async fn update_once(
|
||||
"CODEX_INSTALL_IF_LATEST",
|
||||
)
|
||||
};
|
||||
let started = std::time::Instant::now();
|
||||
let script = tokio::select! {
|
||||
result = fetch_installer_script(http) => result?,
|
||||
result = fetch_installer_script(http) => crate::diagnostics::result("installer_fetch", started, result)?,
|
||||
_ = terminate.recv() => return Ok((UpdateLoopControl::Stop, None)),
|
||||
};
|
||||
anyhow::ensure!(
|
||||
@@ -383,16 +387,21 @@ async fn update_once(
|
||||
crate::managed_install::package_root(codex_home) == package_root,
|
||||
"daemon package root changed during the update; retry the command"
|
||||
);
|
||||
let started = std::time::Instant::now();
|
||||
#[cfg(unix)]
|
||||
if matches!(
|
||||
run_installer_script(&script, installer_mode, &package_root, terminate.recv()).await?,
|
||||
crate::diagnostics::result(
|
||||
"installer_run",
|
||||
started,
|
||||
run_installer_script(&script, installer_mode, &package_root, terminate.recv()).await
|
||||
)?,
|
||||
UpdateLoopControl::Stop
|
||||
) {
|
||||
return Ok((UpdateLoopControl::Stop, None));
|
||||
}
|
||||
#[cfg(windows)]
|
||||
tokio::select! {
|
||||
result = run_installer_script(&script, installer_mode, &package_root) => { result?; },
|
||||
result = run_installer_script(&script, installer_mode, &package_root) => { crate::diagnostics::result("installer_run", started, result)?; },
|
||||
_ = terminate.recv() => return Ok((UpdateLoopControl::Stop, None)),
|
||||
}
|
||||
anyhow::ensure!(
|
||||
@@ -405,6 +414,13 @@ async fn update_once(
|
||||
|
||||
let managed_codex_bin =
|
||||
resolved_managed_codex_bin(&daemon.current_managed_codex_bin()?).await?;
|
||||
crate::diagnostics::event(
|
||||
"package_selected",
|
||||
serde_json::json!({
|
||||
"release": selected_release(daemon).ok().map(|(_, _, release)| release),
|
||||
"trigger": if trigger == UpdateTrigger::Scheduled { "scheduled" } else { "manual" },
|
||||
}),
|
||||
);
|
||||
let restart_mode = match trigger {
|
||||
// The package can contain different resources even when its CLI binary
|
||||
// is identical. A release change must also replace the running process.
|
||||
|
||||
@@ -268,10 +268,15 @@ impl FeedbackRequestProcessor {
|
||||
|
||||
let session_source = self.thread_manager.session_source();
|
||||
let runtime_handle = tokio::runtime::Handle::current();
|
||||
let codex_home = self.config.codex_home.clone();
|
||||
|
||||
let upload_result = tokio::task::spawn_blocking(move || {
|
||||
// Cancelling the RPC waiter must not release a still-running upload's slot.
|
||||
let _permit = permit;
|
||||
if include_logs {
|
||||
extra_attachments
|
||||
.extend(codex_feedback::daemon_log_attachments(codex_home.as_path()));
|
||||
}
|
||||
let tags = (!upload_tags.is_empty()).then_some(&upload_tags);
|
||||
runtime_handle.block_on(snapshot.upload_feedback(
|
||||
FeedbackUploadOptions {
|
||||
|
||||
@@ -107,6 +107,24 @@ async fn feedback_upload_includes_sqlite_flush_and_query_failures() -> Result<()
|
||||
|
||||
// Terminate HTTPS locally and capture real Sentry envelopes. Never forward uploads.
|
||||
let home = tempfile::tempdir()?;
|
||||
let daemon_logs = home.path().join("app-server-daemon");
|
||||
std::fs::create_dir(&daemon_logs)?;
|
||||
std::fs::write(
|
||||
daemon_logs.join("daemon.stderr.log"),
|
||||
"current daemon diagnostic",
|
||||
)?;
|
||||
std::fs::write(
|
||||
daemon_logs.join("daemon-updater.stderr.log.previous"),
|
||||
"previous updater failure",
|
||||
)?;
|
||||
std::fs::write(
|
||||
daemon_logs.join("daemon.stderr.log.previous"),
|
||||
"previous daemon startup failure",
|
||||
)?;
|
||||
std::fs::write(
|
||||
daemon_logs.join("daemon-updater.stderr.log"),
|
||||
"current updater diagnostic",
|
||||
)?;
|
||||
let cert = rcgen::generate_simple_self_signed(vec!["o33249.ingest.us.sentry.io".to_string()])?;
|
||||
let cert_path = home.path().join("feedback-ca.pem");
|
||||
std::fs::write(&cert_path, cert.cert.pem())?;
|
||||
@@ -268,6 +286,16 @@ async fn feedback_upload_includes_sqlite_flush_and_query_failures() -> Result<()
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(!attachments.contains_key("daemon.stderr.log"));
|
||||
assert!(!attachments.contains_key("daemon.stderr.log.previous"));
|
||||
assert_eq!(
|
||||
attachments["daemon-updater.stderr.log"],
|
||||
"current updater diagnostic"
|
||||
);
|
||||
assert_eq!(
|
||||
attachments["daemon-updater.stderr.log.previous"],
|
||||
"previous updater failure"
|
||||
);
|
||||
let logs = &attachments["codex-logs.log"];
|
||||
if phase == Phase::Corrupt {
|
||||
assert!(
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
//! Collects bounded current and previous updater log tails after feedback log consent.
|
||||
//! Read failures omit individual files; only fixed regular-file names are considered.
|
||||
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::io::Read;
|
||||
use std::io::Seek;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::FeedbackAttachment;
|
||||
|
||||
const MAX_LOG_BYTES: u64 = 256 * 1024;
|
||||
const LOG_NAMES: [&str; 2] = ["daemon-updater.stderr.log", "app-server-updater.stderr.log"];
|
||||
|
||||
/// Collect at most two 256 KiB updater tails (512 KiB total) from the app-server host.
|
||||
/// Call only after feedback consent includes logs, on a blocking worker.
|
||||
pub fn daemon_log_attachments(codex_home: &Path) -> Vec<FeedbackAttachment> {
|
||||
let directory = codex_home.join("app-server-daemon");
|
||||
let mut attachments = Vec::new();
|
||||
for suffix in ["", ".previous"] {
|
||||
for name in LOG_NAMES {
|
||||
let filename = format!("{name}{suffix}");
|
||||
let path = directory.join(&filename);
|
||||
let result = (|| -> io::Result<Vec<u8>> {
|
||||
// Reject symlinks and nonregular files before opening (e.g. FIFOs).
|
||||
if !fs::symlink_metadata(&path)?.is_file() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let mut file = fs::File::open(&path)?;
|
||||
let metadata = file.metadata()?;
|
||||
if !metadata.is_file() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
file.seek(io::SeekFrom::Start(
|
||||
metadata.len().saturating_sub(MAX_LOG_BYTES),
|
||||
))?;
|
||||
let mut bytes = Vec::new();
|
||||
file.take(MAX_LOG_BYTES).read_to_end(&mut bytes)?;
|
||||
Ok(bytes)
|
||||
})();
|
||||
match result {
|
||||
Ok(buffer) if !buffer.is_empty() => {
|
||||
attachments.push(FeedbackAttachment {
|
||||
filename,
|
||||
content_type: Some("text/plain".to_string()),
|
||||
buffer,
|
||||
});
|
||||
// Legacy filenames are a fallback, not additional attachments.
|
||||
break;
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => {}
|
||||
Err(error) => {
|
||||
tracing::warn!(filename, error_kind = ?error.kind(), "failed to read daemon feedback log; skipping attachment")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
attachments
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "daemon_logs_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,55 @@
|
||||
use super::*;
|
||||
use codex_protocol::ThreadId;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn collects_bounded_tails_and_skips_missing_empty_and_nonregular_files() -> io::Result<()> {
|
||||
let home = std::env::temp_dir().join(format!("codex-feedback-daemon-{}", ThreadId::new()));
|
||||
let directory = home.join("app-server-daemon");
|
||||
assert!(daemon_log_attachments(&home).is_empty());
|
||||
fs::create_dir_all(&directory)?;
|
||||
let bytes = vec![b'x'; MAX_LOG_BYTES as usize + 100];
|
||||
fs::write(directory.join("daemon.stderr.log"), b"already included")?;
|
||||
fs::write(directory.join("daemon-updater.stderr.log"), &bytes)?;
|
||||
fs::write(
|
||||
directory.join("daemon.stderr.log.previous"),
|
||||
b"failed restart",
|
||||
)?;
|
||||
fs::write(
|
||||
directory.join("app-server-updater.stderr.log"),
|
||||
b"legacy updater",
|
||||
)?;
|
||||
fs::write(
|
||||
directory.join("app-server-updater.stderr.log.previous"),
|
||||
b"previous updater",
|
||||
)?;
|
||||
fs::create_dir(directory.join("app-server.stderr.log.previous"))?;
|
||||
let attachments = daemon_log_attachments(&home);
|
||||
let actual: Vec<_> = attachments
|
||||
.into_iter()
|
||||
.map(|attachment| {
|
||||
(
|
||||
attachment.filename,
|
||||
attachment.content_type,
|
||||
attachment.buffer,
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
actual,
|
||||
vec![
|
||||
(
|
||||
"daemon-updater.stderr.log".to_string(),
|
||||
Some("text/plain".to_string()),
|
||||
bytes[100..].to_vec()
|
||||
),
|
||||
(
|
||||
"app-server-updater.stderr.log.previous".to_string(),
|
||||
Some("text/plain".to_string()),
|
||||
b"previous updater".to_vec()
|
||||
),
|
||||
]
|
||||
);
|
||||
fs::remove_dir_all(home)?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -31,6 +31,8 @@ use tracing_subscriber::filter::Targets;
|
||||
use tracing_subscriber::fmt::writer::MakeWriter;
|
||||
use tracing_subscriber::registry::LookupSpan;
|
||||
|
||||
mod daemon_logs;
|
||||
pub use daemon_logs::daemon_log_attachments;
|
||||
pub(crate) mod feedback_diagnostics;
|
||||
mod guardian;
|
||||
mod report_upload;
|
||||
|
||||
Reference in New Issue
Block a user