diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index d9b95b0af9..52b6db48bb 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -3718,14 +3718,10 @@ impl App { category, include_logs, } => { - self.chat_widget.open_feedback_note( - category, - include_logs, - app_server.request_handle(), - ); + self.chat_widget.open_feedback_note(category, include_logs); } AppEvent::OpenFeedbackConsent { category } => { - self.chat_widget.open_feedback_consent(category, None); + self.chat_widget.open_feedback_consent(category); } AppEvent::LaunchExternalEditor => { if self.chat_widget.external_editor_state() == ExternalEditorState::Active { diff --git a/codex-rs/tui_app_server/src/bottom_pane/feedback_view.rs b/codex-rs/tui_app_server/src/bottom_pane/feedback_view.rs index a27c083de2..4b56761246 100644 --- a/codex-rs/tui_app_server/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui_app_server/src/bottom_pane/feedback_view.rs @@ -1,3 +1,6 @@ +use std::cell::RefCell; +use std::path::PathBuf; + use codex_feedback::feedback_diagnostics::FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME; use codex_feedback::feedback_diagnostics::FeedbackDiagnostics; use crossterm::event::KeyCode; @@ -12,13 +15,13 @@ use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::StatefulWidgetRef; use ratatui::widgets::Widget; -use std::cell::RefCell; use crate::app_event::AppEvent; use crate::app_event::FeedbackCategory; use crate::app_event_sender::AppEventSender; use crate::history_cell; use crate::render::renderable::Renderable; +use codex_protocol::protocol::SessionSource; use super::CancellationEvent; use super::bottom_pane_view::BottomPaneView; @@ -46,7 +49,7 @@ pub(crate) enum FeedbackAudience { pub(crate) struct FeedbackNoteView { category: FeedbackCategory, snapshot: codex_feedback::FeedbackSnapshot, - app_server_request_handle: Option, + rollout_path: Option, app_event_tx: AppEventSender, include_logs: bool, feedback_audience: FeedbackAudience, @@ -61,7 +64,7 @@ impl FeedbackNoteView { pub(crate) fn new( category: FeedbackCategory, snapshot: codex_feedback::FeedbackSnapshot, - app_server_request_handle: Option, + rollout_path: Option, app_event_tx: AppEventSender, include_logs: bool, feedback_audience: FeedbackAudience, @@ -69,7 +72,7 @@ impl FeedbackNoteView { Self { category, snapshot, - app_server_request_handle, + rollout_path, app_event_tx, include_logs, feedback_audience, @@ -81,107 +84,89 @@ impl FeedbackNoteView { fn submit(&mut self) { let note = self.textarea.text().trim().to_string(); - let reason = (!note.is_empty()).then_some(note); - let classification = feedback_classification(self.category); - let thread_id = self.snapshot.thread_id.clone(); - let include_logs = self.include_logs; - let category = self.category; - let feedback_audience = self.feedback_audience; - let app_event_tx = self.app_event_tx.clone(); - let Some(app_server_request_handle) = self.app_server_request_handle.clone() else { - self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::new_error_event( - "Failed to upload feedback: missing app-server request handle".to_string(), - ), - ))); - self.complete = true; - return; + let reason_opt = if note.is_empty() { + None + } else { + Some(note.as_str()) }; + let attachment_paths = if self.include_logs { + self.rollout_path.iter().cloned().collect::>() + } else { + Vec::new() + }; + let classification = feedback_classification(self.category); + let mut thread_id = self.snapshot.thread_id.clone(); - tokio::spawn(async move { - let request_id = codex_app_server_protocol::RequestId::String(format!( - "feedback-upload-{}", - uuid::Uuid::new_v4() - )); - let result = app_server_request_handle - .request_typed::( - codex_app_server_protocol::ClientRequest::FeedbackUpload { - request_id, - params: codex_app_server_protocol::FeedbackUploadParams { - classification: classification.to_string(), - reason, - thread_id: Some(thread_id), - include_logs, - extra_log_files: None, - }, - }, - ) - .await; + let result = self.snapshot.upload_feedback( + classification, + reason_opt, + self.include_logs, + &attachment_paths, + Some(SessionSource::Cli), + /*logs_override*/ None, + ); - match result { - Ok(response) => { - let mut thread_id = response.thread_id; - let prefix = if include_logs { - "• Feedback uploaded." - } else { - "• Feedback recorded (no logs)." - }; - let issue_url = issue_url_for_category(category, &thread_id, feedback_audience); - let mut lines = vec![Line::from(match issue_url.as_ref() { - Some(_) if feedback_audience == FeedbackAudience::OpenAiEmployee => { - format!("{prefix} Please report this in #codex-feedback:") - } - Some(_) => { - format!("{prefix} Please open an issue using the following URL:") - } - None => format!("{prefix} Thanks for the feedback!"), - })]; - match issue_url { - Some(url) if feedback_audience == FeedbackAudience::OpenAiEmployee => { - lines.extend([ - "".into(), - Line::from(vec![" ".into(), url.cyan().underlined()]), - "".into(), - Line::from(" Share this and add some info about your problem:"), - Line::from(vec![ - " ".into(), - format!("https://go/codex-feedback/{thread_id}").bold(), - ]), - ]); - } - Some(url) => { - lines.extend([ - "".into(), - Line::from(vec![" ".into(), url.cyan().underlined()]), - "".into(), - Line::from(vec![ - " Or mention your thread ID ".into(), - std::mem::take(&mut thread_id).bold(), - " in an existing issue.".into(), - ]), - ]); - } - None => { - lines.extend([ - "".into(), - Line::from(vec![ - " Thread ID: ".into(), - std::mem::take(&mut thread_id).bold(), - ]), - ]); - } + match result { + Ok(()) => { + let prefix = if self.include_logs { + "• Feedback uploaded." + } else { + "• Feedback recorded (no logs)." + }; + let issue_url = + issue_url_for_category(self.category, &thread_id, self.feedback_audience); + let mut lines = vec![Line::from(match issue_url.as_ref() { + Some(_) if self.feedback_audience == FeedbackAudience::OpenAiEmployee => { + format!("{prefix} Please report this in #codex-feedback:") + } + Some(_) => format!("{prefix} Please open an issue using the following URL:"), + None => format!("{prefix} Thanks for the feedback!"), + })]; + match issue_url { + Some(url) if self.feedback_audience == FeedbackAudience::OpenAiEmployee => { + lines.extend([ + "".into(), + Line::from(vec![" ".into(), url.cyan().underlined()]), + "".into(), + Line::from(" Share this and add some info about your problem:"), + Line::from(vec![ + " ".into(), + format!("https://go/codex-feedback/{thread_id}").bold(), + ]), + ]); + } + Some(url) => { + lines.extend([ + "".into(), + Line::from(vec![" ".into(), url.cyan().underlined()]), + "".into(), + Line::from(vec![ + " Or mention your thread ID ".into(), + std::mem::take(&mut thread_id).bold(), + " in an existing issue.".into(), + ]), + ]); + } + None => { + lines.extend([ + "".into(), + Line::from(vec![ + " Thread ID: ".into(), + std::mem::take(&mut thread_id).bold(), + ]), + ]); } - app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::PlainHistoryCell::new(lines), - ))); - } - Err(err) => { - app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::new_error_event(format!("Failed to upload feedback: {err}")), - ))); } + self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::PlainHistoryCell::new(lines), + ))); } - }); + Err(e) => { + self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::new_error_event(format!("Failed to upload feedback: {e}")), + ))); + } + } self.complete = true; } } @@ -516,7 +501,6 @@ pub(crate) fn feedback_upload_consent_params( app_event_tx: AppEventSender, category: FeedbackCategory, rollout_path: Option, - guardian_rollout_path: Option, feedback_diagnostics: &FeedbackDiagnostics, ) -> super::SelectionViewParams { use super::popup_consts::standard_popup_hint_line; @@ -549,17 +533,10 @@ pub(crate) fn feedback_upload_consent_params( Line::from("The following files will be sent:".dim()).into(), Line::from(vec![" • ".into(), "codex-logs.log".into()]).into(), ]; - for rollout_path in rollout_path - .iter() - .chain(guardian_rollout_path.iter()) - .map(std::path::PathBuf::as_path) + if let Some(path) = rollout_path.as_deref() + && let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) { - if let Some(name) = rollout_path - .file_name() - .map(|s| s.to_string_lossy().to_string()) - { - header_lines.push(Line::from(vec![" • ".into(), name.into()]).into()); - } + header_lines.push(Line::from(vec![" • ".into(), name.into()]).into()); } if !feedback_diagnostics.is_empty() { header_lines.push( diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 8ed1826cf8..b233527faf 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -1896,10 +1896,9 @@ impl ChatWidget { &mut self, category: crate::app_event::FeedbackCategory, include_logs: bool, - app_server_request_handle: codex_app_server_client::AppServerRequestHandle, ) { let snapshot = self.feedback.snapshot(self.thread_id); - self.show_feedback_note(category, include_logs, snapshot, app_server_request_handle); + self.show_feedback_note(category, include_logs, snapshot); } fn show_feedback_note( @@ -1907,12 +1906,16 @@ impl ChatWidget { category: crate::app_event::FeedbackCategory, include_logs: bool, snapshot: codex_feedback::FeedbackSnapshot, - app_server_request_handle: codex_app_server_client::AppServerRequestHandle, ) { + let rollout = if include_logs { + self.current_rollout_path.clone() + } else { + None + }; let view = crate::bottom_pane::FeedbackNoteView::new( category, snapshot, - Some(app_server_request_handle), + rollout, self.app_event_tx.clone(), include_logs, self.feedback_audience, @@ -1927,17 +1930,12 @@ impl ChatWidget { self.request_redraw(); } - pub(crate) fn open_feedback_consent( - &mut self, - category: crate::app_event::FeedbackCategory, - guardian_rollout_path: Option, - ) { + pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) { let snapshot = self.feedback.snapshot(self.thread_id); let params = crate::bottom_pane::feedback_upload_consent_params( self.app_event_tx.clone(), category, self.current_rollout_path.clone(), - guardian_rollout_path, snapshot.feedback_diagnostics(), ); self.bottom_pane.show_selection_view(params); diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index 7961b489eb..2b14dac16d 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -8735,7 +8735,6 @@ async fn feedback_upload_consent_popup_snapshot() { chat.app_event_tx.clone(), crate::app_event::FeedbackCategory::Bug, chat.current_rollout_path.clone(), - None, &codex_feedback::feedback_diagnostics::FeedbackDiagnostics::new(vec![ codex_feedback::feedback_diagnostics::FeedbackDiagnostic { headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(), @@ -8756,7 +8755,6 @@ async fn feedback_good_result_consent_popup_includes_connectivity_diagnostics_fi chat.app_event_tx.clone(), crate::app_event::FeedbackCategory::GoodResult, chat.current_rollout_path.clone(), - None, &codex_feedback::feedback_diagnostics::FeedbackDiagnostics::new(vec![ codex_feedback::feedback_diagnostics::FeedbackDiagnostic { headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),