rm struct

codex-rs/core/src/approval_coordinator.rs

@@ -1,225 +0,0 @@
-//! Central approval policy-stage execution and reviewer routing.
-
-use std::sync::Arc;
-
-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::hook_runtime::run_permission_request_hooks;
-use crate::session::session::Session;
-use crate::session::turn_context::TurnContext;
-use crate::tools::flat_tool_name;
-use crate::tools::sandboxing::ApprovalCtx;
-use crate::tools::sandboxing::ToolCtx;
-use crate::tools::sandboxing::ToolError;
-use crate::tools::sandboxing::ToolRuntime;
-use codex_config::types::ApprovalsReviewer;
-use codex_hooks::PermissionRequestDecision;
-use codex_otel::ToolDecisionSource;
-use codex_protocol::protocol::AskForApproval;
-use codex_protocol::protocol::NetworkPolicyRuleAction;
-use codex_protocol::protocol::ReviewDecision;
-
-pub(crate) type ApprovalAction = crate::guardian::GuardianApprovalRequest;
-
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub(crate) enum ApprovalReviewer {
-    Guardian,
-    User,
-}
-
-impl ApprovalReviewer {
-    pub(crate) fn for_turn(turn: &TurnContext) -> Self {
-        Self::for_reviewer(turn, turn.config.approvals_reviewer)
-    }
-
-    pub(crate) fn for_reviewer(turn: &TurnContext, reviewer: ApprovalsReviewer) -> Self {
-        if Self::routes_to_guardian(turn, reviewer) {
-            Self::Guardian
-        } else {
-            Self::User
-        }
-    }
-
-    fn routes_to_guardian(turn: &TurnContext, reviewer: ApprovalsReviewer) -> bool {
-        matches!(
-            turn.approval_policy.value(),
-            AskForApproval::OnRequest | AskForApproval::Granular(_)
-        ) && reviewer == ApprovalsReviewer::AutoReview
-    }
-}
-
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub(crate) enum ApprovalResolutionSource {
-    Hook,
-    Guardian,
-    User,
-}
-
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub(crate) struct ApprovalResolution {
-    pub(crate) decision: ReviewDecision,
-    pub(crate) rejection: Option<String>,
-    pub(crate) source: ApprovalResolutionSource,
-}
-
-impl ApprovalResolution {
-    pub(crate) fn into_tool_result(self) -> Result<ReviewDecision, ToolError> {
-        if let Some(rejection) = self.rejection {
-            Err(ToolError::Rejected(rejection))
-        } else {
-            Ok(self.decision)
-        }
-    }
-}
-
-pub(crate) struct ApprovalCoordinator;
-
-impl ApprovalCoordinator {
-    pub(crate) async fn resolve_tool<Rq, Out, T>(
-        tool: &mut T,
-        req: &Rq,
-        permission_request_run_id: &str,
-        ctx: ApprovalCtx<'_>,
-        tool_ctx: &ToolCtx,
-        reviewer: ApprovalReviewer,
-        otel: &codex_otel::SessionTelemetry,
-    ) -> Result<ApprovalResolution, ToolError>
-    where
-        T: ToolRuntime<Rq, Out>,
-    {
-        if let Some(permission_request) = tool.permission_request_payload(req) {
-            match run_permission_request_hooks(
-                ctx.session,
-                ctx.turn,
-                permission_request_run_id,
-                permission_request,
-            )
-            .await
-            {
-                Some(PermissionRequestDecision::Allow) => {
-                    let resolution = ApprovalResolution {
-                        decision: ReviewDecision::Approved,
-                        rejection: None,
-                        source: ApprovalResolutionSource::Hook,
-                    };
-                    Self::record_resolution(otel, tool_ctx, &resolution);
-                    return Ok(resolution);
-                }
-                Some(PermissionRequestDecision::Deny { message }) => {
-                    let resolution = ApprovalResolution {
-                        decision: ReviewDecision::Denied,
-                        rejection: Some(message),
-                        source: ApprovalResolutionSource::Hook,
-                    };
-                    Self::record_resolution(otel, tool_ctx, &resolution);
-                    return Ok(resolution);
-                }
-                None => {}
-            }
-        }
-
-        let resolution = match reviewer {
-            ApprovalReviewer::Guardian => {
-                let review_id = new_guardian_review_id();
-                let action = match tool.approval_action(req, &ctx) {
-                    Ok(action) => action,
-                    Err(err) => {
-                        tracing::error!(%err, "failed to build automatic approval action");
-                        let resolution = ApprovalResolution {
-                            decision: ReviewDecision::Abort,
-                            rejection: Some(
-                                "automatic approval review could not prepare the action"
-                                    .to_string(),
-                            ),
-                            source: ApprovalResolutionSource::Guardian,
-                        };
-                        Self::record_resolution(otel, tool_ctx, &resolution);
-                        return Ok(resolution);
-                    }
-                };
-                let decision = review_approval_request(
-                    ctx.session,
-                    ctx.turn,
-                    review_id.clone(),
-                    action,
-                    ctx.retry_reason.clone(),
-                )
-                .await;
-                Self::normalize_guardian(ctx.session, review_id, decision).await
-            }
-            ApprovalReviewer::User => ApprovalResolution {
-                decision: tool.start_approval_async(req, ctx.clone()).await,
-                rejection: None,
-                source: ApprovalResolutionSource::User,
-            },
-        };
-        let resolution = Self::normalize_user_rejection(resolution);
-        Self::record_resolution(otel, tool_ctx, &resolution);
-        Ok(resolution)
-    }
-
-    async fn normalize_guardian(
-        session: &Arc<Session>,
-        review_id: String,
-        decision: ReviewDecision,
-    ) -> ApprovalResolution {
-        let rejection = match &decision {
-            ReviewDecision::Approved
-            | ReviewDecision::ApprovedForSession
-            | ReviewDecision::ApprovedExecpolicyAmendment { .. } => None,
-            ReviewDecision::NetworkPolicyAmendment {
-                network_policy_amendment,
-            } if network_policy_amendment.action == NetworkPolicyRuleAction::Allow => None,
-            ReviewDecision::TimedOut => Some(guardian_timeout_message()),
-            ReviewDecision::NetworkPolicyAmendment { .. }
-            | ReviewDecision::Denied
-            | ReviewDecision::Abort => {
-                Some(guardian_rejection_message(session.as_ref(), &review_id).await)
-            }
-        };
-        ApprovalResolution {
-            decision,
-            rejection,
-            source: ApprovalResolutionSource::Guardian,
-        }
-    }
-
-    fn normalize_user_rejection(mut resolution: ApprovalResolution) -> ApprovalResolution {
-        if resolution.source == ApprovalResolutionSource::User {
-            resolution.rejection = match &resolution.decision {
-                ReviewDecision::Approved
-                | ReviewDecision::ApprovedForSession
-                | ReviewDecision::ApprovedExecpolicyAmendment { .. } => None,
-                ReviewDecision::NetworkPolicyAmendment {
-                    network_policy_amendment,
-                } if network_policy_amendment.action == NetworkPolicyRuleAction::Allow => None,
-                ReviewDecision::NetworkPolicyAmendment { .. }
-                | ReviewDecision::Denied
-                | ReviewDecision::Abort => Some("rejected by user".to_string()),
-                ReviewDecision::TimedOut => Some("approval request timed out".to_string()),
-            };
-        }
-        resolution
-    }
-
-    fn record_resolution(
-        otel: &codex_otel::SessionTelemetry,
-        tool_ctx: &ToolCtx,
-        resolution: &ApprovalResolution,
-    ) {
-        let source = match resolution.source {
-            ApprovalResolutionSource::Hook => ToolDecisionSource::Config,
-            ApprovalResolutionSource::Guardian => ToolDecisionSource::AutomatedReviewer,
-            ApprovalResolutionSource::User => ToolDecisionSource::User,
-        };
-        let tool_name = flat_tool_name(&tool_ctx.tool_name);
-        otel.tool_decision(
-            tool_name.as_ref(),
-            &tool_ctx.call_id,
-            &resolution.decision,
-            source,
-        );
-    }
-}

codex-rs/core/src/lib.rs

@@ -6,7 +6,6 @@
 #![deny(clippy::print_stdout, clippy::print_stderr)]
 
 mod apply_patch;
-mod approval_coordinator;
 mod apps;
 mod client;
 mod client_common;

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

@@ -0,0 +1,220 @@
+//! Central approval policy-stage execution and reviewer routing.
+
+use std::sync::Arc;
+
+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::hook_runtime::run_permission_request_hooks;
+use crate::session::session::Session;
+use crate::session::turn_context::TurnContext;
+use crate::tools::flat_tool_name;
+use crate::tools::sandboxing::ApprovalCtx;
+use crate::tools::sandboxing::ToolCtx;
+use crate::tools::sandboxing::ToolError;
+use crate::tools::sandboxing::ToolRuntime;
+use codex_config::types::ApprovalsReviewer;
+use codex_hooks::PermissionRequestDecision;
+use codex_otel::ToolDecisionSource;
+use codex_protocol::protocol::AskForApproval;
+use codex_protocol::protocol::NetworkPolicyRuleAction;
+use codex_protocol::protocol::ReviewDecision;
+
+pub(super) type ApprovalAction = crate::guardian::GuardianApprovalRequest;
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(super) enum ApprovalReviewer {
+    Guardian,
+    User,
+}
+
+impl ApprovalReviewer {
+    pub(super) fn for_turn(turn: &TurnContext) -> Self {
+        Self::for_reviewer(turn, turn.config.approvals_reviewer)
+    }
+
+    fn for_reviewer(turn: &TurnContext, reviewer: ApprovalsReviewer) -> Self {
+        if Self::routes_to_guardian(turn, reviewer) {
+            Self::Guardian
+        } else {
+            Self::User
+        }
+    }
+
+    fn routes_to_guardian(turn: &TurnContext, reviewer: ApprovalsReviewer) -> bool {
+        matches!(
+            turn.approval_policy.value(),
+            AskForApproval::OnRequest | AskForApproval::Granular(_)
+        ) && reviewer == ApprovalsReviewer::AutoReview
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+enum ApprovalResolutionSource {
+    Hook,
+    Guardian,
+    User,
+}
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+struct ApprovalResolution {
+    decision: ReviewDecision,
+    rejection: Option<String>,
+    source: ApprovalResolutionSource,
+}
+
+impl ApprovalResolution {
+    fn into_tool_result(self) -> Result<ReviewDecision, ToolError> {
+        if let Some(rejection) = self.rejection {
+            Err(ToolError::Rejected(rejection))
+        } else {
+            Ok(self.decision)
+        }
+    }
+}
+
+pub(super) async fn resolve_tool_apporval<Rq, Out, T>(
+    tool: &mut T,
+    req: &Rq,
+    permission_request_run_id: &str,
+    ctx: ApprovalCtx<'_>,
+    tool_ctx: &ToolCtx,
+    reviewer: ApprovalReviewer,
+    otel: &codex_otel::SessionTelemetry,
+) -> Result<ReviewDecision, ToolError>
+where
+    T: ToolRuntime<Rq, Out>,
+{
+    if let Some(permission_request) = tool.permission_request_payload(req) {
+        match run_permission_request_hooks(
+            ctx.session,
+            ctx.turn,
+            permission_request_run_id,
+            permission_request,
+        )
+        .await
+        {
+            Some(PermissionRequestDecision::Allow) => {
+                let resolution = ApprovalResolution {
+                    decision: ReviewDecision::Approved,
+                    rejection: None,
+                    source: ApprovalResolutionSource::Hook,
+                };
+                record_resolution(otel, tool_ctx, &resolution);
+                return resolution.into_tool_result();
+            }
+            Some(PermissionRequestDecision::Deny { message }) => {
+                let resolution = ApprovalResolution {
+                    decision: ReviewDecision::Denied,
+                    rejection: Some(message),
+                    source: ApprovalResolutionSource::Hook,
+                };
+                record_resolution(otel, tool_ctx, &resolution);
+                return resolution.into_tool_result();
+            }
+            None => {}
+        }
+    }
+
+    let resolution = match reviewer {
+        ApprovalReviewer::Guardian => {
+            let review_id = new_guardian_review_id();
+            let action = match tool.approval_action(req, &ctx) {
+                Ok(action) => action,
+                Err(err) => {
+                    tracing::error!(%err, "failed to build automatic approval action");
+                    let resolution = ApprovalResolution {
+                        decision: ReviewDecision::Abort,
+                        rejection: Some(
+                            "automatic approval review could not prepare the action".to_string(),
+                        ),
+                        source: ApprovalResolutionSource::Guardian,
+                    };
+                    record_resolution(otel, tool_ctx, &resolution);
+                    return resolution.into_tool_result();
+                }
+            };
+            let decision = review_approval_request(
+                ctx.session,
+                ctx.turn,
+                review_id.clone(),
+                action,
+                ctx.retry_reason.clone(),
+            )
+            .await;
+            normalize_guardian(ctx.session, review_id, decision).await
+        }
+        ApprovalReviewer::User => ApprovalResolution {
+            decision: tool.start_approval_async(req, ctx.clone()).await,
+            rejection: None,
+            source: ApprovalResolutionSource::User,
+        },
+    };
+    let resolution = normalize_user_rejection(resolution);
+    record_resolution(otel, tool_ctx, &resolution);
+    resolution.into_tool_result()
+}
+
+async fn normalize_guardian(
+    session: &Arc<Session>,
+    review_id: String,
+    decision: ReviewDecision,
+) -> ApprovalResolution {
+    let rejection = match &decision {
+        ReviewDecision::Approved
+        | ReviewDecision::ApprovedForSession
+        | ReviewDecision::ApprovedExecpolicyAmendment { .. } => None,
+        ReviewDecision::NetworkPolicyAmendment {
+            network_policy_amendment,
+        } if network_policy_amendment.action == NetworkPolicyRuleAction::Allow => None,
+        ReviewDecision::TimedOut => Some(guardian_timeout_message()),
+        ReviewDecision::NetworkPolicyAmendment { .. }
+        | ReviewDecision::Denied
+        | ReviewDecision::Abort => {
+            Some(guardian_rejection_message(session.as_ref(), &review_id).await)
+        }
+    };
+    ApprovalResolution {
+        decision,
+        rejection,
+        source: ApprovalResolutionSource::Guardian,
+    }
+}
+
+fn normalize_user_rejection(mut resolution: ApprovalResolution) -> ApprovalResolution {
+    if resolution.source == ApprovalResolutionSource::User {
+        resolution.rejection = match &resolution.decision {
+            ReviewDecision::Approved
+            | ReviewDecision::ApprovedForSession
+            | ReviewDecision::ApprovedExecpolicyAmendment { .. } => None,
+            ReviewDecision::NetworkPolicyAmendment {
+                network_policy_amendment,
+            } if network_policy_amendment.action == NetworkPolicyRuleAction::Allow => None,
+            ReviewDecision::NetworkPolicyAmendment { .. }
+            | ReviewDecision::Denied
+            | ReviewDecision::Abort => Some("rejected by user".to_string()),
+            ReviewDecision::TimedOut => Some("approval request timed out".to_string()),
+        };
+    }
+    resolution
+}
+
+fn record_resolution(
+    otel: &codex_otel::SessionTelemetry,
+    tool_ctx: &ToolCtx,
+    resolution: &ApprovalResolution,
+) {
+    let source = match resolution.source {
+        ApprovalResolutionSource::Hook => ToolDecisionSource::Config,
+        ApprovalResolutionSource::Guardian => ToolDecisionSource::AutomatedReviewer,
+        ApprovalResolutionSource::User => ToolDecisionSource::User,
+    };
+    let tool_name = flat_tool_name(&tool_ctx.tool_name);
+    otel.tool_decision(
+        tool_name.as_ref(),
+        &tool_ctx.call_id,
+        &resolution.decision,
+        source,
+    );
+}

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

@@ -1,3 +1,4 @@
+mod approvals;
 pub(crate) mod code_mode;
 pub(crate) mod context;
 pub(crate) mod events;

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

@@ -6,8 +6,8 @@ simple sequence for any ToolRuntime: approval → select sandbox → attempt →
 retry with an escalated sandbox strategy on denial (no re‑approval thanks to
 caching).
 */
-use crate::approval_coordinator::ApprovalCoordinator;
-use crate::approval_coordinator::ApprovalReviewer;
+use super::approvals::ApprovalReviewer;
+use super::approvals::resolve_tool_apporval;
 use crate::network_policy_decision::network_approval_context_from_payload;
 use crate::tools::flat_tool_name;
 use crate::tools::network_approval::ActiveNetworkApproval;
@@ -519,7 +519,7 @@ impl ToolOrchestrator {
     where
         T: ToolRuntime<Rq, Out>,
     {
-        ApprovalCoordinator::resolve_tool(
+        resolve_tool_apporval(
             tool,
             req,
             permission_request_run_id,
@@ -528,8 +528,7 @@ impl ToolOrchestrator {
             reviewer,
             otel,
         )
-        .await?
-        .into_tool_result()
+        .await
     }
 }
 

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

@@ -126,7 +126,7 @@ pub(crate) struct ApprovalCtx<'a> {
     pub network_approval_context: Option<NetworkApprovalContext>,
 }
 
-pub(crate) type ApprovalAction = crate::approval_coordinator::ApprovalAction;
+pub(crate) type ApprovalAction = super::approvals::ApprovalAction;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) struct PermissionRequestPayload {