From b9944d8d0aba91a97cc80345630c5f7e39ae4dd0 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Mon, 10 Nov 2025 13:18:11 -0800 Subject: [PATCH] [app-server] define new v2 approval request --- codex-rs/Cargo.lock | 1 - codex-rs/Cargo.toml | 1 - codex-rs/app-server-protocol/Cargo.toml | 1 - .../src/protocol/common.rs | 144 ++++++------- .../app-server-protocol/src/protocol/v1.rs | 44 ++++ .../app-server-protocol/src/protocol/v2.rs | 199 +++++++++++++++++- 6 files changed, 302 insertions(+), 88 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 35424d822a..9b5fad7618 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -873,7 +873,6 @@ dependencies = [ "clap", "codex-protocol", "mcp-types", - "paste", "pretty_assertions", "schemars 0.8.22", "serde", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index c50c69aa3f..c0bef483c9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -149,7 +149,6 @@ opentelemetry-semantic-conventions = "0.30.0" opentelemetry_sdk = "0.30.0" os_info = "3.12.0" owo-colors = "4.2.0" -paste = "1.0.15" path-absolutize = "3.1.1" pathdiff = "0.2" portable-pty = "0.9.0" diff --git a/codex-rs/app-server-protocol/Cargo.toml b/codex-rs/app-server-protocol/Cargo.toml index 5aa1c765e7..4d1afadaa1 100644 --- a/codex-rs/app-server-protocol/Cargo.toml +++ b/codex-rs/app-server-protocol/Cargo.toml @@ -15,7 +15,6 @@ anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-protocol = { workspace = true } mcp-types = { workspace = true } -paste = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index f754ece562..c29d59946b 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1,6 +1,4 @@ -use std::collections::HashMap; use std::path::Path; -use std::path::PathBuf; use crate::JSONRPCNotification; use crate::JSONRPCRequest; @@ -9,12 +7,6 @@ use crate::export::GeneratedSchema; use crate::export::write_json_schema; use crate::protocol::v1; use crate::protocol::v2; -use codex_protocol::ConversationId; -use codex_protocol::parse_command::ParsedCommand; -use codex_protocol::protocol::FileChange; -use codex_protocol::protocol::ReviewDecision; -use codex_protocol::protocol::SandboxCommandAssessment; -use paste::paste; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -277,34 +269,36 @@ macro_rules! server_request_definitions { ( $( $(#[$variant_meta:meta])* - $variant:ident + $variant:ident $(=> $wire:literal)? { + params: $params:ty, + response: $response:ty, + } ),* $(,)? ) => { - paste! { - /// Request initiated from the server and sent to the client. - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] - #[serde(tag = "method", rename_all = "camelCase")] - pub enum ServerRequest { - $( - $(#[$variant_meta])* - $variant { - #[serde(rename = "id")] - request_id: RequestId, - params: [<$variant Params>], - }, - )* - } + /// Request initiated from the server and sent to the client. + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] + #[serde(tag = "method", rename_all = "camelCase")] + pub enum ServerRequest { + $( + $(#[$variant_meta])* + $(#[serde(rename = $wire)] #[ts(rename = $wire)])? + $variant { + #[serde(rename = "id")] + request_id: RequestId, + params: $params, + }, + )* + } - #[derive(Debug, Clone, PartialEq, JsonSchema)] - pub enum ServerRequestPayload { - $( $variant([<$variant Params>]), )* - } + #[derive(Debug, Clone, PartialEq, JsonSchema)] + pub enum ServerRequestPayload { + $( $variant($params), )* + } - impl ServerRequestPayload { - pub fn request_with_id(self, request_id: RequestId) -> ServerRequest { - match self { - $(Self::$variant(params) => ServerRequest::$variant { request_id, params },)* - } + impl ServerRequestPayload { + pub fn request_with_id(self, request_id: RequestId) -> ServerRequest { + match self { + $(Self::$variant(params) => ServerRequest::$variant { request_id, params },)* } } } @@ -312,9 +306,9 @@ macro_rules! server_request_definitions { pub fn export_server_responses( out_dir: &::std::path::Path, ) -> ::std::result::Result<(), ::ts_rs::ExportError> { - paste! { - $(<[<$variant Response>] as ::ts_rs::TS>::export_all_to(out_dir)?;)* - } + $( + <$response as ::ts_rs::TS>::export_all_to(out_dir)?; + )* Ok(()) } @@ -323,9 +317,12 @@ macro_rules! server_request_definitions { out_dir: &Path, ) -> ::anyhow::Result> { let mut schemas = Vec::new(); - paste! { - $(schemas.push(crate::export::write_json_schema::<[<$variant Response>]>(out_dir, stringify!([<$variant Response>]))?);)* - } + $( + schemas.push(crate::export::write_json_schema::<$response>( + out_dir, + concat!(stringify!($variant), "Response"), + )?); + )* Ok(schemas) } @@ -334,9 +331,12 @@ macro_rules! server_request_definitions { out_dir: &Path, ) -> ::anyhow::Result> { let mut schemas = Vec::new(); - paste! { - $(schemas.push(crate::export::write_json_schema::<[<$variant Params>]>(out_dir, stringify!([<$variant Params>]))?);)* - } + $( + schemas.push(crate::export::write_json_schema::<$params>( + out_dir, + concat!(stringify!($variant), "Params"), + )?); + )* Ok(schemas) } }; @@ -426,49 +426,24 @@ impl TryFrom for ServerRequest { } server_request_definitions! { + /// NEW APIs + /// Sent when approval is requested for a specific item (e.g. file edit, command execution). + ItemRequestApproval => "item/requestApproval" { + params: v2::ItemRequestApprovalParams, + response: v2::ItemRequestApprovalResponse, + }, + + /// DEPRECATED APIs below /// Request to approve a patch. - ApplyPatchApproval, + ApplyPatchApproval { + params: v1::ApplyPatchApprovalParams, + response: v1::ApplyPatchApprovalResponse, + }, /// Request to exec a command. - ExecCommandApproval, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -pub struct ApplyPatchApprovalParams { - pub conversation_id: ConversationId, - /// Use to correlate this with [codex_core::protocol::PatchApplyBeginEvent] - /// and [codex_core::protocol::PatchApplyEndEvent]. - pub call_id: String, - pub file_changes: HashMap, - /// Optional explanatory reason (e.g. request for extra write access). - pub reason: Option, - /// When set, the agent is asking the user to allow writes under this root - /// for the remainder of the session (unclear if this is honored today). - pub grant_root: Option, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -pub struct ExecCommandApprovalParams { - pub conversation_id: ConversationId, - /// Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] - /// and [codex_core::protocol::ExecCommandEndEvent]. - pub call_id: String, - pub command: Vec, - pub cwd: PathBuf, - pub reason: Option, - pub risk: Option, - pub parsed_cmd: Vec, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -pub struct ExecCommandApprovalResponse { - pub decision: ReviewDecision, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -pub struct ApplyPatchApprovalResponse { - pub decision: ReviewDecision, + ExecCommandApproval { + params: v1::ExecCommandApprovalParams, + response: v1::ExecCommandApprovalResponse, + }, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -530,10 +505,13 @@ client_notification_definitions! { mod tests { use super::*; use anyhow::Result; + use codex_protocol::ConversationId; use codex_protocol::account::PlanType; + use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::AskForApproval; use pretty_assertions::assert_eq; use serde_json::json; + use std::path::PathBuf; #[test] fn serialize_new_conversation() -> Result<()> { @@ -613,7 +591,7 @@ mod tests { #[test] fn serialize_server_request() -> Result<()> { let conversation_id = ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?; - let params = ExecCommandApprovalParams { + let params = v1::ExecCommandApprovalParams { conversation_id, call_id: "call-42".to_string(), command: vec!["echo".to_string(), "hello".to_string()], diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index d518abc1bd..54f80c9fd4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -8,8 +8,12 @@ use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::Verbosity; use codex_protocol::models::ResponseItem; +use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::FileChange; +use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::SandboxCommandAssessment; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::TurnAbortReason; @@ -191,6 +195,46 @@ pub struct GitDiffToRemoteResponse { pub diff: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +pub struct ApplyPatchApprovalParams { + pub conversation_id: ConversationId, + /// Use to correlate this with [codex_core::protocol::PatchApplyBeginEvent] + /// and [codex_core::protocol::PatchApplyEndEvent]. + pub call_id: String, + pub file_changes: HashMap, + /// Optional explanatory reason (e.g. request for extra write access). + pub reason: Option, + /// When set, the agent is asking the user to allow writes under this root + /// for the remainder of the session (unclear if this is honored today). + pub grant_root: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +pub struct ApplyPatchApprovalResponse { + pub decision: ReviewDecision, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +pub struct ExecCommandApprovalParams { + pub conversation_id: ConversationId, + /// Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] + /// and [codex_core::protocol::ExecCommandEndEvent]. + pub call_id: String, + pub command: Vec, + pub cwd: PathBuf, + pub reason: Option, + pub risk: Option, + pub parsed_cmd: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +pub struct ExecCommandApprovalResponse { + pub decision: ReviewDecision, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] pub struct CancelLoginChatGptParams { diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 10196884aa..fafbeb75e5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -4,11 +4,14 @@ use std::path::PathBuf; use crate::protocol::common::AuthMode; use codex_protocol::ConversationId; use codex_protocol::account::PlanType; +use codex_protocol::approvals::SandboxCommandAssessment as CoreSandboxCommandAssessment; use codex_protocol::config_types::ReasoningEffort; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::items::AgentMessageContent as CoreAgentMessageContent; use codex_protocol::items::TurnItem as CoreTurnItem; use codex_protocol::models::ResponseItem; +use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; +use codex_protocol::protocol::FileChange as CoreFileChange; use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::user_input::UserInput as CoreUserInput; @@ -20,7 +23,7 @@ use serde_json::Value as JsonValue; use ts_rs::TS; // Macro to declare a camelCased API v2 enum mirroring a core enum which -// tends to use kebab-case. +// tends to use either snake_case or kebab-case. macro_rules! v2_enum_from_core { ( pub enum $Name:ident from $Src:path { $( $Variant:ident ),+ $(,)? } @@ -56,6 +59,23 @@ v2_enum_from_core!( } ); +v2_enum_from_core!( + pub enum ReviewDecision from codex_protocol::protocol::ReviewDecision { + Approved, + ApprovedForSession, + Denied, + Abort + } +); + +v2_enum_from_core!( + pub enum SandboxRiskLevel from codex_protocol::approvals::SandboxRiskLevel { + Low, + Medium, + High + } +); + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(tag = "mode", rename_all = "camelCase")] #[ts(tag = "mode")] @@ -119,6 +139,131 @@ impl From for SandboxPolicy { } } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SandboxCommandAssessment { + pub description: String, + pub risk_level: SandboxRiskLevel, +} + +impl SandboxCommandAssessment { + pub fn into_core(self) -> CoreSandboxCommandAssessment { + CoreSandboxCommandAssessment { + description: self.description, + risk_level: self.risk_level.to_core(), + } + } +} + +impl From for SandboxCommandAssessment { + fn from(value: CoreSandboxCommandAssessment) -> Self { + Self { + description: value.description, + risk_level: SandboxRiskLevel::from(value.risk_level), + } + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(tag = "type", rename_all = "camelCase")] +#[ts(tag = "type")] +#[ts(export_to = "v2/")] +pub enum ParsedCommand { + Read { + cmd: String, + name: String, + path: PathBuf, + }, + ListFiles { + cmd: String, + path: Option, + }, + Search { + cmd: String, + query: Option, + path: Option, + }, + Unknown { + cmd: String, + }, +} + +impl ParsedCommand { + pub fn into_core(self) -> CoreParsedCommand { + match self { + ParsedCommand::Read { cmd, name, path } => CoreParsedCommand::Read { cmd, name, path }, + ParsedCommand::ListFiles { cmd, path } => CoreParsedCommand::ListFiles { cmd, path }, + ParsedCommand::Search { cmd, query, path } => { + CoreParsedCommand::Search { cmd, query, path } + } + ParsedCommand::Unknown { cmd } => CoreParsedCommand::Unknown { cmd }, + } + } +} + +impl From for ParsedCommand { + fn from(value: CoreParsedCommand) -> Self { + match value { + CoreParsedCommand::Read { cmd, name, path } => ParsedCommand::Read { cmd, name, path }, + CoreParsedCommand::ListFiles { cmd, path } => ParsedCommand::ListFiles { cmd, path }, + CoreParsedCommand::Search { cmd, query, path } => { + ParsedCommand::Search { cmd, query, path } + } + CoreParsedCommand::Unknown { cmd } => ParsedCommand::Unknown { cmd }, + } + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(tag = "type", rename_all = "camelCase")] +#[ts(tag = "type")] +#[ts(export_to = "v2/")] +pub enum FileChange { + Add { + content: String, + }, + Delete { + content: String, + }, + Update { + unified_diff: String, + move_path: Option, + }, +} + +impl FileChange { + pub fn into_core(self) -> CoreFileChange { + match self { + FileChange::Add { content } => CoreFileChange::Add { content }, + FileChange::Delete { content } => CoreFileChange::Delete { content }, + FileChange::Update { + unified_diff, + move_path, + } => CoreFileChange::Update { + unified_diff, + move_path, + }, + } + } +} + +impl From for FileChange { + fn from(value: CoreFileChange) -> Self { + match value { + CoreFileChange::Add { content } => FileChange::Add { content }, + CoreFileChange::Delete { content } => FileChange::Delete { content }, + CoreFileChange::Update { + unified_diff, + move_path, + } => FileChange::Update { + unified_diff, + move_path, + }, + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type")] @@ -279,7 +424,7 @@ pub struct ThreadStartParams { pub cwd: Option, pub approval_policy: Option, pub sandbox: Option, - pub config: Option>, + pub config: Option>, pub base_instructions: Option, pub developer_instructions: Option, } @@ -735,6 +880,56 @@ pub struct McpToolCallProgressNotification { pub message: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ItemRequestApprovalParams { + pub thread_id: String, + pub turn_id: String, + pub item_id: String, + pub request: ItemApprovalRequest, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(tag = "type", rename_all = "camelCase")] +#[ts(tag = "type")] +#[ts(export_to = "v2/")] +pub enum ItemApprovalRequest { + CommandExecution(CommandExecutionRequest), + FileEdit(FileEditRequest), +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct CommandExecutionRequest { + pub call_id: String, + pub command: Vec, + pub cwd: PathBuf, + /// Optional explanatory reason (e.g. request for network access). + pub reason: Option, + pub risk: Option, + pub parsed_cmd: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct FileEditRequest { + pub call_id: String, + pub file_changes: HashMap, + /// Optional explanatory reason (e.g. request for extra write access). + pub reason: Option, + pub grant_root: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ItemRequestApprovalResponse { + pub decision: ReviewDecision, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")]