From 88abbf58cec9843a7cebe250d71146314ebbcf1a Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Fri, 24 Oct 2025 23:07:40 -0700 Subject: [PATCH] Followup feedback (#5663) - Added files to be uploaded - Refactored - Updated title --- codex-rs/feedback/src/lib.rs | 123 +++++------------- codex-rs/tui/src/bottom_pane/feedback_view.rs | 40 ++++-- codex-rs/tui/src/chatwidget.rs | 7 +- ..._tests__feedback_upload_consent_popup.snap | 3 + 4 files changed, 69 insertions(+), 104 deletions(-) diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index fc696f2a..b1adb3db 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -167,14 +167,13 @@ impl CodexLogSnapshot { Ok(path) } - /// Uploads feedback to Sentry with both the in-memory Codex logs and an optional - /// rollout file attached. Also records metadata such as classification, - /// reason (free-form note), and CLI version as Sentry tags or message. - pub fn upload_feedback_with_rollout( + /// Upload feedback to Sentry with optional attachments. + pub fn upload_feedback( &self, classification: &str, reason: Option<&str>, cli_version: &str, + include_logs: bool, rollout_path: Option<&std::path::Path>, ) -> Result<()> { use std::collections::BTreeMap; @@ -194,129 +193,73 @@ impl CodexLogSnapshot { // Build Sentry client let client = Client::from_config(ClientOptions { - dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {}", e))?), + dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {e}"))?), transport: Some(Arc::new(DefaultTransportFactory {})), ..Default::default() }); - // Tags: thread id, classification, cli_version let mut tags = BTreeMap::from([ (String::from("thread_id"), self.thread_id.to_string()), (String::from("classification"), classification.to_string()), (String::from("cli_version"), cli_version.to_string()), ]); - - // Reason (freeform) – include entire note as a tag; keep title in message. if let Some(r) = reason { tags.insert(String::from("reason"), r.to_string()); } - // Elevate level for error-like classifications let level = match classification { "bug" | "bad_result" => Level::Error, _ => Level::Info, }; let mut envelope = Envelope::new(); - // Title is the message in Sentry: "[Classification]: Codex session " let title = format!( "[{}]: Codex session {}", display_classification(classification), self.thread_id ); - let event = Event { + + let mut event = Event { level, - message: Some(title), + message: Some(title.clone()), tags, ..Default::default() }; + if let Some(r) = reason { + use sentry::protocol::Exception; + use sentry::protocol::Values; + + event.exception = Values::from(vec![Exception { + ty: title.clone(), + value: Some(r.to_string()), + ..Default::default() + }]); + } envelope.add_item(EnvelopeItem::Event(event)); - // Attachment 1: Codex logs snapshot - envelope.add_item(EnvelopeItem::Attachment(Attachment { - buffer: self.bytes.clone(), - filename: String::from("codex-logs.log"), - content_type: Some("text/plain".to_string()), - ty: None, - })); - - // Attachment 2: rollout file (if provided and readable) - if let Some((path, data)) = rollout_path.and_then(|p| fs::read(p).ok().map(|d| (p, d))) { - // Name the file by suffix so users can spot it. - let fname = path - .file_name() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "rollout.jsonl".to_string()); + if include_logs { envelope.add_item(EnvelopeItem::Attachment(Attachment { - buffer: data, - filename: fname, - content_type: Some("application/jsonl".to_string()), + buffer: self.bytes.clone(), + filename: String::from("codex-logs.log"), + content_type: Some("text/plain".to_string()), ty: None, })); } - client.send_envelope(envelope); - client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS))); - - Ok(()) - } - - /// Upload a metadata-only feedback event (no attachments). Includes classification, - /// optional reason, CLI version and thread ID as tags. - pub fn upload_feedback_metadata_only( - &self, - classification: &str, - reason: Option<&str>, - cli_version: &str, - ) -> Result<()> { - use std::collections::BTreeMap; - use std::str::FromStr; - use std::sync::Arc; - - use sentry::Client; - use sentry::ClientOptions; - use sentry::protocol::Envelope; - use sentry::protocol::EnvelopeItem; - use sentry::protocol::Event; - use sentry::protocol::Level; - use sentry::transports::DefaultTransportFactory; - use sentry::types::Dsn; - - let client = Client::from_config(ClientOptions { - dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {}", e))?), - transport: Some(Arc::new(DefaultTransportFactory {})), - ..Default::default() - }); - - let mut tags = BTreeMap::from([ - (String::from("thread_id"), self.thread_id.to_string()), - (String::from("classification"), classification.to_string()), - (String::from("cli_version"), cli_version.to_string()), - ]); - if let Some(r) = reason { - tags.insert(String::from("reason"), r.to_string()); + if let Some((path, data)) = rollout_path.and_then(|p| fs::read(p).ok().map(|d| (p, d))) { + let fname = path + .file_name() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_else(|| "rollout.jsonl".to_string()); + let content_type = "text/plain".to_string(); + envelope.add_item(EnvelopeItem::Attachment(Attachment { + buffer: data, + filename: fname, + content_type: Some(content_type), + ty: None, + })); } - let level = match classification { - "bug" | "bad_result" => Level::Error, - _ => Level::Info, - }; - - let mut envelope = Envelope::new(); - // Title is the message in Sentry: "[Classification]: Codex session " - let title = format!( - "[{}]: Codex session {}", - display_classification(classification), - self.thread_id - ); - let event = Event { - level, - message: Some(title), - tags, - ..Default::default() - }; - envelope.add_item(EnvelopeItem::Event(event)); - client.send_envelope(envelope); client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS))); Ok(()) diff --git a/codex-rs/tui/src/bottom_pane/feedback_view.rs b/codex-rs/tui/src/bottom_pane/feedback_view.rs index fce3a795..234dbbd2 100644 --- a/codex-rs/tui/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui/src/bottom_pane/feedback_view.rs @@ -76,17 +76,17 @@ impl FeedbackNoteView { let cli_version = crate::version::CODEX_CLI_VERSION; let mut thread_id = self.snapshot.thread_id.clone(); - let result = if self.include_logs { - self.snapshot.upload_feedback_with_rollout( - classification, - reason_opt, - cli_version, - rollout_path_ref, - ) - } else { - self.snapshot - .upload_feedback_metadata_only(classification, reason_opt, cli_version) - }; + let result = self.snapshot.upload_feedback( + classification, + reason_opt, + cli_version, + self.include_logs, + if self.include_logs { + rollout_path_ref + } else { + None + }, + ); match result { Ok(()) => { @@ -380,6 +380,7 @@ fn make_feedback_item( pub(crate) fn feedback_upload_consent_params( app_event_tx: AppEventSender, category: FeedbackCategory, + rollout_path: Option, ) -> super::SelectionViewParams { use super::popup_consts::standard_popup_hint_line; let yes_action: super::SelectionAction = Box::new({ @@ -404,8 +405,20 @@ pub(crate) fn feedback_upload_consent_params( } }); + // Build header listing files that would be sent if user consents. + let mut header_lines: Vec> = vec![ + Line::from("Upload logs?".bold()).into(), + Line::from("").into(), + Line::from("The following files will be sent:".dim()).into(), + Line::from(vec![" • ".into(), "codex-logs.log".into()]).into(), + ]; + if let Some(path) = rollout_path.as_deref() + && let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) + { + header_lines.push(Line::from(vec![" • ".into(), name.into()]).into()); + } + super::SelectionViewParams { - title: Some("Upload logs?".to_string()), footer_hint: Some(standard_popup_hint_line()), items: vec![ super::SelectionItem { @@ -426,6 +439,9 @@ pub(crate) fn feedback_upload_consent_params( ..Default::default() }, ], + header: Box::new(crate::render::renderable::ColumnRenderable::with( + header_lines, + )), ..Default::default() } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 81661aa3..fdebe158 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -370,8 +370,11 @@ impl ChatWidget { } pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) { - let params = - crate::bottom_pane::feedback_upload_consent_params(self.app_event_tx.clone(), category); + let params = crate::bottom_pane::feedback_upload_consent_params( + self.app_event_tx.clone(), + category, + self.current_rollout_path.clone(), + ); self.bottom_pane.show_selection_view(params); self.request_redraw(); } 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 092e961d..cc3d8e37 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 @@ -4,6 +4,9 @@ expression: popup --- Upload logs? + The following files will be sent: + • codex-logs.log + › 1. Yes Share the current Codex session logs with the team for troubleshooting. 2. No