diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 13f5bb7eb3d7..31350e216d47 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -293,6 +293,7 @@ use codex_features::FEATURES; use codex_features::Feature; use codex_features::Stage; use codex_feedback::CodexFeedback; +use codex_feedback::FeedbackAttachmentPath; use codex_feedback::FeedbackUploadOptions; use codex_git_utils::git_diff_to_remote; use codex_git_utils::resolve_root_git_project_for_trust; @@ -7733,14 +7734,33 @@ impl CodexMessageProcessor { continue; }; if seen_attachment_paths.insert(rollout_path.clone()) { - attachment_paths.push(rollout_path); + attachment_paths.push(FeedbackAttachmentPath { + path: rollout_path, + attachment_filename_override: None, + }); } } + if let Some(conversation_id) = conversation_id + && let Ok(conversation) = self.thread_manager.get_thread(conversation_id).await + && let Some(guardian_rollout_path) = + conversation.guardian_trunk_rollout_path().await + && seen_attachment_paths.insert(guardian_rollout_path.clone()) + { + attachment_paths.push(FeedbackAttachmentPath { + path: guardian_rollout_path, + attachment_filename_override: Some(auto_review_rollout_filename( + conversation_id, + )), + }); + } } if let Some(extra_log_files) = extra_log_files { for extra_log_file in extra_log_files { if seen_attachment_paths.insert(extra_log_file.clone()) { - attachment_paths.push(extra_log_file); + attachment_paths.push(FeedbackAttachmentPath { + path: extra_log_file, + attachment_filename_override: None, + }); } } } @@ -7885,6 +7905,10 @@ impl CodexMessageProcessor { } } +fn auto_review_rollout_filename(thread_id: ThreadId) -> String { + format!("auto-review-rollout-{thread_id}.jsonl") +} + fn normalize_thread_list_cwd_filters( cwd: Option, ) -> Result>, JSONRPCErrorError> { diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 47f1b99ce5de..10429d58909b 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -369,6 +369,14 @@ impl CodexThread { self.rollout_path.clone() } + pub async fn guardian_trunk_rollout_path(&self) -> Option { + self.codex + .session + .guardian_review_session + .trunk_rollout_path() + .await + } + pub fn state_db(&self) -> Option { self.codex.state_db() } diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 22651c23d8e6..4725d3d63276 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -259,6 +259,18 @@ impl Drop for EphemeralReviewCleanup { } impl GuardianReviewSessionManager { + pub(crate) async fn trunk_rollout_path(&self) -> Option { + let trunk = self.state.lock().await.trunk.clone()?; + trunk.codex.session.ensure_rollout_materialized().await; + match trunk.codex.session.current_rollout_path().await { + Ok(path) => path, + Err(err) => { + warn!("failed to resolve guardian trunk rollout path: {err}"); + None + } + } + } + pub(crate) async fn shutdown(&self) { let (review_session, ephemeral_reviews) = { let mut state = self.state.lock().await; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 731987faa9e5..94b7f99486d2 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4660,6 +4660,38 @@ async fn shutdown_and_wait_shuts_down_cached_guardian_subagent() { .expect("guardian subagent should receive a shutdown op"); } +#[tokio::test] +async fn cached_guardian_subagent_exposes_its_rollout_path() { + let (parent_session, _parent_turn_context) = make_session_and_context().await; + let parent_session = Arc::new(parent_session); + + let (mut child_session, _child_turn_context) = make_session_and_context().await; + let child_rollout_path = attach_thread_persistence(&mut child_session).await; + let (child_tx_sub, _child_rx_sub) = async_channel::bounded(4); + let (_child_tx_event, child_rx_event) = async_channel::unbounded(); + let (_child_status_tx, child_agent_status) = watch::channel(AgentStatus::PendingInit); + let child_session_loop_handle = tokio::spawn(async {}); + let child_codex = Codex { + tx_sub: child_tx_sub, + rx_event: child_rx_event, + agent_status: child_agent_status, + session: Arc::new(child_session), + session_loop_termination: session_loop_termination_from_handle(child_session_loop_handle), + }; + parent_session + .guardian_review_session + .cache_for_test(child_codex) + .await; + + assert_eq!( + parent_session + .guardian_review_session + .trunk_rollout_path() + .await, + Some(child_rollout_path) + ); +} + #[tokio::test] async fn shutdown_and_wait_shuts_down_tracked_ephemeral_guardian_review() { let (parent_session, parent_turn_context) = make_session_and_context().await; diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index ff6b7963581b..884bed588c3e 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -338,12 +338,18 @@ pub struct FeedbackSnapshot { pub thread_id: String, } +pub struct FeedbackAttachmentPath { + pub path: PathBuf, + /// Optional filename to use for the uploaded attachment instead of `path`'s basename. + pub attachment_filename_override: Option, +} + pub struct FeedbackUploadOptions<'a> { pub classification: &'a str, pub reason: Option<&'a str>, pub tags: Option<&'a BTreeMap>, pub include_logs: bool, - pub extra_attachment_paths: &'a [PathBuf], + pub extra_attachment_paths: &'a [FeedbackAttachmentPath], pub session_source: Option, pub logs_override: Option>, } @@ -501,7 +507,7 @@ impl FeedbackSnapshot { fn feedback_attachments( &self, include_logs: bool, - extra_attachment_paths: &[PathBuf], + extra_attachment_paths: &[FeedbackAttachmentPath], logs_override: Option>, ) -> Vec { use sentry::protocol::Attachment; @@ -526,22 +532,28 @@ impl FeedbackSnapshot { }); } - for path in extra_attachment_paths { - let data = match fs::read(path) { + for attachment_path in extra_attachment_paths { + let data = match fs::read(&attachment_path.path) { Ok(data) => data, Err(err) => { tracing::warn!( - path = %path.display(), + path = %attachment_path.path.display(), error = %err, "failed to read log attachment; skipping" ); continue; } }; - let filename = path - .file_name() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "extra-log.log".to_string()); + let filename = attachment_path + .attachment_filename_override + .clone() + .unwrap_or_else(|| { + attachment_path + .path + .file_name() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_else(|| "extra-log.log".to_string()) + }); attachments.push(Attachment { buffer: data, filename, @@ -676,6 +688,10 @@ mod tests { fn feedback_attachments_gate_connectivity_diagnostics() { let extra_filename = format!("codex-feedback-extra-{}.jsonl", ThreadId::new()); let extra_path = std::env::temp_dir().join(&extra_filename); + let extra_attachment_path = FeedbackAttachmentPath { + path: extra_path.clone(), + attachment_filename_override: None, + }; fs::write(&extra_path, "rollout").expect("extra attachment should be written"); let snapshot_with_diagnostics = CodexFeedback::new() @@ -688,7 +704,7 @@ mod tests { let attachments_with_diagnostics = snapshot_with_diagnostics.feedback_attachments( /*include_logs*/ true, - std::slice::from_ref(&extra_path), + std::slice::from_ref(&extra_attachment_path), Some(vec![1]), ); @@ -715,6 +731,7 @@ mod tests { ); let attachments_without_diagnostics = CodexFeedback::new() .snapshot(/*session_id*/ None) + .with_feedback_diagnostics(FeedbackDiagnostics::default()) .feedback_attachments(/*include_logs*/ true, &[], Some(vec![1])); assert_eq!( diff --git a/codex-rs/tui/src/bottom_pane/feedback_view.rs b/codex-rs/tui/src/bottom_pane/feedback_view.rs index f40a21cdfcdb..59e30aba7b42 100644 --- a/codex-rs/tui/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui/src/bottom_pane/feedback_view.rs @@ -470,6 +470,7 @@ pub(crate) fn feedback_upload_consent_params( app_event_tx: AppEventSender, category: FeedbackCategory, rollout_path: Option, + auto_review_rollout_filename: Option, feedback_diagnostics: &FeedbackDiagnostics, ) -> super::SelectionViewParams { use super::popup_consts::standard_popup_hint_line; @@ -507,6 +508,9 @@ pub(crate) fn feedback_upload_consent_params( { header_lines.push(Line::from(vec![" • ".into(), name.into()]).into()); } + if let Some(filename) = auto_review_rollout_filename { + header_lines.push(Line::from(vec![" • ".into(), filename.into()]).into()); + } if !feedback_diagnostics.is_empty() { header_lines.push( Line::from(vec![ diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 2b1871886038..b0b3e10028ac 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2557,6 +2557,8 @@ impl ChatWidget { self.app_event_tx.clone(), category, self.current_rollout_path.clone(), + self.thread_id + .map(|thread_id| format!("auto-review-rollout-{thread_id}.jsonl")), snapshot.feedback_diagnostics(), ); self.bottom_pane.show_selection_view(params); 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 4529d6d478ad..ffc3e7f15a6f 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 + • auto-review-rollout-thread-1.jsonl • codex-connectivity-diagnostics.txt › 1. Yes Share the current Codex session logs with the team for 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 b625ad0a8df4..01f1175f3c39 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 + • auto-review-rollout-thread-1.jsonl • codex-connectivity-diagnostics.txt Connectivity diagnostics 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 728cbf01709b..7aeba5714cd9 100644 --- a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs +++ b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs @@ -2256,6 +2256,7 @@ async fn feedback_upload_consent_popup_snapshot() { chat.app_event_tx.clone(), crate::app_event::FeedbackCategory::Bug, chat.current_rollout_path.clone(), + Some("auto-review-rollout-thread-1.jsonl".to_string()), &codex_feedback::FeedbackDiagnostics::new(vec![codex_feedback::FeedbackDiagnostic { headline: "Proxy environment variables are set and may affect connectivity." .to_string(), @@ -2275,6 +2276,7 @@ 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(), + Some("auto-review-rollout-thread-1.jsonl".to_string()), &codex_feedback::FeedbackDiagnostics::new(vec![codex_feedback::FeedbackDiagnostic { headline: "Proxy environment variables are set and may affect connectivity." .to_string(),