Followup feedback (#5663)

- Added files to be uploaded
- Refactored
- Updated title

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 <thread_id>"
         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 <thread_id>"
-        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(())

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<std::path::PathBuf>,
 ) -> 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<Box<dyn crate::render::renderable::Renderable>> = 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()
     }
 }

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();
     }

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