diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 3d4268f409..eab62a26b9 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -103,7 +103,6 @@ use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::FeedbackUploadParams; -use codex_app_server_protocol::FeedbackUploadResponse; use codex_app_server_protocol::GetAccountRateLimitsResponse; use codex_app_server_protocol::HooksListEntry; use codex_app_server_protocol::ListMcpServerStatusParams; @@ -223,6 +222,7 @@ mod empty_state_policy; mod event_dispatch; mod exit_summary; mod experimental_features; +mod feedback_upload; mod file_change_approvals; mod history_pagination; mod history_ui; diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index bfa33203da..2219ee0e1c 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -4,6 +4,7 @@ //! limits, add-credit nudges, and feedback uploads. Results are routed back through `AppEvent` so //! the main event loop remains single-threaded. +use super::feedback_upload::fetch_feedback_upload; use super::plugin_mentions::fetch_plugin_mentions; use super::*; use crate::app_event::ConnectorsSnapshot; @@ -604,8 +605,10 @@ impl App { turn_id, include_logs, ); + let codex_home = app_server.codex_home_path(&self.config.codex_home); + let feedback = self.feedback.clone(); tokio::spawn(async move { - let result = fetch_feedback_upload(request_handle, params) + let result = fetch_feedback_upload(request_handle, codex_home, params, feedback) .await .map(|response| response.thread_id) .map_err(|err| err.to_string()); @@ -1278,17 +1281,6 @@ pub(super) fn build_feedback_upload_params( } } -pub(super) async fn fetch_feedback_upload( - request_handle: AppServerRequestHandle, - params: FeedbackUploadParams, -) -> Result { - let request_id = RequestId::String(format!("feedback-upload-{}", Uuid::new_v4())); - request_handle - .request_typed(ClientRequest::FeedbackUpload { request_id, params }) - .await - .wrap_err("feedback/upload failed in TUI") -} - /// Convert flat `McpServerStatus` responses into the per-server maps used by the /// in-process MCP subsystem (tools keyed as `mcp__{server}__{tool}`, plus /// per-server resource/template/auth maps). Test-only because the TUI diff --git a/codex-rs/tui/src/app/feedback_upload.rs b/codex-rs/tui/src/app/feedback_upload.rs new file mode 100644 index 0000000000..13ba9b9d10 --- /dev/null +++ b/codex-rs/tui/src/app/feedback_upload.rs @@ -0,0 +1,76 @@ +//! Stage bounded TUI diagnostics on the app-server host for a single feedback upload. +//! Log consent gates all staging; cleanup runs after staging or upload failures too. + +use crate::app_server_session::fs::AppServerFileSystem; +use codex_app_server_client::AppServerPath; +use codex_app_server_client::AppServerRequestHandle; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::FeedbackUploadParams; +use codex_app_server_protocol::FeedbackUploadResponse; +use codex_app_server_protocol::RequestId; +use codex_feedback::CodexFeedback; +use color_eyre::eyre::Result; +use color_eyre::eyre::WrapErr; +use color_eyre::eyre::eyre; +use std::path::PathBuf; +use uuid::Uuid; + +// Base64 adds only a third to this size, keeping fs/writeFile below the 16 MiB frame limit. +const MAX_CLIENT_LOG_SIZE: usize = 1024 * 1024; + +pub(super) async fn fetch_feedback_upload( + request_handle: AppServerRequestHandle, + codex_home: Option, + mut params: FeedbackUploadParams, + feedback: CodexFeedback, +) -> Result { + let fs = AppServerFileSystem { request_handle }; + let staging_dir = codex_home.filter(|_| params.include_logs).map(|home| { + home.join("tmp") + .join(format!("tui-feedback-{}", Uuid::new_v4())) + }); + if params.include_logs { + let staged = async { + let directory = staging_dir.as_ref().ok_or_else(|| { + eyre!("App server did not report $CODEX_HOME; cannot stage feedback logs") + })?; + // These RPCs use the app-server's local filesystem, where feedback/upload + // reads extraLogFiles, independently of the thread's executor environment. + fs.fs_create_directory_all_path(directory).await?; + let path = directory.join("client-logs.txt"); + let mut logs = feedback + .snapshot(/*session_id*/ None) + .log_attachment(/*logs_override*/ None) + .buffer; + logs.drain(..logs.len().saturating_sub(MAX_CLIENT_LOG_SIZE)); + fs.fs_write_file_path(&path, logs).await?; + Ok::<_, color_eyre::Report>(path) + } + .await; + match staged { + Ok(path) => params + .extra_log_files + .get_or_insert_with(Vec::new) + .push(PathBuf::from(path.as_str())), + Err(err) => { + tracing::warn!("failed to stage TUI feedback logs: {err}"); + params.reason.get_or_insert_with(String::new).push_str( + "\n\nTUI client logs were omitted because they could not be staged on the app-server.", + ); + } + } + } + let request_id = RequestId::String(format!("feedback-upload-{}", Uuid::new_v4())); + let result = fs + .request_handle + .request_typed(ClientRequest::FeedbackUpload { request_id, params }) + .await + .wrap_err("feedback/upload failed in TUI"); + + if let Some(directory) = staging_dir + && let Err(err) = fs.fs_remove_path(&directory).await + { + tracing::warn!("failed to remove staged feedback logs: {err}"); + } + result +} diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index bdfc3aa23e..4438a377cb 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -7185,8 +7185,90 @@ fn request_user_input_request(thread_id: ThreadId, turn_id: &str, item_id: &str) } #[tokio::test] -async fn feedback_submission_without_thread_emits_error_history_cell() { +async fn feedback_submission_stages_logs_cleans_up_and_emits_error_history_cell() -> Result<()> { + use base64::Engine; + use base64::engine::general_purpose::STANDARD; + use std::io::Write; + use tracing_subscriber::fmt::writer::MakeWriter; + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let (server, requests, proxy) = + session_lifecycle_requests::start_recording_remote_app_server(&app.config).await?; + let diagnostic = "SQLITE LOG WRITE FAILURE: disk full\n"; + let expected_logs = format!("{}{diagnostic}", "x".repeat(1024 * 1024 - diagnostic.len())); + let mut writer = app.feedback.make_writer().make_writer(); + writer.write_all(&vec![b'x'; 1024 * 1024])?; + writer.write_all(diagnostic.as_bytes())?; + for (include_logs, staging_fails) in [(true, false), (false, false), (true, true)] { + let home = tempdir()?; + let tmp = home.path().join("tmp"); + if staging_fails { + std::fs::write(&tmp, "not a directory")?; + } + let mut params = background_requests::build_feedback_upload_params( + /*origin_thread_id*/ None, + Some(PathBuf::from("existing-rollout.jsonl")), + FeedbackCategory::Bug, + /*reason*/ None, + /*turn_id*/ None, + include_logs, + ); + // Reject before network upload, after the client has staged its diagnostics. + params.thread_id = Some("invalid-thread-id".into()); + let error = feedback_upload::fetch_feedback_upload( + server.request_handle(), + Some(AppServerPath::from_app_server( + home.path().display().to_string(), + )), + params, + app.feedback.clone(), + ) + .await + .unwrap_err(); + assert!(format!("{error:#}").contains("invalid thread id")); + let recorded = std::mem::take(&mut *requests.lock().unwrap()); + let upload = recorded + .iter() + .find(|r| r.method == "feedback/upload") + .unwrap(); + let upload: FeedbackUploadParams = serde_json::from_value(upload.params.clone().unwrap())?; + if include_logs && !staging_fails { + let write = recorded + .iter() + .find(|r| r.method == "fs/writeFile") + .unwrap(); + let write = write.params.as_ref().unwrap(); + assert_eq!( + STANDARD.decode(write["dataBase64"].as_str().unwrap())?, + expected_logs.as_bytes() + ); + assert_eq!( + upload.extra_log_files, + Some(vec![ + PathBuf::from("existing-rollout.jsonl"), + PathBuf::from(write["path"].as_str().unwrap()) + ]) + ); + assert_eq!(std::fs::read_dir(&tmp)?.count(), 0); + } else if staging_fails { + assert_eq!( + upload.extra_log_files, + Some(vec![PathBuf::from("existing-rollout.jsonl")]) + ); + assert!( + upload + .reason + .unwrap() + .contains("TUI client logs were omitted") + ); + } else { + assert_eq!(recorded.len(), 1); + assert_eq!(upload.extra_log_files, None); + assert!(!tmp.exists()); + } + } + server.shutdown().await?; + proxy.await??; app.handle_feedback_submitted( /*origin_thread_id*/ None, @@ -7204,6 +7286,7 @@ async fn feedback_submission_without_thread_emits_error_history_cell() { lines_to_single_string(&cell.display_lines(/*width*/ 120)), "■ Failed to upload feedback: boom" ); + Ok(()) } #[tokio::test] diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index 1dd2c9f3e7..b46853dba8 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -343,7 +343,7 @@ async fn cleanup_materialized_goal_files( output_dir: Option, ) { if let Some(output_dir) = output_dir - && let Err(err) = app_server.fs_remove_path(&output_dir).await + && let Err(err) = app_server.file_system().fs_remove_path(&output_dir).await { tracing::warn!("failed to clean up materialized goal files at {output_dir}: {err}"); } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 84df511e1a..322cdb2445 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -4,7 +4,7 @@ //! request/response plumbing out of `App` and `ChatWidget`. mod external_agent_config; -mod fs; +pub(crate) mod fs; mod history; mod models; mod realtime; diff --git a/codex-rs/tui/src/app_server_session/fs.rs b/codex-rs/tui/src/app_server_session/fs.rs index 74d0e075ef..3b1efcd8ce 100644 --- a/codex-rs/tui/src/app_server_session/fs.rs +++ b/codex-rs/tui/src/app_server_session/fs.rs @@ -1,3 +1,5 @@ +//! Filesystem RPCs on the app-server host, usable by sessions and background requests. + use super::AppServerSession; use base64::Engine; use base64::engine::general_purpose::STANDARD; @@ -19,12 +21,22 @@ use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; use serde::de::DeserializeOwned; use serde_json::json; +use uuid::Uuid; impl AppServerSession { - pub(crate) async fn fs_create_directory_all_path( - &mut self, - path: &AppServerPath, - ) -> Result<()> { + pub(crate) fn file_system(&self) -> AppServerFileSystem { + AppServerFileSystem { + request_handle: self.request_handle(), + } + } +} + +pub(crate) struct AppServerFileSystem { + pub(crate) request_handle: AppServerRequestHandle, +} + +impl AppServerFileSystem { + pub(crate) async fn fs_create_directory_all_path(&self, path: &AppServerPath) -> Result<()> { self.request_fs_path::( "fs/createDirectory", path, @@ -42,7 +54,7 @@ impl AppServerSession { } pub(crate) async fn fs_write_file_path( - &mut self, + &self, path: &AppServerPath, bytes: Vec, ) -> Result<()> { @@ -63,7 +75,7 @@ impl AppServerSession { .map(drop) } - pub(crate) async fn fs_read_file_path(&mut self, path: &AppServerPath) -> Result> { + pub(crate) async fn fs_read_file_path(&self, path: &AppServerPath) -> Result> { let response: FsReadFileResponse = self .request_fs_path( "fs/readFile", @@ -80,7 +92,7 @@ impl AppServerSession { .wrap_err("fs/readFile returned invalid base64 data") } - pub(crate) async fn fs_remove_path(&mut self, path: &AppServerPath) -> Result<()> { + pub(crate) async fn fs_remove_path(&self, path: &AppServerPath) -> Result<()> { self.request_fs_path::( "fs/remove", path, @@ -99,14 +111,14 @@ impl AppServerSession { } async fn request_fs_path( - &mut self, + &self, method: &str, path: &AppServerPath, local_request: impl FnOnce(RequestId, AbsolutePathBuf) -> ClientRequest, remote_params: impl FnOnce() -> serde_json::Value, ) -> Result { - let request_id = self.next_request_id(); - match self.request_handle() { + let request_id = RequestId::String(format!("tui-fs-{}", Uuid::new_v4())); + match &self.request_handle { AppServerRequestHandle::Remote(handle) => { let response = handle .request_json_rpc(JSONRPCRequest { @@ -125,7 +137,7 @@ impl AppServerSession { AppServerRequestHandle::InProcess(_) => { let path = AbsolutePathBuf::from_absolute_path_checked(path.as_str()) .wrap_err_with(|| format!("invalid local app-server fs path {path}"))?; - self.client + self.request_handle .request_typed(local_request(request_id, path)) .await .wrap_err_with(|| format!("{method} failed in TUI")) diff --git a/codex-rs/tui/src/bottom_pane/feedback_view.rs b/codex-rs/tui/src/bottom_pane/feedback_view.rs index 3ceba06831..544fce1baa 100644 --- a/codex-rs/tui/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui/src/bottom_pane/feedback_view.rs @@ -253,6 +253,7 @@ pub(crate) fn feedback_upload_consent_params( Line::from("").into(), Line::from("The following files will be sent:".dim()).into(), Line::from(vec![" • ".into(), "codex-logs.log".into()]).into(), + Line::from(vec![" • ".into(), "client-logs.txt (if available)".into()]).into(), Line::from(vec![ " • ".into(), DOCTOR_REPORT_ATTACHMENT_FILENAME.into(), 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 c5abceac69..bbc1737568 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 @@ -7,6 +7,7 @@ Upload logs? The following files will be sent: • codex-logs.log + • client-logs.txt (if available) • codex-doctor-report.json • codex-apps-tools-cache.json (if available) • codex-app-directory-cache.json (if available) 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 index 68474648aa..edb8291a8e 100644 --- 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 @@ -7,6 +7,7 @@ Upload logs? The following files will be sent: • codex-logs.log + • client-logs.txt (if available) • codex-doctor-report.json • codex-apps-tools-cache.json (if available) • codex-app-directory-cache.json (if available) diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_good_result_consent_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_good_result_consent_popup.snap index 47c4b29d1d..dd81733077 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_good_result_consent_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_good_result_consent_popup.snap @@ -6,6 +6,7 @@ expression: popup The following files will be sent: • codex-logs.log + • client-logs.txt (if available) • codex-doctor-report.json • codex-apps-tools-cache.json (if available) • codex-app-directory-cache.json (if available) 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 9d8b365ada..3f27314c86 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 @@ -6,6 +6,7 @@ expression: popup The following files will be sent: • codex-logs.log + • client-logs.txt (if available) • codex-doctor-report.json • codex-apps-tools-cache.json (if available) • codex-app-directory-cache.json (if available) diff --git a/codex-rs/tui/src/goal_files.rs b/codex-rs/tui/src/goal_files.rs index b07b648e9a..163ec282dc 100644 --- a/codex-rs/tui/src/goal_files.rs +++ b/codex-rs/tui/src/goal_files.rs @@ -126,7 +126,7 @@ pub(crate) async fn materialize_goal_draft( Ok(reference) => reference, Err(err) => { if let Some(output_dir) = output_dir.as_ref() { - let _ = app_server.fs_remove_path(output_dir).await; + let _ = app_server.file_system().fs_remove_path(output_dir).await; } return Err(err); } @@ -146,6 +146,7 @@ pub(crate) async fn objective_text_for_edit( return Ok(objective.to_string()); }; let bytes = app_server + .file_system() .fs_read_file_path(&path) .await .map_err(|err| anyhow::anyhow!("{err}")) @@ -196,6 +197,7 @@ async fn ensure_goal_output_dir( .join(GOAL_ATTACHMENT_DIR) .join(Uuid::new_v4().to_string()); app_server + .file_system() .fs_create_directory_all_path(&path) .await .map_err(|err| anyhow::anyhow!("{err}")) @@ -210,6 +212,7 @@ async fn write_goal_file( bytes: Vec, ) -> Result<()> { app_server + .file_system() .fs_write_file_path(&path, bytes) .await .map_err(|err| anyhow::anyhow!("{err}"))