mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
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
This commit is contained in:
+1
-2
@@ -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
|
||||
+1
-2
@@ -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)
|
||||
|
||||
@@ -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())
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<String>| {
|
||||
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(|_| "<unrenderable guardian action>".to_string());
|
||||
history_cell::new_guardian_approved_action_request(summary)
|
||||
};
|
||||
|
||||
self.add_boxed_history(cell);
|
||||
self.request_redraw();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -310,16 +310,6 @@ pub fn new_guardian_denied_action_request(summary: String) -> Box<dyn HistoryCel
|
||||
Box::new(PrefixedWrappedHistoryCell::new(line, "✗ ".red(), " "))
|
||||
}
|
||||
|
||||
pub fn new_guardian_approved_action_request(summary: String) -> Box<dyn HistoryCell> {
|
||||
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<String>) -> Box<dyn HistoryCell> {
|
||||
let mut summary = vec![
|
||||
"Review ".into(),
|
||||
|
||||
Reference in New Issue
Block a user