diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 12c5d1497a..3eb0519761 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -950,7 +950,6 @@ async fn on_exec_approval_response( .submit(Op::ExecApproval { id: event_turn_id, decision: response.decision, - allow_prefix: None, }) .await { @@ -1155,7 +1154,6 @@ async fn on_command_execution_request_approval_response( .submit(Op::ExecApproval { id: event_turn_id, decision, - allow_prefix: None, }) .await { diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index b9b36a0d8a..5cf0ecee70 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -71,7 +71,7 @@ pub(crate) async fn apply_patch( .await; match rx_approve.await.unwrap_or_default() { ReviewDecision::Approved - | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedAllowPrefix { .. } | ReviewDecision::ApprovedForSession => { InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec { action, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index d1345150fb..1352475c81 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1470,12 +1470,8 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv handlers::user_input_or_turn(&sess, sub.id.clone(), sub.op, &mut previous_context) .await; } - Op::ExecApproval { - id, - decision, - allow_prefix, - } => { - handlers::exec_approval(&sess, id, decision, allow_prefix).await; + Op::ExecApproval { id, decision } => { + handlers::exec_approval(&sess, id, decision).await; } Op::PatchApproval { id, decision } => { handlers::patch_approval(&sess, id, decision).await; @@ -1672,11 +1668,9 @@ mod handlers { sess: &Arc, id: String, decision: ReviewDecision, - allow_prefix: Option>, ) { - if let Some(prefix) = allow_prefix - && matches!(decision, ReviewDecision::ApprovedAllowPrefix) - && let Err(err) = sess.persist_command_allow_prefix(&prefix).await + if let ReviewDecision::ApprovedAllowPrefix { allow_prefix } = &decision + && let Err(err) = sess.persist_command_allow_prefix(allow_prefix).await { let message = format!("Failed to update execpolicy allow list: {err}"); tracing::warn!("{message}"); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index e4cf077785..0e1140febd 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -285,13 +285,7 @@ async fn handle_exec_approval( ) .await; - let _ = codex - .submit(Op::ExecApproval { - id, - decision, - allow_prefix: None, - }) - .await; + let _ = codex.submit(Op::ExecApproval { id, decision }).await; } /// Handle an ApplyPatchApprovalRequest by consulting the parent session and replying. diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 00416748e1..ac471a18cb 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -88,14 +88,14 @@ impl ToolOrchestrator { }; let decision = tool.start_approval_async(req, approval_ctx).await; - otel.tool_decision(otel_tn, otel_ci, decision, otel_user.clone()); + otel.tool_decision(otel_tn, otel_ci, decision.clone(), otel_user.clone()); match decision { ReviewDecision::Denied | ReviewDecision::Abort => { return Err(ToolError::Rejected("rejected by user".to_string())); } ReviewDecision::Approved - | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedAllowPrefix { .. } | ReviewDecision::ApprovedForSession => {} } already_approved = true; @@ -171,14 +171,14 @@ impl ToolOrchestrator { }; let decision = tool.start_approval_async(req, approval_ctx).await; - otel.tool_decision(otel_tn, otel_ci, decision, otel_user); + otel.tool_decision(otel_tn, otel_ci, decision.clone(), otel_user); match decision { ReviewDecision::Denied | ReviewDecision::Abort => { return Err(ToolError::Rejected("rejected by user".to_string())); } ReviewDecision::Approved - | ReviewDecision::ApprovedAllowPrefix + | ReviewDecision::ApprovedAllowPrefix { .. } | ReviewDecision::ApprovedForSession => {} } } diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index c0ddea7d0c..5bd0b8dc86 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -1523,8 +1523,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { test.codex .submit(Op::ExecApproval { id: "0".into(), - decision: *decision, - allow_prefix: None, + decision: decision.clone(), }) .await?; wait_for_completion(&test).await; @@ -1545,7 +1544,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { test.codex .submit(Op::PatchApproval { id: "0".into(), - decision: *decision, + decision: decision.clone(), }) .await?; wait_for_completion(&test).await; diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index 24d3a6e4cd..f5fe1a7df9 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -95,7 +95,6 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, - allow_prefix: None, }) .await .expect("submit exec approval"); diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index 045c42e941..8665d3a8ea 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -843,7 +843,6 @@ async fn handle_container_exec_user_approved_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, - allow_prefix: None, }) .await .unwrap(); @@ -902,7 +901,6 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::ApprovedForSession, - allow_prefix: None, }) .await .unwrap(); @@ -961,7 +959,6 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, - allow_prefix: None, }) .await .unwrap(); @@ -1020,7 +1017,6 @@ async fn handle_container_exec_user_denies_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Denied, - allow_prefix: None, }) .await .unwrap(); @@ -1079,7 +1075,6 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::ApprovedForSession, - allow_prefix: None, }) .await .unwrap(); @@ -1139,7 +1134,6 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Denied, - allow_prefix: None, }) .await .unwrap(); diff --git a/codex-rs/mcp-server/src/exec_approval.rs b/codex-rs/mcp-server/src/exec_approval.rs index e5b2e00f45..033523ac0d 100644 --- a/codex-rs/mcp-server/src/exec_approval.rs +++ b/codex-rs/mcp-server/src/exec_approval.rs @@ -150,7 +150,6 @@ async fn on_exec_approval_response( .submit(Op::ExecApproval { id: event_id, decision: response.decision, - allow_prefix: None, }) .await { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index f606f15fff..78cce24cd4 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -146,9 +146,6 @@ pub enum Op { id: String, /// The user's decision in response to the request. decision: ReviewDecision, - /// When set, persist this prefix to the execpolicy allow list. - #[serde(default, skip_serializing_if = "Option::is_none")] - allow_prefix: Option>, }, /// Approve a code patch @@ -1649,9 +1646,7 @@ pub struct SessionConfiguredEvent { } /// User's decision in response to an ExecApprovalRequest. -#[derive( - Debug, Default, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Display, JsonSchema, TS, -)] +#[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, Eq, Display, JsonSchema, TS)] #[serde(rename_all = "snake_case")] pub enum ReviewDecision { /// User has approved this command and the agent should execute it. @@ -1659,7 +1654,7 @@ pub enum ReviewDecision { /// User has approved this command and wants to add the command prefix to /// the execpolicy allow list so future matching commands are permitted. - ApprovedAllowPrefix, + ApprovedAllowPrefix { allow_prefix: Vec }, /// User has approved this command and wants to automatically approve any /// future identical instances (`command` and `cwd` match exactly) for the diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index afaa88fedc..f285d69d78 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -162,11 +162,17 @@ impl ApprovalOverlay { }; if let Some(variant) = self.current_variant.as_ref() { match (variant, &option.decision) { - (ApprovalVariant::Exec { id, command, .. }, ApprovalDecision::Review(decision)) => { - self.handle_exec_decision(id, command, *decision, option.allow_prefix.clone()); + ( + ApprovalVariant::Exec { id, command, .. }, + ApprovalDecision::Review(decision), + ) => { + self.handle_exec_decision(id, command, decision.clone()); } - (ApprovalVariant::ApplyPatch { id, .. }, ApprovalDecision::Review(decision)) => { - self.handle_patch_decision(id, *decision); + ( + ApprovalVariant::ApplyPatch { id, .. }, + ApprovalDecision::Review(decision), + ) => { + self.handle_patch_decision(id, decision.clone()); } ( ApprovalVariant::McpElicitation { @@ -185,19 +191,14 @@ impl ApprovalOverlay { self.advance_queue(); } - fn handle_exec_decision( - &self, - id: &str, - command: &[String], - decision: ReviewDecision, - allow_prefix: Option>, - ) { - let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision); + fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) { + let decision_for_history = decision.clone(); + let cell = + history_cell::new_approval_decision_cell(command.to_vec(), decision_for_history); self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval { id: id.to_string(), decision, - allow_prefix, })); } @@ -282,7 +283,7 @@ impl BottomPaneView for ApprovalOverlay { { match &variant { ApprovalVariant::Exec { id, command, .. } => { - self.handle_exec_decision(id, command, ReviewDecision::Abort, None); + self.handle_exec_decision(id, command, ReviewDecision::Abort); } ApprovalVariant::ApplyPatch { id, .. } => { self.handle_patch_decision(id, ReviewDecision::Abort); @@ -467,7 +468,6 @@ struct ApprovalOption { decision: ApprovalDecision, display_shortcut: Option, additional_shortcuts: Vec, - allow_prefix: Option>, } impl ApprovalOption { @@ -485,30 +485,26 @@ fn exec_options(allow_prefix: Option>) -> Vec { decision: ApprovalDecision::Review(ReviewDecision::Approved), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - allow_prefix: None, }, ApprovalOption { label: "Yes, and don't ask again for this command".to_string(), decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], - allow_prefix: None, }, ] .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::ApprovedAllowPrefix), + decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix { allow_prefix: prefix }), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - allow_prefix: Some(prefix), })) .chain([ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Abort), display_shortcut: Some(key_hint::plain(KeyCode::Esc)), additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - allow_prefix: None, }]) .collect() } @@ -520,14 +516,12 @@ fn patch_options() -> Vec { decision: ApprovalDecision::Review(ReviewDecision::Approved), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - allow_prefix: None, }, ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Abort), display_shortcut: Some(key_hint::plain(KeyCode::Esc)), additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - allow_prefix: None, }, ] } @@ -539,21 +533,18 @@ fn elicitation_options() -> Vec { decision: ApprovalDecision::McpElicitation(ElicitationAction::Accept), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - allow_prefix: None, }, ApprovalOption { label: "No, but continue without it".to_string(), decision: ApprovalDecision::McpElicitation(ElicitationAction::Decline), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - allow_prefix: None, }, ApprovalOption { label: "Cancel this request".to_string(), decision: ApprovalDecision::McpElicitation(ElicitationAction::Cancel), display_shortcut: Some(key_hint::plain(KeyCode::Esc)), additional_shortcuts: vec![key_hint::plain(KeyCode::Char('c'))], - allow_prefix: None, }, ] } @@ -621,14 +612,13 @@ mod tests { view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE)); let mut saw_op = false; while let Ok(ev) = rx.try_recv() { - if let AppEvent::CodexOp(Op::ExecApproval { - allow_prefix, - decision, - .. - }) = ev - { - assert_eq!(decision, ReviewDecision::ApprovedAllowPrefix); - assert_eq!(allow_prefix, Some(vec!["echo".to_string()])); + if let AppEvent::CodexOp(Op::ExecApproval { decision, .. }) = ev { + assert_eq!( + decision, + ReviewDecision::ApprovedAllowPrefix { + allow_prefix: vec!["echo".to_string()] + } + ); saw_op = true; break; } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 96bea11460..eac163b8fb 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -408,7 +408,7 @@ pub fn new_approval_decision_cell( ], ) } - ApprovedAllowPrefix => { + ApprovedAllowPrefix { .. } => { let snippet = Span::from(exec_snippet(&command)).dim(); ( "✔ ".green(),