From 0ae02915bd1bfdd8207631f65aa31987368c7c8b Mon Sep 17 00:00:00 2001 From: jif Date: Fri, 4 Sep 2026 17:23:16 +0000 Subject: [PATCH] Add request-scoped Guardian approval decisions (#42807) ## What changed - Extend `ApprovalReviewContributor` with a `decide` hook that can allow an action, return a synchronous review result, or hand the request to the user. - Pass contributors the request's approval state and a callback bound to the existing synchronous reviewer. Review results satisfy the review gate while the host continues to enforce permissions. - Add structured reasons for requesting a fresh Guardian review and select the first contributor decision in registration order. GitOrigin-RevId: ec84474b3fcbcf1027c8502aa1c2947c10149f9d --- codex-rs/core/src/guardian/mod.rs | 1 + codex-rs/core/src/guardian/review.rs | 30 ++++++++++++++ codex-rs/core/src/guardian/runtime.rs | 32 +++++++++++++++ .../ext/extension-api/src/contributors.rs | 11 ++++++ .../src/contributors/approval_review.rs | 39 ++++++++++++++++++- codex-rs/ext/extension-api/src/lib.rs | 3 ++ codex-rs/ext/extension-api/src/registry.rs | 13 +++++++ codex-rs/protocol/src/approvals.rs | 17 ++++++++ 8 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 codex-rs/core/src/guardian/runtime.rs diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index d3d40a7491..9adbe215d2 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -18,6 +18,7 @@ mod metrics; mod prompt; mod review; mod review_session; +mod runtime; use std::sync::Arc; use std::time::Duration; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index d1132afb22..09d4d01a52 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -405,6 +405,35 @@ async fn run_guardian_review( return decision; } + let runtime = super::runtime::ReviewRuntime { + session, + context, + review_id, + request, + reasons, + options, + }; + codex_extension_api::SynchronousApprovalReviewer::review( + &runtime, + codex_protocol::approvals::GuardianReviewReason::Policy, + ) + .await +} + +/// Runs the existing synchronous reviewer without choosing policy or reusing a score. +pub(super) async fn run_synchronous_review( + runtime: super::runtime::ReviewRuntime, + _review_reason: codex_protocol::approvals::GuardianReviewReason, +) -> ReviewDecision { + let super::runtime::ReviewRuntime { + session, + context, + review_id, + request, + reasons, + options, + } = runtime; + let turn = Arc::clone(context.turn()); let GuardianReviewOptions { plugin_attribution_override, approval_request_source, @@ -777,6 +806,7 @@ async fn run_guardian_review( } } +#[derive(Clone)] pub(crate) struct GuardianReviewOptions { pub(crate) plugin_attribution_override: Option, pub(crate) approval_request_source: GuardianApprovalRequestSource, diff --git a/codex-rs/core/src/guardian/runtime.rs b/codex-rs/core/src/guardian/runtime.rs new file mode 100644 index 0000000000..d4b353371e --- /dev/null +++ b/codex-rs/core/src/guardian/runtime.rs @@ -0,0 +1,32 @@ +//! Binds the existing synchronous reviewer to one captured approval action. + +use codex_extension_api::ExtensionFuture; +use codex_extension_api::SynchronousApprovalReviewer; +use codex_protocol::protocol::ReviewDecision; +use std::sync::Arc; + +use super::ApprovalRequestReasons; +use super::GuardianApprovalRequest; +use super::GuardianReviewContext; +use super::GuardianReviewOptions; +use super::review::run_synchronous_review; +use crate::session::session::Session; + +#[derive(Clone)] +pub(super) struct ReviewRuntime { + pub(super) session: Arc, + pub(super) context: GuardianReviewContext, + pub(super) review_id: String, + pub(super) request: GuardianApprovalRequest, + pub(super) reasons: ApprovalRequestReasons, + pub(super) options: GuardianReviewOptions, +} + +impl SynchronousApprovalReviewer for ReviewRuntime { + fn review( + &self, + reason: codex_protocol::approvals::GuardianReviewReason, + ) -> ExtensionFuture<'_, ReviewDecision> { + Box::pin(run_synchronous_review(self.clone(), reason)) + } +} diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index ebb180755e..0f57a7441e 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -24,9 +24,12 @@ mod turn_lifecycle; mod world_state; pub use approval_review::ApprovalAssessment; +pub use approval_review::ApprovalDecision; +pub use approval_review::ApprovalDecisionInput; pub use approval_review::ApprovalReviewError; pub use approval_review::ApprovalReviewInput; pub use approval_review::GuardianV2Enabled; +pub use approval_review::SynchronousApprovalReviewer; pub use context::TurnContextContributionInput; pub use mcp::McpServerContribution; pub use mcp::McpServerContributionContext; @@ -352,6 +355,14 @@ pub trait ToolLifecycleContributor: Send + Sync { /// a full structured review, or support both paths. Returning `None` leaves the /// request available to the next contributor or the host's fallback path. pub trait ApprovalReviewContributor: Send + Sync { + /// Claims one request, including a handoff to the user. + fn decide<'a>( + &'a self, + _input: &'a ApprovalDecisionInput<'_>, + ) -> ExtensionFuture<'a, Option> { + Box::pin(std::future::ready(None)) + } + /// Returns an available approval decision without performing a full review. fn fast_decision<'a>( &'a self, diff --git a/codex-rs/ext/extension-api/src/contributors/approval_review.rs b/codex-rs/ext/extension-api/src/contributors/approval_review.rs index 38693fac8e..a090962ec8 100644 --- a/codex-rs/ext/extension-api/src/contributors/approval_review.rs +++ b/codex-rs/ext/extension-api/src/contributors/approval_review.rs @@ -1,3 +1,5 @@ +//! Request-scoped approval decisions. A review only satisfies the review gate; the host enforces permissions. + use std::fmt; use std::sync::Arc; @@ -31,7 +33,7 @@ pub struct ApprovalAssessment { pub enum ApprovalReviewError { /// The reviewer could not produce a valid assessment. Failed(String), - /// The request exceeded its host-owned review deadline. + /// The request exceeded its synchronous review deadline. TimedOut, /// The request was cancelled by its parent approval. Cancelled, @@ -66,3 +68,38 @@ pub struct ApprovalReviewInput<'a> { /// Reason an earlier request is being retried, when applicable. pub retry_reason: Option<&'a str>, } + +/// Guardian's choice for one approval. Synchronous results pass through unchanged. +#[derive(Clone, Debug, PartialEq)] +pub enum ApprovalDecision { + /// Existing async evidence allows this action without synchronous review. + Allow, + Reviewed(codex_protocol::protocol::ReviewDecision), + AskUser, +} + +/// Runs the existing synchronous review for the bound action and cancellation token. +/// Implementations must not resolve policy or reuse an async score. +pub trait SynchronousApprovalReviewer: Send + Sync { + fn review( + &self, + reason: codex_protocol::approvals::GuardianReviewReason, + ) -> crate::ExtensionFuture<'_, codex_protocol::protocol::ReviewDecision>; +} + +/// Inputs to Guardian's policy choice. Conversation and scores stay thread-owned. +pub struct ApprovalDecisionInput<'a> { + pub approval_id: &'a str, + pub action: &'a serde_json::Value, + pub thread_id: ThreadId, + pub thread_store: &'a ExtensionData, + pub category: codex_protocol::openai_models::GuardianScope, + pub approval_policy: codex_protocol::protocol::AskForApproval, + pub approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, + pub require_guardian: bool, + /// Existing retry and sensitive-action rules require a synchronous review. + pub require_fresh_review: bool, + pub full_access: bool, + pub metrics: Option>, + pub synchronous_reviewer: &'a dyn SynchronousApprovalReviewer, +} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index a8ba8f1f20..9e8478679e 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -41,6 +41,8 @@ pub use codex_tools::TurnItemEmitter; pub use codex_tools::parse_tool_input_schema; pub use codex_tools::parse_tool_input_schema_without_compaction; pub use contributors::ApprovalAssessment; +pub use contributors::ApprovalDecision; +pub use contributors::ApprovalDecisionInput; pub use contributors::ApprovalReviewContributor; pub use contributors::ApprovalReviewError; pub use contributors::ApprovalReviewInput; @@ -63,6 +65,7 @@ pub use contributors::SelectedPluginSnapshot; pub use contributors::SkillInvocationContributor; pub use contributors::SkillInvocationInput; pub use contributors::SkillInvocationKind; +pub use contributors::SynchronousApprovalReviewer; pub use contributors::ThreadIdleCause; pub use contributors::ThreadIdleInput; pub use contributors::ThreadLifecycleContributor; diff --git a/codex-rs/ext/extension-api/src/registry.rs b/codex-rs/ext/extension-api/src/registry.rs index 8a8ae707ef..be706b5061 100644 --- a/codex-rs/ext/extension-api/src/registry.rs +++ b/codex-rs/ext/extension-api/src/registry.rs @@ -241,6 +241,19 @@ impl ExtensionRegistry { None } + /// Returns the first claimed decision in registration order. + pub async fn decide_approval( + &self, + input: &crate::ApprovalDecisionInput<'_>, + ) -> Option { + for contributor in &self.approval_review_contributors { + if let Some(decision) = contributor.decide(input).await { + return Some(decision); + } + } + None + } + /// Returns the registered prompt contributors. pub fn context_contributors(&self) -> &[Arc] { &self.context_contributors diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index c051430ec4..3fa3bdeaff 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -185,6 +185,23 @@ pub struct NetworkPolicyAmendment { pub action: NetworkPolicyRuleAction, } +/// Why this approval needs a fresh Guardian assessment. +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum GuardianReviewReason { + Policy, + FreshRequired, + MissingScore, + StaleScore, + InvalidScore, + IncompatibleCompaction, + ElevatedRisk, + ScoringFailure, + AuthorizationChanged, + #[serde(other)] + Unknown, +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)] pub struct GuardianAssessmentEvent { /// Stable identifier for this guardian review lifecycle.