From 8f4a2c99dd56e136894c2ef2221bd7f24f760dd7 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 11 Aug 2026 15:58:35 +0000 Subject: [PATCH] Hide approved Guardian assessments from TUI history (#38032) ## What changed - Silently complete approved Guardian assessments after clearing their live review status, instead of adding an approval entry to conversation history. - Continue rendering denied and timed-out assessments in history. ## Testing - Update TUI snapshots to verify that approved command and permission-request assessments add no history entry and that the status returns to `Working`. GitOrigin-RevId: 32df1f2e3d4e23f0b792dcfdfcf7fbe95f33c703 --- ...approved_exec_is_hidden_from_history.snap} | 3 +-- ...ssions_clears_status_without_history.snap} | 3 +-- codex-rs/tui/src/chatwidget/tests/guardian.rs | 21 ++++++++--------- codex-rs/tui/src/chatwidget/tool_requests.rs | 23 ++++--------------- codex-rs/tui/src/history_cell/approvals.rs | 10 -------- 5 files changed, 15 insertions(+), 45 deletions(-) rename codex-rs/tui/src/chatwidget/snapshots/{codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap => codex_tui__chatwidget__tests__guardian_approved_exec_is_hidden_from_history.snap} (70%) rename codex-rs/tui/src/chatwidget/snapshots/{codex_tui__chatwidget__tests__guardian_approved_request_permissions_renders_request_summary.snap => codex_tui__chatwidget__tests__guardian_approved_request_permissions_clears_status_without_history.snap} (72%) diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_is_hidden_from_history.snap similarity index 70% rename from codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap rename to codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_is_hidden_from_history.snap index bd272de13b..651376f853 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_is_hidden_from_history.snap @@ -7,8 +7,7 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) -✔ Auto-reviewer approved codex to run rm -f /tmp/guardian-approved.sqlite this - time + › Ask Codex to do anything diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_request_permissions_renders_request_summary.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_request_permissions_clears_status_without_history.snap similarity index 72% rename from codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_request_permissions_renders_request_summary.snap rename to codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_request_permissions_clears_status_without_history.snap index 4a05153625..417c1164ea 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_request_permissions_renders_request_summary.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_request_permissions_clears_status_without_history.snap @@ -5,8 +5,7 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) -✔ Request approved for permission request: Need write access for generated - report assets. + • Working (0s • esc to interrupt) diff --git a/codex-rs/tui/src/chatwidget/tests/guardian.rs b/codex-rs/tui/src/chatwidget/tests/guardian.rs index bedba04b90..5956af872e 100644 --- a/codex-rs/tui/src/chatwidget/tests/guardian.rs +++ b/codex-rs/tui/src/chatwidget/tests/guardian.rs @@ -159,7 +159,7 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() { } #[tokio::test] -async fn guardian_approved_exec_renders_approved_request() { +async fn guardian_approved_exec_is_hidden_from_history() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.show_welcome_banner = false; @@ -192,10 +192,7 @@ async fn guardian_approved_exec_renders_approved_request() { let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); term.set_viewport_area(viewport); - for lines in drain_insert_history(&mut rx) { - crate::insert_history::insert_history_lines(&mut term, lines) - .expect("Failed to insert history lines in test"); - } + assert!(drain_insert_history(&mut rx).is_empty()); term.draw(|f| { chat.render(f.area(), f.buffer_mut()); @@ -203,13 +200,13 @@ async fn guardian_approved_exec_renders_approved_request() { .expect("draw guardian approval history"); assert_chatwidget_snapshot!( - "guardian_approved_exec_renders_approved_request", + "guardian_approved_exec_is_hidden_from_history", normalize_snapshot_paths(term.backend().vt100().screen().contents()) ); } #[tokio::test] -async fn guardian_approved_request_permissions_renders_request_summary() { +async fn guardian_approved_request_permissions_clears_status_without_history() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.show_welcome_banner = false; let action = GuardianAssessmentAction::RequestPermissions { @@ -265,6 +262,9 @@ async fn guardian_approved_request_permissions_renders_request_summary() { action, }); + assert!(chat.status_state.pending_guardian_review_status.is_empty()); + assert_eq!(chat.status_state.current_status.header, "Working"); + let width: u16 = 110; let ui_height: u16 = chat.desired_height(width); let vt_height: u16 = ui_height.saturating_add(1).max(12); @@ -274,10 +274,7 @@ async fn guardian_approved_request_permissions_renders_request_summary() { let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); term.set_viewport_area(viewport); - for lines in drain_insert_history(&mut rx) { - crate::insert_history::insert_history_lines(&mut term, lines) - .expect("Failed to insert history lines in test"); - } + assert!(drain_insert_history(&mut rx).is_empty()); term.draw(|f| { chat.render(f.area(), f.buffer_mut()); @@ -285,7 +282,7 @@ async fn guardian_approved_request_permissions_renders_request_summary() { .expect("draw guardian request permissions approval history"); assert_chatwidget_snapshot!( - "guardian_approved_request_permissions_renders_request_summary", + "guardian_approved_request_permissions_clears_status_without_history", normalize_snapshot_paths(term.backend().vt100().screen().contents()) ); } diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index b9ba8108f3..029a1279d0 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -31,8 +31,8 @@ impl ChatWidget { /// In-progress assessments temporarily own the live status footer so the /// user can see what is being reviewed, including parallel review /// aggregation. Terminal assessments clear or update that footer state and - /// render the final approved/denied history cell when guardian returns a - /// decision. + /// render denied or timed-out decisions in history. Approved assessments + /// silently complete after updating the footer. pub(super) fn on_guardian_assessment(&mut self, ev: GuardianAssessmentEvent) { let permission_request_summary = |subject: &str, reason: &Option| { reason @@ -119,8 +119,8 @@ impl ChatWidget { return; } - // Terminal assessments remove the matching pending footer entry first, - // then render the final approved/denied history cell below. + // Terminal assessments remove the matching pending footer entry before + // any decision-specific history handling. if self .status_state .pending_guardian_review_status @@ -147,21 +147,6 @@ impl ChatWidget { } if ev.status == GuardianAssessmentStatus::Approved { - let cell = if let Some(command) = guardian_command(&ev.action) { - history_cell::new_approval_decision_cell( - history_cell::ApprovalDecisionSubject::Command(command), - crate::history_cell::ReviewDecision::Approved, - history_cell::ApprovalDecisionActor::Guardian, - ) - } else if let Some(summary) = guardian_action_summary(&ev.action) { - history_cell::new_guardian_approved_action_request(summary) - } else { - let summary = serde_json::to_string(&ev.action) - .unwrap_or_else(|_| "".to_string()); - history_cell::new_guardian_approved_action_request(summary) - }; - - self.add_boxed_history(cell); self.request_redraw(); return; } diff --git a/codex-rs/tui/src/history_cell/approvals.rs b/codex-rs/tui/src/history_cell/approvals.rs index 14c5ad24a8..ef10ad6c98 100644 --- a/codex-rs/tui/src/history_cell/approvals.rs +++ b/codex-rs/tui/src/history_cell/approvals.rs @@ -310,16 +310,6 @@ pub fn new_guardian_denied_action_request(summary: String) -> Box Box { - let line = Line::from(vec![ - "Request ".into(), - "approved".bold(), - " for ".into(), - Span::from(summary).dim(), - ]); - Box::new(PrefixedWrappedHistoryCell::new(line, "✔ ".green(), " ")) -} - pub fn new_guardian_timed_out_patch_request(files: Vec) -> Box { let mut summary = vec![ "Review ".into(),