diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 613a45872d..bed54ea512 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2926,6 +2926,8 @@ dependencies = [ "codex-tools", "codex-utils-absolute-path", "pretty_assertions", + "serde", + "serde_json", "tokio", ] diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index 86b49d375a..56994f0ec1 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -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, diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index fba227834a..11e90b6b67 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -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, - cwd: AbsolutePathBuf, - sandbox_permissions: crate::sandboxing::SandboxPermissions, - additional_permissions: Option, - justification: Option, - }, - ExecCommand { - id: String, - command: Vec, - cwd: AbsolutePathBuf, - sandbox_permissions: crate::sandboxing::SandboxPermissions, - additional_permissions: Option, - justification: Option, - tty: bool, - }, - #[cfg(unix)] - Execve { - id: String, - source: GuardianCommandSource, - program: String, - argv: Vec, - cwd: AbsolutePathBuf, - additional_permissions: Option, - }, - ApplyPatch { - id: String, - cwd: AbsolutePathBuf, - files: Vec, - patch: String, - }, - NetworkAccess { - id: String, - turn_id: String, - target: String, - host: String, - protocol: NetworkApprovalProtocol, - port: u16, - trigger: Option, - }, - McpToolCall { - id: String, - server: String, - tool_name: String, - arguments: Option, - connector_id: Option, - connector_name: Option, - connector_description: Option, - tool_title: Option, - tool_description: Option, - annotations: Option, - }, - RequestPermissions { - id: String, - turn_id: String, - reason: Option, - 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, - pub(crate) cwd: AbsolutePathBuf, - pub(crate) sandbox_permissions: crate::sandboxing::SandboxPermissions, - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) additional_permissions: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) justification: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) tty: Option, -} - -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub(crate) struct GuardianMcpAnnotations { - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) destructive_hint: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) open_world_hint: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) read_only_hint: Option, -} - #[derive(Serialize)] struct CommandApprovalAction<'a> { tool: &'a str, diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 721c55d6ac..f444327ee8 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -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; diff --git a/codex-rs/core/src/tools/approval_dispatch.rs b/codex-rs/core/src/tools/approval_dispatch.rs index 6d5849229c..4b058aa031 100644 --- a/codex-rs/core/src/tools/approval_dispatch.rs +++ b/codex-rs/core/src/tools/approval_dispatch.rs @@ -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, diff --git a/codex-rs/ext/extension-api/Cargo.toml b/codex-rs/ext/extension-api/Cargo.toml index 1a5a9742fb..6cc769d0fa 100644 --- a/codex-rs/ext/extension-api/Cargo.toml +++ b/codex-rs/ext/extension-api/Cargo.toml @@ -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 } diff --git a/codex-rs/ext/extension-api/src/approval_review.rs b/codex-rs/ext/extension-api/src/approval_review.rs index c1f0c35805..a40e1865cf 100644 --- a/codex-rs/ext/extension-api/src/approval_review.rs +++ b/codex-rs/ext/extension-api/src/approval_review.rs @@ -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 diff --git a/codex-rs/ext/extension-api/src/approval_review_request.rs b/codex-rs/ext/extension-api/src/approval_review_request.rs new file mode 100644 index 0000000000..fac5b6ea0c --- /dev/null +++ b/codex-rs/ext/extension-api/src/approval_review_request.rs @@ -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, + cwd: AbsolutePathBuf, + sandbox_permissions: SandboxPermissions, + additional_permissions: Option, + justification: Option, + }, + ExecCommand { + id: String, + command: Vec, + cwd: AbsolutePathBuf, + sandbox_permissions: SandboxPermissions, + additional_permissions: Option, + justification: Option, + tty: bool, + }, + #[cfg(unix)] + Execve { + id: String, + source: GuardianCommandSource, + program: String, + argv: Vec, + cwd: AbsolutePathBuf, + additional_permissions: Option, + }, + ApplyPatch { + id: String, + cwd: AbsolutePathBuf, + files: Vec, + patch: String, + }, + NetworkAccess { + id: String, + turn_id: String, + target: String, + host: String, + protocol: NetworkApprovalProtocol, + port: u16, + trigger: Option, + }, + McpToolCall { + id: String, + server: String, + tool_name: String, + arguments: Option, + connector_id: Option, + connector_name: Option, + connector_description: Option, + tool_title: Option, + tool_description: Option, + annotations: Option, + }, + RequestPermissions { + id: String, + turn_id: String, + reason: Option, + 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, + pub cwd: AbsolutePathBuf, + pub sandbox_permissions: SandboxPermissions, + #[serde(skip_serializing_if = "Option::is_none")] + pub additional_permissions: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub justification: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub tty: Option, +} + +/// 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, + #[serde(skip_serializing_if = "Option::is_none")] + pub open_world_hint: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub read_only_hint: Option, +} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index 1af84950c1..d790f006d0 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -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; diff --git a/codex-rs/ext/extension-api/tests/registry.rs b/codex-rs/ext/extension-api/tests/registry.rs index b41c86d9a4..7e980aa16e 100644 --- a/codex-rs/ext/extension-api/tests/registry.rs +++ b/codex-rs/ext/extension-api/tests/registry.rs @@ -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, prompt: String, action: GuardianAssessmentAction, + request: ApprovalReviewRequest, reviewer: ApprovalsReviewer, approval_policy: AskForApproval, retry_reason: Option, @@ -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,