From 9826581e7b2c130bc389f4490cd746851eb79af0 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Tue, 26 May 2026 15:59:25 -0700 Subject: [PATCH] Attach Windows sandbox log to feedback reports (#24623) ## Why Windows sandbox diagnostics are currently hard to recover from `/feedback` even though they are often the most useful artifact when debugging sandbox behavior. Now that sandbox logging uses daily rolling files, feedback can safely include the current day's sandbox log without uploading the old ever-growing legacy `sandbox.log`. ## What changed - Add a `codex-windows-sandbox` helper that resolves the current daily sandbox log from `codex_home`. - When feedback is submitted with logs enabled on Windows, app-server attaches today's sandbox log if it exists. - Upload the attachment under the stable filename `windows-sandbox.log`, independent of the dated on-disk filename. - Keep existing raw `extra_log_files` behavior unchanged for rollout and desktop log attachments. ## Verification - `cargo fmt -p codex-app-server -p codex-windows-sandbox` - `cargo test -p codex-windows-sandbox current_log_file_path_for_codex_home_uses_sandbox_dir` - `cargo test -p codex-app-server windows_sandbox_log_attachment_uses_current_log` - Manual CLI/TUI `/feedback` test confirmed Sentry received `windows-sandbox.log`. --- codex-rs/Cargo.lock | 1 + codex-rs/app-server/Cargo.toml | 3 ++ .../request_processors/feedback_processor.rs | 51 +++++++++++++++++++ codex-rs/feedback/src/lib.rs | 2 + codex-rs/tui/src/bottom_pane/feedback_view.rs | 33 ++++++++++++ ...ck_upload_consent_lists_doctor_report.snap | 2 +- ...sts_windows_sandbox_log_when_included.snap | 13 +++++ codex-rs/tui/src/chatwidget.rs | 7 +++ ..._tests__feedback_upload_consent_popup.snap | 2 + .../chatwidget/tests/popups_and_settings.rs | 2 + codex-rs/windows-sandbox-rs/src/lib.rs | 2 + codex-rs/windows-sandbox-rs/src/logging.rs | 14 +++++ 12 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_windows_sandbox_log_when_included.snap diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 044146d9d..1b1bc0e59 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1929,6 +1929,7 @@ dependencies = [ "codex-utils-json-to-toml", "codex-utils-pty", "codex-web-search-extension", + "codex-windows-sandbox", "core_test_support", "flate2", "futures", diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 17ecd7dc3..09e3197b7 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -94,6 +94,9 @@ tracing = { workspace = true, features = ["log"] } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt", "json"] } uuid = { workspace = true, features = ["serde", "v7"] } +[target.'cfg(windows)'.dependencies] +codex-windows-sandbox = { workspace = true } + [dev-dependencies] app_test_support = { workspace = true } axum = { workspace = true, default-features = false, features = [ 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 18f519d3c..73da67f73 100644 --- a/codex-rs/app-server/src/request_processors/feedback_processor.rs +++ b/codex-rs/app-server/src/request_processors/feedback_processor.rs @@ -1,4 +1,6 @@ use super::*; +#[cfg(target_os = "windows")] +use codex_feedback::WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME; #[derive(Clone)] pub(crate) struct FeedbackRequestProcessor { @@ -186,6 +188,12 @@ impl FeedbackRequestProcessor { )), }); } + if let Some(sandbox_log_attachment) = + windows_sandbox_log_attachment(&self.config.codex_home) + && seen_attachment_paths.insert(sandbox_log_attachment.path.clone()) + { + attachment_paths.push(sandbox_log_attachment); + } } if let Some(extra_log_files) = extra_log_files { for extra_log_file in extra_log_files { @@ -264,3 +272,46 @@ impl FeedbackRequestProcessor { fn auto_review_rollout_filename(thread_id: ThreadId) -> String { format!("auto-review-rollout-{thread_id}.jsonl") } + +#[cfg(target_os = "windows")] +fn windows_sandbox_log_attachment(codex_home: &Path) -> Option { + let sandbox_log_path = codex_windows_sandbox::current_log_file_path_for_codex_home(codex_home); + sandbox_log_path + .is_file() + .then_some(FeedbackAttachmentPath { + path: sandbox_log_path, + attachment_filename_override: Some(WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME.to_string()), + }) +} + +#[cfg(not(target_os = "windows"))] +fn windows_sandbox_log_attachment(_codex_home: &Path) -> Option { + None +} + +#[cfg(all(test, target_os = "windows"))] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn windows_sandbox_log_attachment_uses_current_log() { + let codex_home = tempfile::tempdir().expect("create tempdir"); + let sandbox_dir = codex_windows_sandbox::sandbox_dir(codex_home.path()); + std::fs::create_dir_all(&sandbox_dir).expect("create sandbox dir"); + let sandbox_log_path = + codex_windows_sandbox::current_log_file_path_for_codex_home(codex_home.path()); + std::fs::write(&sandbox_log_path, "sandbox log").expect("write sandbox log"); + + let attachment = windows_sandbox_log_attachment(codex_home.path()) + .map(|attachment| (attachment.path, attachment.attachment_filename_override)); + + assert_eq!( + attachment, + Some(( + sandbox_log_path, + Some(WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME.to_string()) + )) + ); + } +} diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index 9a8caf489..7c27d2b3b 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -29,6 +29,8 @@ pub use feedback_diagnostics::FeedbackDiagnostics; /// Filename used for the redacted `codex doctor --json` feedback attachment. pub const DOCTOR_REPORT_ATTACHMENT_FILENAME: &str = "codex-doctor-report.json"; +/// Filename used for the Windows sandbox log feedback attachment. +pub const WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME: &str = "windows-sandbox.log"; const DEFAULT_MAX_BYTES: usize = 4 * 1024 * 1024; // 4 MiB const SENTRY_DSN: &str = "https://ae32ed50620d7a7792c1ce5df38b3e3e@o33249.ingest.us.sentry.io/4510195390611458"; diff --git a/codex-rs/tui/src/bottom_pane/feedback_view.rs b/codex-rs/tui/src/bottom_pane/feedback_view.rs index 04bcbd0b0..2770598fc 100644 --- a/codex-rs/tui/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui/src/bottom_pane/feedback_view.rs @@ -1,6 +1,7 @@ use codex_feedback::DOCTOR_REPORT_ATTACHMENT_FILENAME; use codex_feedback::FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME; use codex_feedback::FeedbackDiagnostics; +use codex_feedback::WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -472,6 +473,7 @@ pub(crate) fn feedback_upload_consent_params( category: FeedbackCategory, rollout_path: Option, auto_review_rollout_filename: Option, + include_windows_sandbox_log: bool, feedback_diagnostics: &FeedbackDiagnostics, ) -> super::SelectionViewParams { use super::popup_consts::standard_popup_hint_line; @@ -509,6 +511,15 @@ pub(crate) fn feedback_upload_consent_params( ]) .into(), ]; + if include_windows_sandbox_log { + header_lines.push( + Line::from(vec![ + " • ".into(), + WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME.into(), + ]) + .into(), + ); + } if let Some(path) = rollout_path.as_deref() && let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) { @@ -696,6 +707,7 @@ mod tests { FeedbackCategory::Bug, Some(std::path::PathBuf::from("rollout.jsonl")), Some("auto-review-rollout.jsonl".to_string()), + /*include_windows_sandbox_log*/ false, &FeedbackDiagnostics::default(), ); @@ -704,6 +716,27 @@ mod tests { insta::assert_snapshot!("feedback_upload_consent_lists_doctor_report", rendered); } + #[test] + fn feedback_upload_consent_lists_windows_sandbox_log_when_included() { + let (tx_raw, _rx) = tokio::sync::mpsc::unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let params = feedback_upload_consent_params( + tx, + FeedbackCategory::Bug, + Some(std::path::PathBuf::from("rollout.jsonl")), + Some("auto-review-rollout.jsonl".to_string()), + /*include_windows_sandbox_log*/ true, + &FeedbackDiagnostics::default(), + ); + + let rendered = render_renderable(params.header.as_ref(), /*width*/ 60); + + insta::assert_snapshot!( + "feedback_upload_consent_lists_windows_sandbox_log_when_included", + rendered + ); + } + #[test] fn submit_feedback_emits_submit_event_with_trimmed_note() { let (tx_raw, mut rx) = tokio::sync::mpsc::unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_doctor_report.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_doctor_report.snap index 6cca54542..e91d736ec 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_doctor_report.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_doctor_report.snap @@ -1,6 +1,6 @@ --- source: tui/src/bottom_pane/feedback_view.rs -assertion_line: 704 +assertion_line: 716 expression: rendered --- Upload logs? diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_windows_sandbox_log_when_included.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_windows_sandbox_log_when_included.snap new file mode 100644 index 000000000..741505d5f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_upload_consent_lists_windows_sandbox_log_when_included.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/feedback_view.rs +assertion_line: 734 +expression: rendered +--- +Upload logs? + +The following files will be sent: + • codex-logs.log + • codex-doctor-report.json + • windows-sandbox.log + • rollout.jsonl + • auto-review-rollout.jsonl diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b9c510118..16ac980f3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -977,12 +977,19 @@ impl ChatWidget { pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) { let snapshot = self.feedback.snapshot(self.thread_id); + #[cfg(target_os = "windows")] + let include_windows_sandbox_log = + codex_windows_sandbox::current_log_file_path_for_codex_home(&self.config.codex_home) + .is_file(); + #[cfg(not(target_os = "windows"))] + let include_windows_sandbox_log = false; let params = crate::bottom_pane::feedback_upload_consent_params( self.app_event_tx.clone(), category, self.current_rollout_path.clone(), self.thread_id .map(|thread_id| format!("auto-review-rollout-{thread_id}.jsonl")), + include_windows_sandbox_log, snapshot.feedback_diagnostics(), ); self.bottom_pane.show_selection_view(params); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap index 354e473a1..b860cf8fb 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests/popups_and_settings.rs +assertion_line: 2562 expression: popup --- Upload logs? @@ -7,6 +8,7 @@ expression: popup The following files will be sent: • codex-logs.log • codex-doctor-report.json + • windows-sandbox.log • auto-review-rollout-thread-1.jsonl • codex-connectivity-diagnostics.txt diff --git a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs index 3a4bdd132..55aadefb6 100644 --- a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs +++ b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs @@ -2621,6 +2621,7 @@ async fn feedback_upload_consent_popup_snapshot() { crate::app_event::FeedbackCategory::Bug, chat.current_rollout_path.clone(), Some("auto-review-rollout-thread-1.jsonl".to_string()), + /*include_windows_sandbox_log*/ true, &codex_feedback::FeedbackDiagnostics::new(vec![codex_feedback::FeedbackDiagnostic { headline: "Proxy environment variables are set and may affect connectivity." .to_string(), @@ -2641,6 +2642,7 @@ async fn feedback_good_result_consent_popup_includes_connectivity_diagnostics_fi crate::app_event::FeedbackCategory::GoodResult, chat.current_rollout_path.clone(), Some("auto-review-rollout-thread-1.jsonl".to_string()), + /*include_windows_sandbox_log*/ false, &codex_feedback::FeedbackDiagnostics::new(vec![codex_feedback::FeedbackDiagnostic { headline: "Proxy environment variables are set and may affect connectivity." .to_string(), diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 03d2393ed..26e22dde0 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -177,6 +177,8 @@ pub use ipc_framed::write_frame; #[cfg(target_os = "windows")] pub use logging::current_log_file_path; #[cfg(target_os = "windows")] +pub use logging::current_log_file_path_for_codex_home; +#[cfg(target_os = "windows")] pub use logging::log_file_path_for_utc_date; #[cfg(target_os = "windows")] pub use logging::log_note; diff --git a/codex-rs/windows-sandbox-rs/src/logging.rs b/codex-rs/windows-sandbox-rs/src/logging.rs index f5e839970..7108a1de3 100644 --- a/codex-rs/windows-sandbox-rs/src/logging.rs +++ b/codex-rs/windows-sandbox-rs/src/logging.rs @@ -43,6 +43,10 @@ pub fn current_log_file_path(base_dir: &Path) -> PathBuf { log_file_path_for_utc_date(base_dir, chrono::Utc::now().date_naive()) } +pub fn current_log_file_path_for_codex_home(codex_home: &Path) -> PathBuf { + current_log_file_path(&crate::sandbox_dir(codex_home)) +} + pub fn log_writer(base_dir: &Path) -> Option { if !base_dir.is_dir() { return None; @@ -142,4 +146,14 @@ mod tests { PathBuf::from("logs").join("sandbox.2026-05-21.log") ); } + + #[test] + fn current_log_file_path_for_codex_home_uses_sandbox_dir() { + let codex_home = Path::new("codex-home"); + + assert_eq!( + current_log_file_path_for_codex_home(codex_home), + current_log_file_path(&codex_home.join(".sandbox")) + ); + } }