openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 17:38:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

extension-api: own the rich approval review request

Browse Source
This commit is contained in:
Dylan Hurd
2026-06-12 12:09:53 -07:00
parent 7112558bfd
commit fd870447b4
10 changed files with 149 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+2
View File
@@ -2926,6 +2926,8 @@ dependencies = [
"codex-tools",
"codex-utils-absolute-path",
"pretty_assertions",
"serde",
"serde_json",
"tokio",
]
codex-rs/app-server/src/extensions.rs
+8
View File
@@ -178,6 +178,7 @@ mod tests {
use codex_extension_api::ApprovalReviewError;
use codex_extension_api::ApprovalReviewInput;
use codex_extension_api::ApprovalReviewOutcome;
use codex_extension_api::ApprovalReviewRequest;
use codex_extension_api::ApprovalReviewRunner;
use codex_extension_api::ApprovalReviewSource;
use codex_extension_api::ExtensionData;
@@ -218,6 +219,12 @@ mod tests {
reason: Some("run command".to_string()),
permissions: Default::default(),
};
let request = ApprovalReviewRequest::RequestPermissions {
id: "request".to_string(),
turn_id: "turn".to_string(),
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 {
@@ -227,6 +234,7 @@ mod tests {
review_id: "review",
turn_id: "turn",
target_item_id: None,
request: &request,
prompt: "review command",
action: &action,
reviewer,
codex-rs/core/src/guardian/approval_request.rs
+3 -89
View File
@@ -1,6 +1,9 @@
use std::path::Path;
use codex_analytics::GuardianReviewedAction;
use codex_extension_api::ApprovalReviewMcpAnnotations as GuardianMcpAnnotations;
use codex_extension_api::ApprovalReviewNetworkAccessTrigger as GuardianNetworkAccessTrigger;
use codex_extension_api::ApprovalReviewRequest as GuardianApprovalRequest;
use codex_protocol::approvals::GuardianAssessmentAction;
use codex_protocol::approvals::GuardianCommandSource;
use codex_protocol::approvals::NetworkApprovalProtocol;
@@ -13,95 +16,6 @@ use serde_json::Value;
use super::GUARDIAN_MAX_ACTION_STRING_TOKENS;
use super::prompt::guardian_truncate_text;
#[derive(Debug, Clone, PartialEq)]
pub(crate) enum GuardianApprovalRequest {
Shell {
id: String,
command: Vec<String>,
cwd: AbsolutePathBuf,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
justification: Option<String>,
},
ExecCommand {
id: String,
command: Vec<String>,
cwd: AbsolutePathBuf,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
justification: Option<String>,
tty: bool,
},
#[cfg(unix)]
Execve {
id: String,
source: GuardianCommandSource,
program: String,
argv: Vec<String>,
cwd: AbsolutePathBuf,
additional_permissions: Option<AdditionalPermissionProfile>,
},
ApplyPatch {
id: String,
cwd: AbsolutePathBuf,
files: Vec<AbsolutePathBuf>,
patch: String,
},
NetworkAccess {
id: String,
turn_id: String,
target: String,
host: String,
protocol: NetworkApprovalProtocol,
port: u16,
trigger: Option<GuardianNetworkAccessTrigger>,
},
McpToolCall {
id: String,
server: String,
tool_name: String,
arguments: Option<Value>,
connector_id: Option<String>,
connector_name: Option<String>,
connector_description: Option<String>,
tool_title: Option<String>,
tool_description: Option<String>,
annotations: Option<GuardianMcpAnnotations>,
},
RequestPermissions {
id: String,
turn_id: String,
reason: Option<String>,
permissions: RequestPermissionProfile,
},
}
#[derive(Debug, Clone, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct GuardianNetworkAccessTrigger {
pub(crate) call_id: String,
pub(crate) tool_name: String,
pub(crate) command: Vec<String>,
pub(crate) cwd: AbsolutePathBuf,
pub(crate) sandbox_permissions: crate::sandboxing::SandboxPermissions,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) additional_permissions: Option<AdditionalPermissionProfile>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) justification: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) tty: Option<bool>,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub(crate) struct GuardianMcpAnnotations {
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) destructive_hint: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) open_world_hint: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) read_only_hint: Option<bool>,
}
#[derive(Serialize)]
struct CommandApprovalAction<'a> {
tool: &'a str,
codex-rs/core/src/guardian/mod.rs
+4 -3
View File
@@ -24,15 +24,16 @@ use codex_protocol::protocol::GuardianAssessmentOutcome;
use serde::Deserialize;
use serde::Serialize;
pub(crate) use approval_request::GuardianApprovalRequest;
pub(crate) use approval_request::GuardianMcpAnnotations;
pub(crate) use approval_request::GuardianNetworkAccessTrigger;
pub(crate) use approval_request::format_guardian_action_pretty;
#[cfg(test)]
pub(crate) use approval_request::guardian_approval_request_to_json;
pub(crate) use approval_request::guardian_assessment_action;
pub(crate) use approval_request::guardian_request_target_item_id;
pub(crate) use approval_request::guardian_request_turn_id;
// Compatibility aliases while core call sites migrate to extension-api names.
pub(crate) use codex_extension_api::ApprovalReviewMcpAnnotations as GuardianMcpAnnotations;
pub(crate) use codex_extension_api::ApprovalReviewNetworkAccessTrigger as GuardianNetworkAccessTrigger;
pub(crate) use codex_extension_api::ApprovalReviewRequest as GuardianApprovalRequest;
pub(crate) use review::guardian_rejection_message;
pub(crate) use review::guardian_timeout_message;
pub(crate) use review::is_guardian_reviewer_source;
codex-rs/core/src/tools/approval_dispatch.rs
+2
View File
@@ -147,6 +147,7 @@ pub(crate) async fn request_automated_approval(
review_id: &review_id,
turn_id: guardian_request_turn_id(&request, &turn.sub_id),
target_item_id: guardian_request_target_item_id(&request),
request: &request,
prompt: &prompt,
action: &action,
reviewer,
@@ -222,6 +223,7 @@ pub(crate) async fn request_automated_approval_with_cancel(
review_id: &review_id,
turn_id: guardian_request_turn_id(&request, &turn.sub_id),
target_item_id: guardian_request_target_item_id(&request),
request: &request,
prompt: &prompt,
action: &action,
reviewer,
codex-rs/ext/extension-api/Cargo.toml
+2
View File
@@ -19,6 +19,8 @@ codex-context-fragments = { workspace = true }
codex-protocol = { workspace = true }
codex-tools = { workspace = true }
codex-utils-absolute-path = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
codex-rs/ext/extension-api/src/approval_review.rs
+2
View File
@@ -6,6 +6,7 @@ use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use crate::ApprovalReviewRequest;
use crate::ExtensionData;
use crate::ExtensionFuture;
@@ -37,6 +38,7 @@ pub struct ApprovalReviewInput<'a> {
pub review_id: &'a str,
pub turn_id: &'a str,
pub target_item_id: Option<&'a str>,
pub request: &'a ApprovalReviewRequest,
/// Bounded review prompt rendered by the host.
///
/// The action is useful assessment metadata but does not yet carry all
codex-rs/ext/extension-api/src/approval_review_request.rs
+100
View File
@@ -0,0 +1,100 @@
use codex_protocol::approvals::GuardianCommandSource;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::SandboxPermissions;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Serialize;
use serde_json::Value;
/// Complete host-neutral description of an action offered for approval review.
#[derive(Debug, Clone, PartialEq)]
pub enum ApprovalReviewRequest {
Shell {
id: String,
command: Vec<String>,
cwd: AbsolutePathBuf,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
justification: Option<String>,
},
ExecCommand {
id: String,
command: Vec<String>,
cwd: AbsolutePathBuf,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
justification: Option<String>,
tty: bool,
},
#[cfg(unix)]
Execve {
id: String,
source: GuardianCommandSource,
program: String,
argv: Vec<String>,
cwd: AbsolutePathBuf,
additional_permissions: Option<AdditionalPermissionProfile>,
},
ApplyPatch {
id: String,
cwd: AbsolutePathBuf,
files: Vec<AbsolutePathBuf>,
patch: String,
},
NetworkAccess {
id: String,
turn_id: String,
target: String,
host: String,
protocol: NetworkApprovalProtocol,
port: u16,
trigger: Option<ApprovalReviewNetworkAccessTrigger>,
},
McpToolCall {
id: String,
server: String,
tool_name: String,
arguments: Option<Value>,
connector_id: Option<String>,
connector_name: Option<String>,
connector_description: Option<String>,
tool_title: Option<String>,
tool_description: Option<String>,
annotations: Option<ApprovalReviewMcpAnnotations>,
},
RequestPermissions {
id: String,
turn_id: String,
reason: Option<String>,
permissions: RequestPermissionProfile,
},
}
/// Command context that caused a network approval request.
#[derive(Debug, Clone, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct ApprovalReviewNetworkAccessTrigger {
pub call_id: String,
pub tool_name: String,
pub command: Vec<String>,
pub cwd: AbsolutePathBuf,
pub sandbox_permissions: SandboxPermissions,
#[serde(skip_serializing_if = "Option::is_none")]
pub additional_permissions: Option<AdditionalPermissionProfile>,
#[serde(skip_serializing_if = "Option::is_none")]
pub justification: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub tty: Option<bool>,
}
/// MCP tool hints retained for approval review.
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub struct ApprovalReviewMcpAnnotations {
#[serde(skip_serializing_if = "Option::is_none")]
pub destructive_hint: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub open_world_hint: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub read_only_hint: Option<bool>,
}
codex-rs/ext/extension-api/src/lib.rs
+4
View File
@@ -1,4 +1,5 @@
mod approval_review;
mod approval_review_request;
mod capabilities;
mod contributors;
mod registry;
@@ -10,6 +11,9 @@ pub use approval_review::ApprovalReviewInput;
pub use approval_review::ApprovalReviewOutcome;
pub use approval_review::ApprovalReviewRunner;
pub use approval_review::ApprovalReviewSource;
pub use approval_review_request::ApprovalReviewMcpAnnotations;
pub use approval_review_request::ApprovalReviewNetworkAccessTrigger;
pub use approval_review_request::ApprovalReviewRequest;
pub use capabilities::AgentSpawnFuture;
pub use capabilities::AgentSpawner;
pub use capabilities::ExtensionEventSink;
codex-rs/ext/extension-api/tests/registry.rs
+22
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::ApprovalReviewRequest;
use codex_extension_api::ApprovalReviewRunner;
use codex_extension_api::ApprovalReviewSource;
use codex_extension_api::ConfigContributor;
@@ -145,6 +146,7 @@ async fn build_round_trips_every_contributor_category() {
&ExtensionData::new("thread"),
&ExtensionData::new("turn"),
&test_action(),
&test_request(),
&AskForApproval::OnRequest,
))
.await,
@@ -249,6 +251,7 @@ struct ApprovalCall {
target_item_id: Option<String>,
prompt: String,
action: GuardianAssessmentAction,
request: ApprovalReviewRequest,
reviewer: ApprovalsReviewer,
approval_policy: AskForApproval,
retry_reason: Option<String>,
@@ -290,6 +293,7 @@ impl ApprovalReviewContributor for RecordingApprovalContributor {
target_item_id: input.target_item_id.map(str::to_string),
prompt: input.prompt.to_string(),
action: input.action.clone(),
request: input.request.clone(),
reviewer: input.reviewer,
approval_policy: *input.approval_policy,
retry_reason: input.retry_reason.map(str::to_string),
@@ -307,11 +311,21 @@ fn test_action() -> GuardianAssessmentAction {
}
}
fn test_request() -> ApprovalReviewRequest {
ApprovalReviewRequest::RequestPermissions {
id: "request-1".to_string(),
turn_id: "turn-1".to_string(),
reason: Some("run delegated command".to_string()),
permissions: Default::default(),
}
}
fn approval_review_input<'a>(
session_store: &'a ExtensionData,
thread_store: &'a ExtensionData,
turn_store: &'a ExtensionData,
action: &'a GuardianAssessmentAction,
request: &'a ApprovalReviewRequest,
approval_policy: &'a AskForApproval,
) -> ApprovalReviewInput<'a> {
static RUNNER: UnusedApprovalRunner = UnusedApprovalRunner;
@@ -322,6 +336,7 @@ fn approval_review_input<'a>(
review_id: "review-1",
turn_id: "turn-1",
target_item_id: Some("item-1"),
request,
prompt: "allow delegated command?",
action,
reviewer: ApprovalsReviewer::AutoReview,
@@ -358,6 +373,7 @@ async fn approval_review_returns_first_claim_and_short_circuits() {
let thread_store = ExtensionData::new("thread-1");
let turn_store = ExtensionData::new("turn-store-1");
let action = test_action();
let request = test_request();
let approval_policy = AskForApproval::OnRequest;
let decision = registry
@@ -366,6 +382,7 @@ async fn approval_review_returns_first_claim_and_short_circuits() {
&thread_store,
&turn_store,
&action,
&request,
&approval_policy,
))
.await;
@@ -384,6 +401,7 @@ async fn approval_review_returns_first_claim_and_short_circuits() {
target_item_id: Some("item-1".to_string()),
prompt: "allow delegated command?".to_string(),
action: action.clone(),
request: request.clone(),
reviewer: ApprovalsReviewer::AutoReview,
approval_policy: AskForApproval::OnRequest,
retry_reason: Some("initial review timed out".to_string()),
@@ -418,6 +436,7 @@ async fn approval_review_error_stops_dispatch() {
let thread_store = ExtensionData::new("thread");
let turn_store = ExtensionData::new("turn");
let action = test_action();
let request = test_request();
let approval_policy = AskForApproval::OnRequest;
let result = registry
@@ -426,6 +445,7 @@ async fn approval_review_error_stops_dispatch() {
&thread_store,
&turn_store,
&action,
&request,
&approval_policy,
))
.await;
@@ -491,6 +511,7 @@ async fn empty_registry_does_not_claim_approval_review() {
let thread_store = ExtensionData::new("thread");
let turn_store = ExtensionData::new("turn");
let action = test_action();
let request = test_request();
let approval_policy = AskForApproval::OnRequest;
assert_eq!(
@@ -500,6 +521,7 @@ async fn empty_registry_does_not_claim_approval_review() {
&thread_store,
&turn_store,
&action,
&request,
&approval_policy,
))
.await,