core: dispatch MCP tool approval reviews through extensions

codex-rs/core/src/mcp_tool_call.rs

@@ -9,10 +9,7 @@ use crate::config::edit::ConfigEditsBuilder;
 use crate::connectors;
 use crate::guardian::GuardianApprovalRequest;
 use crate::guardian::GuardianMcpAnnotations;
-use crate::guardian::guardian_rejection_message;
-use crate::guardian::guardian_timeout_message;
 use crate::guardian::new_guardian_review_id;
-use crate::guardian::review_approval_request;
 use crate::guardian::routes_approval_to_guardian_with_reviewer;
 use crate::hook_runtime::run_permission_request_hooks;
 use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files;
@@ -20,6 +17,7 @@ use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam;
 use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template;
 use crate::session::session::Session;
 use crate::session::turn_context::TurnContext;
+use crate::tools::approval_dispatch::request_automated_approval;
 use crate::tools::hook_names::HookToolName;
 use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::turn_metadata::McpTurnMetadataContext;
@@ -33,6 +31,7 @@ use codex_app_server_protocol::McpServerElicitationRequest;
 use codex_app_server_protocol::McpServerElicitationRequestParams;
 use codex_config::types::AppToolApproval;
 use codex_config::types::ApprovalsReviewer;
+use codex_extension_api::ApprovalReviewSource;
 use codex_features::Feature;
 use codex_hooks::PermissionRequestDecision;
 use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
@@ -1207,16 +1206,22 @@ async fn maybe_request_mcp_tool_approval(
         .enabled(Feature::ToolCallMcpElicitation);
 
     if routes_approval_to_guardian_with_reviewer(turn_context, approvals_reviewer) {
-        let review_id = new_guardian_review_id();
-        let decision = review_approval_request(
+        let automated = request_automated_approval(
             sess,
             turn_context,
-            review_id.clone(),
+            new_guardian_review_id(),
             build_guardian_mcp_tool_review_request(call_id, invocation, metadata),
+            approvals_reviewer,
             /*retry_reason*/ None,
+            ApprovalReviewSource::MainTurn,
         )
         .await;
-        let decision = mcp_tool_approval_decision_from_guardian(sess, &review_id, decision).await;
+        let decision = match automated {
+            Ok(automated) => mcp_tool_approval_decision_from_automated(&automated),
+            Err(message) => McpToolApprovalDecision::Decline {
+                message: Some(message),
+            },
+        };
         apply_mcp_tool_approval_decision(
             sess,
             turn_context,
@@ -1382,21 +1387,19 @@ pub(crate) fn build_guardian_mcp_tool_review_request(
     }
 }
 
-async fn mcp_tool_approval_decision_from_guardian(
-    sess: &Session,
-    review_id: &str,
-    decision: ReviewDecision,
+fn mcp_tool_approval_decision_from_automated(
+    automated: &crate::tools::approval_dispatch::AutomatedApprovalDecision,
 ) -> McpToolApprovalDecision {
-    match decision {
+    match automated.decision.clone() {
         ReviewDecision::Approved
         | ReviewDecision::ApprovedExecpolicyAmendment { .. }
         | ReviewDecision::NetworkPolicyAmendment { .. } => McpToolApprovalDecision::Accept,
         ReviewDecision::ApprovedForSession => McpToolApprovalDecision::AcceptForSession,
         ReviewDecision::Denied => McpToolApprovalDecision::Decline {
-            message: Some(guardian_rejection_message(sess, review_id).await),
+            message: Some(automated.denial_message()),
         },
         ReviewDecision::TimedOut => McpToolApprovalDecision::Decline {
-            message: Some(guardian_timeout_message()),
+            message: Some(automated.denial_message()),
         },
         ReviewDecision::Abort => McpToolApprovalDecision::Decline { message: None },
     }

codex-rs/core/src/mcp_tool_call_tests.rs

@@ -5,6 +5,8 @@ use crate::session::tests::make_session_and_context;
 use crate::session::tests::make_session_and_context_with_rx;
 use crate::state::ActiveTurn;
 use crate::test_support::models_manager_with_provider;
+use crate::tools::approval_dispatch::AutomatedApprovalDecision;
+use crate::tools::approval_dispatch::AutomatedApprovalSource;
 use crate::tools::hook_names::HookToolName;
 use crate::turn_metadata::McpTurnMetadataContext;
 use codex_config::CONFIG_TOML_FILE;
@@ -16,6 +18,12 @@ use codex_config::types::ApprovalsReviewer;
 use codex_config::types::AppsConfigToml;
 use codex_config::types::McpServerConfig;
 use codex_config::types::McpServerToolConfig;
+use codex_extension_api::ApprovalReviewContributor;
+use codex_extension_api::ApprovalReviewError;
+use codex_extension_api::ApprovalReviewInput;
+use codex_extension_api::ApprovalReviewOutcome;
+use codex_extension_api::ExtensionFuture;
+use codex_extension_api::ExtensionRegistryBuilder;
 use codex_features::Features;
 use codex_hooks::Hooks;
 use codex_hooks::HooksConfig;
@@ -52,6 +60,21 @@ use tracing::Level;
 use tracing_subscriber::fmt::format::FmtSpan;
 use tracing_test::internal::MockWriter;
 
+struct DenyingMcpApprovalReviewContributor {
+    rationale: String,
+    seen_reviewer: Arc<std::sync::Mutex<Option<ApprovalsReviewer>>>,
+}
+
+impl ApprovalReviewContributor for DenyingMcpApprovalReviewContributor {
+    fn review<'a>(
+        &'a self,
+        input: ApprovalReviewInput<'a>,
+    ) -> ExtensionFuture<'a, Result<ApprovalReviewOutcome, ApprovalReviewError>> {
+        *self.seen_reviewer.lock().expect("reviewer lock") = Some(input.reviewer);
+        Box::pin(async move { Ok(ApprovalReviewOutcome::denied(self.rationale.clone())) })
+    }
+}
+
 fn annotations(
     read_only: Option<bool>,
     destructive: Option<bool>,
@@ -1654,62 +1677,42 @@ fn guardian_mcp_review_request_includes_annotations_when_present() {
     );
 }
 
-#[tokio::test(flavor = "current_thread")]
-async fn guardian_review_decision_maps_to_mcp_tool_decision() {
-    let (session, _) = make_session_and_context().await;
-    let session = Arc::new(session);
-
+#[test]
+fn automated_review_decision_maps_to_mcp_tool_decision() {
     assert_eq!(
-        mcp_tool_approval_decision_from_guardian(
-            session.as_ref(),
-            "review-id",
-            ReviewDecision::Approved
-        )
-        .await,
+        mcp_tool_approval_decision_from_automated(&AutomatedApprovalDecision {
+            decision: ReviewDecision::Approved,
+            denial_message: None,
+            source: AutomatedApprovalSource::Guardian,
+        }),
         McpToolApprovalDecision::Accept
     );
-    session.services.guardian_rejections.lock().await.insert(
-        "review-id".to_string(),
-        crate::guardian::GuardianRejection {
-            rationale: "too risky".to_string(),
-            source: codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
-        },
-    );
-    let denial = mcp_tool_approval_decision_from_guardian(
-        session.as_ref(),
-        "review-id",
-        ReviewDecision::Denied,
-    )
-    .await;
-    let McpToolApprovalDecision::Decline {
-        message: Some(message),
-    } = denial
-    else {
-        panic!("guardian denial should carry a rejection message");
-    };
-    assert!(message.contains("Reason: too risky"));
-    assert!(message.contains("The agent must not attempt to achieve the same outcome"));
-    let timeout = mcp_tool_approval_decision_from_guardian(
-        session.as_ref(),
-        "review-id",
-        ReviewDecision::TimedOut,
-    )
-    .await;
-    let McpToolApprovalDecision::Decline {
-        message: Some(message),
-    } = timeout
-    else {
-        panic!("guardian timeout should carry a timeout message");
-    };
-    assert!(message.contains("did not finish before its deadline"));
-    assert!(!message.contains("unacceptable risk"));
     assert_eq!(
-        mcp_tool_approval_decision_from_guardian(
-            session.as_ref(),
-            "review-id",
-            ReviewDecision::Abort
-        )
-        .await,
+        mcp_tool_approval_decision_from_automated(&AutomatedApprovalDecision {
+            decision: ReviewDecision::Denied,
+            denial_message: Some("extension denied this MCP tool".to_string()),
+            source: AutomatedApprovalSource::Extension,
+        }),
+        McpToolApprovalDecision::Decline {
+            message: Some("extension denied this MCP tool".to_string()),
+        }
+    );
+    assert_eq!(
+        mcp_tool_approval_decision_from_automated(&AutomatedApprovalDecision {
+            decision: ReviewDecision::TimedOut,
+            denial_message: None,
+            source: AutomatedApprovalSource::Extension,
+        }),
+        McpToolApprovalDecision::Decline {
+            message: Some(crate::guardian::guardian_timeout_message()),
+        }
+    );
+    assert_eq!(
+        mcp_tool_approval_decision_from_automated(&AutomatedApprovalDecision {
+            decision: ReviewDecision::Abort,
+            denial_message: Some("cancelled".to_string()),
+            source: AutomatedApprovalSource::Guardian,
+        }),
         McpToolApprovalDecision::Decline { message: None }
     );
 }
@@ -2675,6 +2678,69 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
     );
 }
 
+#[tokio::test]
+async fn approval_review_extension_denies_mcp_tool_with_effective_reviewer() {
+    let (mut session, mut turn_context) = make_session_and_context().await;
+    turn_context
+        .approval_policy
+        .set(AskForApproval::OnRequest)
+        .expect("test setup should allow updating approval policy");
+    let mut config = (*turn_context.config).clone();
+    config.approvals_reviewer = ApprovalsReviewer::AutoReview;
+    turn_context.config = Arc::new(config);
+
+    let rationale = "the connector request exceeds the user's authorization";
+    let seen_reviewer = Arc::new(std::sync::Mutex::new(None));
+    let mut extensions = ExtensionRegistryBuilder::<crate::config::Config>::new();
+    extensions.approval_review_contributor(Arc::new(DenyingMcpApprovalReviewContributor {
+        rationale: rationale.to_string(),
+        seen_reviewer: Arc::clone(&seen_reviewer),
+    }));
+    session.services.extensions = Arc::new(extensions.build());
+
+    let session = Arc::new(session);
+    let turn_context = Arc::new(turn_context);
+    let invocation = McpInvocation {
+        server: "custom_server".to_string(),
+        tool: "dangerous_tool".to_string(),
+        arguments: Some(serde_json::json!({ "calendar_id": "primary" })),
+    };
+    let metadata = McpToolApprovalMetadata {
+        annotations: Some(annotations(Some(false), Some(true), Some(true))),
+        connector_id: None,
+        connector_name: None,
+        connector_description: None,
+        plugin_id: None,
+        tool_title: Some("Dangerous Tool".to_string()),
+        tool_description: Some("Reads calendar data.".to_string()),
+        mcp_app_resource_uri: None,
+        codex_apps_meta: None,
+        openai_file_input_params: None,
+    };
+
+    let decision = maybe_request_mcp_tool_approval(
+        &session,
+        &turn_context,
+        "call-extension-deny",
+        &invocation,
+        &HookToolName::new("mcp__test__tool"),
+        Some(&metadata),
+        AppToolApproval::Auto,
+    )
+    .await;
+
+    assert_eq!(
+        decision,
+        Some(McpToolApprovalDecision::Decline {
+            message: Some(rationale.to_string()),
+        })
+    );
+    assert_eq!(
+        *seen_reviewer.lock().expect("reviewer lock"),
+        Some(ApprovalsReviewer::AutoReview)
+    );
+}
+
 #[tokio::test]
 async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval() {
     let (session, turn_context, _rx_event) = make_session_and_context_with_rx().await;

codex-rs/core/src/tools/approval_dispatch.rs

@@ -37,9 +37,11 @@ pub(crate) struct AutomatedApprovalDecision {
 
 impl AutomatedApprovalDecision {
     pub(crate) fn denial_message(&self) -> String {
-        self.denial_message
-            .clone()
-            .unwrap_or_else(|| match self.source {
+        self.denial_message.clone().unwrap_or_else(|| {
+            if self.decision == ReviewDecision::TimedOut {
+                return guardian_timeout_message();
+            }
+            match self.source {
                 AutomatedApprovalSource::Extension => {
                     "automatic approval reviewer denied the action".to_string()
                 }
@@ -47,7 +49,8 @@ impl AutomatedApprovalDecision {
                     "automatic approval review failed".to_string()
                 }
                 AutomatedApprovalSource::Guardian => "Guardian denied this request.".to_string(),
-            })
+            }
+        })
     }
 }