diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 1142612ae3..93de2dc5d2 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -700,9 +700,9 @@ async fn maybe_auto_review_mcp_request_user_input( ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(), - ReviewDecision::Denied | ReviewDecision::Abort => { - MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string() - } + ReviewDecision::ApprovedOverrideCommand { .. } + | ReviewDecision::Denied + | ReviewDecision::Abort => MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string(), }; Some(RequestUserInputResponse { answers: HashMap::from([( diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 9900fea43e..9246ebc377 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -687,11 +687,12 @@ pub(crate) fn build_guardian_mcp_tool_review_request( fn mcp_tool_approval_decision_from_guardian(decision: ReviewDecision) -> McpToolApprovalDecision { match decision { ReviewDecision::Approved - | ReviewDecision::ApprovedOverrideCommand { .. } | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => McpToolApprovalDecision::Accept, ReviewDecision::ApprovedForSession => McpToolApprovalDecision::AcceptForSession, - ReviewDecision::Denied | ReviewDecision::Abort => McpToolApprovalDecision::Decline, + ReviewDecision::ApprovedOverrideCommand { .. } + | ReviewDecision::Denied + | ReviewDecision::Abort => McpToolApprovalDecision::Decline, } } diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 62292b43f3..c808cda66f 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -602,6 +602,12 @@ fn guardian_review_decision_maps_to_mcp_tool_decision() { mcp_tool_approval_decision_from_guardian(ReviewDecision::Approved), McpToolApprovalDecision::Accept ); + assert_eq!( + mcp_tool_approval_decision_from_guardian(ReviewDecision::ApprovedOverrideCommand { + command: vec!["echo".to_string(), "override".to_string()], + }), + McpToolApprovalDecision::Decline + ); assert_eq!( mcp_tool_approval_decision_from_guardian(ReviewDecision::Denied), McpToolApprovalDecision::Decline diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index b47922ed65..52c3851fcf 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -123,10 +123,11 @@ fn pending_decision_for_network_review( review_decision: &ReviewDecision, ) -> Option { match review_decision { - ReviewDecision::Approved => Some(PendingApprovalDecision::AllowOnce), + ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + Some(PendingApprovalDecision::AllowOnce) + } ReviewDecision::ApprovedForSession => Some(PendingApprovalDecision::AllowForSession), ReviewDecision::ApprovedOverrideCommand { .. } - | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::Denied | ReviewDecision::Abort => Some(PendingApprovalDecision::Deny), ReviewDecision::NetworkPolicyAmendment { .. } => None, diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index e633013a08..21f8505c8f 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -178,7 +178,7 @@ fn only_never_policy_disables_network_approval_flow() { } #[test] -fn network_review_rejects_command_and_execpolicy_overrides() { +fn network_review_rejects_command_override_but_allows_execpolicy_amendment() { assert_eq!( pending_decision_for_network_review(&ReviewDecision::ApprovedOverrideCommand { command: vec!["echo".to_string(), "override".to_string()], @@ -192,7 +192,7 @@ fn network_review_rejects_command_and_execpolicy_overrides() { "override".to_string(), ]), }), - Some(PendingApprovalDecision::Deny) + Some(PendingApprovalDecision::AllowOnce) ); } @@ -200,7 +200,9 @@ fn network_review_rejects_command_and_execpolicy_overrides() { async fn inline_network_review_rejects_command_override_at_runtime() { let service = Arc::new(NetworkApprovalService::default()); let (session, turn_context, rx) = make_session_and_context_with_rx().await; - service.register_call("registration-1".to_string()).await; + service + .register_call("registration-1".to_string(), "turn-1".to_string()) + .await; session .spawn_task( Arc::clone(&turn_context),