From ea9fc79deccbf0320561356c0a55f8287d058dbe Mon Sep 17 00:00:00 2001 From: kevin zhao Date: Thu, 20 Nov 2025 19:16:12 -0500 Subject: [PATCH] Add explicit prefix-approval decision and wire it through execpolicy/UI snapshots --- codex-rs/core/src/apply_patch.rs | 4 +++- codex-rs/core/src/codex.rs | 4 +++- codex-rs/core/src/tools/orchestrator.rs | 8 ++++++-- codex-rs/protocol/src/protocol.rs | 4 ++++ codex-rs/tui/src/bottom_pane/approval_overlay.rs | 4 ++-- ...tui__chatwidget__tests__approval_modal_exec.snap | 3 ++- ...idget__tests__approval_modal_exec_no_reason.snap | 3 ++- ...et__tests__status_widget_and_approval_modal.snap | 4 ++-- codex-rs/tui/src/history_cell.rs | 13 +++++++++++++ 9 files changed, 37 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index dffe94be61..b9b36a0d8a 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -70,7 +70,9 @@ pub(crate) async fn apply_patch( ) .await; match rx_approve.await.unwrap_or_default() { - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => { + ReviewDecision::Approved + | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedForSession => { InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec { action, user_explicitly_approved_this_action: true, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 09d6872d2a..69539ffb7f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1671,7 +1671,9 @@ mod handlers { if let Some(prefix) = allow_prefix && matches!( decision, - ReviewDecision::Approved | ReviewDecision::ApprovedForSession + ReviewDecision::Approved + | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedForSession ) && let Err(err) = sess.persist_command_allow_prefix(&prefix).await { diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 93b914e570..00416748e1 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -94,7 +94,9 @@ impl ToolOrchestrator { ReviewDecision::Denied | ReviewDecision::Abort => { return Err(ToolError::Rejected("rejected by user".to_string())); } - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {} + ReviewDecision::Approved + | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedForSession => {} } already_approved = true; } @@ -175,7 +177,9 @@ impl ToolOrchestrator { ReviewDecision::Denied | ReviewDecision::Abort => { return Err(ToolError::Rejected("rejected by user".to_string())); } - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {} + ReviewDecision::Approved + | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedForSession => {} } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2c4d793a4b..f606f15fff 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1657,6 +1657,10 @@ pub enum ReviewDecision { /// User has approved this command and the agent should execute it. Approved, + /// User has approved this command and wants to add the command prefix to + /// the execpolicy allow list so future matching commands are permitted. + ApprovedAllowPrefix, + /// User has approved this command and wants to automatically approve any /// future identical instances (`command` and `cwd` match exactly) for the /// remainder of the session. diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 80859dcd84..afaa88fedc 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -498,7 +498,7 @@ fn exec_options(allow_prefix: Option>) -> Vec { .into_iter() .chain(allow_prefix.map(|prefix| ApprovalOption { label: "Yes, and don't ask again for commands with this prefix".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), + decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], allow_prefix: Some(prefix), @@ -627,7 +627,7 @@ mod tests { .. }) = ev { - assert_eq!(decision, ReviewDecision::ApprovedForSession); + assert_eq!(decision, ReviewDecision::ApprovedAllowPrefix); assert_eq!(allow_prefix, Some(vec!["echo".to_string()])); saw_op = true; break; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap index b84588e337..9a0cf18f7f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap @@ -11,6 +11,7 @@ expression: terminal.backend().vt100().screen().contents() › 1. Yes, proceed (y) 2. Yes, and don't ask again for this command (a) - 3. No, and tell Codex what to do differently (esc) + 3. Yes, and don't ask again for commands with this prefix (p) + 4. No, and tell Codex what to do differently (esc) Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap index 543d367d23..3571f5153f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap @@ -8,6 +8,7 @@ expression: terminal.backend().vt100().screen().contents() › 1. Yes, proceed (y) 2. Yes, and don't ask again for this command (a) - 3. No, and tell Codex what to do differently (esc) + 3. Yes, and don't ask again for commands with this prefix (p) + 4. No, and tell Codex what to do differently (esc) Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap index f98c807878..990aa83680 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap @@ -1,6 +1,5 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 1548 expression: terminal.backend() --- " " @@ -14,6 +13,7 @@ expression: terminal.backend() " " "› 1. Yes, proceed (y) " " 2. Yes, and don't ask again for this command (a) " -" 3. No, and tell Codex what to do differently (esc) " +" 3. Yes, and don't ask again for commands with this prefix (p) " +" 4. No, and tell Codex what to do differently (esc) " " " " Press enter to confirm or esc to cancel " diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 02ab0d243b..96bea11460 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -408,6 +408,19 @@ pub fn new_approval_decision_cell( ], ) } + ApprovedAllowPrefix => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✔ ".green(), + vec![ + "You ".into(), + "approved".bold(), + " codex to run ".into(), + snippet, + " and added its prefix to your allow list".bold(), + ], + ) + } ApprovedForSession => { let snippet = Span::from(exec_snippet(&command)).dim(); (