Attach guardian trunk rollout to feedback uploads

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Charles Cunningham
2026-03-19 17:58:12 -07:00
co-authored by Codex
parent d796ac977b
commit 03c95508fb
12 changed files with 274 additions and 106 deletions
@@ -7074,6 +7074,24 @@ impl CodexMessageProcessor {
None
};
let mut attachment_paths = validated_rollout_path.into_iter().collect::<Vec<_>>();
if include_logs && let Some(conversation_id) = conversation_id {
match self.thread_manager.get_thread(conversation_id).await {
Ok(thread) => {
if let Some(path) = thread.guardian_trunk_rollout_path().await
&& !attachment_paths.contains(&path)
{
attachment_paths.push(path);
}
}
Err(err) => {
warn!(
thread_id = %conversation_id,
error = %err,
"failed to load live thread while resolving guardian rollout attachment"
);
}
}
}
if let Some(extra_log_files) = extra_log_files {
attachment_paths.extend(extra_log_files);
}
+8
View File
@@ -155,6 +155,14 @@ impl CodexThread {
self.codex.state_db()
}
pub async fn guardian_trunk_rollout_path(&self) -> Option<PathBuf> {
self.codex
.session
.guardian_review_session
.trunk_rollout_path()
.await
}
pub async fn config_snapshot(&self) -> ThreadConfigSnapshot {
self.codex.thread_config_snapshot().await
}
@@ -313,6 +313,13 @@ impl GuardianReviewSessionManager {
}
}
pub(crate) async fn trunk_rollout_path(&self) -> Option<PathBuf> {
let trunk = self.state.lock().await.trunk.as_ref().cloned();
let trunk = trunk?;
trunk.codex.session.flush_rollout().await;
trunk.codex.session.current_rollout_path().await
}
pub(crate) fn spawn_initialize_trunk_if_needed(
&self,
parent_session: Arc<Session>,
+55
View File
@@ -736,6 +736,61 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
Ok(())
}
async fn guardian_session_can_initialize_before_first_approval() -> anyhow::Result<()> {
let server = start_mock_server().await;
let _startup_prewarm = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-guardian-startup"),
ev_completed("resp-guardian-startup"),
]),
)
.await;
let (session, turn) = guardian_test_session_and_turn(&server).await;
assert!(!session.guardian_review_session.has_trunk_for_test().await);
session
.guardian_review_session
.initialize_trunk_if_needed(Arc::clone(&session), Arc::clone(&turn))
.await;
assert!(session.guardian_review_session.has_trunk_for_test().await);
session.guardian_review_session.shutdown().await;
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn guardian_session_trunk_rollout_path_is_persisted() -> anyhow::Result<()> {
let server = start_mock_server().await;
let _startup_prewarm = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-guardian-startup"),
ev_completed("resp-guardian-startup"),
]),
)
.await;
let (session, turn) = guardian_test_session_and_turn(&server).await;
session
.guardian_review_session
.initialize_trunk_if_needed(Arc::clone(&session), Arc::clone(&turn))
.await;
assert!(
session
.guardian_review_session
.trunk_rollout_path()
.await
.is_some()
);
session.guardian_review_session.shutdown().await;
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn guardian_active_fork_cap_waits_for_any_fork_slot() -> anyhow::Result<()> {
let fork_cap = 2;
+25 -2
View File
@@ -1399,6 +1399,21 @@ impl App {
self.active_thread_id.or(self.chat_widget.thread_id())
}
async fn current_guardian_rollout_path(&self) -> Option<PathBuf> {
let thread_id = self.current_displayed_thread_id()?;
match self.server.get_thread(thread_id).await {
Ok(thread) => thread.guardian_trunk_rollout_path().await,
Err(err) => {
tracing::warn!(
thread_id = %thread_id,
error = %err,
"failed to load thread while resolving guardian rollout path for feedback"
);
None
}
}
}
/// Mirrors the visible thread into the contextual footer row.
///
/// The footer sometimes shows ambient context instead of an instructional hint. In multi-agent
@@ -2834,10 +2849,18 @@ impl App {
category,
include_logs,
} => {
self.chat_widget.open_feedback_note(category, include_logs);
let guardian_rollout_path = if include_logs {
self.current_guardian_rollout_path().await
} else {
None
};
self.chat_widget
.open_feedback_note(category, include_logs, guardian_rollout_path);
}
AppEvent::OpenFeedbackConsent { category } => {
self.chat_widget.open_feedback_consent(category);
let guardian_rollout_path = self.current_guardian_rollout_path().await;
self.chat_widget
.open_feedback_consent(category, guardian_rollout_path);
}
AppEvent::LaunchExternalEditor => {
if self.chat_widget.external_editor_state() == ExternalEditorState::Active {
+21 -4
View File
@@ -50,6 +50,7 @@ pub(crate) struct FeedbackNoteView {
category: FeedbackCategory,
snapshot: codex_feedback::FeedbackSnapshot,
rollout_path: Option<PathBuf>,
guardian_rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
feedback_audience: FeedbackAudience,
@@ -65,6 +66,7 @@ impl FeedbackNoteView {
category: FeedbackCategory,
snapshot: codex_feedback::FeedbackSnapshot,
rollout_path: Option<PathBuf>,
guardian_rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
feedback_audience: FeedbackAudience,
@@ -73,6 +75,7 @@ impl FeedbackNoteView {
category,
snapshot,
rollout_path,
guardian_rollout_path,
app_event_tx,
include_logs,
feedback_audience,
@@ -90,7 +93,11 @@ impl FeedbackNoteView {
Some(note.as_str())
};
let attachment_paths = if self.include_logs {
self.rollout_path.iter().cloned().collect::<Vec<_>>()
self.rollout_path
.iter()
.chain(self.guardian_rollout_path.iter())
.cloned()
.collect::<Vec<_>>()
} else {
Vec::new()
};
@@ -502,6 +509,7 @@ pub(crate) fn feedback_upload_consent_params(
app_event_tx: AppEventSender,
category: FeedbackCategory,
rollout_path: Option<std::path::PathBuf>,
guardian_rollout_path: Option<std::path::PathBuf>,
feedback_diagnostics: &FeedbackDiagnostics,
) -> super::SelectionViewParams {
use super::popup_consts::standard_popup_hint_line;
@@ -534,10 +542,17 @@ 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(),
];
if let Some(path) = rollout_path.as_deref()
&& let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string())
for rollout_path in rollout_path
.iter()
.chain(guardian_rollout_path.iter())
.map(std::path::PathBuf::as_path)
{
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
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());
}
}
if !feedback_diagnostics.is_empty() {
header_lines.push(
@@ -633,6 +648,7 @@ mod tests {
category,
snapshot,
None,
None,
tx,
true,
FeedbackAudience::External,
@@ -696,6 +712,7 @@ mod tests {
FeedbackCategory::Bug,
snapshot,
None,
None,
tx,
false,
FeedbackAudience::External,
+10 -2
View File
@@ -1519,6 +1519,7 @@ impl ChatWidget {
&mut self,
category: crate::app_event::FeedbackCategory,
include_logs: bool,
guardian_rollout_path: Option<PathBuf>,
) {
if let Some(chatgpt_user_id) = self
.auth_manager
@@ -1528,13 +1529,14 @@ impl ChatWidget {
tracing::info!(target: "feedback_tags", chatgpt_user_id);
}
let snapshot = self.feedback.snapshot(self.thread_id);
self.show_feedback_note(category, include_logs, snapshot);
self.show_feedback_note(category, include_logs, guardian_rollout_path, snapshot);
}
fn show_feedback_note(
&mut self,
category: crate::app_event::FeedbackCategory,
include_logs: bool,
guardian_rollout_path: Option<PathBuf>,
snapshot: codex_feedback::FeedbackSnapshot,
) {
let rollout = if include_logs {
@@ -1546,6 +1548,7 @@ impl ChatWidget {
category,
snapshot,
rollout,
guardian_rollout_path,
self.app_event_tx.clone(),
include_logs,
self.feedback_audience,
@@ -1560,7 +1563,11 @@ impl ChatWidget {
self.request_redraw();
}
pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) {
pub(crate) fn open_feedback_consent(
&mut self,
category: crate::app_event::FeedbackCategory,
guardian_rollout_path: Option<PathBuf>,
) {
if let Some(chatgpt_user_id) = self
.auth_manager
.auth_cached()
@@ -1573,6 +1580,7 @@ impl ChatWidget {
self.app_event_tx.clone(),
category,
self.current_rollout_path.clone(),
guardian_rollout_path,
snapshot.feedback_diagnostics(),
);
self.bottom_pane.show_selection_view(params);
+2
View File
@@ -8191,6 +8191,7 @@ 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(),
@@ -8211,6 +8212,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(),
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(),
+6 -2
View File
@@ -3718,10 +3718,14 @@ impl App {
category,
include_logs,
} => {
self.chat_widget.open_feedback_note(category, include_logs);
self.chat_widget.open_feedback_note(
category,
include_logs,
app_server.request_handle(),
);
}
AppEvent::OpenFeedbackConsent { category } => {
self.chat_widget.open_feedback_consent(category);
self.chat_widget.open_feedback_consent(category, None);
}
AppEvent::LaunchExternalEditor => {
if self.chat_widget.external_editor_state() == ExternalEditorState::Active {
@@ -1,6 +1,3 @@
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;
@@ -15,13 +12,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;
@@ -49,7 +46,7 @@ pub(crate) enum FeedbackAudience {
pub(crate) struct FeedbackNoteView {
category: FeedbackCategory,
snapshot: codex_feedback::FeedbackSnapshot,
rollout_path: Option<PathBuf>,
app_server_request_handle: Option<codex_app_server_client::AppServerRequestHandle>,
app_event_tx: AppEventSender,
include_logs: bool,
feedback_audience: FeedbackAudience,
@@ -64,7 +61,7 @@ impl FeedbackNoteView {
pub(crate) fn new(
category: FeedbackCategory,
snapshot: codex_feedback::FeedbackSnapshot,
rollout_path: Option<PathBuf>,
app_server_request_handle: Option<codex_app_server_client::AppServerRequestHandle>,
app_event_tx: AppEventSender,
include_logs: bool,
feedback_audience: FeedbackAudience,
@@ -72,7 +69,7 @@ impl FeedbackNoteView {
Self {
category,
snapshot,
rollout_path,
app_server_request_handle,
app_event_tx,
include_logs,
feedback_audience,
@@ -84,90 +81,107 @@ impl FeedbackNoteView {
fn submit(&mut self) {
let note = self.textarea.text().trim().to_string();
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::<Vec<_>>()
} else {
Vec::new()
};
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 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::FeedbackUploadResponse>(
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(()) => {
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(),
]),
]);
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(),
]),
]);
}
}
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;
}
}
@@ -502,6 +516,7 @@ pub(crate) fn feedback_upload_consent_params(
app_event_tx: AppEventSender,
category: FeedbackCategory,
rollout_path: Option<std::path::PathBuf>,
guardian_rollout_path: Option<std::path::PathBuf>,
feedback_diagnostics: &FeedbackDiagnostics,
) -> super::SelectionViewParams {
use super::popup_consts::standard_popup_hint_line;
@@ -534,10 +549,17 @@ 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(),
];
if let Some(path) = rollout_path.as_deref()
&& let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string())
for rollout_path in rollout_path
.iter()
.chain(guardian_rollout_path.iter())
.map(std::path::PathBuf::as_path)
{
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
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());
}
}
if !feedback_diagnostics.is_empty() {
header_lines.push(
+10 -8
View File
@@ -1896,9 +1896,10 @@ 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);
self.show_feedback_note(category, include_logs, snapshot, app_server_request_handle);
}
fn show_feedback_note(
@@ -1906,16 +1907,12 @@ 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,
rollout,
Some(app_server_request_handle),
self.app_event_tx.clone(),
include_logs,
self.feedback_audience,
@@ -1930,12 +1927,17 @@ impl ChatWidget {
self.request_redraw();
}
pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) {
pub(crate) fn open_feedback_consent(
&mut self,
category: crate::app_event::FeedbackCategory,
guardian_rollout_path: Option<PathBuf>,
) {
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);
@@ -8735,6 +8735,7 @@ 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(),
@@ -8755,6 +8756,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(),
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(),