From b430c5ce9545f236884076245b3327443ee39f19 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Sun, 16 Nov 2025 20:13:43 -0800 Subject: [PATCH 1/5] [app-server] feat: add command execution approval flow --- codex-rs/Cargo.lock | 1 - codex-rs/Cargo.toml | 1 - codex-rs/app-server-protocol/Cargo.toml | 1 - .../src/protocol/common.rs | 147 ++++++------- .../app-server-protocol/src/protocol/v1.rs | 44 ++++ .../app-server-protocol/src/protocol/v2.rs | 132 ++++++++++- .../app-server/src/bespoke_event_handling.rs | 208 ++++++++++++++---- .../app-server/src/codex_message_processor.rs | 9 +- .../app-server/tests/suite/v2/turn_start.rs | 69 +++--- codex-rs/core/src/codex.rs | 1 + codex-rs/core/src/tasks/user_shell.rs | 34 ++- codex-rs/core/src/tools/events.rs | 207 +++++++++-------- .../tests/event_processor_with_json_output.rs | 38 +++- codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/protocol/src/approvals.rs | 4 + codex-rs/protocol/src/protocol.rs | 17 ++ codex-rs/tui/src/chatwidget/tests.rs | 134 ++++++----- 17 files changed, 731 insertions(+), 317 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4209b8f39f..a675a885a7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -874,7 +874,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 5e2bd05e47..b19bf76608 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -153,7 +153,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 d36e81be02..5791a3b5bd 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,27 @@ impl TryFrom for ServerRequest { } server_request_definitions! { + /// NEW APIs + /// Sent when approval is requested for a specific command execution. + /// This request is used for Turns started via turn/start. + CommandExecutionRequestApproval => "item/commandExecution/requestApproval" { + params: v2::CommandExecutionRequestApprovalParams, + response: v2::CommandExecutionRequestApprovalResponse, + }, + + /// DEPRECATED APIs below /// Request to approve a patch. - ApplyPatchApproval, + /// This request is used for Turns started via the legacy APIs (i.e. SendUserTurn, SendUserMessage). + 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, + /// This request is used for Turns started via the legacy APIs (i.e. SendUserTurn, SendUserMessage). + ExecCommandApproval { + params: v1::ExecCommandApprovalParams, + response: v1::ExecCommandApprovalResponse, + }, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -533,10 +511,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<()> { @@ -616,7 +597,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 c93d4dc64b..78fe157445 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -4,11 +4,13 @@ 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::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::user_input::UserInput as CoreUserInput; @@ -20,7 +22,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 +58,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 = "type", rename_all = "camelCase")] #[ts(tag = "type")] @@ -121,6 +140,82 @@ 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")] @@ -281,7 +376,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, } @@ -527,10 +622,18 @@ pub enum ThreadItem { #[ts(rename_all = "camelCase")] CommandExecution { id: String, + /// The command to be executed. command: String, - aggregated_output: String, - exit_code: Option, + /// The command's working directory if not the default cwd for the agent. + cwd: PathBuf, status: CommandExecutionStatus, + /// A best-effort parsing of the command to identify the type of command and its arguments. + parsed_cmd: Vec, + /// The command's output, aggregated from stdout and stderr. + aggregated_output: Option, + /// The command's exit code. + exit_code: Option, + /// The duration of the command execution in milliseconds. duration_ms: Option, }, #[serde(rename_all = "camelCase")] @@ -762,6 +865,27 @@ pub struct McpToolCallProgressNotification { pub message: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct CommandExecutionRequestApprovalParams { + pub thread_id: String, + pub turn_id: String, + pub item_id: String, + /// Optional explanatory reason (e.g. request for network access). + pub reason: Option, + /// Optional model-provided risk assessment describing the blocked command. + pub risk: Option, + /// A best-effort parsing of the command to identify the type of command and its arguments. + pub parsed_cmd: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[ts(export_to = "v2/")] +pub struct CommandExecutionRequestApprovalResponse { + pub decision: ReviewDecision, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 3f6abb0976..62b373a45a 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -5,6 +5,10 @@ use codex_app_server_protocol::AccountRateLimitsUpdatedNotification; use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; +use codex_app_server_protocol::CommandExecutionOutputDeltaNotification; +use codex_app_server_protocol::CommandExecutionRequestApprovalParams; +use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; +use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::ExecCommandApprovalParams; use codex_app_server_protocol::ExecCommandApprovalResponse; use codex_app_server_protocol::InterruptConversationResponse; @@ -13,9 +17,12 @@ use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::McpToolCallError; use codex_app_server_protocol::McpToolCallResult; use codex_app_server_protocol::McpToolCallStatus; +use codex_app_server_protocol::ParsedCommand as V2ParsedCommand; use codex_app_server_protocol::ReasoningSummaryPartAddedNotification; use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; use codex_app_server_protocol::ReasoningTextDeltaNotification; +use codex_app_server_protocol::ReviewDecision as V2ReviewDecision; +use codex_app_server_protocol::SandboxCommandAssessment as V2SandboxCommandAssessment; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::ThreadItem; @@ -25,16 +32,18 @@ use codex_core::protocol::ApplyPatchApprovalRequestEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::ExecApprovalRequestEvent; +use codex_core::protocol::ExecCommandEndEvent; use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; use codex_protocol::ConversationId; +use std::convert::TryFrom; use std::sync::Arc; use tokio::sync::oneshot; use tracing::error; -type JsonRpcResult = serde_json::Value; +type JsonValue = serde_json::Value; pub(crate) async fn apply_bespoke_event_handling( event: Event, @@ -42,6 +51,7 @@ pub(crate) async fn apply_bespoke_event_handling( conversation: Arc, outgoing: Arc, pending_interrupts: PendingInterrupts, + api_version: ApiVersion, ) { let Event { id: event_id, msg } = event; match msg { @@ -61,11 +71,58 @@ pub(crate) async fn apply_bespoke_event_handling( let rx = outgoing .send_request(ServerRequestPayload::ApplyPatchApproval(params)) .await; - // TODO(mbolin): Enforce a timeout so this task does not live indefinitely? tokio::spawn(async move { on_patch_approval_response(event_id, rx, conversation).await; }); } + EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { + call_id, + turn_id, + command, + cwd, + reason, + risk, + parsed_cmd, + }) => match api_version { + ApiVersion::V1 => { + let params = ExecCommandApprovalParams { + conversation_id, + call_id, + command, + cwd, + reason, + risk, + parsed_cmd, + }; + let rx = outgoing + .send_request(ServerRequestPayload::ExecCommandApproval(params)) + .await; + tokio::spawn(async move { + on_exec_approval_response(event_id, rx, conversation).await; + }); + } + ApiVersion::V2 => { + let params = CommandExecutionRequestApprovalParams { + thread_id: conversation_id.to_string(), + turn_id: turn_id.clone(), + // Until we migrate the core to be aware of a first class CommandExecutionItem + // and emit the corresponding EventMsg, we repurpose the call_id as the item_id. + item_id: call_id.clone(), + reason, + risk: risk.map(V2SandboxCommandAssessment::from), + parsed_cmd: parsed_cmd.into_iter().map(V2ParsedCommand::from).collect(), + }; + let rx = outgoing + .send_request(ServerRequestPayload::CommandExecutionRequestApproval( + params, + )) + .await; + tokio::spawn(async move { + on_command_execution_request_approval_response(event_id, rx, conversation) + .await; + }); + } + }, // TODO(celia): properly construct McpToolCall TurnItem in core. EventMsg::McpToolCallBegin(begin_event) => { let notification = construct_mcp_tool_call_notification(begin_event).await; @@ -121,32 +178,6 @@ pub(crate) async fn apply_bespoke_event_handling( )) .await; } - EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { - call_id, - command, - cwd, - reason, - risk, - parsed_cmd, - }) => { - let params = ExecCommandApprovalParams { - conversation_id, - call_id, - command, - cwd, - reason, - risk, - parsed_cmd, - }; - let rx = outgoing - .send_request(ServerRequestPayload::ExecCommandApproval(params)) - .await; - - // TODO(mbolin): Enforce a timeout so this task does not live indefinitely? - tokio::spawn(async move { - on_exec_approval_response(event_id, rx, conversation).await; - }); - } EventMsg::TokenCount(token_count_event) => { if let Some(rate_limits) = token_count_event.rate_limits { outgoing @@ -172,6 +203,79 @@ pub(crate) async fn apply_bespoke_event_handling( .send_server_notification(ServerNotification::ItemCompleted(notification)) .await; } + EventMsg::ExecCommandBegin(exec_command_begin_event) => { + let item = ThreadItem::CommandExecution { + id: exec_command_begin_event.call_id.clone(), + command: exec_command_begin_event.command.join(" "), + cwd: exec_command_begin_event.cwd, + status: CommandExecutionStatus::InProgress, + parsed_cmd: exec_command_begin_event + .parsed_cmd + .into_iter() + .map(V2ParsedCommand::from) + .collect(), + aggregated_output: None, + exit_code: None, + duration_ms: None, + }; + let notification = ItemStartedNotification { item }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(notification)) + .await; + } + EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => { + let notification = CommandExecutionOutputDeltaNotification { + item_id: exec_command_output_delta_event.call_id.clone(), + delta: String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string(), + }; + outgoing + .send_server_notification(ServerNotification::CommandExecutionOutputDelta( + notification, + )) + .await; + } + EventMsg::ExecCommandEnd(exec_command_end_event) => { + let ExecCommandEndEvent { + call_id, + command, + cwd, + parsed_cmd, + aggregated_output, + exit_code, + duration, + .. + } = exec_command_end_event; + + let status = if exit_code == 0 { + CommandExecutionStatus::Completed + } else { + CommandExecutionStatus::Failed + }; + + let aggregated_output = if aggregated_output.is_empty() { + None + } else { + Some(aggregated_output) + }; + + let duration_ms = i64::try_from(duration.as_millis()).unwrap_or(i64::MAX); + + let item = ThreadItem::CommandExecution { + id: call_id, + command: command.join(" "), + cwd, + status, + parsed_cmd: parsed_cmd.into_iter().map(V2ParsedCommand::from).collect(), + aggregated_output, + exit_code: Some(exit_code), + duration_ms: Some(duration_ms), + }; + + let notification = ItemCompletedNotification { item }; + outgoing + .send_server_notification(ServerNotification::ItemCompleted(notification)) + .await; + } // If this is a TurnAborted, reply to any pending interrupt requests. EventMsg::TurnAborted(turn_aborted_event) => { let pending = { @@ -202,7 +306,7 @@ pub(crate) async fn apply_bespoke_event_handling( async fn on_patch_approval_response( event_id: String, - receiver: oneshot::Receiver, + receiver: oneshot::Receiver, codex: Arc, ) { let response = receiver.await; @@ -244,7 +348,7 @@ async fn on_patch_approval_response( async fn on_exec_approval_response( event_id: String, - receiver: oneshot::Receiver, + receiver: oneshot::Receiver, conversation: Arc, ) { let response = receiver.await; @@ -278,6 +382,40 @@ async fn on_exec_approval_response( } } +async fn on_command_execution_request_approval_response( + event_id: String, + receiver: oneshot::Receiver, + conversation: Arc, +) { + let response = receiver.await; + let value = match response { + Ok(value) => value, + Err(err) => { + error!("request failed: {err:?}"); + return; + } + }; + + let response = serde_json::from_value::(value) + .unwrap_or_else(|err| { + error!("failed to deserialize CommandExecutionRequestApprovalResponse: {err}"); + CommandExecutionRequestApprovalResponse { + decision: V2ReviewDecision::Denied, + } + }); + + let decision = response.decision.to_core(); + if let Err(err) = conversation + .submit(Op::ExecApproval { + id: event_id, + decision, + }) + .await + { + error!("failed to submit ExecApproval: {err}"); + } +} + /// similar to handle_mcp_tool_call_begin in exec async fn construct_mcp_tool_call_notification( begin_event: McpToolCallBeginEvent, @@ -287,10 +425,7 @@ async fn construct_mcp_tool_call_notification( server: begin_event.invocation.server, tool: begin_event.invocation.tool, status: McpToolCallStatus::InProgress, - arguments: begin_event - .invocation - .arguments - .unwrap_or(JsonRpcResult::Null), + arguments: begin_event.invocation.arguments.unwrap_or(JsonValue::Null), result: None, error: None, }; @@ -328,10 +463,7 @@ async fn construct_mcp_tool_call_end_notification( server: end_event.invocation.server, tool: end_event.invocation.tool, status, - arguments: end_event - .invocation - .arguments - .unwrap_or(JsonRpcResult::Null), + arguments: end_event.invocation.arguments.unwrap_or(JsonValue::Null), result, error, }; diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c694486187..c5ac6a14af 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1245,7 +1245,7 @@ impl CodexMessageProcessor { // Auto-attach a conversation listener when starting a thread. // Use the same behavior as the v1 API with experimental_raw_events=false. if let Err(err) = self - .attach_conversation_listener(conversation_id, false) + .attach_conversation_listener(conversation_id, false, ApiVersion::V2) .await { tracing::warn!( @@ -1523,7 +1523,7 @@ impl CodexMessageProcessor { }) => { // Auto-attach a conversation listener when resuming a thread. if let Err(err) = self - .attach_conversation_listener(conversation_id, false) + .attach_conversation_listener(conversation_id, false, ApiVersion::V2) .await { tracing::warn!( @@ -2376,7 +2376,7 @@ impl CodexMessageProcessor { experimental_raw_events, } = params; match self - .attach_conversation_listener(conversation_id, experimental_raw_events) + .attach_conversation_listener(conversation_id, experimental_raw_events, ApiVersion::V1) .await { Ok(subscription_id) => { @@ -2417,6 +2417,7 @@ impl CodexMessageProcessor { &mut self, conversation_id: ConversationId, experimental_raw_events: bool, + api_version: ApiVersion, ) -> Result { let conversation = match self .conversation_manager @@ -2440,6 +2441,7 @@ impl CodexMessageProcessor { let outgoing_for_task = self.outgoing.clone(); let pending_interrupts = self.pending_interrupts.clone(); + let api_version_for_task = api_version; tokio::spawn(async move { loop { tokio::select! { @@ -2495,6 +2497,7 @@ impl CodexMessageProcessor { conversation.clone(), outgoing_for_task.clone(), pending_interrupts.clone(), + api_version_for_task, ) .await; } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index b26f01a9f1..3d3032285e 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -5,10 +5,14 @@ use app_test_support::create_mock_chat_completions_server; use app_test_support::create_mock_chat_completions_server_unchecked; use app_test_support::create_shell_sse_response; use app_test_support::to_response; +use codex_app_server_protocol::CommandExecutionStatus; +use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::ParsedCommand; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; @@ -17,9 +21,6 @@ use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::UserInput as V2UserInput; use codex_core::protocol_config_types::ReasoningEffort; use codex_core::protocol_config_types::ReasoningSummary; -use codex_protocol::parse_command::ParsedCommand; -use codex_protocol::protocol::Event; -use codex_protocol::protocol::EventMsg; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; use std::path::Path; @@ -235,7 +236,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { .await??; let ThreadStartResponse { thread } = to_response::(start_resp)?; - // turn/start — expect ExecCommandApproval request from server + // turn/start — expect CommandExecutionRequestApproval request from server let first_turn_id = mcp .send_turn_start_request(TurnStartParams { thread_id: thread.id.clone(), @@ -258,14 +259,14 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { mcp.read_stream_until_request_message(), ) .await??; - let ServerRequest::ExecCommandApproval { request_id, params } = server_req else { - panic!("expected ExecCommandApproval request"); + let ServerRequest::CommandExecutionRequestApproval { request_id, params } = server_req else { + panic!("expected CommandExecutionRequestApproval request"); }; - assert_eq!(params.call_id, "call1"); + assert_eq!(params.item_id, "call1"); assert_eq!( params.parsed_cmd, vec![ParsedCommand::Unknown { - cmd: "python3 -c 'print(42)'".to_string() + cmd: "python3 -c 'print(42)'".to_string(), }] ); @@ -302,7 +303,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { ) .await??; - // Ensure we do NOT receive an ExecCommandApproval request before task completes + // Ensure we do NOT receive a CommandExecutionRequestApproval request before task completes timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), @@ -314,8 +315,6 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { #[tokio::test] async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { - // When returning Result from a test, pass an Ok(()) to the skip macro - // so the early return type matches. The no-arg form returns unit. skip_if_no_network!(Ok(())); let tmp = TempDir::new()?; @@ -424,29 +423,35 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { ) .await??; - let exec_begin_notification = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("codex/event/exec_command_begin"), - ) + let command_exec_item = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let item_started_notification = mcp + .read_stream_until_notification_message("item/started") + .await?; + let params = item_started_notification + .params + .clone() + .expect("item/started params"); + let item_started: ItemStartedNotification = + serde_json::from_value(params).expect("deserialize item/started notification"); + if matches!(item_started.item, ThreadItem::CommandExecution { .. }) { + return Ok::(item_started.item); + } + } + }) .await??; - let params = exec_begin_notification - .params - .clone() - .expect("exec_command_begin params"); - let event: Event = serde_json::from_value(params).expect("deserialize exec begin event"); - let exec_begin = match event.msg { - EventMsg::ExecCommandBegin(exec_begin) => exec_begin, - other => panic!("expected ExecCommandBegin event, got {other:?}"), + let ThreadItem::CommandExecution { + cwd, + command, + status, + .. + } = command_exec_item + else { + unreachable!("loop ensures we break on command execution items"); }; - assert_eq!(exec_begin.cwd, second_cwd); - assert_eq!( - exec_begin.command, - vec![ - "bash".to_string(), - "-lc".to_string(), - "echo second turn".to_string() - ] - ); + assert_eq!(cwd, second_cwd); + assert_eq!(command, "bash -lc echo second turn"); + assert_eq!(status, CommandExecutionStatus::InProgress); timeout( DEFAULT_READ_TIMEOUT, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8bafc13aa6..3196176823 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -863,6 +863,7 @@ impl Session { let parsed_cmd = parse_command(&command); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { call_id, + turn_id: turn_context.sub_id.clone(), command, cwd, reason, diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 4ba4b5ef94..465517adba 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -65,22 +65,24 @@ impl SessionTask for UserShellCommandTask { // allows commands that use shell features (pipes, &&, redirects, etc.). // We do not source rc files or otherwise reformat the script. let use_login_shell = true; - let shell_invocation = session + let command = session .user_shell() .derive_exec_args(&self.command, use_login_shell); let call_id = Uuid::new_v4().to_string(); let raw_command = self.command.clone(); + let cwd = turn_context.cwd.clone(); - let parsed_cmd = parse_command(&shell_invocation); + let parsed_cmd = parse_command(&command); session .send_event( turn_context.as_ref(), EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: call_id.clone(), - command: shell_invocation.clone(), - cwd: turn_context.cwd.clone(), - parsed_cmd, + turn_id: turn_context.sub_id.clone(), + command: command.clone(), + cwd: cwd.clone(), + parsed_cmd: parsed_cmd.clone(), source: ExecCommandSource::UserShell, interaction_input: None, }), @@ -88,8 +90,8 @@ impl SessionTask for UserShellCommandTask { .await; let exec_env = ExecEnv { - command: shell_invocation, - cwd: turn_context.cwd.clone(), + command: command.clone(), + cwd: cwd.clone(), env: create_env(&turn_context.shell_environment_policy), timeout_ms: None, sandbox: SandboxType::None, @@ -129,6 +131,12 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, + turn_id: turn_context.sub_id.clone(), + command: command.clone(), + cwd: cwd.clone(), + parsed_cmd: parsed_cmd.clone(), + source: ExecCommandSource::UserShell, + interaction_input: None, stdout: String::new(), stderr: aborted_message.clone(), aggregated_output: aborted_message.clone(), @@ -145,6 +153,12 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: call_id.clone(), + turn_id: turn_context.sub_id.clone(), + command: command.clone(), + cwd: cwd.clone(), + parsed_cmd: parsed_cmd.clone(), + source: ExecCommandSource::UserShell, + interaction_input: None, stdout: output.stdout.text.clone(), stderr: output.stderr.text.clone(), aggregated_output: output.aggregated_output.text.clone(), @@ -176,6 +190,12 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, + turn_id: turn_context.sub_id.clone(), + command, + cwd, + parsed_cmd, + source: ExecCommandSource::UserShell, + interaction_input: None, stdout: exec_output.stdout.text.clone(), stderr: exec_output.stderr.text.clone(), aggregated_output: exec_output.aggregated_output.text.clone(), diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 3f8a9accfe..26dc397dc9 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -15,6 +15,7 @@ use crate::protocol::PatchApplyEndEvent; use crate::protocol::TurnDiffEvent; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::sandboxing::ToolError; +use codex_protocol::parse_command::ParsedCommand; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -61,6 +62,7 @@ pub(crate) async fn emit_exec_command_begin( ctx: ToolEventCtx<'_>, command: &[String], cwd: &Path, + parsed_cmd: &[ParsedCommand], source: ExecCommandSource, interaction_input: Option, ) { @@ -69,9 +71,10 @@ pub(crate) async fn emit_exec_command_begin( ctx.turn, EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: ctx.call_id.to_string(), + turn_id: ctx.turn.sub_id.clone(), command: command.to_vec(), cwd: cwd.to_path_buf(), - parsed_cmd: parse_command(command), + parsed_cmd: parsed_cmd.to_vec(), source, interaction_input, }), @@ -84,6 +87,7 @@ pub(crate) enum ToolEmitter { command: Vec, cwd: PathBuf, source: ExecCommandSource, + parsed_cmd: Vec, }, ApplyPatch { changes: HashMap, @@ -94,15 +98,18 @@ pub(crate) enum ToolEmitter { cwd: PathBuf, source: ExecCommandSource, interaction_input: Option, + parsed_cmd: Vec, }, } impl ToolEmitter { pub fn shell(command: Vec, cwd: PathBuf, source: ExecCommandSource) -> Self { + let parsed_cmd = parse_command(&command); Self::Shell { command, cwd, source, + parsed_cmd, } } @@ -119,11 +126,13 @@ impl ToolEmitter { source: ExecCommandSource, interaction_input: Option, ) -> Self { + let parsed_cmd = parse_command(command); Self::UnifiedExec { command: command.to_vec(), cwd, source, interaction_input, + parsed_cmd, } } @@ -134,44 +143,14 @@ impl ToolEmitter { command, cwd, source, + parsed_cmd, }, - ToolEventStage::Begin, + stage, ) => { - emit_exec_command_begin(ctx, command, cwd.as_path(), *source, None).await; - } - (Self::Shell { .. }, ToolEventStage::Success(output)) => { - emit_exec_end( + emit_exec_stage( ctx, - output.stdout.text.clone(), - output.stderr.text.clone(), - output.aggregated_output.text.clone(), - output.exit_code, - output.duration, - format_exec_output_str(&output), - ) - .await; - } - (Self::Shell { .. }, ToolEventStage::Failure(ToolEventFailure::Output(output))) => { - emit_exec_end( - ctx, - output.stdout.text.clone(), - output.stderr.text.clone(), - output.aggregated_output.text.clone(), - output.exit_code, - output.duration, - format_exec_output_str(&output), - ) - .await; - } - (Self::Shell { .. }, ToolEventStage::Failure(ToolEventFailure::Message(message))) => { - emit_exec_end( - ctx, - String::new(), - (*message).to_string(), - (*message).to_string(), - -1, - Duration::ZERO, - message.clone(), + ExecCommandInput::new(command, cwd.as_path(), parsed_cmd, *source, None), + stage, ) .await; } @@ -231,57 +210,20 @@ impl ToolEmitter { cwd, source, interaction_input, + parsed_cmd, }, - ToolEventStage::Begin, - ) => { - emit_exec_command_begin( - ctx, - command, - cwd.as_path(), - *source, - interaction_input.clone(), - ) - .await; - } - (Self::UnifiedExec { .. }, ToolEventStage::Success(output)) => { - emit_exec_end( - ctx, - output.stdout.text.clone(), - output.stderr.text.clone(), - output.aggregated_output.text.clone(), - output.exit_code, - output.duration, - format_exec_output_str(&output), - ) - .await; - } - ( - Self::UnifiedExec { .. }, - ToolEventStage::Failure(ToolEventFailure::Output(output)), - ) => { - emit_exec_end( - ctx, - output.stdout.text.clone(), - output.stderr.text.clone(), - output.aggregated_output.text.clone(), - output.exit_code, - output.duration, - format_exec_output_str(&output), - ) - .await; - } - ( - Self::UnifiedExec { .. }, - ToolEventStage::Failure(ToolEventFailure::Message(message)), + stage, ) => { - emit_exec_end( + emit_exec_stage( ctx, - String::new(), - (*message).to_string(), - (*message).to_string(), - -1, - Duration::ZERO, - message.clone(), + ExecCommandInput::new( + command, + cwd.as_path(), + parsed_cmd, + *source, + interaction_input.as_deref(), + ), + stage, ) .await; } @@ -340,26 +282,107 @@ impl ToolEmitter { } } -async fn emit_exec_end( - ctx: ToolEventCtx<'_>, +struct ExecCommandInput<'a> { + command: &'a [String], + cwd: &'a Path, + parsed_cmd: &'a [ParsedCommand], + source: ExecCommandSource, + interaction_input: Option<&'a str>, +} + +impl<'a> ExecCommandInput<'a> { + fn new( + command: &'a [String], + cwd: &'a Path, + parsed_cmd: &'a [ParsedCommand], + source: ExecCommandSource, + interaction_input: Option<&'a str>, + ) -> Self { + Self { + command, + cwd, + parsed_cmd, + source, + interaction_input, + } + } +} + +struct ExecCommandResult { stdout: String, stderr: String, aggregated_output: String, exit_code: i32, duration: Duration, formatted_output: String, +} + +async fn emit_exec_stage( + ctx: ToolEventCtx<'_>, + exec_input: ExecCommandInput<'_>, + stage: ToolEventStage, +) { + match stage { + ToolEventStage::Begin => { + emit_exec_command_begin( + ctx, + exec_input.command, + exec_input.cwd, + exec_input.parsed_cmd, + exec_input.source, + exec_input.interaction_input.map(str::to_owned), + ) + .await; + } + ToolEventStage::Success(output) + | ToolEventStage::Failure(ToolEventFailure::Output(output)) => { + let exec_result = ExecCommandResult { + stdout: output.stdout.text.clone(), + stderr: output.stderr.text.clone(), + aggregated_output: output.aggregated_output.text.clone(), + exit_code: output.exit_code, + duration: output.duration, + formatted_output: format_exec_output_str(&output), + }; + emit_exec_end(ctx, exec_input, exec_result).await; + } + ToolEventStage::Failure(ToolEventFailure::Message(message)) => { + let text = message.to_string(); + let exec_result = ExecCommandResult { + stdout: String::new(), + stderr: text.clone(), + aggregated_output: text.clone(), + exit_code: -1, + duration: Duration::ZERO, + formatted_output: text, + }; + emit_exec_end(ctx, exec_input, exec_result).await; + } + } +} + +async fn emit_exec_end( + ctx: ToolEventCtx<'_>, + exec_input: ExecCommandInput<'_>, + exec_result: ExecCommandResult, ) { ctx.session .send_event( ctx.turn, EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: ctx.call_id.to_string(), - stdout, - stderr, - aggregated_output, - exit_code, - duration, - formatted_output, + turn_id: ctx.turn.sub_id.clone(), + command: exec_input.command.to_vec(), + cwd: exec_input.cwd.to_path_buf(), + parsed_cmd: exec_input.parsed_cmd.to_vec(), + source: exec_input.source, + interaction_input: exec_input.interaction_input.map(str::to_owned), + stdout: exec_result.stdout, + stderr: exec_result.stderr, + aggregated_output: exec_result.aggregated_output, + exit_code: exec_result.exit_code, + duration: exec_result.duration, + formatted_output: exec_result.formatted_output, }), ) .await; diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index f9258a2b77..7a6245ae77 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -618,15 +618,19 @@ fn error_followed_by_task_complete_produces_turn_failed() { #[test] fn exec_command_end_success_produces_completed_command_item() { let mut ep = EventProcessorWithJsonOutput::new(None); + let command = vec!["bash".to_string(), "-lc".to_string(), "echo hi".to_string()]; + let cwd = std::env::current_dir().unwrap(); + let parsed_cmd = Vec::new(); // Begin -> no output let begin = event( "c1", EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: "1".to_string(), - command: vec!["bash".to_string(), "-lc".to_string(), "echo hi".to_string()], - cwd: std::env::current_dir().unwrap(), - parsed_cmd: Vec::new(), + turn_id: "turn-1".to_string(), + command: command.clone(), + cwd: cwd.clone(), + parsed_cmd: parsed_cmd.clone(), source: ExecCommandSource::Agent, interaction_input: None, }), @@ -652,6 +656,12 @@ fn exec_command_end_success_produces_completed_command_item() { "c2", EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "1".to_string(), + turn_id: "turn-1".to_string(), + command, + cwd, + parsed_cmd, + source: ExecCommandSource::Agent, + interaction_input: None, stdout: String::new(), stderr: String::new(), aggregated_output: "hi\n".to_string(), @@ -680,15 +690,19 @@ fn exec_command_end_success_produces_completed_command_item() { #[test] fn exec_command_end_failure_produces_failed_command_item() { let mut ep = EventProcessorWithJsonOutput::new(None); + let command = vec!["sh".to_string(), "-c".to_string(), "exit 1".to_string()]; + let cwd = std::env::current_dir().unwrap(); + let parsed_cmd = Vec::new(); // Begin -> no output let begin = event( "c1", EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: "2".to_string(), - command: vec!["sh".to_string(), "-c".to_string(), "exit 1".to_string()], - cwd: std::env::current_dir().unwrap(), - parsed_cmd: Vec::new(), + turn_id: "turn-1".to_string(), + command: command.clone(), + cwd: cwd.clone(), + parsed_cmd: parsed_cmd.clone(), source: ExecCommandSource::Agent, interaction_input: None, }), @@ -713,6 +727,12 @@ fn exec_command_end_failure_produces_failed_command_item() { "c2", EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "2".to_string(), + turn_id: "turn-1".to_string(), + command, + cwd, + parsed_cmd, + source: ExecCommandSource::Agent, + interaction_input: None, stdout: String::new(), stderr: String::new(), aggregated_output: String::new(), @@ -747,6 +767,12 @@ fn exec_command_end_without_begin_is_ignored() { "c1", EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "no-begin".to_string(), + turn_id: "turn-1".to_string(), + command: Vec::new(), + cwd: PathBuf::from("."), + parsed_cmd: Vec::new(), + source: ExecCommandSource::Agent, + interaction_input: None, stdout: String::new(), stderr: String::new(), aggregated_output: String::new(), diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index be9cbaaf7d..93dc7764dc 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -174,6 +174,7 @@ async fn run_codex_tool_session_inner( match event.msg { EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { + turn_id: _, command, cwd, call_id, diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 3227ddd1a3..f7c5fc6049 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -36,6 +36,10 @@ impl SandboxRiskLevel { pub struct ExecApprovalRequestEvent { /// Identifier for the associated exec call, if available. pub call_id: String, + /// Turn ID that this command belongs to. + /// Uses `#[serde(default)]` for backwards compatibility. + #[serde(default)] + pub turn_id: String, /// The command to be executed. pub command: Vec, /// The command's working directory. diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4a4a610441..553f6437a6 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1241,6 +1241,8 @@ impl Default for ExecCommandSource { pub struct ExecCommandBeginEvent { /// Identifier so this can be paired with the ExecCommandEnd event. pub call_id: String, + /// Turn ID that this command belongs to. + pub turn_id: String, /// The command to be executed. pub command: Vec, /// The command's working directory if not the default cwd for the agent. @@ -1259,6 +1261,21 @@ pub struct ExecCommandBeginEvent { pub struct ExecCommandEndEvent { /// Identifier for the ExecCommandBegin that finished. pub call_id: String, + /// Turn ID that this command belongs to. + pub turn_id: String, + /// The command that was executed. + pub command: Vec, + /// The command's working directory if not the default cwd for the agent. + pub cwd: PathBuf, + pub parsed_cmd: Vec, + /// Where the command originated. Defaults to Agent for backward compatibility. + #[serde(default)] + pub source: ExecCommandSource, + /// Raw input sent to a unified exec session (if this is an interaction event). + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub interaction_input: Option, + /// Captured stdout pub stdout: String, /// Captured stderr diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 554acf2f4e..4bcae99ef1 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -473,6 +473,7 @@ fn exec_approval_emits_proposed_command_and_decision_history() { // Trigger an exec approval request with a short, single-line command let ev = ExecApprovalRequestEvent { call_id: "call-short".into(), + turn_id: "turn-short".into(), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: Some( @@ -516,6 +517,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() { // Multiline command: modal should show full command, history records decision only let ev_multi = ExecApprovalRequestEvent { call_id: "call-multi".into(), + turn_id: "turn-multi".into(), command: vec!["bash".into(), "-lc".into(), "echo line1\necho line2".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: Some( @@ -567,6 +569,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() { let long = format!("echo {}", "a".repeat(200)); let ev_long = ExecApprovalRequestEvent { call_id: "call-long".into(), + turn_id: "turn-long".into(), command: vec!["bash".into(), "-lc".into(), long], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, @@ -598,38 +601,64 @@ fn begin_exec_with_source( call_id: &str, raw_cmd: &str, source: ExecCommandSource, -) { +) -> ExecCommandBeginEvent { // Build the full command vec and parse it using core's parser, // then convert to protocol variants for the event payload. let command = vec!["bash".to_string(), "-lc".to_string(), raw_cmd.to_string()]; let parsed_cmd: Vec = codex_core::parse_command::parse_command(&command); + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let interaction_input = None; + let event = ExecCommandBeginEvent { + call_id: call_id.to_string(), + turn_id: "turn-1".to_string(), + command, + cwd, + parsed_cmd, + source, + interaction_input, + }; chat.handle_codex_event(Event { id: call_id.to_string(), - msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { - call_id: call_id.to_string(), - command, - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd, - source, - interaction_input: None, - }), + msg: EventMsg::ExecCommandBegin(event.clone()), }); + event } -fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) { - begin_exec_with_source(chat, call_id, raw_cmd, ExecCommandSource::Agent); +fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) -> ExecCommandBeginEvent { + begin_exec_with_source(chat, call_id, raw_cmd, ExecCommandSource::Agent) } -fn end_exec(chat: &mut ChatWidget, call_id: &str, stdout: &str, stderr: &str, exit_code: i32) { +fn end_exec( + chat: &mut ChatWidget, + begin_event: ExecCommandBeginEvent, + stdout: &str, + stderr: &str, + exit_code: i32, +) { let aggregated = if stderr.is_empty() { stdout.to_string() } else { format!("{stdout}{stderr}") }; - chat.handle_codex_event(Event { - id: call_id.to_string(), + let ExecCommandBeginEvent { + call_id, + turn_id, + command, + cwd, + parsed_cmd, + source, + interaction_input, + } = begin_event; + chat.handle_codex_event(Event { + id: call_id.clone(), msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { - call_id: call_id.to_string(), + call_id, + turn_id, + command, + cwd, + parsed_cmd, + source, + interaction_input, stdout: stdout.to_string(), stderr: stderr.to_string(), aggregated_output: aggregated.clone(), @@ -803,13 +832,13 @@ fn exec_history_cell_shows_working_then_completed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); // Begin command - begin_exec(&mut chat, "call-1", "echo done"); + let begin = begin_exec(&mut chat, "call-1", "echo done"); let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 0, "no exec cell should have been flushed yet"); // End command successfully - end_exec(&mut chat, "call-1", "done", "", 0); + end_exec(&mut chat, begin, "done", "", 0); let cells = drain_insert_history(&mut rx); // Exec end now finalizes and flushes the exec cell immediately. @@ -833,12 +862,12 @@ fn exec_history_cell_shows_working_then_failed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); // Begin command - begin_exec(&mut chat, "call-2", "false"); + let begin = begin_exec(&mut chat, "call-2", "false"); let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 0, "no exec cell should have been flushed yet"); // End command with failure - end_exec(&mut chat, "call-2", "", "Bloop", 2); + end_exec(&mut chat, begin, "", "Bloop", 2); let cells = drain_insert_history(&mut rx); // Exec end with failure should also flush immediately. @@ -856,7 +885,7 @@ fn exec_history_cell_shows_working_then_failed() { fn exec_history_shows_unified_exec_startup_commands() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); - begin_exec_with_source( + let begin = begin_exec_with_source( &mut chat, "call-startup", "echo unified exec startup", @@ -867,13 +896,7 @@ fn exec_history_shows_unified_exec_startup_commands() { "exec begin should not flush until completion" ); - end_exec( - &mut chat, - "call-startup", - "echo unified exec startup\n", - "", - 0, - ); + end_exec(&mut chat, begin, "echo unified exec startup\n", "", 0); let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 1, "expected finalized exec cell to flush"); @@ -1588,29 +1611,29 @@ fn exec_history_extends_previous_when_consecutive() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); // 1) Start "ls -la" (List) - begin_exec(&mut chat, "call-ls", "ls -la"); + let begin_ls = begin_exec(&mut chat, "call-ls", "ls -la"); assert_snapshot!("exploring_step1_start_ls", active_blob(&chat)); // 2) Finish "ls -la" - end_exec(&mut chat, "call-ls", "", "", 0); + end_exec(&mut chat, begin_ls, "", "", 0); assert_snapshot!("exploring_step2_finish_ls", active_blob(&chat)); // 3) Start "cat foo.txt" (Read) - begin_exec(&mut chat, "call-cat-foo", "cat foo.txt"); + let begin_cat_foo = begin_exec(&mut chat, "call-cat-foo", "cat foo.txt"); assert_snapshot!("exploring_step3_start_cat_foo", active_blob(&chat)); // 4) Complete "cat foo.txt" - end_exec(&mut chat, "call-cat-foo", "hello from foo", "", 0); + end_exec(&mut chat, begin_cat_foo, "hello from foo", "", 0); assert_snapshot!("exploring_step4_finish_cat_foo", active_blob(&chat)); // 5) Start & complete "sed -n 100,200p foo.txt" (treated as Read of foo.txt) - begin_exec(&mut chat, "call-sed-range", "sed -n 100,200p foo.txt"); - end_exec(&mut chat, "call-sed-range", "chunk", "", 0); + let begin_sed_range = begin_exec(&mut chat, "call-sed-range", "sed -n 100,200p foo.txt"); + end_exec(&mut chat, begin_sed_range, "chunk", "", 0); assert_snapshot!("exploring_step5_finish_sed_range", active_blob(&chat)); // 6) Start & complete "cat bar.txt" - begin_exec(&mut chat, "call-cat-bar", "cat bar.txt"); - end_exec(&mut chat, "call-cat-bar", "hello from bar", "", 0); + let begin_cat_bar = begin_exec(&mut chat, "call-cat-bar", "cat bar.txt"); + end_exec(&mut chat, begin_cat_bar, "hello from bar", "", 0); assert_snapshot!("exploring_step6_finish_cat_bar", active_blob(&chat)); } @@ -1647,6 +1670,7 @@ fn approval_modal_exec_snapshot() { // Inject an exec approval request to display the approval modal. let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd".into(), + turn_id: "turn-approve-cmd".into(), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: Some( @@ -1694,6 +1718,7 @@ fn approval_modal_exec_without_reason_snapshot() { let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-noreason".into(), + turn_id: "turn-approve-cmd-noreason".into(), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, @@ -1903,6 +1928,7 @@ fn status_widget_and_approval_modal_snapshot() { // Now show an approval modal (e.g. exec approval). let ev = ExecApprovalRequestEvent { call_id: "call-approve-exec".into(), + turn_id: "turn-approve-exec".into(), command: vec!["echo".into(), "hello world".into()], cwd: PathBuf::from("/tmp"), reason: Some( @@ -2604,24 +2630,28 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() { msg: EventMsg::AgentMessage(AgentMessageEvent { message: "I’m going to search the repo for where “Change Approved” is rendered to update that view.".into() }), }); + let command = vec!["bash".into(), "-lc".into(), "rg \"Change Approved\"".into()]; + let parsed_cmd = vec![ + ParsedCommand::Search { + query: Some("Change Approved".into()), + path: None, + cmd: "rg \"Change Approved\"".into(), + }, + ParsedCommand::Read { + name: "diff_render.rs".into(), + cmd: "cat diff_render.rs".into(), + path: "diff_render.rs".into(), + }, + ]; + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); chat.handle_codex_event(Event { id: "c1".into(), msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: "c1".into(), - command: vec!["bash".into(), "-lc".into(), "rg \"Change Approved\"".into()], - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd: vec![ - ParsedCommand::Search { - query: Some("Change Approved".into()), - path: None, - cmd: "rg \"Change Approved\"".into(), - }, - ParsedCommand::Read { - name: "diff_render.rs".into(), - cmd: "cat diff_render.rs".into(), - path: "diff_render.rs".into(), - }, - ], + turn_id: "turn-1".into(), + command: command.clone(), + cwd: cwd.clone(), + parsed_cmd: parsed_cmd.clone(), source: ExecCommandSource::Agent, interaction_input: None, }), @@ -2630,6 +2660,12 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() { id: "c1".into(), msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "c1".into(), + turn_id: "turn-1".into(), + command, + cwd, + parsed_cmd, + source: ExecCommandSource::Agent, + interaction_input: None, stdout: String::new(), stderr: String::new(), aggregated_output: String::new(), From ca071a26ec04159f953d146c973a88703f01b5a3 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Mon, 17 Nov 2025 11:13:31 -0800 Subject: [PATCH 2/5] use shlex_join --- codex-rs/app-server/src/bespoke_event_handling.rs | 5 +++-- codex-rs/core/src/parse_command.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 62b373a45a..be1e1fe201 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -28,6 +28,7 @@ use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::TurnInterruptResponse; use codex_core::CodexConversation; +use codex_core::parse_command::shlex_join; use codex_core::protocol::ApplyPatchApprovalRequestEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; @@ -206,7 +207,7 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::ExecCommandBegin(exec_command_begin_event) => { let item = ThreadItem::CommandExecution { id: exec_command_begin_event.call_id.clone(), - command: exec_command_begin_event.command.join(" "), + command: shlex_join(&exec_command_begin_event.command), cwd: exec_command_begin_event.cwd, status: CommandExecutionStatus::InProgress, parsed_cmd: exec_command_begin_event @@ -262,7 +263,7 @@ pub(crate) async fn apply_bespoke_event_handling( let item = ThreadItem::CommandExecution { id: call_id, - command: command.join(" "), + command: shlex_join(&command), cwd, status, parsed_cmd: parsed_cmd.into_iter().map(V2ParsedCommand::from).collect(), diff --git a/codex-rs/core/src/parse_command.rs b/codex-rs/core/src/parse_command.rs index 61454db462..f9819ba83b 100644 --- a/codex-rs/core/src/parse_command.rs +++ b/codex-rs/core/src/parse_command.rs @@ -6,7 +6,7 @@ use shlex::split as shlex_split; use shlex::try_join as shlex_try_join; use std::path::PathBuf; -fn shlex_join(tokens: &[String]) -> String { +pub fn shlex_join(tokens: &[String]) -> String { shlex_try_join(tokens.iter().map(String::as_str)) .unwrap_or_else(|_| "".to_string()) } From d35431fd0eacbbf7e876ebd98c8af80e0daa3436 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Mon, 17 Nov 2025 11:32:45 -0800 Subject: [PATCH 3/5] use camelCase annotations --- codex-rs/app-server-protocol/src/protocol/v2.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 78fe157445..5fd173684c 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -881,6 +881,7 @@ pub struct CommandExecutionRequestApprovalParams { } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct CommandExecutionRequestApprovalResponse { pub decision: ReviewDecision, From 54db61b20701babb8a0dee646a239317adae1883 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Mon, 17 Nov 2025 14:16:56 -0800 Subject: [PATCH 4/5] address PR feedback --- .../app-server-protocol/src/protocol/v2.rs | 34 ++++++++++++------- .../app-server/src/bespoke_event_handling.rs | 19 +++++++++-- .../app-server/tests/suite/v2/turn_start.rs | 2 +- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 5fd173684c..36b8444de6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -59,22 +59,22 @@ 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 { + pub enum CommandRiskLevel from codex_protocol::approvals::SandboxRiskLevel { Low, Medium, High } ); +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub enum ApprovalDecision { + Accept, + Decline, + Cancel, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type")] @@ -145,7 +145,7 @@ impl From for SandboxPolicy { #[ts(export_to = "v2/")] pub struct SandboxCommandAssessment { pub description: String, - pub risk_level: SandboxRiskLevel, + pub risk_level: CommandRiskLevel, } impl SandboxCommandAssessment { @@ -161,7 +161,7 @@ impl From for SandboxCommandAssessment { fn from(value: CoreSandboxCommandAssessment) -> Self { Self { description: value.description, - risk_level: SandboxRiskLevel::from(value.risk_level), + risk_level: CommandRiskLevel::from(value.risk_level), } } } @@ -880,11 +880,19 @@ pub struct CommandExecutionRequestApprovalParams { pub parsed_cmd: Vec, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct CommandExecutionRequestAcceptSettings { + pub for_session: bool, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct CommandExecutionRequestApprovalResponse { - pub decision: ReviewDecision, + pub decision: ApprovalDecision, + pub accept_settings: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index be1e1fe201..a3927b6b81 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -5,6 +5,7 @@ use codex_app_server_protocol::AccountRateLimitsUpdatedNotification; use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; +use codex_app_server_protocol::ApprovalDecision; use codex_app_server_protocol::CommandExecutionOutputDeltaNotification; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; @@ -21,7 +22,6 @@ use codex_app_server_protocol::ParsedCommand as V2ParsedCommand; use codex_app_server_protocol::ReasoningSummaryPartAddedNotification; use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; use codex_app_server_protocol::ReasoningTextDeltaNotification; -use codex_app_server_protocol::ReviewDecision as V2ReviewDecision; use codex_app_server_protocol::SandboxCommandAssessment as V2SandboxCommandAssessment; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; @@ -401,11 +401,24 @@ async fn on_command_execution_request_approval_response( .unwrap_or_else(|err| { error!("failed to deserialize CommandExecutionRequestApprovalResponse: {err}"); CommandExecutionRequestApprovalResponse { - decision: V2ReviewDecision::Denied, + decision: ApprovalDecision::Decline, + accept_settings: None, } }); - let decision = response.decision.to_core(); + let CommandExecutionRequestApprovalResponse { + decision, + accept_settings, + } = response; + + let decision = match (decision, accept_settings) { + (ApprovalDecision::Accept, Some(settings)) if settings.for_session => { + ReviewDecision::ApprovedForSession + } + (ApprovalDecision::Accept, _) => ReviewDecision::Approved, + (ApprovalDecision::Decline, _) => ReviewDecision::Denied, + (ApprovalDecision::Cancel, _) => ReviewDecision::Abort, + }; if let Err(err) = conversation .submit(Op::ExecApproval { id: event_id, diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 3d3032285e..5b776878d4 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -450,7 +450,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { unreachable!("loop ensures we break on command execution items"); }; assert_eq!(cwd, second_cwd); - assert_eq!(command, "bash -lc echo second turn"); + assert_eq!(command, "bash -lc 'echo second turn'"); assert_eq!(status, CommandExecutionStatus::InProgress); timeout( From 47546f40e0c2860efb8257724b2ae49da91e0c28 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Mon, 17 Nov 2025 16:10:01 -0800 Subject: [PATCH 5/5] rename some vars --- .../app-server-protocol/src/protocol/v2.rs | 65 ++++++++++++------- .../app-server/src/bespoke_event_handling.rs | 7 +- .../app-server/tests/suite/v2/turn_start.rs | 7 -- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 36b8444de6..a2b9cee3fe 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -170,48 +170,64 @@ impl From for SandboxCommandAssessment { #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type")] #[ts(export_to = "v2/")] -pub enum ParsedCommand { +pub enum CommandAction { Read { - cmd: String, + command: String, name: String, path: PathBuf, }, ListFiles { - cmd: String, + command: String, path: Option, }, Search { - cmd: String, + command: String, query: Option, path: Option, }, Unknown { - cmd: String, + command: String, }, } -impl ParsedCommand { +impl CommandAction { 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 } + CommandAction::Read { + command: cmd, + name, + path, + } => CoreParsedCommand::Read { cmd, name, path }, + CommandAction::ListFiles { command: cmd, path } => { + CoreParsedCommand::ListFiles { cmd, path } } - ParsedCommand::Unknown { cmd } => CoreParsedCommand::Unknown { cmd }, + CommandAction::Search { + command: cmd, + query, + path, + } => CoreParsedCommand::Search { cmd, query, path }, + CommandAction::Unknown { command: cmd } => CoreParsedCommand::Unknown { cmd }, } } } -impl From for ParsedCommand { +impl From for CommandAction { 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::Read { cmd, name, path } => CommandAction::Read { + command: cmd, + name, + path, + }, + CoreParsedCommand::ListFiles { cmd, path } => { + CommandAction::ListFiles { command: cmd, path } } - CoreParsedCommand::Unknown { cmd } => ParsedCommand::Unknown { cmd }, + CoreParsedCommand::Search { cmd, query, path } => CommandAction::Search { + command: cmd, + query, + path, + }, + CoreParsedCommand::Unknown { cmd } => CommandAction::Unknown { command: cmd }, } } } @@ -624,11 +640,13 @@ pub enum ThreadItem { id: String, /// The command to be executed. command: String, - /// The command's working directory if not the default cwd for the agent. + /// The command's working directory. cwd: PathBuf, status: CommandExecutionStatus, - /// A best-effort parsing of the command to identify the type of command and its arguments. - parsed_cmd: Vec, + /// A best-effort parsing of the command to understand the action(s) it will perform. + /// This returns a list of CommandAction objects because a single shell command may + /// be composed of many commands piped together. + command_actions: Vec, /// The command's output, aggregated from stdout and stderr. aggregated_output: Option, /// The command's exit code. @@ -876,14 +894,14 @@ pub struct CommandExecutionRequestApprovalParams { pub reason: Option, /// Optional model-provided risk assessment describing the blocked command. pub risk: Option, - /// A best-effort parsing of the command to identify the type of command and its arguments. - pub parsed_cmd: Vec, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct CommandExecutionRequestAcceptSettings { + /// If true, automatically approve this command for the duration of the session. + #[serde(default)] pub for_session: bool, } @@ -892,6 +910,9 @@ pub struct CommandExecutionRequestAcceptSettings { #[ts(export_to = "v2/")] pub struct CommandExecutionRequestApprovalResponse { pub decision: ApprovalDecision, + /// Optional approval settings for when the decision is `accept`. + /// Ignored if the decision is `decline` or `cancel`. + #[serde(default)] pub accept_settings: Option, } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index a3927b6b81..8ed343f03a 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -6,6 +6,7 @@ use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; use codex_app_server_protocol::ApprovalDecision; +use codex_app_server_protocol::CommandAction as V2ParsedCommand; use codex_app_server_protocol::CommandExecutionOutputDeltaNotification; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; @@ -18,7 +19,6 @@ use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::McpToolCallError; use codex_app_server_protocol::McpToolCallResult; use codex_app_server_protocol::McpToolCallStatus; -use codex_app_server_protocol::ParsedCommand as V2ParsedCommand; use codex_app_server_protocol::ReasoningSummaryPartAddedNotification; use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; use codex_app_server_protocol::ReasoningTextDeltaNotification; @@ -111,7 +111,6 @@ pub(crate) async fn apply_bespoke_event_handling( item_id: call_id.clone(), reason, risk: risk.map(V2SandboxCommandAssessment::from), - parsed_cmd: parsed_cmd.into_iter().map(V2ParsedCommand::from).collect(), }; let rx = outgoing .send_request(ServerRequestPayload::CommandExecutionRequestApproval( @@ -210,7 +209,7 @@ pub(crate) async fn apply_bespoke_event_handling( command: shlex_join(&exec_command_begin_event.command), cwd: exec_command_begin_event.cwd, status: CommandExecutionStatus::InProgress, - parsed_cmd: exec_command_begin_event + command_actions: exec_command_begin_event .parsed_cmd .into_iter() .map(V2ParsedCommand::from) @@ -266,7 +265,7 @@ pub(crate) async fn apply_bespoke_event_handling( command: shlex_join(&command), cwd, status, - parsed_cmd: parsed_cmd.into_iter().map(V2ParsedCommand::from).collect(), + command_actions: parsed_cmd.into_iter().map(V2ParsedCommand::from).collect(), aggregated_output, exit_code: Some(exit_code), duration_ms: Some(duration_ms), diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 5b776878d4..433c7b4486 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -9,7 +9,6 @@ use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCResponse; -use codex_app_server_protocol::ParsedCommand; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::ThreadItem; @@ -263,12 +262,6 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { panic!("expected CommandExecutionRequestApproval request"); }; assert_eq!(params.item_id, "call1"); - assert_eq!( - params.parsed_cmd, - vec![ParsedCommand::Unknown { - cmd: "python3 -c 'print(42)'".to_string(), - }] - ); // Approve and wait for task completion mcp.send_response(