diff --git a/codex-rs/app-server-daemon/src/backend/mod.rs b/codex-rs/app-server-daemon/src/backend/mod.rs index 1788e9b347..19622b7e4c 100644 --- a/codex-rs/app-server-daemon/src/backend/mod.rs +++ b/codex-rs/app-server-daemon/src/backend/mod.rs @@ -1,4 +1,5 @@ mod pid; +mod stderr_log; #[cfg(windows)] pub(crate) mod windows; diff --git a/codex-rs/app-server-daemon/src/backend/pid.rs b/codex-rs/app-server-daemon/src/backend/pid.rs index 0e69a3f0e9..fce30642d5 100644 --- a/codex-rs/app-server-daemon/src/backend/pid.rs +++ b/codex-rs/app-server-daemon/src/backend/pid.rs @@ -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 { 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))] diff --git a/codex-rs/app-server-daemon/src/backend/pid_start.rs b/codex-rs/app-server-daemon/src/backend/pid_start.rs index 26d90bc152..10a49962b9 100644 --- a/codex-rs/app-server-daemon/src/backend/pid_start.rs +++ b/codex-rs/app-server-daemon/src/backend/pid_start.rs @@ -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() { diff --git a/codex-rs/app-server-daemon/src/backend/pid_tests.rs b/codex-rs/app-server-daemon/src/backend/pid_tests.rs index dbd6a4075a..41de1d2313 100644 --- a/codex-rs/app-server-daemon/src/backend/pid_tests.rs +++ b/codex-rs/app-server-daemon/src/backend/pid_tests.rs @@ -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(()) +} diff --git a/codex-rs/app-server-daemon/src/backend/stderr_log.rs b/codex-rs/app-server-daemon/src/backend/stderr_log.rs new file mode 100644 index 0000000000..4e619735f1 --- /dev/null +++ b/codex-rs/app-server-daemon/src/backend/stderr_log.rs @@ -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; diff --git a/codex-rs/app-server-daemon/src/backend/stderr_log_tests.rs b/codex-rs/app-server-daemon/src/backend/stderr_log_tests.rs new file mode 100644 index 0000000000..85603d2f5d --- /dev/null +++ b/codex-rs/app-server-daemon/src/backend/stderr_log_tests.rs @@ -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(()) +} diff --git a/codex-rs/app-server-daemon/src/diagnostics.rs b/codex-rs/app-server-daemon/src/diagnostics.rs new file mode 100644 index 0000000000..a000bce055 --- /dev/null +++ b/codex-rs/app-server-daemon/src/diagnostics.rs @@ -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, +} + +impl From<&anyhow::Error> for Failure { + fn from(error: &anyhow::Error) -> Self { + match error.downcast_ref::() { + 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 { + timestamp_ms: u128, + process_pid: u32, + #[serde(flatten)] + payload: Payload, +} + +#[derive(Serialize)] +#[serde(untagged)] +enum Payload { + Event { + event: &'static str, + details: D, + }, + Result { + stage: &'static str, + outcome: Outcome, + #[serde(rename = "elapsedMs")] + elapsed_ms: u128, + failure: Option, + }, +} + +#[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(stage: &'static str, started: Instant, result: Result) -> Result { + 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) { + 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; diff --git a/codex-rs/app-server-daemon/src/diagnostics_tests.rs b/codex-rs/app-server-daemon/src/diagnostics_tests.rs new file mode 100644 index 0000000000..a5f00df8e2 --- /dev/null +++ b/codex-rs/app-server-daemon/src/diagnostics_tests.rs @@ -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 }, + }), + ); +} diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index cc77d8b814..24df96a586 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -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 }; diff --git a/codex-rs/app-server-daemon/src/update_loop.rs b/codex-rs/app-server-daemon/src/update_loop.rs index 3a07699953..499acd113e 100644 --- a/codex-rs/app-server-daemon/src/update_loop.rs +++ b/codex-rs/app-server-daemon/src/update_loop.rs @@ -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. diff --git a/codex-rs/app-server/src/request_processors/feedback_processor.rs b/codex-rs/app-server/src/request_processors/feedback_processor.rs index 9bb2b461ec..3d4763770d 100644 --- a/codex-rs/app-server/src/request_processors/feedback_processor.rs +++ b/codex-rs/app-server/src/request_processors/feedback_processor.rs @@ -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 { diff --git a/codex-rs/app-server/tests/suite/v2/feedback.rs b/codex-rs/app-server/tests/suite/v2/feedback.rs index fbe609df32..b7a411941d 100644 --- a/codex-rs/app-server/tests/suite/v2/feedback.rs +++ b/codex-rs/app-server/tests/suite/v2/feedback.rs @@ -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!( diff --git a/codex-rs/feedback/src/daemon_logs.rs b/codex-rs/feedback/src/daemon_logs.rs new file mode 100644 index 0000000000..b5e274379f --- /dev/null +++ b/codex-rs/feedback/src/daemon_logs.rs @@ -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 { + 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> { + // 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; diff --git a/codex-rs/feedback/src/daemon_logs_tests.rs b/codex-rs/feedback/src/daemon_logs_tests.rs new file mode 100644 index 0000000000..3f93900eb9 --- /dev/null +++ b/codex-rs/feedback/src/daemon_logs_tests.rs @@ -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(()) +} diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index 5f6519368a..4c9919c1cb 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -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;