openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-06 15:29:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

guardian: own approval review routing in the extension

Browse Source
This commit is contained in:
Dylan Hurd
2026-06-11 23:00:11 -07:00
parent f7cd9e8497
commit ac735a879a
6 changed files with 232 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

75
codex-rs/app-server/src/extensions.rs
View File

@@ -171,8 +171,21 @@ pub(crate) fn guardian_agent_spawner(
#[cfg(test)]
mod tests {
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;
use std::time::Duration;
use codex_extension_api::ApprovalReviewError;
use codex_extension_api::ApprovalReviewInput;
use codex_extension_api::ApprovalReviewOutcome;
use codex_extension_api::ApprovalReviewRunner;
use codex_extension_api::ApprovalReviewSource;
use codex_extension_api::ExtensionData;
use codex_extension_api::ExtensionFuture;
use codex_protocol::approvals::GuardianAssessmentAction;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::ThreadGoal as CoreThreadGoal;
use codex_protocol::protocol::ThreadGoalStatus;
use codex_protocol::protocol::ThreadGoalUpdatedEvent;
@@ -182,6 +195,68 @@ mod tests {
use super::*;
struct RecordingApprovalRunner(AtomicUsize);
impl ApprovalReviewRunner for RecordingApprovalRunner {
fn run(&self) -> ExtensionFuture<'_, Result<ApprovalReviewOutcome, ApprovalReviewError>> {
self.0.fetch_add(1, Ordering::Relaxed);
Box::pin(std::future::ready(Ok(ApprovalReviewOutcome::decision(
ReviewDecision::Approved,
))))
}
}
#[tokio::test]
async fn installed_guardian_claims_only_auto_review_requests() {
let mut builder = ExtensionRegistryBuilder::<Config>::new();
codex_guardian::install(&mut builder, ());
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new(ThreadId::default().to_string());
let turn_store = ExtensionData::new("turn");
let action = GuardianAssessmentAction::RequestPermissions {
reason: Some("run command".to_string()),
permissions: Default::default(),
};
let approval_policy = AskForApproval::OnRequest;
let runner = RecordingApprovalRunner(AtomicUsize::new(0));
let input = |reviewer, approval_policy| ApprovalReviewInput {
session_store: &session_store,
thread_store: &thread_store,
turn_store: &turn_store,
review_id: "review",
turn_id: "turn",
target_item_id: None,
prompt: "review command",
action: &action,
reviewer,
approval_policy,
retry_reason: None,
source: ApprovalReviewSource::MainTurn,
runner: &runner,
};
assert_eq!(
registry
.approval_review(input(ApprovalsReviewer::AutoReview, &approval_policy))
.await,
Ok(ApprovalReviewOutcome::decision(ReviewDecision::Approved))
);
assert_eq!(
registry
.approval_review(input(ApprovalsReviewer::User, &approval_policy))
.await,
Ok(ApprovalReviewOutcome::Abstain)
);
assert_eq!(
registry
.approval_review(input(ApprovalsReviewer::AutoReview, &AskForApproval::Never))
.await,
Ok(ApprovalReviewOutcome::Abstain)
);
assert_eq!(runner.0.load(Ordering::Relaxed), 1);
}
#[tokio::test]
async fn app_server_event_sink_uses_listener_fifo_for_goal_updates_and_clears() {
let (outgoing_tx, _outgoing_rx) = mpsc::channel(4);

134
codex-rs/core/src/tools/approval_dispatch.rs
View File

@@ -16,7 +16,9 @@ use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use codex_extension_api::ApprovalReviewInput;
use codex_extension_api::ApprovalReviewOutcome;
use codex_extension_api::ApprovalReviewRunner;
use codex_extension_api::ApprovalReviewSource;
use codex_extension_api::ExtensionFuture;
use codex_hooks::PermissionRequestDecision;
use codex_otel::ToolDecisionSource;
use codex_protocol::protocol::ReviewDecision;
@@ -55,6 +57,65 @@ impl AutomatedApprovalDecision {
}
}
struct CoreApprovalReviewRunner<'a> {
session: &'a std::sync::Arc<crate::session::session::Session>,
turn: &'a std::sync::Arc<crate::session::turn_context::TurnContext>,
review_id: &'a str,
request: &'a GuardianApprovalRequest,
retry_reason: Option<&'a str>,
source: ApprovalReviewSource,
cancellation_token: Option<&'a CancellationToken>,
}
impl ApprovalReviewRunner for CoreApprovalReviewRunner<'_> {
fn run(
&self,
) -> ExtensionFuture<'_, Result<ApprovalReviewOutcome, codex_extension_api::ApprovalReviewError>>
{
Box::pin(async move {
let decision = if let Some(cancellation_token) = self.cancellation_token {
let review_rx = crate::guardian::spawn_approval_request_review(
std::sync::Arc::clone(self.session),
std::sync::Arc::clone(self.turn),
self.review_id.to_string(),
self.request.clone(),
self.retry_reason.map(str::to_string),
match self.source {
ApprovalReviewSource::MainTurn => {
codex_analytics::GuardianApprovalRequestSource::MainTurn
}
ApprovalReviewSource::DelegatedSubagent => {
codex_analytics::GuardianApprovalRequestSource::DelegatedSubagent
}
},
cancellation_token.clone(),
);
review_rx.await.unwrap_or(ReviewDecision::Denied)
} else {
review_approval_request(
self.session,
self.turn,
self.review_id.to_string(),
self.request.clone(),
self.retry_reason.map(str::to_string),
)
.await
};
let denial_message = match decision {
ReviewDecision::Denied | ReviewDecision::Abort => {
Some(guardian_rejection_message(self.session, self.review_id).await)
}
ReviewDecision::TimedOut => Some(guardian_timeout_message()),
_ => None,
};
Ok(ApprovalReviewOutcome::Decision {
decision,
denial_message,
})
})
}
}
#[allow(clippy::too_many_arguments)]
pub(crate) async fn request_automated_approval(
session: &std::sync::Arc<crate::session::session::Session>,
@@ -70,6 +131,15 @@ pub(crate) async fn request_automated_approval(
.map_err(|error| format!("failed to render review prompt: {error}"))?
.text;
let approval_policy = turn.approval_policy.value();
let runner = CoreApprovalReviewRunner {
session,
turn,
review_id: &review_id,
request: &request,
retry_reason: retry_reason.as_deref(),
source,
cancellation_token: None,
};
let review_input = ApprovalReviewInput {
session_store: &session.services.session_extension_data,
thread_store: &session.services.thread_extension_data,
@@ -83,6 +153,7 @@ pub(crate) async fn request_automated_approval(
approval_policy: &approval_policy,
retry_reason: retry_reason.as_deref(),
source,
runner: &runner,
};
match session
.services
@@ -99,15 +170,12 @@ pub(crate) async fn request_automated_approval(
source: AutomatedApprovalSource::Extension,
}),
Ok(ApprovalReviewOutcome::Abstain) => {
let decision =
review_approval_request(session, turn, review_id.clone(), request, retry_reason)
.await;
let denial_message = match decision {
ReviewDecision::Denied | ReviewDecision::Abort => {
Some(guardian_rejection_message(session, &review_id).await)
}
ReviewDecision::TimedOut => Some(guardian_timeout_message()),
_ => None,
let ApprovalReviewOutcome::Decision {
decision,
denial_message,
} = runner.run().await.map_err(|error| error.to_string())?
else {
return Err("core approval reviewer unexpectedly abstained".to_string());
};
Ok(AutomatedApprovalDecision {
decision,
@@ -138,6 +206,15 @@ pub(crate) async fn request_automated_approval_with_cancel(
}
};
let approval_policy = turn.approval_policy.value();
let runner = CoreApprovalReviewRunner {
session,
turn,
review_id: &review_id,
request: &request,
retry_reason: retry_reason.as_deref(),
source,
cancellation_token: Some(&cancellation_token),
};
let review_input = ApprovalReviewInput {
session_store: &session.services.session_extension_data,
thread_store: &session.services.thread_extension_data,
@@ -151,6 +228,7 @@ pub(crate) async fn request_automated_approval_with_cancel(
approval_policy: &approval_policy,
retry_reason: retry_reason.as_deref(),
source,
runner: &runner,
};
let extension_outcome = tokio::select! {
@@ -168,33 +246,23 @@ pub(crate) async fn request_automated_approval_with_cancel(
source: AutomatedApprovalSource::Extension,
})),
Ok(ApprovalReviewOutcome::Abstain) => {
let review_rx = crate::guardian::spawn_approval_request_review(
std::sync::Arc::clone(session),
std::sync::Arc::clone(turn),
review_id.clone(),
request,
retry_reason,
match source {
ApprovalReviewSource::MainTurn => {
codex_analytics::GuardianApprovalRequestSource::MainTurn
}
ApprovalReviewSource::DelegatedSubagent => {
codex_analytics::GuardianApprovalRequestSource::DelegatedSubagent
}
},
cancellation_token.clone(),
);
let decision = tokio::select! {
let outcome = tokio::select! {
biased;
_ = cancellation_token.cancelled() => return None,
decision = review_rx => decision.unwrap_or(ReviewDecision::Denied),
outcome = runner.run() => outcome,
};
let denial_message = match decision {
ReviewDecision::Denied | ReviewDecision::Abort => {
Some(guardian_rejection_message(session, &review_id).await)
}
ReviewDecision::TimedOut => Some(guardian_timeout_message()),
_ => None,
let outcome = match outcome {
Ok(outcome) => outcome,
Err(error) => return Some(Err(error.to_string())),
};
let ApprovalReviewOutcome::Decision {
decision,
denial_message,
} = outcome
else {
return Some(Err(
"core approval reviewer unexpectedly abstained".to_string()
));
};
Some(Ok(AutomatedApprovalDecision {
decision,

11
codex-rs/ext/extension-api/src/approval_review.rs
View File

@@ -7,6 +7,16 @@ use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use crate::ExtensionData;
use crate::ExtensionFuture;
/// Request-scoped host capability that executes the configured approval review.
///
/// Contributors decide whether they own a request before invoking this runner.
/// The host retains responsibility for the underlying review runtime while the
/// contributor owns routing policy and the returned decision.
pub trait ApprovalReviewRunner: Send + Sync {
fn run(&self) -> ExtensionFuture<'_, Result<ApprovalReviewOutcome, ApprovalReviewError>>;
}
/// Identifies the runtime that originated an approval-review request.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -37,6 +47,7 @@ pub struct ApprovalReviewInput<'a> {
pub approval_policy: &'a AskForApproval,
pub retry_reason: Option<&'a str>,
pub source: ApprovalReviewSource,
pub runner: &'a dyn ApprovalReviewRunner,
}
/// Result of offering an approval request to one extension contributor.

1
codex-rs/ext/extension-api/src/lib.rs
View File

@@ -8,6 +8,7 @@ mod user_instructions;
pub use approval_review::ApprovalReviewError;
pub use approval_review::ApprovalReviewInput;
pub use approval_review::ApprovalReviewOutcome;
pub use approval_review::ApprovalReviewRunner;
pub use approval_review::ApprovalReviewSource;
pub use capabilities::AgentSpawnFuture;
pub use capabilities::AgentSpawner;

13
codex-rs/ext/extension-api/tests/registry.rs
View File

@@ -5,6 +5,7 @@ use codex_extension_api::ApprovalReviewContributor;
use codex_extension_api::ApprovalReviewError;
use codex_extension_api::ApprovalReviewInput;
use codex_extension_api::ApprovalReviewOutcome;
use codex_extension_api::ApprovalReviewRunner;
use codex_extension_api::ApprovalReviewSource;
use codex_extension_api::ConfigContributor;
use codex_extension_api::ContextContributor;
@@ -260,6 +261,16 @@ struct RecordingApprovalContributor {
calls: Arc<Mutex<Vec<ApprovalCall>>>,
}
struct UnusedApprovalRunner;
impl ApprovalReviewRunner for UnusedApprovalRunner {
fn run(&self) -> ExtensionFuture<'_, Result<ApprovalReviewOutcome, ApprovalReviewError>> {
Box::pin(std::future::ready(Err(ApprovalReviewError::new(
"test contributor should not invoke the runner",
))))
}
}
impl ApprovalReviewContributor for RecordingApprovalContributor {
fn review<'a>(
&'a self,
@@ -303,6 +314,7 @@ fn approval_review_input<'a>(
action: &'a GuardianAssessmentAction,
approval_policy: &'a AskForApproval,
) -> ApprovalReviewInput<'a> {
static RUNNER: UnusedApprovalRunner = UnusedApprovalRunner;
ApprovalReviewInput {
session_store,
thread_store,
@@ -316,6 +328,7 @@ fn approval_review_input<'a>(
approval_policy,
retry_reason: Some("initial review timed out"),
source: ApprovalReviewSource::DelegatedSubagent,
runner: &RUNNER,
}
}

32
codex-rs/ext/guardian/src/lib.rs
View File

@@ -3,11 +3,16 @@ use std::sync::Arc;
use codex_core::config::Config;
use codex_extension_api::AgentSpawnFuture;
use codex_extension_api::AgentSpawner;
use codex_extension_api::ApprovalReviewContributor;
use codex_extension_api::ApprovalReviewInput;
use codex_extension_api::ApprovalReviewOutcome;
use codex_extension_api::ExtensionFuture;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_extension_api::ThreadLifecycleContributor;
use codex_extension_api::ThreadStartInput;
use codex_protocol::ThreadId;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::AskForApproval;
/// Guardian extension dependencies supplied by the host at construction time.
#[derive(Clone, Debug)]
@@ -68,10 +73,35 @@ where
}
}
impl<S> ApprovalReviewContributor for GuardianExtension<S>
where
S: Send + Sync,
{
fn review<'a>(
&'a self,
input: ApprovalReviewInput<'a>,
) -> ExtensionFuture<'a, Result<ApprovalReviewOutcome, codex_extension_api::ApprovalReviewError>>
{
Box::pin(async move {
if input.reviewer != ApprovalsReviewer::AutoReview
|| !matches!(
input.approval_policy,
AskForApproval::OnRequest | AskForApproval::Granular(_)
)
{
return Ok(ApprovalReviewOutcome::Abstain);
}
input.runner.run().await
})
}
}
/// Installs the guardian contributors into the extension registry.
pub fn install<S>(registry: &mut ExtensionRegistryBuilder<Config>, agent_spawner: S)
where
S: Send + Sync + 'static,
{
registry.thread_lifecycle_contributor(Arc::new(GuardianExtension::new(agent_spawner)));
let extension = Arc::new(GuardianExtension::new(agent_spawner));
registry.thread_lifecycle_contributor(extension.clone());
registry.approval_review_contributor(extension);
}